View Issue Details

IDProjectCategoryView StatusLast Update
0016301ScribusScripterpublic2020-11-07 14:52
Reporterale Assigned Tojghali  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version1.5.6.svn 
Fixed in Version1.5.6.svn 
Summary0016301: Fix scripter getAllObjects() documentation
DescriptiongetAllObjects accepts two optional arguments:

- type: what is this?
- pageNr: that would be useful

now:

- the scripter documentation in the manual does not mention any argument
- the docstring mentions one single argument: page

it took me some time to understand that the type was the itemtype (get all text frames on a page...).
while it is for sure useful and common to ask for a specific item type, i don't think that anybody has ever done it this way (in all public available scripts i've ever seen everybody is just querying all items from the current page and then filter the ones that match the type or do different actions depending on the type) and i'm not sure that having the page number as the second optional argument is a good interface, when both arguments are optional.

my suggestion:

- deprecate getAllObjects
- create a new function getAllItems(page:string = None)
- maybe add getAllItemsByType(type, page:string = None)
- make getAllObjects an alias of getAllItems since this is the only documented usage (even if it was not possible to use it)
TagsNo tags attached.
PatchNo

Activities

jghali

2020-11-02 10:37

administrator   ~0048288

Well currently getAllObjects () works at least as documented in the html doc : a getAllObjects() call will indeed get objects on current page ;)

More seriously, "type" corresponds to the item type. This argument is already there in 1.4.x, but apparently not documented. It has been kept for backward compatibility. With current getAllObjects() implementation, you can already get objects on a specific page using the "page" keyword : getAllObjects(page=1). As already specified by the doc string, the page index is 0-based.

ale

2020-11-02 11:19

manager   ~0048289

yes, one can say that getAllObjects() does work for the documented features.

but the way i read the docstring is that i am allowed to do getAllObjects(1) and get the objects of page 1.
and i guess that everybody that has not studied the c++ code will read it that way.
because the usage of named argument is not common in the scribus scripting.
(i don't think i've ever seen anybody using named arguments in a scribus script and i've discovered myself that it was a thing a couple of hours ago...)

i really have to think that nobody is using getAllObjects() with an argument.
so i think that getAllObjects() should be simplified to match its usage.

but, of course, we can also go the other way and add a getAllItems() with three named arguments: page, itemtype, layer .
i've already implemented a getAllLayerItems(), which imo better fits the current scribus API
But if in the future we want to go for named arguments, i think it's better to get my code merged into getAllObjects(), plan a the renaming to getAllItems() and fix the docstrings.

ale

2020-11-02 13:24

manager   ~0048291

as discussed, if done correctly, i'm fine with keeping the named arguments... and adding the layer in here...

jghali

2020-11-03 10:24

administrator   ~0048308

Last edited: 2020-11-03 10:27

I've fixed getAllObjects() documentation and added support for retrieving items on a specific layer as part of r24159. So now you can do such calls:
* objectList = getAllObjects() -> get all objects on current page
* objectList = getAllObjects(layer = "New Layer") -> get all objects located on current page and layer "New Layer"
* objectList = getAllObjects(page = 1) -> get all objects on second page (index is 0 - based)
* objectList = getAllObjects(page = 1, layer = "New Layer") -> get all objects located on second page and layer "New Layer"

Issue History

Date Modified Username Field Change
2020-11-02 08:26 ale New Issue
2020-11-02 10:37 jghali Note Added: 0048288
2020-11-02 11:19 ale Note Added: 0048289
2020-11-02 13:24 ale Note Added: 0048291
2020-11-03 10:16 jghali Summary fix the arguments for getAllObjects and rename it to getAllItems => Fix getAllObjects documentation
2020-11-03 10:24 jghali Assigned To => jghali
2020-11-03 10:24 jghali Status new => resolved
2020-11-03 10:24 jghali Resolution open => fixed
2020-11-03 10:24 jghali Fixed in Version => 1.5.6.svn
2020-11-03 10:24 jghali Note Added: 0048308
2020-11-03 10:25 jghali Summary Fix getAllObjects documentation => Fix scripter getAllObjects() documentation
2020-11-03 10:27 jghali Note Edited: 0048308
2020-11-07 14:52 cbradney Status resolved => closed