← 返回首页
gh-97607: Fix content parsing in the impl-detail reST directive by CAM-Gerlach · Pull Request #97652 · python/cpython · GitHub
Skip to content

Navigation Menu

Toggle navigation
Sign in
Appearance settings
Search or jump to...

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Include my email address so I can be contacted
Cancel Submit feedback

Saved searches

Use saved searches to filter your results more quickly

Cancel Create saved search
Appearance settings
Resetting focus

gh-97607: Fix content parsing in the impl-detail reST directive#97652

Merged
ezio-melotti merged 3 commits into
python:mainfrom
CAM-Gerlach:doc-fix-impl-detail-directive
Oct 2, 2022
Merged

gh-97607: Fix content parsing in the impl-detail reST directive#97652
ezio-melotti merged 3 commits into
python:mainfrom
CAM-Gerlach:doc-fix-impl-detail-directive

Conversation

CAM-Gerlach commented Sep 29, 2022 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

This fixes #97607 with the custom impl-detail reST directive in the docs, that results in the text being untranslated in translations (e.g. python/python-docs-zh-tw#187 ) when it isn't written as a separate content paragraph under the directive (as opposed to with no or one line break after the directive start ::, as opposed to two).

In particular, currently the directive treats text in the same paragraph as the directive start by capturing it in an optional argument and manually processing it, which results in missing metadata (e.g. raw source, file and line number) in the parsed doctree nodes (that in turn breaks the translation code that requires this metadata). This PR fixes the directive to use the same internal logic as the similar builtin docutils/Sphinx directives (e.g. the various admonition types), taking no arguments and just treating that text as directive content by default, regardless of the number of line breaks.

All three forms (0, 1, and 2 line breaks) are used for e.g. note:: in the os module doc source, and all three render correctly in the translated os module docs, and I also confirmed that the parsed doctree structure, content and attributes are now identical for all three to that previously generated for separate paragraphs, and the untranslated HTML is byte-identical both between source forms and before and after this change. In addition to fixing the issue, this also simplifies the directive code and logic.

Additionally, I also was able to eliminate the related unused branch from the directive code that handled the situation where the directive is used alone with no content at all. This isn't used anywhere in the docs and seems to lack a clear use case, and its use would be more likely to just an unintentional mistake (e.g. due to an indentation mismatch). The behavior is now likewise consistent with the aforementioned built-in directive, being flagged as an error at build time, and the code/logic is further simplified from two (originally three) code paths to one.

@StevenHsuYL , could you test to confirm that this PR fixes the issue you experienced in python/python-docs-zh-tw#187 and reported in #97607 ? Thanks!

Fixes #97607
Closes #97635

Don't parse content as arg in the impl-detail directive …
00e5bcc
This does not change the (untranslated) output, but ensures that the doctree node metadata is correct. which fixes pythongh-97607 with the text not being translated. It also simplifies the code and logic and makes it consistant with the docutils built-in directives.
Remove unused branch from impl-detail directive handling no-content case …
9dcab9c
This is not used anywhere in the docs and lacks a clear use case, and is more likely a mistake which is now flagged at build time. This simplifies the logic from two code paths to one, and makes the behavior consistant with similar built-in directives (e.g. the various admonition types).
CAM-Gerlach added docs Documentation in the Doc dir skip news needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes labels Sep 29, 2022

CAM-Gerlach commented Sep 29, 2022

Copy link
Copy Markdown
Member Author

FYI, @pablogsal in case this isn't backported to the release branch by default as a docs change, this will need to be to ensure translations for these blocks work, given the reported bug.

CAM-Gerlach mentioned this pull request Sep 29, 2022
Closed
AA-Turner approved these changes Sep 29, 2022
View reviewed changes
Comment thread Doc/tools/extensions/pyspecific.py Outdated Show resolved Hide resolved

AA-Turner commented Sep 29, 2022

Copy link
Copy Markdown
Member

I agree zero-content impl-detail doesn't make sense.

A

ezio-melotti approved these changes Sep 30, 2022
View reviewed changes

ezio-melotti left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

@StevenHsuYL , could you test to confirm that this PR fixes the issue you experienced in python/python-docs-zh-tw#187 and reported in #97607 ? Thanks!

I tested this and can confirm that after updating Doc/library/queue.rst(including the change suggested by @AA-Turner), the translation of the implementation detail is included correctly:

I also tried rebuilding the .po for Doc/library/readline.rst and it added the implementation note that was previously missing:

Expand diff
$ git diff library/readline.po diff --git a/library/readline.po b/library/readline.po index 6781a16..cfd0e1c 100644 --- a/library/readline.po +++ b/library/readline.po @@ -7,7 +7,7 @@ msgid "" msgstr "" "Project-Id-Version: Python 3.10\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2021-10-26 16:47+0000\n" +"POT-Creation-Date: 2022-09-30 04:06+0200\n" "PO-Revision-Date: 2018-05-23 16:09+0000\n" "Last-Translator: Adrian Liaw <adrianliaw2000@gmail.com>\n" "Language-Team: Chinese - TAIWAN (https://github.com/python/python-docs-zh-" @@ -205,6 +205,12 @@ msgid "" "when true, enables auto history, and that when false, disables auto history." msgstr "" +#: ../../library/readline.rst:188 +msgid "" +"Auto history is enabled by default, and changes to this do not persist " +"across multiple sessions." +msgstr "" + #: ../../library/readline.rst:193 msgid "Startup hooks" msgstr ""

CAM-Gerlach commented Sep 30, 2022

Copy link
Copy Markdown
Member Author

Warning, treated as error:
Unknown platform(s) or syntax 'systems with POSIX threads' in '.. availability:: Windows, systems with POSIX threads.', see > /home/hchsu/github/cpython/Doc/tools/extensions/pyspecific.py:Availability.known_platforms for a set known platforms.

That's an error from the availability custom directive, unrelated to the impl-detail custom directive modified here. Specifically, you're seeing that because that directive was changed in recent versions to be more strict with the input it accepts (only names of known platforms), and evidently the corresponding docs changes weren't backported. If you're going to apply them manually, just copy the ImplementationDetail class, which is the only one that was actually changed here (or merge this branch with 3.10 with Git).

StevenHsuYL commented Sep 30, 2022

Copy link
Copy Markdown
Contributor

Thanks for the reply.
I applied the change in this PR to my 3.10 branch, which works well for translation page generation.
The described issue has disappeared.

Thanks a lot!

CAM-Gerlach commented Sep 30, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

Great, thanks! I've also tested again with the suggested changes locally to confirm.

This was referenced Sep 30, 2022
Merged
Closed
ezio-melotti merged commit e8165d4 into python:main Oct 2, 2022

miss-islington commented Oct 2, 2022

Copy link
Copy Markdown
Contributor

Thanks @CAM-Gerlach for the PR, and @ezio-melotti for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖

bedevere-bot commented Oct 2, 2022

Copy link
Copy Markdown

GH-97723 is a backport of this pull request to the 3.11 branch.

bedevere-bot removed the needs backport to 3.11 only security fixes label Oct 2, 2022
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 2, 2022
pythongh-97607: Fix content parsing in the impl-detail reST directive (… …
8ca39d5
…pythonGH-97652) * Don't parse content as arg in the impl-detail directive This does not change the (untranslated) output, but ensures that the doctree node metadata is correct. which fixes pythongh-97607 with the text not being translated. It also simplifies the code and logic and makes it consistant with the docutils built-in directives. * Remove unused branch from impl-detail directive handling no-content case This is not used anywhere in the docs and lacks a clear use case, and is more likely a mistake which is now flagged at build time. This simplifies the logic from two code paths to one, and makes the behavior consistant with similar built-in directives (e.g. the various admonition types). * Further simplify impl-detail reST directive code (cherry picked from commit e8165d4) Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>

bedevere-bot commented Oct 2, 2022

Copy link
Copy Markdown

GH-97724 is a backport of this pull request to the 3.10 branch.

bedevere-bot removed the needs backport to 3.10 only security fixes label Oct 2, 2022
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 2, 2022
pythongh-97607: Fix content parsing in the impl-detail reST directive (… …
c7957dc
…pythonGH-97652) * Don't parse content as arg in the impl-detail directive This does not change the (untranslated) output, but ensures that the doctree node metadata is correct. which fixes pythongh-97607 with the text not being translated. It also simplifies the code and logic and makes it consistant with the docutils built-in directives. * Remove unused branch from impl-detail directive handling no-content case This is not used anywhere in the docs and lacks a clear use case, and is more likely a mistake which is now flagged at build time. This simplifies the logic from two code paths to one, and makes the behavior consistant with similar built-in directives (e.g. the various admonition types). * Further simplify impl-detail reST directive code (cherry picked from commit e8165d4) Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
miss-islington added a commit that referenced this pull request Oct 2, 2022
gh-97607: Fix content parsing in the impl-detail reST directive (GH-9… …
5ee8344
…7652) * Don't parse content as arg in the impl-detail directive This does not change the (untranslated) output, but ensures that the doctree node metadata is correct. which fixes gh-97607 with the text not being translated. It also simplifies the code and logic and makes it consistant with the docutils built-in directives. * Remove unused branch from impl-detail directive handling no-content case This is not used anywhere in the docs and lacks a clear use case, and is more likely a mistake which is now flagged at build time. This simplifies the logic from two code paths to one, and makes the behavior consistant with similar built-in directives (e.g. the various admonition types). * Further simplify impl-detail reST directive code (cherry picked from commit e8165d4) Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
miss-islington added a commit that referenced this pull request Oct 2, 2022
gh-97607: Fix content parsing in the impl-detail reST directive (GH-9… …
8c528ef
…7652) * Don't parse content as arg in the impl-detail directive This does not change the (untranslated) output, but ensures that the doctree node metadata is correct. which fixes gh-97607 with the text not being translated. It also simplifies the code and logic and makes it consistant with the docutils built-in directives. * Remove unused branch from impl-detail directive handling no-content case This is not used anywhere in the docs and lacks a clear use case, and is more likely a mistake which is now flagged at build time. This simplifies the logic from two code paths to one, and makes the behavior consistant with similar built-in directives (e.g. the various admonition types). * Further simplify impl-detail reST directive code (cherry picked from commit e8165d4) Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
serhiy-storchaka pushed a commit to serhiy-storchaka/cpython that referenced this pull request Oct 2, 2022
pythongh-97607: Fix content parsing in the impl-detail reST directive (… …
299dd41
…python#97652) * Don't parse content as arg in the impl-detail directive This does not change the (untranslated) output, but ensures that the doctree node metadata is correct. which fixes pythongh-97607 with the text not being translated. It also simplifies the code and logic and makes it consistant with the docutils built-in directives. * Remove unused branch from impl-detail directive handling no-content case This is not used anywhere in the docs and lacks a clear use case, and is more likely a mistake which is now flagged at build time. This simplifies the logic from two code paths to one, and makes the behavior consistant with similar built-in directives (e.g. the various admonition types). * Further simplify impl-detail reST directive code
carljm added a commit to carljm/cpython that referenced this pull request Oct 3, 2022
Merge branch 'main' into dictwatch …
e83abac
* main: (2069 commits) pythongh-96512: Move int_max_str_digits setting to PyConfig (python#96944) pythongh-94808: Coverage: Check picklablability of calliter (python#95923) pythongh-94808: Add test coverage for PyObject_HasAttrString (python#96627) pythongh-94732: Fix KeyboardInterrupt race in asyncio run_forever() (python#97765) Fix typos in `bltinmodule.c`. (pythonGH-97766) pythongh-94808: `_PyLineTable_StartsLine` was not used (pythonGH-96609) pythongh-97681: Remove Tools/demo/ directory (python#97682) Fix typo in unittest docs (python#97742) pythongh-97728: Argument Clinic: Fix uninitialized variable in the Py_UNICODE converter (pythonGH-97729) pythongh-95913: Fix PEP number in PEP 678 What's New ref label (python#97739) pythongh-95913: Copyedit/improve New Modules What's New section (python#97721) pythongh-97740: Fix bang in Sphinx C domain ref target syntax (python#97741) pythongh-96819: multiprocessing.resource_tracker: check if length of pipe write <= 512 (python#96890) pythongh-97706: multiprocessing tests: Delete unused variable `rand` (python#97707) pythonGH-85447: Clarify docs about awaiting future multiple times (python#97738) [docs] Update logging cookbook with recipe for using a logger like an output… (pythonGH-97730) pythongh-97607: Fix content parsing in the impl-detail reST directive (python#97652) pythongh-95975: Move except/*/finally ref labels to more precise locations (python#95976) pythongh-97591: In `Exception.__setstate__()` acquire strong references before calling `tp_hash` slot (python#97700) pythongh-95588: Drop the safety claim from `ast.literal_eval` docs. (python#95919) ...
pablogsal pushed a commit that referenced this pull request Oct 22, 2022
gh-97607: Fix content parsing in the impl-detail reST directive (GH-9… …
eb5cdf6
…7652) * Don't parse content as arg in the impl-detail directive This does not change the (untranslated) output, but ensures that the doctree node metadata is correct. which fixes gh-97607 with the text not being translated. It also simplifies the code and logic and makes it consistant with the docutils built-in directives. * Remove unused branch from impl-detail directive handling no-content case This is not used anywhere in the docs and lacks a clear use case, and is more likely a mistake which is now flagged at build time. This simplifies the logic from two code paths to one, and makes the behavior consistant with similar built-in directives (e.g. the various admonition types). * Further simplify impl-detail reST directive code (cherry picked from commit e8165d4) Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
This file contains hidden or 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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

hugovk hugovk approved these changes
AA-Turner AA-Turner approved these changes
ezio-melotti ezio-melotti approved these changes

Assignees

No one assigned

Labels

docs Documentation in the Doc dir skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Lack of a blank line after .. impl-detail::

7 participants

Footer

© 2026 GitHub, Inc.