Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

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

Provide feedback

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

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

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

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

CAM-Gerlach
Copy link
Member

@CAM-GerlachCAM-Gerlach commentedSep 29, 2022•
edited by bedevere-bot
Loading

Thisfixes#97607 with thecustomimpl-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 theos module doc source, and all three render correctly in thetranslatedos 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 inpython/python-docs-zh-tw#187 and reported in#97607 ? Thanks!

Fixes#97607
Closes#97635

This does not change the (untranslated) output,but ensures that the doctree node metadata is correct.whichfixespythongh-97607 with the text not being translated.It also simplifies the code and logicand makes it consistant with the docutils built-in directives.
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
Copy link
MemberAuthor

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.

@AA-Turner
Copy link
Member

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

A

Copy link
Member

@ezio-melottiezio-melotti left a comment

Choose a reason for hiding this comment

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

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

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

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

Expand diff
$ git diff library/readline.podiff --git a/library/readline.po b/library/readline.poindex 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 ""

StevenHsuYL and mattwang44 reacted with thumbs up emojiCAM-Gerlach and mattwang44 reacted with hooray emoji
@CAM-Gerlach
Copy link
MemberAuthor

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 theavailability custom directive, unrelated to theimpl-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 theImplementationDetail class, which is the only one that was actually changed here (or merge this branch with 3.10 with Git).

ezio-melotti and StevenHsuYL reacted with thumbs up emoji

@StevenHsuYL
Copy link
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, ezio-melotti, and mattwang44 reacted with thumbs up emojiCAM-Gerlach and ezio-melotti reacted with hooray emoji

@CAM-Gerlach
Copy link
MemberAuthor

CAM-Gerlach commentedSep 30, 2022•
edited
Loading

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

StevenHsuYL reacted with thumbs up emoji

@ezio-melottiezio-melotti merged commite8165d4 intopython:mainOct 2, 2022
@miss-islington
Copy link
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
Copy link

GH-97723 is a backport of this pull request to the3.11 branch.

@bedevere-botbedevere-bot removed the needs backport to 3.11only security fixes labelOct 2, 2022
miss-islington pushed a commit to miss-islington/cpython that referenced this pull requestOct 2, 2022
…pythonGH-97652)* Don't parse content as arg in the impl-detail directiveThis does not change the (untranslated) output,but ensures that the doctree node metadata is correct.whichfixespythongh-97607 with the text not being translated.It also simplifies the code and logicand makes it consistant with the docutils built-in directives.* Remove unused branch from impl-detail directive handling no-content caseThis 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 commite8165d4)Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
@bedevere-bot
Copy link

GH-97724 is a backport of this pull request to the3.10 branch.

@bedevere-botbedevere-bot removed the needs backport to 3.10only security fixes labelOct 2, 2022
miss-islington pushed a commit to miss-islington/cpython that referenced this pull requestOct 2, 2022
…pythonGH-97652)* Don't parse content as arg in the impl-detail directiveThis does not change the (untranslated) output,but ensures that the doctree node metadata is correct.whichfixespythongh-97607 with the text not being translated.It also simplifies the code and logicand makes it consistant with the docutils built-in directives.* Remove unused branch from impl-detail directive handling no-content caseThis 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 commite8165d4)Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
miss-islington added a commit that referenced this pull requestOct 2, 2022
…7652)* Don't parse content as arg in the impl-detail directiveThis does not change the (untranslated) output,but ensures that the doctree node metadata is correct.whichfixesgh-97607 with the text not being translated.It also simplifies the code and logicand makes it consistant with the docutils built-in directives.* Remove unused branch from impl-detail directive handling no-content caseThis 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 commite8165d4)Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
miss-islington added a commit that referenced this pull requestOct 2, 2022
…7652)* Don't parse content as arg in the impl-detail directiveThis does not change the (untranslated) output,but ensures that the doctree node metadata is correct.whichfixesgh-97607 with the text not being translated.It also simplifies the code and logicand makes it consistant with the docutils built-in directives.* Remove unused branch from impl-detail directive handling no-content caseThis 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 commite8165d4)Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
serhiy-storchaka pushed a commit to serhiy-storchaka/cpython that referenced this pull requestOct 2, 2022
…python#97652)* Don't parse content as arg in the impl-detail directiveThis does not change the (untranslated) output,but ensures that the doctree node metadata is correct.whichfixespythongh-97607 with the text not being translated.It also simplifies the code and logicand makes it consistant with the docutils built-in directives.* Remove unused branch from impl-detail directive handling no-content caseThis 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 requestOct 3, 2022
* 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 requestOct 22, 2022
…7652)* Don't parse content as arg in the impl-detail directiveThis does not change the (untranslated) output,but ensures that the doctree node metadata is correct.whichfixesgh-97607 with the text not being translated.It also simplifies the code and logicand makes it consistant with the docutils built-in directives.* Remove unused branch from impl-detail directive handling no-content caseThis 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 commite8165d4)Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@hugovkhugovkhugovk approved these changes

@AA-TurnerAA-TurnerAA-Turner approved these changes

@ezio-melottiezio-melottiezio-melotti approved these changes

Assignees
No one assigned
Labels
docsDocumentation in the Doc dirskip 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
@CAM-Gerlach@AA-Turner@StevenHsuYL@miss-islington@bedevere-bot@hugovk@ezio-melotti
[8]ページ先頭
©2009-2025 Movatter.jp