gh-110631: Fix wrong reST markup and list numbers. by ezio-melotti · Pull Request #110885 · python/cpython

ezio-melotti · 2023-10-15T01:42:00Z

Issue: Fix wrongly indented blocks in the documentation #110631

📚 Documentation preview 📚: https://cpython-previews--110885.org.readthedocs.build/

hugovk · 2023-10-15T09:53:27Z

Doc/reference/compound_stmts.rst

   one by one:

-   I. The keyword is looked up as an attribute on the subject.
+   1. The keyword is looked up as an attribute on the subject.


I expect the different numbering style (Roman numerals) was used here to differentiate this inner numbered list from the outer one.

Before

After

Can we keep it them as Roman numerals, but fix the formatting?

Sphinx <7 doesn't seem to support them, so they are actually just regular paragraphs starting with I. .... Letters like a. b. c. ... are also not supported, so we only have numbers left and have to rely on the indentation to distinguish the levels.

I see.

Well, we need to keep support for older Sphinx for the benefit of Linux distros (and are testing it on CI), but can we use Sphinx 7 for our actual main build and deploy?

The Roman numerals won't cause any errors for for Sphinx <7, and they'll be better rendered for for Sphinx 7.

Re: #99380 and cc @AA-Turner.

I should be able to fix the indentation/rendering of the nested lists while keeping the roman numerals, the only issue is that since they are rendered as <p>s rather than <li>s, it's technically incorrect (at least on Sphinx <7). Not sure if that matters though (maybe for screen readers or similar cases?).

Actually if I fix it for <7, once we upgrade to 7 it will break again.

On <7 I could do:

I. The keyword is looked up as an attribute on the subject. * ... * ...

which will be seen as a paragraph followed by a list, but on 7 it would have to be:

I. The keyword is looked up as an attribute on the subject. * ... * ...

since it will be seen as a list item followed by a sublist that needs to be indented.

If I leave the indentation on <7, it will render an additional blockquote around the sublist, and it will cause sphinx-lint to complain, so switching to numbers might still be the best compromise.

I don't mind too much if the display isn't perfect for <7, as long as it still builds and looks reasonable.

Then we can use 7 for our deploys, and sphinx-lint will be happy too.

Would that work?

sphinx-lint will be sad on 7 too, since the checker currently ignores alphabetic lists (like Sphinx 6 does) and sees this as an incorrectly indented list under a paragraph, regardless of the Sphinx version used.

I tried adding support for alphabetic lists, but it's a can of worms with many false positives.

Hmm, maybe we should ignore the Sphinx Lint warning here?

Suggested change

1. The keyword is looked up as an attribute on the subject.

I. The keyword is looked up as an attribute on the subject.

I haven't looked into this in a while, but if possible we should find a solution that is both rendered correctly and that is not reported by sphinx-lint as error.
Fixing sphinx-lint is also an option if the error is reported mistakenly, but avoiding alphabetic lists and roman numerals might still be a simpler solution.

willingc · 2024-11-04T20:59:21Z

@ezio-melotti @hugovk Is this something that we can move forward? Or should it be closed?

AA-Turner · 2024-11-04T21:06:50Z

The cited issues were with Sphinx 6 and earlier, which we have now dropped.

AA-Turner · 2024-11-04T21:20:05Z

Doc/reference/compound_stmts.rst

   one by one:

-   I. The keyword is looked up as an attribute on the subject.
+   1. The keyword is looked up as an attribute on the subject.


Suggested change

1. The keyword is looked up as an attribute on the subject.

I. The keyword is looked up as an attribute on the subject.

AA-Turner · 2024-11-04T21:20:14Z

Doc/reference/compound_stmts.rst



-   II. If all keyword patterns succeed, the class pattern succeeds.
+   2. If all keyword patterns succeed, the class pattern succeeds.


Suggested change

2. If all keyword patterns succeed, the class pattern succeeds.

II. If all keyword patterns succeed, the class pattern succeeds.

AA-Turner · 2024-11-04T21:20:20Z

Doc/reference/compound_stmts.rst

   ``name_or_attr`` before matching:

-   I. The equivalent of ``getattr(cls, "__match_args__", ())`` is called.
+   1. The equivalent of ``getattr(cls, "__match_args__", ())`` is called.


Suggested change

1. The equivalent of ``getattr(cls, "__match_args__", ())`` is called.

I. The equivalent of ``getattr(cls, "__match_args__", ())`` is called.

AA-Turner · 2024-11-04T21:20:37Z

Doc/reference/compound_stmts.rst

+   2. Once all positional patterns have been converted to keyword patterns,
+      the match proceeds as if there were only keyword patterns.


Suggested change

2. Once all positional patterns have been converted to keyword patterns,

the match proceeds as if there were only keyword patterns.

II. Once all positional patterns have been converted to keyword patterns,

the match proceeds as if there were only keyword patterns.

willingc

Looks good to me. Thanks @ezio-melotti and @AA-Turner for moving this forward.

Fix wrong markup and list numbers.

1a1d99c

ezio-melotti added docs Documentation in the Doc dir skip news needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Oct 15, 2023

ezio-melotti requested review from hugovk and pradyunsg October 15, 2023 01:42

ezio-melotti self-assigned this Oct 15, 2023

bedevere-app bot mentioned this pull request Oct 15, 2023

Fix wrongly indented blocks in the documentation #110631

Closed

bedevere-app bot added the awaiting core review label Oct 15, 2023

hugovk reviewed Oct 15, 2023

View reviewed changes

hugovk mentioned this pull request Oct 17, 2023

GH-99380: Update to Sphinx 7 #99381

Merged

hugovk removed the needs backport to 3.11 only security fixes label Apr 11, 2024

serhiy-storchaka added the needs backport to 3.13 bugs and security fixes label May 9, 2024

Merge branch 'main' into fix-ind-rem

8ca1c11

AA-Turner requested a review from willingc as a code owner November 4, 2024 21:05

AA-Turner reviewed Nov 4, 2024

View reviewed changes

willingc approved these changes Nov 4, 2024

View reviewed changes

bedevere-app bot added awaiting merge and removed awaiting core review labels Nov 4, 2024

hugovk removed the needs backport to 3.12 only security fixes label Apr 10, 2025

serhiy-storchaka added the needs backport to 3.14 bugs and security fixes label May 8, 2025

	1. The keyword is looked up as an attribute on the subject.
	I. The keyword is looked up as an attribute on the subject.



		II. If all keyword patterns succeed, the class pattern succeeds.
		2. If all keyword patterns succeed, the class pattern succeeds.

	1. The equivalent of ``getattr(cls, "__match_args__", ())`` is called.
	I. The equivalent of ``getattr(cls, "__match_args__", ())`` is called.

		2. Once all positional patterns have been converted to keyword patterns,
		the match proceeds as if there were only keyword patterns.

Uh oh!

Conversation

ezio-melotti commented Oct 15, 2023 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Before

After

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ezio-melotti Oct 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hugovk Oct 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

willingc commented Nov 4, 2024

Uh oh!

AA-Turner commented Nov 4, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

willingc left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ezio-melotti commented Oct 15, 2023 •

edited by github-actions bot

Loading

ezio-melotti Oct 16, 2023 •

edited

Loading

hugovk Oct 17, 2023 •

edited

Loading