View Issue Details

IDProjectCategoryView StatusLast Update
0014030ScribusScrapbookpublic2022-06-26 22:10
Reporterale Assigned To 
PrioritynormalSeverityminorReproducibilityalways
Status confirmedResolutionopen 
Product Version1.5.2 
Target Version1.6 milestone 
Summary0014030: cannot add three times an item with the same name to the scrapbook
Description- create a new file
- create a text frame
- add it to the scrapbook (Text1)
- create a new file
- create a text frame
- add it to the scrapbook ("Text1(0)")
- create a new file
- create a text frame
- add it to the scrapbook: conflict in the name

the scrapbook should not change the name of the item (as shown and as it will be reinserted in a document), but it might use an internal postfix for avoiding conflicts when storing the item on disk.

if there is a conflict in the name, it will have to be resolved when inserting the scrapbook item in the document.
Tagsscrapbook
PatchNo

Activities

Kunda

2016-05-14 21:37

updater  

Kunda

2016-05-14 21:38

updater   ~0041059

Confirmed on 10.10.5 1.5.2svn r21311
added gif
assigning to 1.6 milestone

goto_coder

2022-06-12 21:32

reporter   ~0049697

I'd like to try to write a patch for this.

goto_coder

2022-06-17 23:09

reporter   ~0049706

I think I found the spot where the error originates from. With the attached patch the problem seems to disappear. But the numbering will be somewhat arbitrary in subsequent sessions.
With the patch it will basically just check if the name of the object to be added is already occupied and then count upwards until a unique name is found. This seems to work quite reasonable for new documents 'spawned' from within a single Scribus session. But in the next session, the counter is reset and this will lead to e.g. 'Text1(5)' being auto-corrected to 'Text1(5)(0)' instead of 'Text1(6)'.
Albeit not looking pretty, I'm not sure if we can really do any better than this, because we'd need to guess which numbers in a name are supposed to be counted upwards and which are perhaps part of a name the user supplied. I'm afraid this could mean opening a Pandora's Box.
Do you know how this is handled in the commercial software?
scrapbook_funny_numbering_0014030_2022_06_18_goto_coder.diff (2,492 bytes)   
Index: scribus/ui/scrapbookpalette.cpp
===================================================================
--- scribus/ui/scrapbookpalette.cpp	(Revision 25096)
+++ scribus/ui/scrapbookpalette.cpp	(Arbeitskopie)
@@ -1907,8 +1907,8 @@
 
 void Biblio::objFromMainMenu(QString text, int scrapID)
 {
-	QString nam;
-	QString tmp;
+	QString nam; // A String to store the name of the new object
+	QString tmp; // A String to store temporary information
 	int scID = scrapID;
 	if (scID > 0)
 		scID++;
@@ -1916,11 +1916,13 @@
 	if (!actBView->canWrite)
 		return;
 	nam = getObjectName(text);
-	if (nam.isEmpty())
-		nam = tr("Object") + tmp.setNum(actBView->objectMap.count());
-	if (actBView->objectMap.contains(nam))
-		nam += "("+ tmp.setNum(m_tempCount) + ")";
-	Query dia(this, "tt", 1, tr("&Name:"), tr("New Entry"));
+	if (nam.isEmpty()) // If there is no name to the object...
+		nam = tr("Object") + tmp.setNum(actBView->objectMap.count()); // ...name it `object` + number
+	while (actBView->objectMap.contains(nam)){ // If the current object map contains this name...
+		nam += "("+ tmp.setNum(m_tempCount) + ")"; // ...add number in parentheses at the end, e.g. `name (0)`
+        ++m_tempCount; // Increase counter and repeat until unique name is found.
+    }
+	Query dia(this, "tt", 1, tr("&Name:"), tr("New Entry")); // Query to validate string
 	dia.setValidator(QRegExp("[\\w()]+"));
 	dia.setEditText(nam, true);
 	dia.setTestList(activeBView->objectMap.keys());
Index: scribus/ui/scrapbookpalette.h
===================================================================
--- scribus/ui/scrapbookpalette.h	(Revision 25096)
+++ scribus/ui/scrapbookpalette.h	(Arbeitskopie)
@@ -34,6 +34,17 @@
 	BibView( QWidget* parent);
 	~BibView() {};
 
+    /**
+    * Add an object to the scrapbook.
+    * Creates a `struct Elem` from the parameters and 
+    * adds it to the srapbook.
+    * @param name The name of the new object.
+    * @param daten The data associated with the object.
+    * @param Bild Preview image
+    * @param isDir Whether the object is a directory
+    * @param isRaster Whether the object is a raster image.
+    * @param isVector Whether the object is a vector.
+    */
 	void addObject(const QString& name, const QString& daten, const QPixmap& Bild, bool isDir = false, bool isRaster = false, bool isVector = false);
 	void checkForImg(const QDomElement& elem, bool &hasImage);
 	void checkAndChange(const QString& text, const QString& nam, const QString& dir);

cbradney

2022-06-21 18:56

administrator   ~0049714

@goto_coder: I wonder if you could use "bool QString::contains(const QRegularExpression &re, QRegularExpressionMatch *rmatch) const" to try to match "(number)" with regular expression of "\(\d+\)". If true, then the rmatch value will contain the number which once you strip ( and ), you could QString::toInt(), increment, then replace the number in the string?

goto_coder

2022-06-26 22:10

reporter   ~0049722

@cbradney: Thanks a lot for the advice! I tried to implement a new solution based on your suggestion and it seems to work great, as far as I can tell. :-)
scrapbook_funny_numbering_0014030_2022_06_26_goto_coder.diff (3,796 bytes)   
Index: scribus/ui/scrapbookpalette.cpp
===================================================================
--- scribus/ui/scrapbookpalette.cpp	(Revision 25096)
+++ scribus/ui/scrapbookpalette.cpp	(Arbeitskopie)
@@ -1907,8 +1907,8 @@
 
 void Biblio::objFromMainMenu(QString text, int scrapID)
 {
-	QString nam;
-	QString tmp;
+	QString nam; // A String to store the name of the new object
+	QString tmp; // A String to store temporary information
 	int scID = scrapID;
 	if (scID > 0)
 		scID++;
@@ -1916,11 +1916,38 @@
 	if (!actBView->canWrite)
 		return;
 	nam = getObjectName(text);
-	if (nam.isEmpty())
-		nam = tr("Object") + tmp.setNum(actBView->objectMap.count());
-	if (actBView->objectMap.contains(nam))
-		nam += "("+ tmp.setNum(m_tempCount) + ")";
-	Query dia(this, "tt", 1, tr("&Name:"), tr("New Entry"));
+	if (nam.isEmpty()) // If there is no name to the object...
+		nam = tr("Object") + tmp.setNum(actBView->objectMap.count()); // ...name it `object` + number
+
+    // Check whether the chosen name is already in use... 
+	while (actBView->objectMap.contains(nam)){ 
+
+        // If the name already exists and ends with a number in parentheses,
+        // we replace that number by its successor.
+        // Otherwise, append a 'new' number in parentheses. 
+
+        QRegularExpression counterRegEx("(\\()(\\d+)(\\))$"); // Setup RegEx with 'matching groups'
+        QRegularExpressionMatch counterMatch; 
+        if(nam.contains(counterRegEx, &counterMatch)){ // Check if name ends with number in parentheses
+            bool ok{ false };
+            int currentCounter = counterMatch.captured(2).toInt(&ok); // Group #2 should contain the number 
+            if(ok){ // Make sure conversion from `int` to `QString` succeeded
+                ++currentCounter; // Increase counter
+                QString counterStr = "("+QString::number(currentCounter)+")"; // Create new string to append to name.
+                nam = nam.replace(counterRegEx, counterStr); // Replace the part we matched earlier.
+                continue; // We're done; Restart loop and check if we have a unique name now.
+            }
+        }
+
+        // We should only reach this part, if there was no match to the regex, 
+        // or if something went wrong with integer to string conversion.
+        // In this case, we just append a 'new' number.
+		nam += "("+ tmp.setNum(m_tempCount) + ")"; // Append number at the end, e.g. `name (0)`
+        ++m_tempCount; // Increase counter and repeat until unique name is found.
+        
+    } // end while
+
+	Query dia(this, "tt", 1, tr("&Name:"), tr("New Entry")); // Query to validate string
 	dia.setValidator(QRegExp("[\\w()]+"));
 	dia.setEditText(nam, true);
 	dia.setTestList(activeBView->objectMap.keys());
Index: scribus/ui/scrapbookpalette.h
===================================================================
--- scribus/ui/scrapbookpalette.h	(Revision 25096)
+++ scribus/ui/scrapbookpalette.h	(Arbeitskopie)
@@ -34,6 +34,17 @@
 	BibView( QWidget* parent);
 	~BibView() {};
 
+    /**
+    * Add an object to the scrapbook.
+    * Creates a `struct Elem` from the parameters and 
+    * adds it to the srapbook.
+    * @param name The name of the new object.
+    * @param daten The data associated with the object.
+    * @param Bild Preview image
+    * @param isDir Whether the object is a directory
+    * @param isRaster Whether the object is a raster image.
+    * @param isVector Whether the object is a vector.
+    */
 	void addObject(const QString& name, const QString& daten, const QPixmap& Bild, bool isDir = false, bool isRaster = false, bool isVector = false);
 	void checkForImg(const QDomElement& elem, bool &hasImage);
 	void checkAndChange(const QString& text, const QString& nam, const QString& dir);

Issue History

Date Modified Username Field Change
2016-05-06 09:33 ale New Issue
2016-05-06 19:15 Kunda Category - => Scrapbook
2016-05-14 20:53 Kunda Tag Attached: scrapbook
2016-05-14 21:37 Kunda File Added: scrapbook-name-confict-after-3-times.gif
2016-05-14 21:38 Kunda Note Added: 0041059
2016-05-14 21:38 Kunda Status new => confirmed
2016-05-14 21:39 Kunda Target Version => 1.6 milestone
2022-06-12 21:32 goto_coder Note Added: 0049697
2022-06-17 23:09 goto_coder Note Added: 0049706
2022-06-17 23:09 goto_coder File Added: scrapbook_funny_numbering_0014030_2022_06_18_goto_coder.diff
2022-06-21 18:56 cbradney Note Added: 0049714
2022-06-26 22:10 goto_coder Note Added: 0049722
2022-06-26 22:10 goto_coder File Added: scrapbook_funny_numbering_0014030_2022_06_26_goto_coder.diff