From eb6b43ba06698d37e2d8090335e0082f0a6dfdb5 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Tue, 20 Mar 2018 16:29:07 +0000 Subject: [PATCH] sphinxext: Reporting correct lines for errors Previously, we had an issue whereby errors found in the generated source would result in the following, rather unhelpful error messages: :1: WARNING: Unexpected indentation. Turns out that the 'ViewList.append' function takes a third argument, 'offset', to indicate where in the source an error is occurring. Start using this to improve the quality of the error messages we get from poorly formatted options. Given that we don't actually modify the source file, it's also necessary to write the output from the directive to an intermediate temp file. This also fixes the order of imports, given that we're adding new imports as we go. Change-Id: I6f6796629705926dbed5015f20d47187b77c9c50 Related-Bug: #1755783 --- oslo_config/sphinxext.py | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/oslo_config/sphinxext.py b/oslo_config/sphinxext.py index 715eaa4f..4b1423ff 100644 --- a/oslo_config/sphinxext.py +++ b/oslo_config/sphinxext.py @@ -10,6 +10,10 @@ # License for the specific language governing permissions and limitations # under the License. +from __future__ import print_function + +import tempfile + from docutils import nodes from docutils.parsers import rst from docutils.parsers.rst import directives @@ -302,9 +306,29 @@ class ShowOptionsDirective(rst.Directive): ] result = ViewList() - source_name = '<' + __name__ + '>' - for line in _format_option_help(namespaces, split_namespaces): - result.append(line, source_name) + source_name = self.state.document.current_source + + with tempfile.NamedTemporaryFile(suffix='.rst', delete=False) as tmp: + # NOTE(stephenfin): We dump the output to a tempfile to assist + # people in debugging their broken config options. It would be good + # to conditionalize this but that would require access to error + # state from the directive, which we don't have, and would + # necessitate holding the whole file, which could be rather large, + # in memory while we wait on the decision. + LOG.info('dumping output to %r', tmp.name) + offset = 0 + for count, line in enumerate(_format_option_help( + namespaces, split_namespaces)): + # FIXME(stephenfin): Some lines emitted are actually multiple + # lines. This throws off our counter, which is rather annoying. + # We handle this here but we should really handle it higher up. + parts = line.split('\n') + if len(parts) > 1: + offset += len(parts) - 1 + + for part in parts: + result.append(line, source_name, count + offset) + tmp.write(line.encode('utf-8')) node = nodes.section() node.document = self.state.document