View Issue Details

IDProjectCategoryView StatusLast Update
0016317ScribusScripterpublic2020-11-10 13:16
Reporterale Assigned To 
PrioritynormalSeverityfeatureReproducibilityN/A
Status newResolutionopen 
Product Version1.5.6.svn 
Target VersionFixed in Version 
Summary0016317: [PATCH] add GetSingleItem and deprecate GetUniqueItem
DescriptionIt would be handy to have a util to get the named item or the current one, but only if one single item is selected.
currently, you have to first check if the name is not set and the selection is not a single one, before calling GetUniqueItem()... which is way too verbose for a check that so many commands should have.

i would call the function GetSingleItem(name).

the much used GetUniqueItem() does not do that and has an "undefined" behavior, when multiple items are selected.
(one "random" item in the selection is returned.)
which is nice for quick and dirty scripts, but is "dangerous" for scripts that are published for broad consumption and act on the user's selection.

the currently existing GetUniqueItem() should be deprecated. and -- later -- the command using it should be updated to explicitly accept multiple selections or enforce that only one item is selected.

we don't strictly need a new command, but i have the feeling that creating a new one can make the migration easier.
if you prefer to "fix" GetUniqueItem to only accept one single selected item, i'm fine with it...
all the script that were sloppy might then fail, but that's the price to pay when you build your code on undefined behaviors...
TagsNo tags attached.
PatchYes

Relationships

related to 0016300 new [PATCHJ] add moveToPage() to the scripter 

Activities

ale

2020-11-05 10:05

manager  

single-item.diff (1,945 bytes)
diff --git a/scribus/plugins/scriptplugin/cmdutil.cpp b/scribus/plugins/scriptplugin/cmdutil.cpp
index 7e1b80050..3dce42150 100644
--- a/scribus/plugins/scriptplugin/cmdutil.cpp
+++ b/scribus/plugins/scriptplugin/cmdutil.cpp
@@ -111,6 +111,23 @@ PageItem* GetUniqueItem(const QString& name)
 	return getPageItemByName(name);
 }
 
+PageItem* GetSingleItem(const char* name)
+{
+	return GetSingleItem(QString::fromUtf8(name));
+}
+
+PageItem* GetSingleItem(const QString& name)
+{
+	if (!name.isEmpty())
+		return getPageItemByName(name);
+
+	if (ScCore->primaryMainWindow()->doc->m_Selection->count() == 1)
+		return ScCore->primaryMainWindow()->doc->m_Selection->itemAt(0);
+
+	PyErr_SetString(NoValidObjectError, QString("Please define an item name or select one single item").toLocal8Bit().constData());
+	return nullptr;
+}
+
 PageItem* getPageItemByName(const QString& name)
 {
 	if (name.length() == 0)
diff --git a/scribus/plugins/scriptplugin/cmdutil.h b/scribus/plugins/scriptplugin/cmdutil.h
index 33f785dc2..4c92dfd72 100644
--- a/scribus/plugins/scriptplugin/cmdutil.h
+++ b/scribus/plugins/scriptplugin/cmdutil.h
@@ -43,6 +43,25 @@ void ReplaceColor(const QString& col, const QString& rep);
  */
 PageItem* GetUniqueItem(const QString& name);
 
+/*!
+ * @brief Returns named PageItem, single selection, or set exception and NULL if no item.
+ *
+ * Returns a pointer to a PageItem by looking it up by name - on any page.
+ * If `name' is set, return the matching item or set an exception and return nullptr.
+ * Otherwise, if there is not one single item selected, set an exception and return nullptr
+ * Otherwise, return the selected item.
+ *
+ * @author ale rimoldi
+ */
+PageItem* GetSingleItem(const QString& name);
+
+/*!
+ * @brief call GetSingleItem with a QString
+ *
+ * @author ale rimoldi
+ */
+PageItem* GetSingleItem(const char* name);
+
 /*!
  * @brief Returns named PageItem, or exception and NULL if not found.
  *
single-item.diff (1,945 bytes)

Issue History

Date Modified Username Field Change
2020-11-05 09:43 ale New Issue
2020-11-05 10:05 ale File Added: single-item.diff
2020-11-05 10:07 ale Summary add GetSingleItem and deprecate GetUniqueItem => [PATCH] add GetSingleItem and deprecate GetUniqueItem
2020-11-05 10:07 ale Description Updated View Revisions
2020-11-05 10:07 ale Patch No => Yes
2020-11-10 13:16 ale Relationship added related to 0016300