From 16548cced02882e82e02012b28b38219e350abf2 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 7 Aug 2014 00:25:06 +0200 Subject: [PATCH] Simplify and make more efficient building Python docstrings. Make the rules for combining explicitly specified docstring, autodoc one and the one obtained by translating Doxygen comments implicit in the structure of the code itself instead of writing complicated conditions checking them. This results in small changes to the whitespace in the generated Python code when using autodoc, but this makes it PEP 8-compliant, so it is the right thing to do anyhow. Also cache the docstring built from translated Doxygen comments. The existing code seemed to intend to do it, but didn't, really. This helps with performance generally speaking (-10% for a relatively big library using a lot of Doxygen comments) and also makes debugging Doxygen translation code less painful as it's executed only once instead of twice for each comment. Finally, avoid putting "r", used for Python raw strings, into docstrings in C code, it is really not needed there. --- Examples/test-suite/python/autodoc_runme.py | 12 +- .../python/doxygen_basic_notranslate_runme.py | 4 +- .../python/doxygen_basic_translate_runme.py | 4 +- Source/Modules/python.cxx | 137 +++++++++++------- 4 files changed, 88 insertions(+), 69 deletions(-) diff --git a/Examples/test-suite/python/autodoc_runme.py b/Examples/test-suite/python/autodoc_runme.py index 03a01960b..5312802f6 100644 --- a/Examples/test-suite/python/autodoc_runme.py +++ b/Examples/test-suite/python/autodoc_runme.py @@ -124,19 +124,17 @@ if sys.version_info[0:2] > (2, 4): # Python 2.4 does not seem to work commentVerifier.check(A.variable_a.__doc__, "A_variable_a_get(self) -> int") commentVerifier.check(A.variable_b.__doc__, "A_variable_b_get(A self) -> int") - commentVerifier.check(A.variable_c.__doc__, "\n" + commentVerifier.check(A.variable_c.__doc__, "A_variable_c_get(self) -> int\n" "\n" "Parameters:\n" " self: A *\n" - "\n" ) - commentVerifier.check(A.variable_d.__doc__, "\n" + commentVerifier.check(A.variable_d.__doc__, "A_variable_d_get(A self) -> int\n" "\n" "Parameters:\n" " self: A *\n" - "\n" ) commentVerifier.check(B.__doc__, "Proxy of C++ B class") @@ -166,9 +164,9 @@ commentVerifier.check(F.__init__.__doc__, "\n" commentVerifier.check(B.funk.__doc__, "funk(B self, int c, int d) -> int") commentVerifier.check(funk.__doc__, "funk(A e, short arg2, int c, int d) -> int") commentVerifier.check(funkdefaults.__doc__, "\n" -" funkdefaults(A e, short arg2, int c, int d, double f=2) -> int\n" -" funkdefaults(A e, short arg2, int c, int d) -> int\n" -" " +" funkdefaults(A e, short arg2, int c, int d, double f=2) -> int\n" +" funkdefaults(A e, short arg2, int c, int d) -> int\n" +" " ) commentVerifier.check(func_input.__doc__, "func_input(int * INPUT) -> int") diff --git a/Examples/test-suite/python/doxygen_basic_notranslate_runme.py b/Examples/test-suite/python/doxygen_basic_notranslate_runme.py index bb2fae0f5..27dcb4a7e 100644 --- a/Examples/test-suite/python/doxygen_basic_notranslate_runme.py +++ b/Examples/test-suite/python/doxygen_basic_notranslate_runme.py @@ -61,9 +61,7 @@ commentVerifier.check(doxygen_basic_notranslate.function4.__doc__, """ ) commentVerifier.check(doxygen_basic_notranslate.function5.__doc__, - r""" - This is a post comment. - """ + r""" This is a post comment. """ ) commentVerifier.check(doxygen_basic_notranslate.function6.__doc__, r""" diff --git a/Examples/test-suite/python/doxygen_basic_translate_runme.py b/Examples/test-suite/python/doxygen_basic_translate_runme.py index 1e017fb74..b5ba162e2 100644 --- a/Examples/test-suite/python/doxygen_basic_translate_runme.py +++ b/Examples/test-suite/python/doxygen_basic_translate_runme.py @@ -59,9 +59,7 @@ commentVerifier.check(doxygen_basic_translate.function4.__doc__, """ ) commentVerifier.check(doxygen_basic_translate.function5.__doc__, - """ - This is a post comment. - """ + """ This is a post comment.""" ) commentVerifier.check(doxygen_basic_translate.function6.__doc__, """ diff --git a/Source/Modules/python.cxx b/Source/Modules/python.cxx index b76b83ebb..39703bc7e 100644 --- a/Source/Modules/python.cxx +++ b/Source/Modules/python.cxx @@ -1476,38 +1476,76 @@ public: } /* ------------------------------------------------------------ - * docstring() - * Get the docstring text, stripping off {} if neccessary, - * and enclose in triple double quotes. If autodoc is also - * set then it will build a combined docstring. + * build_combined_docstring() + * Build the full docstring which may be a combination of the + * explicit docstring and autodoc string or, if none of them + * is specified, obtained by translating Doxygen comment to + * Python. + * + * Return new string to be deleted by caller (never NIL but + * may be empty if there is no docstring). * ------------------------------------------------------------ */ - - String *docstring(Node *n, autodoc_t ad_type, const String *indent, bool use_triple = true) { - String *str = Getattr(n, "feature:docstring"); - String *doxygen_comment = 0; - bool have_ds = (str && Len(str) > 0); - bool have_auto = (Getattr(n, "feature:autodoc") && !GetFlag(n, "feature:noautodoc")); - bool have_doxygen = doxygen && doxygenTranslator->hasDocumentation(n); - const char *triple_double = use_triple ? "\"\"\"" : ""; - String *autodoc = NULL; - String *doc = NULL; - - if (have_ds) { - char *t = Char(str); + String *build_combined_docstring(Node *n, autodoc_t ad_type) { + String *docstr = Getattr(n, "feature:docstring"); + if (docstr && Len(docstr)) { + docstr = Copy(docstr); + char *t = Char(docstr); if (*t == '{') { - Delitem(str, 0); - Delitem(str, DOH_END); + Delitem(docstr, 0); + Delitem(docstr, DOH_END); } } - if (have_auto) { - autodoc = make_autodoc(n, ad_type); - have_auto = (autodoc && Len(autodoc) > 0); + if (Getattr(n, "feature:autodoc") && !GetFlag(n, "feature:noautodoc")) { + String* autodoc = make_autodoc(n, ad_type); + if (autodoc && Len(autodoc) > 0) { + if (docstr && Len(docstr)) { + Append(autodoc, "\n"); + Append(autodoc, docstr); + } + + String* tmp = autodoc; + autodoc = docstr; + docstr = tmp; + } + + Delete(autodoc); } - if (have_doxygen) { - doxygen_comment = doxygenTranslator->getDocumentation(n); - have_doxygen = (doxygen_comment && Len(doxygen_comment) > 0); + + if (!docstr || !Len(docstr)) { + if (doxygen) { + docstr = Getattr(n, "python:docstring"); + if (!docstr && doxygenTranslator->hasDocumentation(n)) { + docstr = doxygenTranslator->getDocumentation(n); + + // Avoid rebuilding it again the next time: notice that we can't do + // this for the combined doc string as autodoc part of it depends on + // the sym:name of the node and it is changed while handling it, so + // the cached results become incorrect. But Doxygen docstring only + // depends on the comment which is not going to change, so we can + // safely cache it. + Setattr(n, "python:docstring", Copy(docstr)); + } + } } + + if (!docstr) + docstr = NewString(""); + + return docstr; + } + + /* ------------------------------------------------------------ + * docstring() + * Get the docstring text enclosed in triple double quotes. + * ------------------------------------------------------------ */ + + String *docstring(Node *n, autodoc_t ad_type, const String *indent) { + String *docstr = build_combined_docstring(n, ad_type); + if (!Len(docstr)) + return docstr; + + // If there is more than one line then make docstrings like this: // // """ @@ -1517,45 +1555,32 @@ public: // // otherwise, put it all on a single line // - // All comments translated from doxygen are given as raw strings (prefix "r"), + // Notice that all comments are created as raw strings (prefix "r"), // because '\' is used often in comments, but may break Python module from // loading. For example, in doxy comment one may write path in quotes: // // This is path to file "C:\x\file.txt" // - // Python will no load the module with such comment because of illegal + // Python will not load the module with such comment because of illegal // escape '\x'. '\' may additionally appear in verbatim or htmlonly sections // of doxygen doc, Latex expressions, ... - if (have_auto && have_ds) { // Both autodoc and docstring are present - doc = NewString(""); - Printv(doc, "r", triple_double, "\n", pythoncode(autodoc, indent), "\n", pythoncode(str, indent), indent, triple_double, NIL); - } else if (!have_auto && have_ds) { // only docstring - if (Strchr(str, '\n') == 0) { - doc = NewStringf("%s%s%s", triple_double, str, triple_double); - } else { - doc = NewString(""); - Printv(doc, "r", triple_double, "\n", pythoncode(str, indent), indent, triple_double, NIL); - } - } else if (have_auto && !have_ds) { // only autodoc - if (Strchr(autodoc, '\n') == 0) { - doc = NewStringf("%s%s%s", triple_double, autodoc, triple_double); - } else { - doc = NewString(""); - Printv(doc, triple_double, "\n", pythoncode(autodoc, indent), indent, triple_double, NIL); - } - } else if (have_doxygen) { // the lowest priority - doc = NewString(""); - Printv(doc, "r", triple_double, "\n", pythoncode(doxygen_comment, indent), indent, triple_double, NIL); - } - else - doc = NewString(""); + String *doc = NewString(""); + Append(doc, "r\"\"\""); + + if (Strchr(docstr, '\n') == 0) { + Append(doc, docstr); + } else { + Append(doc, "\n"); + Append(doc, pythoncode(docstr, indent)); + Append(doc, indent); + } + + Append(doc, "\"\"\""); + + Delete(docstr); - // Save the generated strings in the parse tree in case they are used later - // by post processing tools - Setattr(n, "python:docstring", doc); - Setattr(n, "python:autodoc", autodoc); return doc; - } + } /* ------------------------------------------------------------ * cdocstring() @@ -1565,7 +1590,7 @@ public: String *cdocstring(Node *n, autodoc_t ad_type) { - String *ds = docstring(n, ad_type, "", false); + String *ds = build_combined_docstring(n, ad_type); Replaceall(ds, "\\", "\\\\"); Replaceall(ds, "\"", "\\\""); Replaceall(ds, "\n", "\\n\"\n\t\t\"");