Skip to content

Conversation

AA-Turner
Copy link
Member

@AA-Turner AA-Turner commented Feb 20, 2025

Split into individual commits for readability. I contend that productionlist nodes are semantically the right thing to use here, the only reason we didn't before was due to a limitation in Sphinx.

A


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

Copy link
Member

@encukou encukou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! This looks like the way to go. I left a few notes inline.

conf.py still has needs_sphinx = '8.1.3', and I got a very worrying result with that version: the entire reference directory was missing, without any error message I could see. Is that something you want to look into?

@encukou
Copy link
Member

encukou commented Feb 26, 2025

The extension defines two directives, one with the new syntax but also a CompatProductionList that overrides Sphinx's .. productionlist::. We have syntax highlighting in existing productionlists too.

I added code to merge the lines of each multi-line production into a single production node.
Upstream Sphinx might need a similar change.

@encukou
Copy link
Member

encukou commented Mar 5, 2025

OK, now each production should should have complete rawsource.

@AA-Turner
Copy link
Member Author

Sorry for the delay on my part. I've refactored to split the parsing of content lines into individual productions into its own method, as I think it is sufficiently complex to warrant a dedicated function (and this massively simplifies the creation of the actual production node).

One open question is if group_dict is the right representation, as my understanding is that each dict must have exactly one key -- perhaps an enum would be a better fit? I'm not sure though, and we mustn't let perfect be the enemy of the good.

A

@encukou
Copy link
Member

encukou commented Mar 17, 2025

Don't worry about delays; I've had more important things to deal with too :)

Do you think a (type, content) tuple makes it clearer? Like this: AA-Turner/cpython@docs/productionlist-grammar...encukou:cpython:docs/productionlist-grammar

I think Enum would be overengineering; we'd need to duplicate the names from the regex (unless we build the regex from the enum -- more overengineering).

@encukou
Copy link
Member

encukou commented Mar 19, 2025

I've pushed this change to the PR, and updated the branch to fix a conflict.

@AA-Turner
Copy link
Member Author

@encukou I don't see any new commits...

@encukou
Copy link
Member

encukou commented Mar 20, 2025

I think that was a GitHub glitch.

@AA-Turner AA-Turner merged commit 443c0cd into python:main Mar 20, 2025
43 checks passed
@github-project-automation github-project-automation bot moved this from Todo to Done in Docs PRs Mar 20, 2025
@AA-Turner AA-Turner deleted the docs/productionlist-grammar branch March 20, 2025 15:35
seehwan pushed a commit to seehwan/cpython that referenced this pull request Apr 16, 2025
…-snippet` directive (python#130376)

Co-authored-by: Petr Viktorin <[email protected]>
Co-authored-by: Blaise Pabon <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants