-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Parser agnostic i18n Locale transform #12238
Open
n-peugnet
wants to merge
30
commits into
sphinx-doc:master
Choose a base branch
from
n-peugnet:new-inline-parse-i18n-logic
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 7 commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
0ab14a0
WIP: New i18n logic based on inline_parse function
n-peugnet 2682f8b
Avoid 'Literal block expected; none found.' warnings
n-peugnet 38c004b
Update gettext builder output in tests
n-peugnet 9a2510a
parse_inline's input string is always a string
n-peugnet 3c97944
Only output a single paragraph in RSTParser's parse_inline
n-peugnet 66b14d3
Image nodes are handled separately, and literal can simply be manuall…
n-peugnet 5994ca5
Fix ruff lints + more useful comment
n-peugnet 0794348
Fix last ruff error
n-peugnet 358c611
Remove unused type:ignore annotation
n-peugnet 94b9b9d
Simplify parse_inline literal block handling for rST
n-peugnet 17455d6
Fix rendering for parsed-literals
n-peugnet c10b0ec
Merge remote-tracking branch 'origin/master' into new-inline-parse-i1…
n-peugnet 0fc3f28
Regression test for parsed literals translation
n-peugnet fe7675f
Regression test for strange markup
n-peugnet 7f94dcf
Directly use Inliner.parse() instead of 'Text' initial_state
n-peugnet 14e4833
Add warning messages to doccument in parse_inline
n-peugnet c81d310
Fix ruff lints
n-peugnet 910dbd7
Apply ruff format patch
n-peugnet 3b92bfb
Refactor to fix MyPy error
n-peugnet 8c40fb6
Add tests for parse_inline
n-peugnet ca54a6c
Fix ruff lints
n-peugnet 7de9a40
Skip parse_inline() tests with docutils < 0.19
n-peugnet 7500a00
Try to ignore incorrect mypy lint
n-peugnet e7e7fad
Remove unneeded inliner, title and section options of memo
n-peugnet dba713d
Add test for substitution reference for parse_inline
n-peugnet 2c37780
Simplify parse_inline for RST
n-peugnet 3d5441d
Add test for titles in parse_inline
n-peugnet 463ac3d
Merge remote-tracking branch 'origin/master' into new-inline-parse-i1…
n-peugnet d84d4e6
Fix <unknown> source file in translated warnings
n-peugnet a39b184
Merge branch 'master' into new-inline-parse-i18n-logic
AA-Turner File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should all be using the
self.inliner
to only parse inline syntaxes https://github.com/live-clones/docutils/blob/d50e1676a87f5a495f1a5a0f447e8da9317e1195/docutils/docutils/parsers/rst/states.py#L614There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well this was my initial idea, but it was a lot more complicated to implement. I don't remember if I managed to make it work in the end, but if I did, then it had the same result as using
Text
as theinitial_state
(the::
without literal block were still causing issues), as I did on line 77sphinx/sphinx/parsers.py
Line 77 in 5994ca5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok yes I indeed managed to make it work, but it looked like this:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Text
also passes definition lists and section titles; its definitely much cleaner to use the proper inline parsing, even if docutils does not make this as easy 😒There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here you should also have
parse_inline
take the actual line number and use that, plus the_
issystem_message
nodes that should be appended to theparagraph
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well.. if this proposal is to be accepted, then really it needs to have proper "generic" tests, not just specific to the
i18n
use case, as obviously it could be used for other use casesThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok I get it. Sorry I was too focused on the problem I was trying to solve, but you are right.
Regardless of the implementation details, what do you think about this proposal? As this is more of a proof of concept than a finished product.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeh I mean, coming from the MyST perspective, in principle I am certainly in favor of removing "rST hard-coded" aspects of the code base 😄 (the other big problematic aspect of sphinx for this is executablebooks/MyST-Parser#228)
But indeed, it is quite a "core" addition to sphinx, more broad reaching than just this use case,
so I would obviously want to be very careful (and have good agreement from other maintainers) before merging anything
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course, I marked it as "draft" to make it clearer that it is not ready yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chrisjsewell
☑️
☑️
☑️
Also I started to add some tests for this new function.