Skip to content
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

gh-97607: a blank line after .. impl-detail:: in the Docs #97635

Closed
wants to merge 1 commit into from

Conversation

StevenHsuYL
Copy link
Contributor

@StevenHsuYL StevenHsuYL commented Sep 29, 2022

Add a blank line after .. impl-detail:: in the documentation for translation work.

If a blank line were missing, Sphinx would not build the paragraph after .. impl-detail:: during HTML file generation.

Add a blank line after `impl-detail`
@bedevere-bot bedevere-bot added awaiting review docs Documentation in the Doc dir skip news labels Sep 29, 2022
@StevenHsuYL StevenHsuYL changed the title gh-97607: a blank line after .. impl-detail:: in the Doc gh-97607: a blank line after .. impl-detail:: in the Docs Sep 29, 2022
Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

Actually, the real problem here isn't with the source syntax, but rather than the custom impl-detail directive not handling it correctly (like the built in Sphinx directives do, e.g. admonition).

I've gone ahead and implemented a fix for that in #97652 , which should ensure translations work correctly for all three cases (0, 1 or 2 line breaks before directive content) anywhere the directive is used, now and in the future, makes the behavior consistent with the builtin directives, and substantially simplifies the directive code and logic, while not modifying either the source or the non-translated HTML output.

Therefore, with the fix to the underlying bug, the source changes in this PR won't be actually be necessary (and I've set that one to close this one, if/when successfully merged).

@StevenHsuYL
Copy link
Contributor Author

The issue will be handled in #97652.
Close this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants