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.

Sign up for GitHub

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

Jump to bottom

gh-109634: Use :samp: role #109635

Merged
merged 3 commits into from
Sep 23, 2023
Merged

gh-109634: Use :samp: role #109635

merged 3 commits into from
Sep 23, 2023

Conversation

serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Sep 21, 2023 •
edited by github-actions bot
Loading

@serhiy-storchaka serhiy-storchaka added needs backport to 3.11 only security fixes needs backport to 3.12 bug and security fixes labels Sep 21, 2023
@bedevere-app bedevere-app bot added awaiting review docs Documentation in the Doc dir skip news labels Sep 21, 2023
@bedevere-app bedevere-app bot mentioned this pull request Sep 21, 2023
Closed
@serhiy-storchaka serhiy-storchaka requested review from vstinner and AA-Turner and removed request for vsajip, corona10 and erlend-aasland September 21, 2023 06:07
vstinner
vstinner reviewed Sep 21, 2023
View reviewed changes
Doc/using/configure.rst
@@ -1012,7 +1012,7 @@ differently depending if the ``Py_BUILD_CORE_MODULE`` macro is defined:
* Use ``Py_IMPORTED_SYMBOL`` otherwise.

If the ``Py_BUILD_CORE_BUILTIN`` macro is used by mistake on a C extension
built as a shared library, its ``PyInit_xxx()`` function is not exported,
Copy link
Member

Choose a reason for hiding this comment

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

You can also use it for -I in: https://docs.python.org/dev/using/configure.html#envvar-CPPFLAGS

Compare with new doc: https://cpython-previews.readthedocs.io/en/latest/using/configure.html#cmdoption-arg-CPPFLAGS

vstinner
vstinner approved these changes Sep 21, 2023
View reviewed changes
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM

AA-Turner
AA-Turner approved these changes Sep 21, 2023
View reviewed changes
Copy link
Member

@AA-Turner AA-Turner left a comment

Choose a reason for hiding this comment

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

A nice improvement!

A

Doc/library/os.rst
@@ -88,8 +88,8 @@ startup by the :c:func:`PyConfig_Read` function: see
On some systems, conversion using the file system encoding may fail. In this
case, Python uses the :ref:`surrogateescape encoding error handler
<surrogateescape>`, which means that undecodable bytes are replaced by a
Unicode character U+DCxx on decoding, and these are again translated to the
original byte on encoding.
Unicode character U+DC\ *xx* on decoding, and these are again
Copy link
Member

Choose a reason for hiding this comment

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

Did the role not work here? (I think backslash-escaped spaces are a little ugly & we should avoid if possible; but not a blocker)

Copy link
Member Author

Choose a reason for hiding this comment

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

I do not think that U+DCxx should be rendered with typewriter font.

Doc/reference/lexical_analysis.rst Outdated
+----------------------+---------------------------------+-------+
| Escape Sequence | Meaning | Notes |
+======================+=================================+=======+
| ``\N{name}`` | Character named *name* in the | \(5) |
Copy link
Member

Choose a reason for hiding this comment

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

:samp:`\N{\\{name\\}}` would work. But ungainly.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you for pointing out that backslash have a special meaning in the :samp: role. All other occurrences of backslash should be doubled.

In this example it should be :samp:`\\N\\{{name}\\}`, because curly braces should be outside of the variable part. I am surprised that \ before { needs to be doubled, but it is how it works.

Doc/whatsnew/2.6.rst
@@ -125,7 +125,7 @@ and to C extension code as :c:data:`!Py_Py3kWarningFlag`.

.. seealso::

The 3xxx series of PEPs, which contains proposals for Python 3.0.
The 3\ *xxx* series of PEPs, which contains proposals for Python 3.0.
Copy link
Member

Choose a reason for hiding this comment

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

Same note on avoiding backslash-escaped spaces if possible

erlend-aasland
erlend-aasland approved these changes Sep 21, 2023
View reviewed changes
Copy link
Contributor

@erlend-aasland erlend-aasland left a comment

Choose a reason for hiding this comment

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

Nice; thanks!

@serhiy-storchaka serhiy-storchaka merged commit 92af0cc into python:main Sep 23, 2023
16 checks passed
@miss-islington
Copy link
Contributor

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖

@serhiy-storchaka serhiy-storchaka deleted the docs-samp-role branch September 23, 2023 06:31
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 23, 2023
@serhiy-storchaka @miss-islington
pythongh-109634: Use :samp: role (pythonGH-109635)
fe88759 
(cherry picked from commit 92af0cc)

Co-authored-by: Serhiy Storchaka <[email protected]>
@miss-islington
Copy link
Contributor

Sorry, @serhiy-storchaka, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 92af0cc580051fd1129c7a86af2cbadeb2aa36dc 3.11

@bedevere-app
Copy link

bedevere-app bot commented Sep 23, 2023

GH-109776 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 bug and security fixes label Sep 23, 2023
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this pull request Sep 23, 2023
@serhiy-storchaka
[3.11] pythongh-109634: Use :samp: role (pythonGH-109635).
5d323f4 
(cherry picked from commit 92af0cc)

Co-authored-by: Serhiy Storchaka <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Sep 23, 2023

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

@bedevere-app bedevere-app bot removed the needs backport to 3.11 only security fixes label Sep 23, 2023
Yhg1s pushed a commit that referenced this pull request Sep 24, 2023
@miss-islington @serhiy-storchaka
[3.12] gh-109634: Use :samp: role (GH-109635) (#109776)
2622b13 
gh-109634: Use :samp: role (GH-109635)
(cherry picked from commit 92af0cc)

Co-authored-by: Serhiy Storchaka <[email protected]>
csm10495 pushed a commit to csm10495/cpython that referenced this pull request Sep 28, 2023
@serhiy-storchaka @csm10495
pythongh-109634: Use :samp: role (pythonGH-109635)
ca1edfe
serhiy-storchaka added a commit that referenced this pull request Sep 29, 2023
@serhiy-storchaka @JacobCoffee
[3.11] gh-109634: Use :samp: role (GH-109635) (GH-109778)
c534637 
(cherry picked from commit 92af0cc)

Co-authored-by: Jacob Coffee <[email protected]>
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
@serhiy-storchaka @Glyphack
pythongh-109634: Use :samp: role (pythonGH-109635)
3703089
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@erlend-aasland erlend-aasland erlend-aasland approved these changes

@AA-Turner AA-Turner AA-Turner approved these changes

@vstinner vstinner vstinner approved these changes

Assignees

@serhiy-storchaka serhiy-storchaka

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.
5 participants
@serhiy-storchaka @miss-islington @vstinner @AA-Turner @erlend-aasland