View Issue Details

IDProjectCategoryView StatusLast Update
0014827ScribusScripterpublic2017-05-29 16:46
Reporterdockattt Assigned To 
PrioritynormalSeveritytweakReproducibilityalways
Status newResolutionopen 
Product Version1.5.4.svn 
Target VersionFixed in Version 
Summary0014827: duplicateObject does not return name of duplicate
Descriptionthe scripting function duplicateObject does not return an id for the newly created duplicate. This causes additional scripting work to be done. The attached patch moves code out of slotEditPaste and adds a couple more lines that returns the newly created PageItems. This allows duplicateObject to get the name of the new duplicate and return it.

Steps To Reproducerun duplicateObject on an unpatched Scribus. A new object is created but no id is returned. Run it with patch and the id is returned.
TagsNo tags attached.
PatchYes

Activities

dockattt

2017-05-29 00:10

reporter  

duplicateobject.patch (5,902 bytes)
Index: Scribus/scribus/plugins/scriptplugin/cmdobj.cpp
===================================================================
--- Scribus/scribus/plugins/scriptplugin/cmdobj.cpp	(revision 22048)
+++ Scribus/scribus/plugins/scriptplugin/cmdobj.cpp	(working copy)
@@ -707,10 +707,10 @@
 		return NULL;
 	// do the duplicate
 	ScCore->primaryMainWindow()->slotEditCopy();
-	ScCore->primaryMainWindow()->slotEditPaste();
-//	Py_INCREF(Py_None);
-//	return Py_None;
-	Py_RETURN_NONE;
+	QList<PageItem*> newitems = ScCore->primaryMainWindow()->pasteObjects();
+	PageItem* newitem = newitems.at(0);
+	PyObject *rv = PyString_FromString(newitem->itemName().toUtf8());
+	return rv;
 }
 
 PyObject *scribus_copyobject(PyObject * /* self */, PyObject *args)
Index: Scribus/scribus/scribus.cpp
===================================================================
--- Scribus/scribus/scribus.cpp	(revision 22048)
+++ Scribus/scribus/scribus.cpp	(working copy)
@@ -4921,7 +4921,7 @@
 	}
 	else
 	{
-		if (ScMimeData::clipboardHasScribusElem() || ScMimeData::clipboardHasScribusFragment() || internalCopy)
+		/**if (ScMimeData::clipboardHasScribusElem() || ScMimeData::clipboardHasScribusFragment() || internalCopy)
 		{
 			view->Deselect(true);
 			uint docItemCount = doc->Items->count();
@@ -4981,7 +4981,8 @@
 				retObj->setXYPos(x, y, true);
 			}
 		}
-		view->DrawNew();
+		view->DrawNew();*/
+		pasteObjectsInternal();
 	}
 	if (activeTransaction)
 		activeTransaction.commit();
@@ -4990,6 +4991,100 @@
 	slotDocCh(false);
 }
 
+QList<PageItem*> ScribusMainWindow::pasteObjects(){
+
+	QList<PageItem*> rlist = QList<PageItem*>();
+	if (!HaveDoc)
+		return rlist;
+	if (doc->appMode == modeEditClip)
+		view->requestMode(submodeEndNodeEdit);
+	UndoTransaction activeTransaction;
+	if (!ScMimeData::clipboardHasScribusData() && (!internalCopy))
+		return rlist;
+	if (UndoManager::undoEnabled())
+		activeTransaction = m_undoManager->beginTransaction(doc->currentPage()->getUName(), 0, Um::Paste, "", Um::IPaste);
+
+	rlist = pasteObjectsInternal();
+
+	if (activeTransaction)
+		activeTransaction.commit();
+	if (doc->notesChanged())
+		doc->notesFramesUpdate();
+	slotDocCh(false);
+	return rlist;
+}
+
+QList<PageItem*> ScribusMainWindow::pasteObjectsInternal(){
+
+	QList<PageItem*> rlist = QList<PageItem*>();
+	if (ScMimeData::clipboardHasScribusElem() || ScMimeData::clipboardHasScribusFragment() || internalCopy)
+	{
+		view->Deselect(true);
+		uint docItemCount = doc->Items->count();
+		bool savedAlignGrid = doc->SnapGrid;
+		bool savedAlignGuides = doc->SnapGuides;
+		bool savedAlignElement = doc->SnapElement;
+		doc->SnapGrid = false;
+		doc->SnapGuides = false;
+		doc->SnapElement = false;
+		if (internalCopy)
+			slotElemRead(internalCopyBuffer, doc->currentPage()->xOffset(), doc->currentPage()->yOffset(), false, true, doc, view);
+		else
+		{
+			QString buffer  = ScMimeData::clipboardScribusElem();
+			slotElemRead(buffer, doc->currentPage()->xOffset(), doc->currentPage()->yOffset(), false, true, doc, view);
+		}
+		// update style lists:
+		m_styleManager->setDoc(doc);
+		propertiesPalette->unsetDoc();
+		propertiesPalette->setDoc(doc);
+		textPalette->unsetDoc();
+		textPalette->setDoc(doc);
+		marksManager->setDoc(doc);
+		nsEditor->setDoc(doc);
+		symbolPalette->unsetDoc();
+		symbolPalette->setDoc(doc);
+		inlinePalette->unsetDoc();
+		inlinePalette->setDoc(doc);
+
+		doc->SnapGrid = savedAlignGrid;
+		doc->SnapGuides = savedAlignGuides;
+		doc->SnapElement = savedAlignElement;
+		doc->m_Selection->delaySignalsOn();
+		for (int as = docItemCount; as < doc->Items->count(); ++as)
+		{
+			PageItem* currItem = doc->Items->at(as);
+			if (currItem->isBookmark)
+				AddBookMark(currItem);
+			doc->m_Selection->addItem(currItem);
+			rlist.append(currItem);
+		}
+		doc->m_Selection->delaySignalsOff();
+		if (doc->m_Selection->count() > 1)
+			doc->m_Selection->setGroupRect();
+		}
+	else if (ScMimeData::clipboardHasKnownData())
+	{
+		QString ext = ScMimeData::clipboardKnownDataExt();
+		QByteArray bitsBits = ScMimeData::clipboardKnownDataData();
+		double x0 = (view->contentsX() / view->scale()) + ((view->visibleWidth() / 2.0) / view->scale());
+		double y0 = (view->contentsY() / view->scale()) + ((view->visibleHeight() / 2.0) / view->scale());
+		PageItem *retObj = getVectorFileFromData(doc, bitsBits, ext, x0, y0);
+		if (retObj != NULL)
+		{
+			double x = (view->contentsX() / view->scale()) + ((view->visibleWidth() / 2.0) / view->scale()) - (retObj->width() / 2.0);
+			double y = (view->contentsY() / view->scale()) + ((view->visibleHeight() / 2.0) / view->scale()) - (retObj->height() / 2.0);
+			retObj->setTextFlowMode(PageItem::TextFlowUsesBoundingBox);
+			retObj->setXYPos(x, y, true);
+			rlist.append(retObj);
+		}
+	}
+	view->DrawNew();
+	return rlist;
+}
+
+
+
 //CB-->Doc ?????
 void ScribusMainWindow::SelectAllOnLayer()
 {
Index: Scribus/scribus/scribus.h
===================================================================
--- Scribus/scribus/scribus.h	(revision 22048)
+++ Scribus/scribus/scribus.h	(working copy)
@@ -159,6 +159,7 @@
 	void applyNewMaster(QString name);
 	void updateRecent(QString fn);
 	void doPasteRecent(QString data);
+	QList<PageItem*> pasteObjects();
 	bool getPDFDriver(const QString & filename, const QString & name, int components, const std::vector<int> & pageNumbers, const QMap<int,QPixmap> & thumbs, QString& error, bool* cancelled = NULL);
 	bool DoSaveAsEps(QString fn, QString& error);
 	QString CFileDialog(QString workingDirectory = ".", QString dialogCaption = "", QString fileFilter = "", QString defNa = "",
@@ -590,6 +591,8 @@
 	bool eventFilter( QObject *o, QEvent *e );
 	virtual void dragEnterEvent( QDragEnterEvent* e);
 	virtual void dropEvent( QDropEvent* e);
+	/** called by slotEditPaste() and pasteObjects()*/
+	QList<PageItem*> pasteObjectsInternal();
 
 private:
     /** init methods */
duplicateobject.patch (5,902 bytes)

ale

2017-05-29 07:23

manager   ~0043952

euh, in my eyes out-commenting code that you want to be deleted is a bit habit...

out-commenting it with a "doc" comment ("/**") is for sure not a good idea : - )

on the other side, if you add two functions, it would be nice to see a short doc for them, that says what the functions do...

 the goal of the patch seems to be a very good one... without being able to see what you are deleting, it's a bit hard to say if it's correct or not...

dockattt

2017-05-29 15:46

reporter   ~0043953

sorry about that. The deleted comments are in cmdobj.cpp. I believe it was right next to Py_RETURN_NONE and had to do with that. I've found that comment in other python functions. I think I deleted it because Py_RETURN_NONE wasn't being used anymore at that spot. The other deletions is the moving of code in one method into a new function. The code moved was replaced by a method call.

I think the function is nicer to use if it returns the name of the duplicated object. I used the unpatched duplicateObject a couple projects back and had to do extra stuff to figure out what was new.

ale

2017-05-29 15:50

manager   ~0043954

hi dockatt

i'm referring to

+++ Scribus/scribus/scribus.cpp (working copy)
@@ -4921,7 +4921,7 @@
     }
     else
     {
- if (ScMimeData::clipboardHasScribusElem() || ScMimeData::clipboardHasScribusFragment() || internalCopy)
+ /**if (ScMimeData::clipboardHasScribusElem() || ScMimeData::clipboardHasScribusFragment() || internalCopy)
...

not the comments you have removed : - )

dockattt

2017-05-29 16:46

reporter   ~0043956

very sorry about this! What happened is that I forgot to put the finished working file onto the svn file. I understand why this was confusing now. I just plopped it in and retested the added function. Here is the patch as it was supposed to look. The first one should function as well. I get positive results for both an individual item and a group.

I'm not sure if the pydoc needs to be updated for the duplicateObject function. It already says that it returns a string I think. Yet it doesn't say it returns the name of the duplicate. Perhaps it should say so?

duplicateobject_2nd.patch (7,995 bytes)
Index: Scribus/scribus/plugins/scriptplugin/cmdobj.cpp
===================================================================
--- Scribus/scribus/plugins/scriptplugin/cmdobj.cpp	(revision 22048)
+++ Scribus/scribus/plugins/scriptplugin/cmdobj.cpp	(working copy)
@@ -707,10 +707,10 @@
 		return NULL;
 	// do the duplicate
 	ScCore->primaryMainWindow()->slotEditCopy();
-	ScCore->primaryMainWindow()->slotEditPaste();
-//	Py_INCREF(Py_None);
-//	return Py_None;
-	Py_RETURN_NONE;
+	QList<PageItem*> newitems = ScCore->primaryMainWindow()->pasteObjects();
+	PageItem* newitem = newitems.at(0);
+	PyObject *rv = PyString_FromString(newitem->itemName().toUtf8());
+	return rv;
 }
 
 PyObject *scribus_copyobject(PyObject * /* self */, PyObject *args)
Index: Scribus/scribus/scribus.cpp
===================================================================
--- Scribus/scribus/scribus.cpp	(revision 22048)
+++ Scribus/scribus/scribus.cpp	(working copy)
@@ -4921,67 +4921,7 @@
 	}
 	else
 	{
-		if (ScMimeData::clipboardHasScribusElem() || ScMimeData::clipboardHasScribusFragment() || internalCopy)
-		{
-			view->Deselect(true);
-			uint docItemCount = doc->Items->count();
-			bool savedAlignGrid = doc->SnapGrid;
-			bool savedAlignGuides = doc->SnapGuides;
-			bool savedAlignElement = doc->SnapElement;
-			doc->SnapGrid = false;
-			doc->SnapGuides = false;
-			doc->SnapElement = false;
-			if (internalCopy)
-				slotElemRead(internalCopyBuffer, doc->currentPage()->xOffset(), doc->currentPage()->yOffset(), false, true, doc, view);
-			else
-			{
-				QString buffer  = ScMimeData::clipboardScribusElem();
-				slotElemRead(buffer, doc->currentPage()->xOffset(), doc->currentPage()->yOffset(), false, true, doc, view);
-			}
-			// update style lists:
-			m_styleManager->setDoc(doc);
-			propertiesPalette->unsetDoc();
-			propertiesPalette->setDoc(doc);
-			textPalette->unsetDoc();
-			textPalette->setDoc(doc);
-			marksManager->setDoc(doc);
-			nsEditor->setDoc(doc);
-			symbolPalette->unsetDoc();
-			symbolPalette->setDoc(doc);
-			inlinePalette->unsetDoc();
-			inlinePalette->setDoc(doc);
-
-			doc->SnapGrid = savedAlignGrid;
-			doc->SnapGuides = savedAlignGuides;
-			doc->SnapElement = savedAlignElement;
-			doc->m_Selection->delaySignalsOn();
-			for (int as = docItemCount; as < doc->Items->count(); ++as)
-			{
-				PageItem* currItem = doc->Items->at(as);
-				if (currItem->isBookmark)
-					AddBookMark(currItem);
-				doc->m_Selection->addItem(currItem);
-			}
-			doc->m_Selection->delaySignalsOff();
-			if (doc->m_Selection->count() > 1)
-				doc->m_Selection->setGroupRect();
-		}
-		else if (ScMimeData::clipboardHasKnownData())
-		{
-			QString ext = ScMimeData::clipboardKnownDataExt();
-			QByteArray bitsBits = ScMimeData::clipboardKnownDataData();
-			double x0 = (view->contentsX() / view->scale()) + ((view->visibleWidth() / 2.0) / view->scale());
-			double y0 = (view->contentsY() / view->scale()) + ((view->visibleHeight() / 2.0) / view->scale());
-			PageItem *retObj = getVectorFileFromData(doc, bitsBits, ext, x0, y0);
-			if (retObj != NULL)
-			{
-				double x = (view->contentsX() / view->scale()) + ((view->visibleWidth() / 2.0) / view->scale()) - (retObj->width() / 2.0);
-				double y = (view->contentsY() / view->scale()) + ((view->visibleHeight() / 2.0) / view->scale()) - (retObj->height() / 2.0);
-				retObj->setTextFlowMode(PageItem::TextFlowUsesBoundingBox);
-				retObj->setXYPos(x, y, true);
-			}
-		}
-		view->DrawNew();
+		pasteObjectsInternal();
 	}
 	if (activeTransaction)
 		activeTransaction.commit();
@@ -4990,6 +4930,100 @@
 	slotDocCh(false);
 }
 
+QList<PageItem*> ScribusMainWindow::pasteObjects(){
+
+	QList<PageItem*> rlist = QList<PageItem*>();
+	if (!HaveDoc)
+		return rlist;
+	if (doc->appMode == modeEditClip)
+		view->requestMode(submodeEndNodeEdit);
+	UndoTransaction activeTransaction;
+	if (!ScMimeData::clipboardHasScribusData() && (!internalCopy))
+		return rlist;
+	if (UndoManager::undoEnabled())
+		activeTransaction = m_undoManager->beginTransaction(doc->currentPage()->getUName(), 0, Um::Paste, "", Um::IPaste);
+
+	rlist = pasteObjectsInternal();
+
+	if (activeTransaction)
+		activeTransaction.commit();
+	if (doc->notesChanged())
+		doc->notesFramesUpdate();
+	slotDocCh(false);
+	return rlist;
+}
+
+QList<PageItem*> ScribusMainWindow::pasteObjectsInternal(){
+
+	QList<PageItem*> rlist = QList<PageItem*>();
+	if (ScMimeData::clipboardHasScribusElem() || ScMimeData::clipboardHasScribusFragment() || internalCopy)
+	{
+		view->Deselect(true);
+		uint docItemCount = doc->Items->count();
+		bool savedAlignGrid = doc->SnapGrid;
+		bool savedAlignGuides = doc->SnapGuides;
+		bool savedAlignElement = doc->SnapElement;
+		doc->SnapGrid = false;
+		doc->SnapGuides = false;
+		doc->SnapElement = false;
+		if (internalCopy)
+			slotElemRead(internalCopyBuffer, doc->currentPage()->xOffset(), doc->currentPage()->yOffset(), false, true, doc, view);
+		else
+		{
+			QString buffer  = ScMimeData::clipboardScribusElem();
+			slotElemRead(buffer, doc->currentPage()->xOffset(), doc->currentPage()->yOffset(), false, true, doc, view);
+		}
+		// update style lists:
+		m_styleManager->setDoc(doc);
+		propertiesPalette->unsetDoc();
+		propertiesPalette->setDoc(doc);
+		textPalette->unsetDoc();
+		textPalette->setDoc(doc);
+		marksManager->setDoc(doc);
+		nsEditor->setDoc(doc);
+		symbolPalette->unsetDoc();
+		symbolPalette->setDoc(doc);
+		inlinePalette->unsetDoc();
+		inlinePalette->setDoc(doc);
+
+		doc->SnapGrid = savedAlignGrid;
+		doc->SnapGuides = savedAlignGuides;
+		doc->SnapElement = savedAlignElement;
+		doc->m_Selection->delaySignalsOn();
+		for (int as = docItemCount; as < doc->Items->count(); ++as)
+		{
+			PageItem* currItem = doc->Items->at(as);
+			if (currItem->isBookmark)
+				AddBookMark(currItem);
+			doc->m_Selection->addItem(currItem);
+			rlist.append(currItem);
+		}
+		doc->m_Selection->delaySignalsOff();
+		if (doc->m_Selection->count() > 1)
+			doc->m_Selection->setGroupRect();
+		}
+	else if (ScMimeData::clipboardHasKnownData())
+	{
+		QString ext = ScMimeData::clipboardKnownDataExt();
+		QByteArray bitsBits = ScMimeData::clipboardKnownDataData();
+		double x0 = (view->contentsX() / view->scale()) + ((view->visibleWidth() / 2.0) / view->scale());
+		double y0 = (view->contentsY() / view->scale()) + ((view->visibleHeight() / 2.0) / view->scale());
+		PageItem *retObj = getVectorFileFromData(doc, bitsBits, ext, x0, y0);
+		if (retObj != NULL)
+		{
+			double x = (view->contentsX() / view->scale()) + ((view->visibleWidth() / 2.0) / view->scale()) - (retObj->width() / 2.0);
+			double y = (view->contentsY() / view->scale()) + ((view->visibleHeight() / 2.0) / view->scale()) - (retObj->height() / 2.0);
+			retObj->setTextFlowMode(PageItem::TextFlowUsesBoundingBox);
+			retObj->setXYPos(x, y, true);
+			rlist.append(retObj);
+		}
+	}
+	view->DrawNew();
+	return rlist;
+}
+
+
+
 //CB-->Doc ?????
 void ScribusMainWindow::SelectAllOnLayer()
 {
Index: Scribus/scribus/scribus.h
===================================================================
--- Scribus/scribus/scribus.h	(revision 22048)
+++ Scribus/scribus/scribus.h	(working copy)
@@ -159,6 +159,7 @@
 	void applyNewMaster(QString name);
 	void updateRecent(QString fn);
 	void doPasteRecent(QString data);
+	QList<PageItem*> pasteObjects();
 	bool getPDFDriver(const QString & filename, const QString & name, int components, const std::vector<int> & pageNumbers, const QMap<int,QPixmap> & thumbs, QString& error, bool* cancelled = NULL);
 	bool DoSaveAsEps(QString fn, QString& error);
 	QString CFileDialog(QString workingDirectory = ".", QString dialogCaption = "", QString fileFilter = "", QString defNa = "",
@@ -590,6 +591,8 @@
 	bool eventFilter( QObject *o, QEvent *e );
 	virtual void dragEnterEvent( QDragEnterEvent* e);
 	virtual void dropEvent( QDropEvent* e);
+	/** called by slotEditPaste() and pasteObjects()*/
+	QList<PageItem*> pasteObjectsInternal();
 
 private:
     /** init methods */

Issue History

Date Modified Username Field Change
2017-05-29 00:10 dockattt New Issue
2017-05-29 00:10 dockattt File Added: duplicateobject.patch
2017-05-29 07:23 ale Note Added: 0043952
2017-05-29 15:46 dockattt Note Added: 0043953
2017-05-29 15:50 ale Note Added: 0043954
2017-05-29 16:46 dockattt File Added: duplicateobject_2nd.patch
2017-05-29 16:46 dockattt Note Added: 0043956