From 04b84e82a019b12d79519082d79e6636238b31a7 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+AA-Turner@users.noreply.github.com> Date: Wed, 1 Jun 2022 00:31:12 +0100 Subject: [PATCH 1/6] Fix `findall` usage in KeyboardTransform --- sphinx/builders/html/transforms.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/sphinx/builders/html/transforms.py b/sphinx/builders/html/transforms.py index ea596cb4b22..7f96c4e60ec 100644 --- a/sphinx/builders/html/transforms.py +++ b/sphinx/builders/html/transforms.py @@ -40,7 +40,10 @@ class KeyboardTransform(SphinxPostTransform): def run(self, **kwargs: Any) -> None: matcher = NodeMatcher(nodes.literal, classes=["kbd"]) - for node in self.document.findall(matcher): # type: nodes.literal + # this list must be pre-created as during iteration new nodes + # are added which match the condition in the NodeMatcher. + nodes_to_expand = list(self.document.findall(matcher)) + for node in nodes_to_expand: # type: nodes.literal parts = self.pattern.split(node[-1].astext()) if len(parts) == 1 or self.is_multiwords_key(parts): continue @@ -61,6 +64,7 @@ def run(self, **kwargs: Any) -> None: node += nodes.Text(sep) except IndexError: pass + _a = 1 def is_multiwords_key(self, parts: List[str]) -> bool: if len(parts) >= 3 and parts[1].strip() == '': From 23a4b614157b3e69d4f43075dc6cacd05fecf3b9 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+AA-Turner@users.noreply.github.com> Date: Wed, 1 Jun 2022 00:35:28 +0100 Subject: [PATCH 2/6] Add a test --- .../test-transforms-post_transforms-keyboard/conf.py | 0 .../test-transforms-post_transforms-keyboard/index.rst | 4 ++++ tests/test_transforms_post_transforms.py | 8 ++++++++ 3 files changed, 12 insertions(+) create mode 100644 tests/roots/test-transforms-post_transforms-keyboard/conf.py create mode 100644 tests/roots/test-transforms-post_transforms-keyboard/index.rst diff --git a/tests/roots/test-transforms-post_transforms-keyboard/conf.py b/tests/roots/test-transforms-post_transforms-keyboard/conf.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/roots/test-transforms-post_transforms-keyboard/index.rst b/tests/roots/test-transforms-post_transforms-keyboard/index.rst new file mode 100644 index 00000000000..76c5c584ea3 --- /dev/null +++ b/tests/roots/test-transforms-post_transforms-keyboard/index.rst @@ -0,0 +1,4 @@ +Regression test for issue 10495 +=============================== + +:kbd:`test - test` diff --git a/tests/test_transforms_post_transforms.py b/tests/test_transforms_post_transforms.py index 272d83e3a02..73cd66758a0 100644 --- a/tests/test_transforms_post_transforms.py +++ b/tests/test_transforms_post_transforms.py @@ -48,3 +48,11 @@ def missing_reference(app, env, node, contnode): content = (app.outdir / 'index.html').read_text(encoding='utf8') assert 'Age' in content + + +@pytest.mark.sphinx('html', testroot='transforms-post_transforms-keyboard', + freshenv=True) +def test_keyboard_issue_10495(app): + """Regression test for issue 10495, we want no crash.""" + app.build() + assert "blah" in (app.outdir / 'index.html').read_text(encoding='utf8') From 244ab1c015ff869fbe0757d5cb872c5bf9a2d656 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+AA-Turner@users.noreply.github.com> Date: Wed, 1 Jun 2022 00:38:55 +0100 Subject: [PATCH 3/6] Add annotation --- sphinx/builders/html/transforms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sphinx/builders/html/transforms.py b/sphinx/builders/html/transforms.py index 7f96c4e60ec..a2944c017d2 100644 --- a/sphinx/builders/html/transforms.py +++ b/sphinx/builders/html/transforms.py @@ -42,7 +42,7 @@ def run(self, **kwargs: Any) -> None: matcher = NodeMatcher(nodes.literal, classes=["kbd"]) # this list must be pre-created as during iteration new nodes # are added which match the condition in the NodeMatcher. - nodes_to_expand = list(self.document.findall(matcher)) + nodes_to_expand: List[nodes.literal] = list(self.document.findall(matcher)) for node in nodes_to_expand: # type: nodes.literal parts = self.pattern.split(node[-1].astext()) if len(parts) == 1 or self.is_multiwords_key(parts): From 6aa2d05111168ea93a82719b3fa99fd203924bb2 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+AA-Turner@users.noreply.github.com> Date: Wed, 1 Jun 2022 00:39:35 +0100 Subject: [PATCH 4/6] Remove temp variable --- sphinx/builders/html/transforms.py | 1 - 1 file changed, 1 deletion(-) diff --git a/sphinx/builders/html/transforms.py b/sphinx/builders/html/transforms.py index a2944c017d2..940b0c0d8e2 100644 --- a/sphinx/builders/html/transforms.py +++ b/sphinx/builders/html/transforms.py @@ -64,7 +64,6 @@ def run(self, **kwargs: Any) -> None: node += nodes.Text(sep) except IndexError: pass - _a = 1 def is_multiwords_key(self, parts: List[str]) -> bool: if len(parts) >= 3 and parts[1].strip() == '': From 91f90a5fb5c7c4e412d966582266f20bbadd46d2 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+AA-Turner@users.noreply.github.com> Date: Wed, 1 Jun 2022 00:50:41 +0100 Subject: [PATCH 5/6] Fix test --- tests/roots/test-transforms-post_transforms-keyboard/index.rst | 2 +- tests/test_transforms_post_transforms.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/roots/test-transforms-post_transforms-keyboard/index.rst b/tests/roots/test-transforms-post_transforms-keyboard/index.rst index 76c5c584ea3..21775782f64 100644 --- a/tests/roots/test-transforms-post_transforms-keyboard/index.rst +++ b/tests/roots/test-transforms-post_transforms-keyboard/index.rst @@ -1,4 +1,4 @@ Regression test for issue 10495 =============================== -:kbd:`test - test` +:kbd:`spanish - inquisition` diff --git a/tests/test_transforms_post_transforms.py b/tests/test_transforms_post_transforms.py index 73cd66758a0..ebd50577637 100644 --- a/tests/test_transforms_post_transforms.py +++ b/tests/test_transforms_post_transforms.py @@ -55,4 +55,5 @@ def missing_reference(app, env, node, contnode): def test_keyboard_issue_10495(app): """Regression test for issue 10495, we want no crash.""" app.build() - assert "blah" in (app.outdir / 'index.html').read_text(encoding='utf8') + assert "spanish" in (app.outdir / 'index.html').read_text(encoding='utf8') + assert "inquisition" in (app.outdir / 'index.html').read_text(encoding='utf8') From efdbe06eea67cba57532160a1eb80aaac2241d50 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+AA-Turner@users.noreply.github.com> Date: Wed, 1 Jun 2022 18:10:55 +0100 Subject: [PATCH 6/6] Review comments --- sphinx/builders/html/transforms.py | 3 +-- tests/test_transforms_post_transforms.py | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/sphinx/builders/html/transforms.py b/sphinx/builders/html/transforms.py index 940b0c0d8e2..1ba6ba85743 100644 --- a/sphinx/builders/html/transforms.py +++ b/sphinx/builders/html/transforms.py @@ -42,8 +42,7 @@ def run(self, **kwargs: Any) -> None: matcher = NodeMatcher(nodes.literal, classes=["kbd"]) # this list must be pre-created as during iteration new nodes # are added which match the condition in the NodeMatcher. - nodes_to_expand: List[nodes.literal] = list(self.document.findall(matcher)) - for node in nodes_to_expand: # type: nodes.literal + for node in list(self.document.findall(matcher)): # type: nodes.literal parts = self.pattern.split(node[-1].astext()) if len(parts) == 1 or self.is_multiwords_key(parts): continue diff --git a/tests/test_transforms_post_transforms.py b/tests/test_transforms_post_transforms.py index ebd50577637..215e6d14fe2 100644 --- a/tests/test_transforms_post_transforms.py +++ b/tests/test_transforms_post_transforms.py @@ -52,7 +52,7 @@ def missing_reference(app, env, node, contnode): @pytest.mark.sphinx('html', testroot='transforms-post_transforms-keyboard', freshenv=True) -def test_keyboard_issue_10495(app): +def test_keyboard_hyphen_spaces(app): """Regression test for issue 10495, we want no crash.""" app.build() assert "spanish" in (app.outdir / 'index.html').read_text(encoding='utf8')