View Issue Details

IDProjectCategoryView StatusLast Update
0016277ScribusScripterpublic2020-11-07 14:52
Reporterale Assigned Toale  
PrioritynormalSeverityminorReproducibilityhave not tried
Status closedResolutionfixed 
Product Version 
Target VersionFixed in Version1.5.6.svn 
Summary0016277: Balance quotes in cmdgetsetprop.h
Description`blah' is not a good idea...
TagsNo tags attached.
PatchYes

Activities

ale

2020-10-23 12:45

manager  

getsetprop-balance-quotes.diff (4,850 bytes)
diff --git a/scribus/plugins/scriptplugin/cmdgetsetprop.h b/scribus/plugins/scriptplugin/cmdgetsetprop.h
index 7a2e50853..aeb184c64 100644
--- a/scribus/plugins/scriptplugin/cmdgetsetprop.h
+++ b/scribus/plugins/scriptplugin/cmdgetsetprop.h
@@ -57,8 +57,8 @@ PyObject* convert_QObjectList_to_PyListObject(QObjectList* origlist);
 /**
  * @brief return the name of the C type of the specified property
  *
- * Look up `propname' on `obj', searching through parent classes if
- * `includsuper' is true. Get and return the name of the C type
+ * Look up `propname` on `obj`, searching through parent classes if
+ * `includsuper` is true. Get and return the name of the C type
  * of the property.
  *
  * If the property cannot be found, a null string is returned. NO
@@ -76,10 +76,10 @@ const char* getpropertytype(QObject* obj, const char* propname, bool includesupe
 PyDoc_STRVAR(scribus_propertyctype__doc__,
 QT_TR_NOOP("getPropertyCType(object, property, includesuper=True)\n\
 \n\
-Returns the name of the C type of `property' of `object'. See getProperty()\n\
+Returns the name of the C type of `property` of `object`. See getProperty()\n\
 for details of arguments.\n\
 \n\
-If `includesuper' is true, search inherited properties too.\n\
+If `includesuper` is true, search inherited properties too.\n\
 "));
 PyObject* scribus_propertyctype(PyObject* /*self*/, PyObject* args, PyObject* kw);
 
@@ -95,8 +95,8 @@ PyObject* scribus_propertyctype(PyObject* /*self*/, PyObject* args, PyObject* kw
 PyDoc_STRVAR(scribus_getpropertynames__doc__,
 QT_TR_NOOP("getPropertyNames(object, includesuper=True)\n\
 \n\
-Return a list of property names supported by `object'.\n\
-If `includesuper' is true, return properties supported\n\
+Return a list of property names supported by `object`.\n\
+If `includesuper` is true, return properties supported\n\
 by parent classes as well.\n\
 "));
 PyObject* scribus_getpropertynames(PyObject* self, PyObject* args, PyObject* kw);
@@ -122,14 +122,14 @@ PyObject* scribus_getpropertynames(PyObject* self, PyObject* args, PyObject* kw)
 PyDoc_STRVAR(scribus_getproperty__doc__,
 QT_TR_NOOP("getProperty(object, property)\n\
 \n\
-Return the value of the property `property' of the passed `object'.\n\
+Return the value of the property `property` of the passed `object`.\n\
 \n\
-The `object' argument may be a string, in which case the named PageItem\n\
+The `object` argument may be a string, in which case the named PageItem\n\
 is searched for. It may also be a PyCObject, which may point to any\n\
 C++ QObject instance.\n\
 \n\
-The `property' argument must be a string, and is the name of the property\n\
-to look up on `object'.\n\
+The `property` argument must be a string, and is the name of the property\n\
+to look up on `object`.\n\
 \n\
 The return value varies depending on the type of the property.\n\
 "));
@@ -157,8 +157,8 @@ PyObject* scribus_getproperty(PyObject* /*self*/, PyObject* args, PyObject* kw);
 PyDoc_STRVAR(scribus_setproperty__doc__,
 QT_TR_NOOP("setProperty(object, property, value)\n\
 \n\
-Set `property' of `object' to `value'. If `value' cannot be converted to a type\n\
-compatible with the type of `property', an exception is raised. An exception may\n\
+Set `property` of `object` to `value`. If `value` cannot be converted to a type\n\
+compatible with the type of `property`, an exception is raised. An exception may\n\
 also be raised if the underlying setter fails.\n\
 \n\
 See getProperty() for more information.\n\
@@ -177,8 +177,8 @@ PyObject* scribus_setproperty(PyObject* /*self*/, PyObject* args, PyObject* kw);
 PyDoc_STRVAR(scribus_getchildren__doc__,
 QT_TR_NOOP("getChildren(object, ofclass=None, ofname=None, regexpmatch=False, recursive=True)\n\
 \n\
-Return a list of children of `object', possibly restricted to children\n\
-of class named `ofclass' or children named `ofname'. If `recursive' is true,\n\
+Return a list of children of `object`, possibly restricted to children\n\
+of class named `ofclass` or children named `ofname`. If `recursive` is true,\n\
 search recursively through children, grandchildren, etc.\n\
 \n\
 See QObject::children() in the Qt docs for more information.\n\
@@ -197,8 +197,8 @@ PyObject* scribus_getchildren(PyObject* self, PyObject* args, PyObject* kw);
 PyDoc_STRVAR(scribus_getchild__doc__,
 QT_TR_NOOP("getChild(object, childname, ofclass=None, recursive=True)\n\
 \n\
-Return the first child of `object' named `childname', possibly restricting\n\
-the search to children of type name `ofclass'. If `recursive' is true,\n\
+Return the first child of `object` named `childname`, possibly restricting\n\
+the search to children of type name `ofclass`. If `recursive` is true,\n\
 search recursively through children, grandchildren, etc.\n\
 "));
 PyObject* scribus_getchild(PyObject* self, PyObject* args, PyObject* kw);

jghali

2020-10-23 14:57

administrator   ~0048184

No sure about the use of ` as quote character in English... I would have used either the single quote (') or double quote (") characters. It seems there is also inconsistencies across files related to the use of quotes. Currently we have a bit of everything.

jghali

2020-10-23 14:58

administrator   ~0048185

It seems wikipedia has a nice page about quotation marks across languages:
https://en.wikipedia.org/wiki/Quotation_mark

jghali

2020-10-23 15:10

administrator   ~0048186

There is also another page about quotation marks in English:
https://en.wikipedia.org/wiki/Quotation_marks_in_English

ale

2020-10-23 16:10

manager   ~0048188

Last edited: 2020-10-23 16:10

View 2 revisions

since those are commands, using ` will produce a kbd markup when the comment is interpreted as markdown...

for this reason, i would use double quotes for normal quotes and ` around commands.

i don't think that the docstring should be in markdown, but where it does not hurt, i would go for what gives the best result.

jghali

2020-10-23 16:31

administrator   ~0048189

>> since those are commands

These are not commands, just function arguments, so I'm not sure a <kbd> markup is suited in this case.

cbradney

2020-10-23 16:56

administrator   ~0048190

This is code, they should all be ' and not `

ale

2020-10-24 08:24

manager   ~0048191

ok, jean, they are function arguments, which are normally layout as <kbd> in html...
i don't think that this changes much my statement...

if you want to allow a nicer output in the generated API documentation, you're welcome to accept the patch as is.

if you prefer to only use ' or ", just replace the `s in the patch.

just don't leave as is because the unmatched `s screw up the html output and does not really make any sense.
(of course it does not urge... but it would be nice to see this fixed...)







as my last words, a longer comment with some background information:

as far as i know, most docstring formats allow or define some formatting

- doxygen (the one informally used by scribus) lists markdown as the first choice but also others support others (https://www.doxygen.nl/manual/docblocks.html#docstructure)
- javadoc uses some sort of html (https://www.oracle.com/technical-resources/articles/java/javadoc-tool.html#format)
- python docstring seem to use ReSt (which is rather verbose and intrusive)

personally, i don't see much interest in having a formatting in the docstring produced by python (nobody really looks at them), but if we can have a nicer html produced from them, i'm all for it.
of course, if this does not request more effort or produces any other issue in the code.
to me, markdown seems to be the least intrusive formatting: it does not hurt when reading/writing the docstring in c++ and does not distract in the python docstring output.

you can see here what happens if you leave it as is, and guess how nicely formatted "scaleToFrame" and "proportional" could be:

http://impagina.org/scribus-scripter-api/image-frame/#setscaleimagetoframe

at the end, i'd like to produce a nice html and qt help for the scripter and achieve this with the least effort for everybody.
i'm still convinced that, for scribus, the best way is to use the docstrings to generate the API documentation.
and i'd be delighted to use some non obtrusive markdown to improve the formatting (correctly format the lists, code tags, clickable links (sometimes they will need <> around them)... not much more).

so i'm happy if you accept the ` quotes for references to the code (commands, arguments, ...) and i don't really care that much if you don't.
just don't leave it as is.

jghali

2020-10-24 21:26

administrator   ~0048195

Last edited: 2020-10-24 21:36

View 2 revisions

Given Craig preference's, I used the single quote character. I also committed a similar fix to cmdmani.h. Given the inconsistencies, we should later decide if we convert all docstrings to single quotes or if we convert them to double quotes.

Issue History

Date Modified Username Field Change
2020-10-23 12:45 ale New Issue
2020-10-23 12:45 ale File Added: getsetprop-balance-quotes.diff
2020-10-23 12:45 ale Summary Balance quotes in cmdgetsetprop.h => [PATCH] Balance quotes in cmdgetsetprop.h
2020-10-23 12:45 ale Patch No => Yes
2020-10-23 14:57 jghali Note Added: 0048184
2020-10-23 14:58 jghali Note Added: 0048185
2020-10-23 15:10 jghali Note Added: 0048186
2020-10-23 16:10 ale Note Added: 0048188
2020-10-23 16:10 ale Note Edited: 0048188 View Revisions
2020-10-23 16:31 jghali Note Added: 0048189
2020-10-23 16:56 cbradney Note Added: 0048190
2020-10-24 08:24 ale Note Added: 0048191
2020-10-24 21:23 jghali Summary [PATCH] Balance quotes in cmdgetsetprop.h => Balance quotes in cmdgetsetprop.h
2020-10-24 21:26 jghali Assigned To => ale
2020-10-24 21:26 jghali Status new => resolved
2020-10-24 21:26 jghali Resolution open => fixed
2020-10-24 21:26 jghali Fixed in Version => 1.5.6.svn
2020-10-24 21:26 jghali Note Added: 0048195
2020-10-24 21:36 jghali Note Edited: 0048195 View Revisions
2020-11-07 14:52 cbradney Status resolved => closed