From a66ea2cc75c600692711f2708eb32b9d39ceb593 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Wed, 28 Aug 2019 19:43:18 +0200 Subject: [PATCH] documentation/python: dying on each error isn't nice. --- documentation/python.py | 77 ++++++++++++------- .../test_python/content/content.html | 24 ++++++ .../test_python/content/content/__init__.py | 5 ++ documentation/test_python/content/docs.rst | 8 ++ .../content_parse_docstrings.html | 4 + .../content_parse_docstrings.py | 11 +++ 6 files changed, 103 insertions(+), 26 deletions(-) diff --git a/documentation/python.py b/documentation/python.py index 1044c6de..0322a05d 100755 --- a/documentation/python.py +++ b/documentation/python.py @@ -791,6 +791,9 @@ def format_value(state: State, referrer_path: List[str], value: str) -> Optional else: return None +def prettify_multiline_error(error: str) -> str: + return ' | {}\n'.format(error.replace('\n', '\n | ')) + def extract_docs(state: State, external_docs, type: EntryType, path: List[str], doc: str, *, signature=None, summary_only=False) -> Tuple[str, str]: path_str = '.'.join(path) # If function signature is supplied, try that first @@ -829,24 +832,29 @@ def extract_docs(state: State, external_docs, type: EntryType, path: List[str], # this one after another; stopping once there's nothing left. If # nothing left, the populated entries should be non-None. for hook in state.hooks_docstring: - doc = hook( - type=type, - path=path, - signature=signature, - doc=doc) - - # The hook could have replaced the entry with a new dict - # instance, fetch it again to avoid looking at stale data below - external_doc_entry = external_docs[path_signature_str] - - if not doc: - # Assuming the doc were non-empty on input, if those are - # empty on output, the hook should be filling both summary - # and content to non-None values (so, in the worst case, - # an empty string) - assert external_doc_entry['summary'] is not None - assert external_doc_entry['content'] is not None - break + try: + doc = hook( + type=type, + path=path, + signature=signature, + doc=doc) + + # The hook could have replaced the entry with a new dict + # instance, fetch it again to avoid looking at stale data below + external_doc_entry = external_docs[path_signature_str] + + if not doc: + # Assuming the doc were non-empty on input, if those are + # empty on output, the hook should be filling both summary + # and content to non-None values (so, in the worst case, + # an empty string) + assert external_doc_entry['summary'] is not None + assert external_doc_entry['content'] is not None + break + + except docutils.utils.SystemMessage: + logging.error("Failed to process a docstring for %s, ignoring:\n%s", path_signature_str, prettify_multiline_error(doc)) + doc = '' # If there's still something left after the hooks (or there are no # hooks), process it as a plain unformatted text. @@ -872,10 +880,10 @@ def extract_docs(state: State, external_docs, type: EntryType, path: List[str], docutils.utils.extract_options = prev_extract_options docutils.utils.assemble_option_dict = prev_assemble_option_dict - # We ain't got nothing. If there isn't anything supplied externally, - # set summary / content to an empty string so this branch isn't entered - # again. - else: + # We ain't got nothing (or the above parse failed). If there isn't + # anything supplied externally, set summary / content to an empty + # string so this branch isn't entered again. + if not doc: if external_doc_entry.get('summary') is None: external_doc_entry['summary'] = '' if external_doc_entry.get('content') is None: @@ -883,10 +891,19 @@ def extract_docs(state: State, external_docs, type: EntryType, path: List[str], # Render. This can't be done just once and then cached because e.g. math # rendering needs to ensure each SVG formula has unique IDs on each page. - summary = render_inline_rst(state, external_doc_entry['summary']) + try: + summary = render_inline_rst(state, external_doc_entry['summary']) + except docutils.utils.SystemMessage: + logging.error("Failed to process summary for %s, ignoring:\n%s", path_signature_str, prettify_multiline_error(external_doc_entry['summary'])) + summary = '' + if summary_only: return summary - content = render_rst(state, external_doc_entry['content']) + try: + content = render_rst(state, external_doc_entry['content']) + except docutils.utils.SystemMessage: + logging.error("Failed to process content for %s, ignoring:\n%s", path_signature_str, prettify_multiline_error(external_doc_entry['content'])) + content = '' # Mark the docs as used (so it can warn about unused docs at the end) external_doc_entry['used'] = True @@ -1326,7 +1343,11 @@ def extract_function_doc(state: State, parent, entry: Empty) -> List[Any]: if param.name != 'self': logging.warning("%s%s parameter %s is not documented", path_str, signature, param.name) continue - param.content = render_inline_rst(state, param_docs[param.name]) + try: + param.content = render_inline_rst(state, param_docs[param.name]) + except docutils.utils.SystemMessage: + logging.error("Failed to process doc for %s param %s, ignoring:\n%s", path_str, param.name, prettify_multiline_error(param_docs[param.name])) + param.content = '' used_params.add(param.name) out.has_param_details = True out.has_details = True @@ -1336,7 +1357,11 @@ def extract_function_doc(state: State, parent, entry: Empty) -> List[Any]: logging.warning("%s%s documents parameter %s, which isn't in the signature", path_str, signature, name) if function_docs.get('return'): - out.return_value = render_inline_rst(state, function_docs['return']) + try: + out.return_value = render_inline_rst(state, function_docs['return']) + except docutils.utils.SystemMessage: + logging.error("Failed to process return doc for %s, ignoring:\n%s", path_str, prettify_multiline_error(function_docs['return'])) + out.return_value = '' out.has_details = True if not state.config['SEARCH_DISABLED']: diff --git a/documentation/test_python/content/content.html b/documentation/test_python/content/content.html index 3fb124db..35622ede 100644 --- a/documentation/test_python/content/content.html +++ b/documentation/test_python/content/content.html @@ -103,6 +103,10 @@ doesn't add any detailed block. def param_docs_wrong(a, b)
Should give warnings
+
+ def this_function_has_bad_docs(a, b) +
+
@@ -247,6 +251,26 @@ or parsed in any way.

The b is not documented, while c isn't in the signature.

+
+

+ def content.this_function_has_bad_docs(a, b) +

+ + + + + + + + + + + + + + +
Parameters
a
b
+

Data documentation

diff --git a/documentation/test_python/content/content/__init__.py b/documentation/test_python/content/content/__init__.py index 9850d0b7..46691793 100644 --- a/documentation/test_python/content/content/__init__.py +++ b/documentation/test_python/content/content/__init__.py @@ -80,6 +80,11 @@ def full_docstring(a, b) -> str: Like this. """ +# This should check we handle reST parsing errors in external docs gracefully. +# Will probably look extra weird in the output tho, but that's okay -- it's an +# error after all. +def this_function_has_bad_docs(a, b): pass + CONSTANT: float = 3.14 DATA_WITH_DETAILS: str = 'heyoo' diff --git a/documentation/test_python/content/docs.rst b/documentation/test_python/content/docs.rst index 76d3d884..902e5f70 100644 --- a/documentation/test_python/content/docs.rst +++ b/documentation/test_python/content/docs.rst @@ -122,6 +122,14 @@ Why it has to be yelling?! +.. This should check we handle reST parsing errors gracefully. +.. py:function:: content.this_function_has_bad_docs + :summary: :nonexistentrole:`summary is all bad` + :param a: :nonexistentrole:`param docs also blow` + :return: :nonexistentrole:`return is terrible` + + :nonexistentrole:`this too` + .. py:module:: thismoduledoesnotexist :summary: This docs get unused and produce a warning diff --git a/documentation/test_python/content_parse_docstrings/content_parse_docstrings.html b/documentation/test_python/content_parse_docstrings/content_parse_docstrings.html index edfdf553..e3d6acb8 100644 --- a/documentation/test_python/content_parse_docstrings/content_parse_docstrings.html +++ b/documentation/test_python/content_parse_docstrings/content_parse_docstrings.html @@ -70,6 +70,10 @@ With a multi-line summary. def summary_only()
This is just a summary.
+
+ def this_function_has_bad_docs(a, b) -> str +
+
diff --git a/documentation/test_python/content_parse_docstrings/content_parse_docstrings.py b/documentation/test_python/content_parse_docstrings/content_parse_docstrings.py index 24600dc0..021628db 100644 --- a/documentation/test_python/content_parse_docstrings/content_parse_docstrings.py +++ b/documentation/test_python/content_parse_docstrings/content_parse_docstrings.py @@ -40,3 +40,14 @@ def empty_docstring(): pass def summary_only(): """This is just a summary.""" + +# This should check we handle reST parsing errors gracefully. Will probably +# look extra weird in the output tho, but that's okay -- it's an error after +# all. +def this_function_has_bad_docs(a, b) -> str: + """This function has bad docs. It's freaking terrible. + Yes. + Really. + + :broken: yes + """ -- 2.30.2