From 1e92335976e1c666ea486d4e01c8251f766d98a2 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Fri, 7 Feb 2025 20:11:48 +0000 Subject: [PATCH 1/4] Fix continuation line indentation in productionlists --- sphinx/writers/html5.py | 5 ++--- sphinx/writers/manpage.py | 5 ++--- sphinx/writers/texinfo.py | 5 ++--- sphinx/writers/text.py | 5 ++--- 4 files changed, 8 insertions(+), 12 deletions(-) diff --git a/sphinx/writers/html5.py b/sphinx/writers/html5.py index c497d6120af..86ab4f4c21e 100644 --- a/sphinx/writers/html5.py +++ b/sphinx/writers/html5.py @@ -696,8 +696,7 @@ def depart_literal(self, node: Element) -> None: def visit_productionlist(self, node: Element) -> None: self.body.append(self.starttag(node, 'pre')) productionlist = cast('Iterable[addnodes.production]', node) - names = (production['tokenname'] for production in productionlist) - maxlen = max(len(name) for name in names) + maxlen = max(len(production['tokenname']) for production in productionlist) lastname = None for production in productionlist: if production['tokenname']: @@ -705,7 +704,7 @@ def visit_productionlist(self, node: Element) -> None: self.body.append(self.starttag(production, 'strong', '')) self.body.append(lastname + ' ::= ') elif lastname is not None: - self.body.append('%s ' % (' ' * len(lastname))) + self.body.append(' ' * (maxlen + 5)) production.walkabout(self) self.body.append('\n') self.body.append('\n') diff --git a/sphinx/writers/manpage.py b/sphinx/writers/manpage.py index 63900584d04..3fa4638c954 100644 --- a/sphinx/writers/manpage.py +++ b/sphinx/writers/manpage.py @@ -277,8 +277,7 @@ def visit_productionlist(self, node: Element) -> None: self.in_productionlist += 1 self.body.append('.sp\n.nf\n') productionlist = cast('Iterable[addnodes.production]', node) - names = (production['tokenname'] for production in productionlist) - maxlen = max(len(name) for name in names) + maxlen = max(len(production['tokenname']) for production in productionlist) lastname = None for production in productionlist: if production['tokenname']: @@ -288,7 +287,7 @@ def visit_productionlist(self, node: Element) -> None: self.body.append(self.defs['strong'][1]) self.body.append(' ::= ') elif lastname is not None: - self.body.append('%s ' % (' ' * len(lastname))) + self.body.append(' ' * (maxlen + 5)) production.walkabout(self) self.body.append('\n') self.body.append('\n.fi\n') diff --git a/sphinx/writers/texinfo.py b/sphinx/writers/texinfo.py index 3bf8b913f0a..d3da34f108f 100644 --- a/sphinx/writers/texinfo.py +++ b/sphinx/writers/texinfo.py @@ -1309,8 +1309,7 @@ def unknown_departure(self, node: Node) -> None: def visit_productionlist(self, node: Element) -> None: self.visit_literal_block(None) productionlist = cast('Iterable[addnodes.production]', node) - names = (production['tokenname'] for production in productionlist) - maxlen = max(len(name) for name in names) + maxlen = max(len(production['tokenname']) for production in productionlist) for production in productionlist: if production['tokenname']: @@ -1318,7 +1317,7 @@ def visit_productionlist(self, node: Element) -> None: self.add_anchor(id, production) s = production['tokenname'].ljust(maxlen) + ' ::=' else: - s = '%s ' % (' ' * maxlen) + s = ' ' * (maxlen + 4) self.body.append(self.escape(s)) self.body.append(self.escape(production.astext() + '\n')) self.depart_literal_block(None) diff --git a/sphinx/writers/text.py b/sphinx/writers/text.py index 7ef6e808ef9..d712fd133ed 100644 --- a/sphinx/writers/text.py +++ b/sphinx/writers/text.py @@ -788,15 +788,14 @@ def depart_caption(self, node: Element) -> None: def visit_productionlist(self, node: Element) -> None: self.new_state() productionlist = cast('Iterable[addnodes.production]', node) - names = (production['tokenname'] for production in productionlist) - maxlen = max(len(name) for name in names) + maxlen = max(len(production['tokenname']) for production in productionlist) lastname = None for production in productionlist: if production['tokenname']: self.add_text(production['tokenname'].ljust(maxlen) + ' ::=') lastname = production['tokenname'] elif lastname is not None: - self.add_text('%s ' % (' ' * len(lastname))) + self.add_text(' ' * (maxlen + 4)) self.add_text(production.astext() + self.nl) self.end_state(wrap=False) raise nodes.SkipNode From b6586301cff2d18c6ec1fbd6eca650aec4f6322c Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Mon, 10 Feb 2025 01:19:54 +0000 Subject: [PATCH 2/4] Split test_directive_productionlist --- .../test_directive_productionlist.py | 92 +++++++++++++++++++ tests/test_domains/test_domain_std.py | 80 +--------------- 2 files changed, 93 insertions(+), 79 deletions(-) create mode 100644 tests/test_directives/test_directive_productionlist.py diff --git a/tests/test_directives/test_directive_productionlist.py b/tests/test_directives/test_directive_productionlist.py new file mode 100644 index 00000000000..e221ba04e61 --- /dev/null +++ b/tests/test_directives/test_directive_productionlist.py @@ -0,0 +1,92 @@ +"""Tests the productionlist directive.""" + +from __future__ import annotations + +import pytest +from docutils import nodes + +from sphinx.addnodes import pending_xref +from sphinx.testing import restructuredtext +from sphinx.testing.util import assert_node, etree_parse + + +@pytest.mark.sphinx('html', testroot='productionlist') +def test_productionlist(app): + app.build(force_all=True) + + warnings = app.warning.getvalue().split('\n') + assert len(warnings) == 2 + assert warnings[-1] == '' + assert ( + 'Dup2.rst:4: WARNING: duplicate token description of Dup, other instance in Dup1' + in warnings[0] + ) + + etree = etree_parse(app.outdir / 'index.html') + nodes = list(etree.iter('ul')) + assert len(nodes) >= 3 + + ul = nodes[2] + cases = [] + for li in list(ul): + li_list = list(li) + assert len(li_list) == 1 + p = li_list[0] + assert p.tag == 'p' + text = str(p.text).strip(' :') + p_list = list(p) + assert len(p_list) == 1 + a = p_list[0] + assert a.tag == 'a' + link = a.get('href') + a_list = list(a) + assert len(a_list) == 1 + code = a_list[0] + assert code.tag == 'code' + code_list = list(code) + assert len(code_list) == 1 + span = code_list[0] + assert span.tag == 'span' + link_text = span.text.strip() + cases.append((text, link, link_text)) + assert cases == [ + ('A', 'Bare.html#grammar-token-A', 'A'), + ('B', 'Bare.html#grammar-token-B', 'B'), + ('P1:A', 'P1.html#grammar-token-P1-A', 'P1:A'), + ('P1:B', 'P1.html#grammar-token-P1-B', 'P1:B'), + ('P2:A', 'P1.html#grammar-token-P1-A', 'P1:A'), + ('P2:B', 'P2.html#grammar-token-P2-B', 'P2:B'), + ('Explicit title A, plain', 'Bare.html#grammar-token-A', 'MyTitle'), + ('Explicit title A, colon', 'Bare.html#grammar-token-A', 'My:Title'), + ('Explicit title P1:A, plain', 'P1.html#grammar-token-P1-A', 'MyTitle'), + ('Explicit title P1:A, colon', 'P1.html#grammar-token-P1-A', 'My:Title'), + ('Tilde A', 'Bare.html#grammar-token-A', 'A'), + ('Tilde P1:A', 'P1.html#grammar-token-P1-A', 'A'), + ('Tilde explicit title P1:A', 'P1.html#grammar-token-P1-A', '~MyTitle'), + ('Tilde, explicit title P1:A', 'P1.html#grammar-token-P1-A', 'MyTitle'), + ('Dup', 'Dup2.html#grammar-token-Dup', 'Dup'), + ('FirstLine', 'firstLineRule.html#grammar-token-FirstLine', 'FirstLine'), + ('SecondLine', 'firstLineRule.html#grammar-token-SecondLine', 'SecondLine'), + ] + + text = (app.outdir / 'LineContinuation.html').read_text(encoding='utf8') + assert 'A ::= B C D E F G' in text + + +@pytest.mark.sphinx('html', testroot='root') +def test_productionlist_xref(app): + text = """\ +.. productionlist:: P2 + A: `:A` `A` + B: `P1:B` `~P1:B` +""" + doctree = restructuredtext.parse(app, text) + refnodes = list(doctree.findall(pending_xref)) + assert_node(refnodes[0], pending_xref, reftarget='A') + assert_node(refnodes[1], pending_xref, reftarget='P2:A') + assert_node(refnodes[2], pending_xref, reftarget='P1:B') + assert_node(refnodes[3], pending_xref, reftarget='P1:B') + assert_node(refnodes[0], [pending_xref, nodes.literal, 'A']) + assert_node(refnodes[1], [pending_xref, nodes.literal, 'A']) + assert_node(refnodes[2], [pending_xref, nodes.literal, 'P1:B']) + assert_node(refnodes[3], [pending_xref, nodes.literal, 'B']) diff --git a/tests/test_domains/test_domain_std.py b/tests/test_domains/test_domain_std.py index b253f28e3d7..aa890f598af 100644 --- a/tests/test_domains/test_domain_std.py +++ b/tests/test_domains/test_domain_std.py @@ -21,7 +21,7 @@ ) from sphinx.domains.std import StandardDomain from sphinx.testing import restructuredtext -from sphinx.testing.util import assert_node, etree_parse +from sphinx.testing.util import assert_node def test_process_doc_handle_figure_caption(): @@ -485,84 +485,6 @@ def test_multiple_cmdoptions(app): assert domain.progoptions['cmd', '--output'] == ('index', 'cmdoption-cmd-o') -@pytest.mark.sphinx('html', testroot='productionlist') -def test_productionlist(app): - app.build(force_all=True) - - warnings = app.warning.getvalue().split('\n') - assert len(warnings) == 2 - assert warnings[-1] == '' - assert ( - 'Dup2.rst:4: WARNING: duplicate token description of Dup, other instance in Dup1' - in warnings[0] - ) - - etree = etree_parse(app.outdir / 'index.html') - nodes = list(etree.iter('ul')) - assert len(nodes) >= 3 - - ul = nodes[2] - cases = [] - for li in list(ul): - li_list = list(li) - assert len(li_list) == 1 - p = li_list[0] - assert p.tag == 'p' - text = str(p.text).strip(' :') - p_list = list(p) - assert len(p_list) == 1 - a = p_list[0] - assert a.tag == 'a' - link = a.get('href') - a_list = list(a) - assert len(a_list) == 1 - code = a_list[0] - assert code.tag == 'code' - code_list = list(code) - assert len(code_list) == 1 - span = code_list[0] - assert span.tag == 'span' - link_text = span.text.strip() - cases.append((text, link, link_text)) - assert cases == [ - ('A', 'Bare.html#grammar-token-A', 'A'), - ('B', 'Bare.html#grammar-token-B', 'B'), - ('P1:A', 'P1.html#grammar-token-P1-A', 'P1:A'), - ('P1:B', 'P1.html#grammar-token-P1-B', 'P1:B'), - ('P2:A', 'P1.html#grammar-token-P1-A', 'P1:A'), - ('P2:B', 'P2.html#grammar-token-P2-B', 'P2:B'), - ('Explicit title A, plain', 'Bare.html#grammar-token-A', 'MyTitle'), - ('Explicit title A, colon', 'Bare.html#grammar-token-A', 'My:Title'), - ('Explicit title P1:A, plain', 'P1.html#grammar-token-P1-A', 'MyTitle'), - ('Explicit title P1:A, colon', 'P1.html#grammar-token-P1-A', 'My:Title'), - ('Tilde A', 'Bare.html#grammar-token-A', 'A'), - ('Tilde P1:A', 'P1.html#grammar-token-P1-A', 'A'), - ('Tilde explicit title P1:A', 'P1.html#grammar-token-P1-A', '~MyTitle'), - ('Tilde, explicit title P1:A', 'P1.html#grammar-token-P1-A', 'MyTitle'), - ('Dup', 'Dup2.html#grammar-token-Dup', 'Dup'), - ('FirstLine', 'firstLineRule.html#grammar-token-FirstLine', 'FirstLine'), - ('SecondLine', 'firstLineRule.html#grammar-token-SecondLine', 'SecondLine'), - ] - - text = (app.outdir / 'LineContinuation.html').read_text(encoding='utf8') - assert 'A ::= B C D E F G' in text - - -@pytest.mark.sphinx('html', testroot='root') -def test_productionlist2(app): - text = '.. productionlist:: P2\n A: `:A` `A`\n B: `P1:B` `~P1:B`\n' - doctree = restructuredtext.parse(app, text) - refnodes = list(doctree.findall(pending_xref)) - assert_node(refnodes[0], pending_xref, reftarget='A') - assert_node(refnodes[1], pending_xref, reftarget='P2:A') - assert_node(refnodes[2], pending_xref, reftarget='P1:B') - assert_node(refnodes[3], pending_xref, reftarget='P1:B') - assert_node(refnodes[0], [pending_xref, nodes.literal, 'A']) - assert_node(refnodes[1], [pending_xref, nodes.literal, 'A']) - assert_node(refnodes[2], [pending_xref, nodes.literal, 'P1:B']) - assert_node(refnodes[3], [pending_xref, nodes.literal, 'B']) - - @pytest.mark.sphinx('html', testroot='root') def test_disabled_docref(app): text = ':doc:`index`\n:doc:`!index`\n' From 147b0ea51e4989dd7927ae69ec09980fb465d5c5 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Mon, 10 Feb 2025 01:44:24 +0000 Subject: [PATCH 3/4] Add test --- CHANGES.rst | 3 + .../test_directive_productionlist.py | 67 +++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index 601c20f3904..991c2848483 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -141,6 +141,9 @@ Bugs fixed Patch by Pavel Holica * #13273, #13318: Properly convert command-line overrides for Boolean types. Patch by Adam Turner. +* #13302, #13319: Use the correct indentation for continuation lines + in :rst:dir:`productionlist` directives. + Patch by Adam Turner. Testing ------- diff --git a/tests/test_directives/test_directive_productionlist.py b/tests/test_directives/test_directive_productionlist.py index e221ba04e61..8fe0b0632c7 100644 --- a/tests/test_directives/test_directive_productionlist.py +++ b/tests/test_directives/test_directive_productionlist.py @@ -90,3 +90,70 @@ def test_productionlist_xref(app): assert_node(refnodes[1], [pending_xref, nodes.literal, 'A']) assert_node(refnodes[2], [pending_xref, nodes.literal, 'P1:B']) assert_node(refnodes[3], [pending_xref, nodes.literal, 'B']) + + +def test_productionlist_continuation_lines(make_app, tmp_path): + text = """ +.. productionlist:: python-grammar + assignment_stmt: (`target_list` "=")+ (`starred_expression` | `yield_expression`) + target_list: `target` ("," `target`)* [","] + target: `identifier` + : | "(" [`target_list`] ")" + : | "[" [`target_list`] "]" + : | `attributeref` + : | `subscription` + : | `slicing` + : | "*" `target` +""" + (tmp_path / 'conf.py').touch() + (tmp_path / 'index.rst').write_text(text, encoding='utf-8') + + app = make_app(buildername='text', srcdir=tmp_path) + app.build(force_all=True) + content = (app.outdir / 'index.txt').read_text(encoding='utf-8') + assert content == """\ + assignment_stmt ::= (target_list "=")+ (starred_expression | yield_expression) + target_list ::= target ("," target)* [","] + target ::= identifier + | "(" [target_list] ")" + | "[" [target_list] "]" + | attributeref + | subscription + | slicing + | "*" target +""" + + app = make_app(buildername='html', srcdir=tmp_path) + app.build(force_all=True) + content = (app.outdir / 'index.html').read_text(encoding='utf-8') + _, _, content = content.partition('
')
+    content, _, _ = content.partition('
') + assert content == """ +assignment_stmt ::= (target_list "=")+ (starred_expression | yield_expression) +target_list ::= target ("," target)* [","] +target ::= identifier + | "(" [target_list] ")" + | "[" [target_list] "]" + | attributeref + | subscription + | slicing + | "*" target +""" + + app = make_app(buildername='man', srcdir=tmp_path) + app.build(force_all=True) + content = (app.outdir / 'projectnamenotset.1').read_text(encoding='utf-8') + _, _, content = content.partition('.nf') + content, _, _ = content.partition('.fi') + assert content == r""" +\fBassignment_stmt\fP ::= (\fI\%target_list\fP \(dq=\(dq)+ (\fBstarred_expression\fP | \fByield_expression\fP) +\fBtarget_list \fP ::= \fI\%target\fP (\(dq,\(dq \fI\%target\fP)* [\(dq,\(dq] +\fBtarget \fP ::= \fBidentifier\fP + | \(dq(\(dq [\fI\%target_list\fP] \(dq)\(dq + | \(dq[\(dq [\fI\%target_list\fP] \(dq]\(dq + | \fBattributeref\fP + | \fBsubscription\fP + | \fBslicing\fP + | \(dq*\(dq \fI\%target\fP + +""" From 852236220086139f873e99ca2121986fc30c5091 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Mon, 10 Feb 2025 01:53:41 +0000 Subject: [PATCH 4/4] fixup! Add test --- .../test_directive_productionlist.py | 40 ++++++++----------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/tests/test_directives/test_directive_productionlist.py b/tests/test_directives/test_directive_productionlist.py index 8fe0b0632c7..2026730c64b 100644 --- a/tests/test_directives/test_directive_productionlist.py +++ b/tests/test_directives/test_directive_productionlist.py @@ -9,9 +9,16 @@ from sphinx.testing import restructuredtext from sphinx.testing.util import assert_node, etree_parse +TYPE_CHECKING = False +if TYPE_CHECKING: + from collections.abc import Callable + from pathlib import Path + + from sphinx.testing.util import SphinxTestApp + @pytest.mark.sphinx('html', testroot='productionlist') -def test_productionlist(app): +def test_productionlist(app: SphinxTestApp) -> None: app.build(force_all=True) warnings = app.warning.getvalue().split('\n') @@ -47,6 +54,7 @@ def test_productionlist(app): assert len(code_list) == 1 span = code_list[0] assert span.tag == 'span' + assert span.text is not None link_text = span.text.strip() cases.append((text, link, link_text)) assert cases == [ @@ -74,7 +82,7 @@ def test_productionlist(app): @pytest.mark.sphinx('html', testroot='root') -def test_productionlist_xref(app): +def test_productionlist_xref(app: SphinxTestApp) -> None: text = """\ .. productionlist:: P2 A: `:A` `A` @@ -92,7 +100,9 @@ def test_productionlist_xref(app): assert_node(refnodes[3], [pending_xref, nodes.literal, 'B']) -def test_productionlist_continuation_lines(make_app, tmp_path): +def test_productionlist_continuation_lines( + make_app: Callable[..., SphinxTestApp], tmp_path: Path +) -> None: text = """ .. productionlist:: python-grammar assignment_stmt: (`target_list` "=")+ (`starred_expression` | `yield_expression`) @@ -111,7 +121,7 @@ def test_productionlist_continuation_lines(make_app, tmp_path): app = make_app(buildername='text', srcdir=tmp_path) app.build(force_all=True) content = (app.outdir / 'index.txt').read_text(encoding='utf-8') - assert content == """\ + expected = """\ assignment_stmt ::= (target_list "=")+ (starred_expression | yield_expression) target_list ::= target ("," target)* [","] target ::= identifier @@ -122,13 +132,14 @@ def test_productionlist_continuation_lines(make_app, tmp_path): | slicing | "*" target """ + assert content == expected app = make_app(buildername='html', srcdir=tmp_path) app.build(force_all=True) content = (app.outdir / 'index.html').read_text(encoding='utf-8') _, _, content = content.partition('
')
     content, _, _ = content.partition('
') - assert content == """ + expected = """ assignment_stmt ::= (target_list "=")+ (starred_expression | yield_expression) target_list ::= target ("," target)* [","] target ::= identifier @@ -139,21 +150,4 @@ def test_productionlist_continuation_lines(make_app, tmp_path): | slicing | "*" target """ - - app = make_app(buildername='man', srcdir=tmp_path) - app.build(force_all=True) - content = (app.outdir / 'projectnamenotset.1').read_text(encoding='utf-8') - _, _, content = content.partition('.nf') - content, _, _ = content.partition('.fi') - assert content == r""" -\fBassignment_stmt\fP ::= (\fI\%target_list\fP \(dq=\(dq)+ (\fBstarred_expression\fP | \fByield_expression\fP) -\fBtarget_list \fP ::= \fI\%target\fP (\(dq,\(dq \fI\%target\fP)* [\(dq,\(dq] -\fBtarget \fP ::= \fBidentifier\fP - | \(dq(\(dq [\fI\%target_list\fP] \(dq)\(dq - | \(dq[\(dq [\fI\%target_list\fP] \(dq]\(dq - | \fBattributeref\fP - | \fBsubscription\fP - | \fBslicing\fP - | \(dq*\(dq \fI\%target\fP - -""" + assert content == expected