View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0017399 | Scribus | General | public | 2025-01-31 16:05 | 2025-04-05 22:22 |
Reporter | tropion | Assigned To | |||
Priority | normal | Severity | minor | Reproducibility | always |
Status | new | Resolution | open | ||
Summary | 0017399: Objects in groups are not accessible via the scriptplugin python API | ||||
Description | I suggest the attached patch | ||||
Steps To Reproduce | Create objects Group the objects Open the script console Type getAllObjects() Press F9 See that the group is returned, but not the elements of the group. Type objectsExists('<name of obj in group>') Press F9 See that the object is not accessible. getGroupItems ('Group1',recursive=true) returns all objects in Group1, but there still is no way to do anything with them. | ||||
Tags | No tags attached. | ||||
Patch | |||||
|
scriptplugin.diff (5,645 bytes)
diff --git a/scribus/plugins/scriptplugin/cmdgetprop.cpp b/scribus/plugins/scriptplugin/cmdgetprop.cpp index 3106dd471..c60a29070 100644 --- a/scribus/plugins/scriptplugin/cmdgetprop.cpp +++ b/scribus/plugins/scriptplugin/cmdgetprop.cpp @@ -383,7 +383,6 @@ PyObject *scribus_getallobjects(PyObject* /* self */, PyObject* args, PyObject * { int itemType = -1; int layerId = -1; - uint counter = 0; if (!checkHaveDocument()) return nullptr; @@ -426,16 +425,24 @@ PyObject *scribus_getallobjects(PyObject* /* self */, PyObject* args, PyObject * return true; }; - int returnedItemCount = std::count_if(currentDoc->Items->begin(), currentDoc->Items->end(), isReturnedItem); - - PyObject* pyItemList = PyList_New(returnedItemCount); + PyObject* pyItemList = PyList_New(0); for (int i = 0; i < currentDoc->Items->count(); ++i) { PageItem* item = currentDoc->Items->at(i); - if (!isReturnedItem(item)) + if (!isReturnedItem(item)) // No items of wrong type or from wrong Layer continue; - PyList_SetItem(pyItemList, counter, PyUnicode_FromString(item->itemName().toUtf8())); - counter++; + PyList_Append(pyItemList, PyUnicode_FromString(item->itemName().toUtf8())); + if (item->isGroup()) + { + QList<PageItem*> allItems; + allItems = item->asGroupFrame()->getAllChildren(); + for (PageItem *it : allItems) + { + if (!isReturnedItem(it)) // No items of wrong type or from wrong Layer + continue; + PyList_Append(pyItemList, PyUnicode_FromString(it->itemName().toUtf8())); + } + } } return pyItemList; } diff --git a/scribus/plugins/scriptplugin/cmdpage.cpp b/scribus/plugins/scriptplugin/cmdpage.cpp index a99b66c1f..dfbb493ea 100644 --- a/scribus/plugins/scriptplugin/cmdpage.cpp +++ b/scribus/plugins/scriptplugin/cmdpage.cpp @@ -244,16 +244,10 @@ PyObject *scribus_getpageitems(PyObject* /* self */) if (currentDoc->Items->count() == 0) return Py_BuildValue("[]"); - uint counter = 0; + int pageNr = currentDoc->currentPageNumber(); - for (int lam2 = 0; lam2 < currentDoc->Items->count(); ++lam2) - { - if (pageNr == currentDoc->Items->at(lam2)->OwnPage) - counter++; - } - PyObject *l = PyList_New(counter); + PyObject* l = PyList_New (0); PyObject *row; - counter = 0; for (int i = 0; i<currentDoc->Items->count(); ++i) { if (pageNr == currentDoc->Items->at(i)->OwnPage) @@ -263,8 +257,26 @@ PyObject *scribus_getpageitems(PyObject* /* self */) currentDoc->Items->at(i)->itemType(), currentDoc->Items->at(i)->uniqueNr ); - PyList_SetItem(l, counter, row); - counter++; + PyList_Append(l, row); + if (currentDoc->Items->at(i)->isGroup()) + { + QList<PageItem*> allItems; + allItems = currentDoc->Items->at(i)->asGroupFrame()->getAllChildren(); + for (PageItem* it : allItems) + { + if (it->OwnPage == pageNr) + { + row = Py_BuildValue("(sii)", + it->itemName().toUtf8().constData(), + it->itemType(), + it->uniqueNr + ); + PyList_Append(l, row); + } + else + continue; + } + } } } // for return l; diff --git a/scribus/plugins/scriptplugin/cmdutil.cpp b/scribus/plugins/scriptplugin/cmdutil.cpp index d3bd6da8f..b15593907 100644 --- a/scribus/plugins/scriptplugin/cmdutil.cpp +++ b/scribus/plugins/scriptplugin/cmdutil.cpp @@ -63,11 +63,23 @@ PageItem *GetItem(const QString& name) ScribusDoc* currentDoc = ScCore->primaryMainWindow()->doc; if (!name.isEmpty()) { + // We do not call getPageItemByName() here, + // bc we do not want to raise an error in case the name is not found for (int i = 0; i < currentDoc->Items->count(); ++i) - { - if (currentDoc->Items->at(i)->itemName() == name) - return currentDoc->Items->at(i); - } + { + if (currentDoc->Items->at(i)->itemName() == name) + return currentDoc->Items->at(i); + if (currentDoc->Items->at(i)->isGroup()) + { + QList<PageItem*> allItems; + allItems = currentDoc->Items->at(i)->asGroupFrame()->getAllChildren(); + for (PageItem* it : allItems) + { + if (name == it->itemName()) + return it; + } + } + } } else { @@ -77,6 +89,7 @@ PageItem *GetItem(const QString& name) return nullptr; } + void ReplaceColor(const QString& col, const QString& rep) { QMap<QString, QString> colorMap; @@ -123,6 +136,16 @@ PageItem* getPageItemByName(const QString& name) { if (name == currentDoc->Items->at(i)->itemName()) return currentDoc->Items->at(i); + if (currentDoc->Items->at(i)->isGroup()) + { + QList<PageItem*> allItems; + allItems = currentDoc->Items->at(i)->asGroupFrame()->getAllChildren(); + for (PageItem* it : allItems) + { + if (name == it->itemName()) + return it; + } + } } PyErr_SetString(NoValidObjectError, QString("Object not found").toLocal8Bit().constData()); @@ -140,13 +163,7 @@ bool ItemExists(const QString& name) if (name.length() == 0) return false; - ScribusDoc* currentDoc = ScCore->primaryMainWindow()->doc; - for (int i = 0; i < currentDoc->Items->count(); ++i) - { - if (name == currentDoc->Items->at(i)->itemName()) - return true; - } - return false; + return GetItem(name) != nullptr; } /*! @@ -201,11 +218,7 @@ bool setSelectedItemsByName(const QStringList& itemNames) { // Search for the named item PageItem* item = nullptr; - for (int j = 0; j < currentDoc->Items->count(); j++) - { - if (*it == currentDoc->Items->at(j)->itemName()) - item = currentDoc->Items->at(j); - } + item = GetItem(*it); if (!item) return false; // And select it |
|
can you explain what exactly your script does? if i understand it correctly, it's patching all the function that have an own implementation of getItem (and getItem itself) to also look inside of groups. if it's the case, that's fine for me and i would be ready to review your patch. i already have one remark: shouldn't also groups in groups be explored? this can be done by creating a stack with the items to be looped and adding the children to it, when a group is found. |
|
You do understand this correctly, it changes getItem and all variations of it to also look inside of groups. Whenever a group is encountered getAllChildren() is called. That should explore groups in groups. |
|
/tmp/scriptplugin.diff:61: trailing whitespace. PyObject* l = PyList_New (0); /tmp/scriptplugin.diff:80: trailing whitespace. if (it->OwnPage == pageNr) /tmp/scriptplugin.diff:104: trailing whitespace. // We do not call getPageItemByName() here, warning: 3 lines add whitespace errors. |
|
I've made a review on Gitlab: https://gitlab.com/scribus/scribus/-/merge_requests/48 The patch seems Ok to me, just a few minor or aesthetic things that can be fixed. |
|
I'm going through this after some time... after having had again the need for modifying the properties of items in a group... i attach a patch that, in my eyes, is slightly better. First, i'm leaving out the changes in getAllObjects() and getPageItems(). i don't think it's a good idea to make them return all items, included the ones that are in a group. 0017052 has added getGroupItems() and it's probably better to go through all the direct items and, if you encounter a group act on its items (if the task demands for it) Also, ReplaceColor() is now completely different. Probably no need to patch it. My version of the patch - adds getPageItemByNameOrNull(), which does not throw a python exception. getPageItemByName() uses and throws an exception on nullptr (or empty name). - removes GetItem() and replace its only usage by getItemByName() (createPathText() throws the same exception anyway, when it gets nullptr; there was already a comment asking for this change) - get ItemExists() is much simplified and by using getPageItemOrNullByName() also checks inside the groups - use getPageItemOrNullByName() instead of ad hoc loops - removed setSelectedItemsByName() because it's unused - move some doxygen doc from .cpp to .h Sadly, git messed up the diff, and the changes in getPageItemByName(), the new getPageItemOrNullByName, and the minimized ItemExists() are mixed together... once the diff applied, the result will be like this: PageItem* getPageItemOrNullByName(const QString& name) { if (name.isEmpty()) { return nullptr; } ScribusDoc* currentDoc = ScCore->primaryMainWindow()->doc; for (int i = 0; i < currentDoc->Items->count(); ++i) { PageItem* pageItem = currentDoc->Items->at(i); if (name == pageItem->itemName()) return pageItem; if (pageItem->isGroup()) { for (const auto& groupItem: pageItem->asGroupFrame()->getAllChildren()) { if (name == groupItem->itemName()) return groupItem; } } } return nullptr; } PageItem* getPageItemByName(const QString& name) { if (name.length() == 0) { PyErr_SetString(PyExc_ValueError, QString("Cannot accept empty name for pageitem").toLocal8Bit().constData()); return nullptr; } PageItem* pageItem = getPageItemOrNullByName(name); if (pageItem == nullptr) { PyErr_SetString(NoValidObjectError, QString("Object not found").toLocal8Bit().constData()); return nullptr; } return pageItem; } bool ItemExists(const QString& name) { return getPageItemOrNullByName(name) != nullptr; } get-page-item-by-name-in-groups.diff (8,460 bytes)
From eb1e069442d1a48b16976a9eb42093a50e8dbb35 Mon Sep 17 00:00:00 2001 From: ale rimoldi <ale@graphicslab.org> Date: Sun, 6 Apr 2025 00:14:57 +0200 Subject: getPageItemByName should also look inside of groups diff --git a/scribus/plugins/scriptplugin/cmdobj.cpp b/scribus/plugins/scriptplugin/cmdobj.cpp index 296b0dbd2..5693c2faa 100644 --- a/scribus/plugins/scriptplugin/cmdobj.cpp +++ b/scribus/plugins/scriptplugin/cmdobj.cpp @@ -470,9 +470,6 @@ PyObject *scribus_createbezierline(PyObject* /* self */, PyObject* args) return PyUnicode_FromString(it->itemName().toUtf8()); } - -/* 03/31/2004 - xception handling - */ PyObject *scribus_createpathtext(PyObject* /* self */, PyObject* args) { double x, y; @@ -481,20 +478,14 @@ PyObject *scribus_createpathtext(PyObject* /* self */, PyObject* args) PyESString polyB; if (!PyArg_ParseTuple(args, "ddeses|es", &x, &y, "utf-8", textB.ptr(), "utf-8", polyB.ptr(), "utf-8", name.ptr())) return nullptr; + if (!checkHaveDocument()) return nullptr; -// if (ItemExists(QString::fromUtf8(name.c_str()))) -// { -// PyErr_SetString(NameExistsError, QObject::tr("An object with the requested name already exists.","python error")); -// return nullptr; -// } - //FIXME: Why use GetItem not GetUniqueItem? Maybe use GetUniqueItem and use the exceptions - // its sets for us? - PageItem *i = GetItem(QString::fromUtf8(textB.c_str())); - PageItem *ii = GetItem(QString::fromUtf8(polyB.c_str())); + + PageItem *i = getPageItemByName(QString::fromUtf8(textB.c_str())); + PageItem *ii = getPageItemByName(QString::fromUtf8(polyB.c_str())); if ((i == nullptr) || (ii == nullptr)) { - PyErr_SetString(NotFoundError, QObject::tr("Object not found.","python error").toLocal8Bit().constData()); return nullptr; } ScCore->primaryMainWindow()->doc->m_Selection->clear(); @@ -502,6 +493,7 @@ PyObject *scribus_createpathtext(PyObject* /* self */, PyObject* args) ScCore->primaryMainWindow()->doc->m_Selection->addItem(ii); ScCore->primaryMainWindow()->view->ToPathText(); ScCore->primaryMainWindow()->doc->moveItem(pageUnitXToDocX(x) - i->xPos(), pageUnitYToDocY(y) - i->yPos(), i); + if (name.length() > 0) { QString objName = QString::fromUtf8(name.c_str()); diff --git a/scribus/plugins/scriptplugin/cmdutil.cpp b/scribus/plugins/scriptplugin/cmdutil.cpp index d3bd6da8f..56f3d89e6 100644 --- a/scribus/plugins/scriptplugin/cmdutil.cpp +++ b/scribus/plugins/scriptplugin/cmdutil.cpp @@ -58,25 +58,6 @@ double docUnitYToPageY(double pageUnitY) return PointToValue(pageUnitY - ScCore->primaryMainWindow()->doc->currentPage()->yOffset()); } -PageItem *GetItem(const QString& name) -{ - ScribusDoc* currentDoc = ScCore->primaryMainWindow()->doc; - if (!name.isEmpty()) - { - for (int i = 0; i < currentDoc->Items->count(); ++i) - { - if (currentDoc->Items->at(i)->itemName() == name) - return currentDoc->Items->at(i); - } - } - else - { - if (currentDoc->m_Selection->count() != 0) - return currentDoc->m_Selection->itemAt(0); - } - return nullptr; -} - void ReplaceColor(const QString& col, const QString& rep) { QMap<QString, QString> colorMap; @@ -110,52 +91,56 @@ PageItem* GetUniqueItem(const QString& name) return getPageItemByName(name); } -PageItem* getPageItemByName(const QString& name) +PageItem* getPageItemOrNullByName(const QString& name) { - if (name.length() == 0) + if (name.isEmpty()) { - PyErr_SetString(PyExc_ValueError, QString("Cannot accept empty name for pageitem").toLocal8Bit().constData()); return nullptr; } ScribusDoc* currentDoc = ScCore->primaryMainWindow()->doc; for (int i = 0; i < currentDoc->Items->count(); ++i) { - if (name == currentDoc->Items->at(i)->itemName()) - return currentDoc->Items->at(i); + PageItem* pageItem = currentDoc->Items->at(i); + if (name == pageItem->itemName()) + return pageItem; + if (pageItem->isGroup()) + { + for (const auto& groupItem: pageItem->asGroupFrame()->getAllChildren()) + { + if (name == groupItem->itemName()) + return groupItem; + } + } } - PyErr_SetString(NoValidObjectError, QString("Object not found").toLocal8Bit().constData()); return nullptr; } - -/*! - * Checks to see if a pageItem named 'name' exists and return true - * if it does exist. Returns false if there is no such object, or - * if the empty string ("") is passed. - */ -bool ItemExists(const QString& name) +PageItem* getPageItemByName(const QString& name) { if (name.length() == 0) - return false; + { + PyErr_SetString(PyExc_ValueError, QString("Cannot accept empty name for pageitem").toLocal8Bit().constData()); + return nullptr; + } - ScribusDoc* currentDoc = ScCore->primaryMainWindow()->doc; - for (int i = 0; i < currentDoc->Items->count(); ++i) + PageItem* pageItem = getPageItemOrNullByName(name); + + if (pageItem == nullptr) { - if (name == currentDoc->Items->at(i)->itemName()) - return true; + PyErr_SetString(NoValidObjectError, QString("Object not found").toLocal8Bit().constData()); + return nullptr; } - return false; + + return pageItem; +} + +bool ItemExists(const QString& name) +{ + return getPageItemOrNullByName(name) != nullptr; } -/*! - * Checks to see if there is a document open. - * If there is an open document, returns true. - * If there is no open document, sets a Python - * exception and returns false. - * 2004-10-27 Craig Ringer - */ bool checkHaveDocument() { if (ScCore->primaryMainWindow()->HaveDoc) @@ -189,31 +174,6 @@ QStringList getSelectedItemsByName() return ScCore->primaryMainWindow()->doc->m_Selection->getSelectedItemsByName(); } -bool setSelectedItemsByName(const QStringList& itemNames) -{ - ScribusDoc* currentDoc = ScCore->primaryMainWindow()->doc; - ScribusView* currentView = ScCore->primaryMainWindow()->view; - - currentView->deselectItems(); - - // For each named item - for (auto it = itemNames.begin() ; it != itemNames.end() ; it++) - { - // Search for the named item - PageItem* item = nullptr; - for (int j = 0; j < currentDoc->Items->count(); j++) - { - if (*it == currentDoc->Items->at(j)->itemName()) - item = currentDoc->Items->at(j); - } - if (!item) - return false; - // And select it - currentView->selectItem(item); - } - return true; -} - TableBorder parseBorder(PyObject* borderLines, bool* ok) { TableBorder border; diff --git a/scribus/plugins/scriptplugin/cmdutil.h b/scribus/plugins/scriptplugin/cmdutil.h index fc2f3f2cd..320631817 100644 --- a/scribus/plugins/scriptplugin/cmdutil.h +++ b/scribus/plugins/scriptplugin/cmdutil.h @@ -27,7 +27,6 @@ double pageUnitYToDocY(double pageUnitY); /// \brief Doc units -> page-relative units double docUnitYToPageY(double pageUnitY); -PageItem *GetItem(const QString& name); void ReplaceColor(const QString& col, const QString& rep); /*! @@ -43,6 +42,11 @@ void ReplaceColor(const QString& col, const QString& rep); */ PageItem* GetUniqueItem(const QString& name); +/*! + * @brief Returns named PageItem, or NULL if not found (no exception). + */ +PageItem* getPageItemOrNullByName(const QString& name); + /*! * @brief Returns named PageItem, or exception and NULL if not found. * @@ -51,7 +55,13 @@ PageItem* GetUniqueItem(const QString& name); */ PageItem* getPageItemByName(const QString& name); -// 2004-10-27 Craig Ringer see cmdutil.cpp for description +/*! + * Checks to see if there is a document open. + * If there is an open document, returns true. + * If there is no open document, sets a Python + * exception and returns false. + * 2004-10-27 Craig Ringer + */ bool checkHaveDocument(); /*! @@ -66,7 +76,11 @@ bool checkHaveDocument(); */ bool checkValidPageNumber(int page); -// 2004-11-12 Craig Ringer see cmdutil.cpp for description +/*! + * Checks to see if a pageItem named 'name' exists and return true + * if it does exist. Returns false if there is no such object, or + * if the empty string ("") is passed. + */ bool ItemExists(const QString& name); /*! @@ -74,14 +88,6 @@ bool ItemExists(const QString& name); */ QStringList getSelectedItemsByName(); -/*! - * @brief Replaces the current selection by selecting all the items named in the passed QStringList - * - * Returns false if one or more items can't be selected, true if all were selected. - * Selection state is undefined on failure. - */ -bool setSelectedItemsByName(const QStringList& itemNames); - /*! * @brief Helper method to parse a border from a list of tuples. */ |
Date Modified | Username | Field | Change |
---|---|---|---|
2025-01-31 16:05 | tropion | New Issue | |
2025-01-31 16:05 | tropion | File Added: scriptplugin.diff | |
2025-02-22 10:39 | ale | Project | Infrastructure => Scribus |
2025-02-22 11:19 | ale | Note Added: 0052099 | |
2025-02-24 09:35 | tropion | Note Added: 0052110 | |
2025-02-24 19:38 | ale | Note Added: 0052112 | |
2025-02-24 21:28 | ale | Note Added: 0052114 | |
2025-04-05 12:01 | ale | Relationship added | related to 0017490 |
2025-04-05 22:16 | ale | Note Added: 0052408 | |
2025-04-05 22:16 | ale | File Added: get-page-item-by-name-in-groups.diff | |
2025-04-05 22:22 | ale | Relationship added | related to 0009054 |