GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-40204: Update documentation formatting for Sphinx 3.0 by jakobandersen · Pull Request #19397 · python/cpython

Conversation

@jakobandersen

@jakobandersen jakobandersen commented Apr 6, 2020 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

In the recently released Sphinx v3.0 the C domain has been rewritten to improve parsing of declarations and cross-referencing capabilities. However, this means that the existing directives has gotten more strict on the type of entities they accept.
This PR updates the documentation to use the new directives and roles. Many of the changes are related to C constructs in running text where a type or expression is "referenced". These have been changed to use the new markup role c:expr which accepts arbitrary types or expressions. This is in opposition to the other roles, like c:type which is for cross-referencing a type declared with a directive.

A few issues I couldn't immediately resovle:

  • The refcount annotation in tools/extensions/c_annoations.py no longer works (though first broken in c domain: Generate node_id for objects in the right way sphinx-doc/sphinx#7269).
  • c-api/module.rst:194: the old m_reload is technically declared inside the scope of m_slots. This is now reflected in its ID/permalink.
  • c-api/call.rst: The two functions PyObject_CallFunctionObjArgs and PyObject_CallMethodObjArgs takes a variable amount of arguments, though the last one must be NULL, indicated explicitly in the declaration. This is not valid C. There could be a configuration option in Sphinx to allow this. Please open an issue for Sphinx if you find this is relevant.
  • howto/instrumentation.rst: The c:function directive is used for some markers, but without a return type. The type void was added. However, python.function.return contains return which is a C keyword, so a warning is generated. The elegant solution would perhaps be to code a custom domain for these things, as they are not really C declarations (as far as I understand).
  • c-api/unicode.rst: The function PyUnicode_Translate is documented twice.
  • c-api/init.rst: There are several instances mentioning "wchar_* string". Should this be :c:expr:`wchar_t*` string?

https://bugs.python.org/issue40204

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

m-aciek and hroncok reacted with thumbs up emoji
All reactions
  • 👍 2 reactions
jakobandersen added 2 commits March 15, 2020 19:43
@jakobandersen
Initial fixes for Sphinx 3
6c604b9
@jakobandersen
More doc fixes for Sphinx 3
965ca55
@jakobandersen
jakobandersen requested a review from tiran as a code owner April 6, 2020 17:41
@the-knights-who-say-ni the-knights-who-say-ni added the CLA not signed label Apr 6, 2020
@the-knights-who-say-ni

the-knights-who-say-ni commented Apr 6, 2020

Copy link
Copy Markdown

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

Recognized GitHub username

We couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames:

@jakobandersen

This might be simply due to a missing "GitHub Name" entry in one's b.p.o account settings. This is necessary for legal reasons before we can look at this contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot added docs Documentation in the Doc dir awaiting review labels Apr 6, 2020
@jakobandersen jakobandersen mentioned this pull request Apr 6, 2020
TypeError in sphinx logging building CPython docs with Sphinx 3.0 sphinx-doc/sphinx#7423
Closed
serhiy-storchaka
serhiy-storchaka reviewed Apr 6, 2020
View reviewed changes
Comment thread Doc/c-api/arg.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

@m-aciek m-aciek mentioned this pull request Apr 8, 2020
Make build-translation job work again python/python-docs-pl#8
Closed
serhiy-storchaka
serhiy-storchaka reviewed Apr 24, 2020
View reviewed changes
Comment thread Doc/c-api/arg.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Doc/c-api/arg.rst

where *object* is the Python object to be converted and *address* is the
:c:type:`void\*` argument that was passed to the :c:func:`PyArg_Parse\*` function.
:c:expr:`void*` argument that was passed to the ``PyArg_Parse*`` functions.

@serhiy-storchaka serhiy-storchaka Apr 24, 2020

Copy link
Copy Markdown
Member

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.

:c:func: not just changes the style and adds a link, but it also adds () after the function name. Please add explicit () in all sites where you changed :c:func: to :c:expr: or `` ``.

Suggested change
:c:expr:`void*` argument that was passed to the ``PyArg_Parse*`` functions.
:c:expr:`void*` argument that was passed to the ``PyArg_Parse*()`` functions.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Doc/c-api/arg.rst
\*`) as its argument and should return a "new" Python object, or ``NULL`` if an
error occurred.
function is called with *anything* (which should be compatible with
:c:expr:`void*`)as its argument and should return a "new" Python object, or

@serhiy-storchaka serhiy-storchaka Apr 24, 2020

Copy link
Copy Markdown
Member

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.

Suggested change
:c:expr:`void*`)as its argument and should return a "new" Python object, or
:c:expr:`void*`) as its argument and should return a "new" Python object, or

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Doc/c-api/arg.rst
error occurred.
function is called with *anything* (which should be compatible with
:c:expr:`void*`)as its argument and should return a "new" Python object, or
``NULL`` if an error occurred.

@serhiy-storchaka serhiy-storchaka Apr 24, 2020

Copy link
Copy Markdown
Member

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.

Seems the tab character is used here. Please replace all tab characters with spaces and check that lines are correctly aligned.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Doc/c-api/exceptions.rst

This is a convenience function to raise :exc:`WindowsError`. If called with
*ierr* of :c:data:`0`, the error code returned by a call to :c:func:`GetLastError`
*ierr* of :c:expr:`0`, the error code returned by a call to :c:func:`GetLastError`

@serhiy-storchaka serhiy-storchaka Apr 24, 2020

Copy link
Copy Markdown
Member

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.

I think it is enough to use

Suggested change
*ierr* of :c:expr:`0`, the error code returned by a call to :c:func:`GetLastError`
*ierr* of ``0``, the error code returned by a call to :c:func:`GetLastError`

as for all literals.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@csabella csabella removed the CLA not signed label May 22, 2020
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label May 22, 2020
bmwiedemann added a commit to bmwiedemann/openSUSE that referenced this pull request Jun 11, 2020
@bmwiedemann
Update python3 to rev 98 via SR 812851
d86f7ea
https://build.opensuse.org/request/show/812851
by user mcepl + dimstar_suse
- add requires python3-base on libpython subpackage (bsc#1167008)

- build against Sphinx 2.x until python is compatible with
  Sphinx 3.x (see gh#python/cpython#19397, bpo#40204)

- Fix build with SQLite 3.32 (bpo#40783)
  add bpo40784-Fix-sqlite3-deterministic-test.patch
vstinner
vstinner requested changes Aug 17, 2020
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

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.

The problem of this PR is that :c:expr: is not supported by Sphinx 2, and so this PR drops Sphinx 2 support. Since Sphinx 3 is not widely adopted yet, I proposed a different approach which keeps Sphinx 2 support, but adds Sphinx 3 support, see: https://bugs.python.org/issue40204 Sphinx 2+3 support can be added to stable branches, it's not a backward incompatible change. But my changes require Sphinx 3.2 which adds a new option to opt-in for Sphinx 2 syntax for the C domain.

This PR contains a few relevant changes which are compatible with Sphinx 2. If you want, please try to extract them and write a new PR.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@bedevere-bot bedevere-bot added awaiting changes and removed awaiting review labels Aug 17, 2020
@bedevere-bot

bedevere-bot commented Aug 17, 2020

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@vstinner vstinner closed this Aug 17, 2020
@vstinner

vstinner commented Aug 17, 2020

Copy link
Copy Markdown
Member

Thanks for your contribution! But your PR couldn't be merged. If you want, please try to extract your changes which are relevant and compatible with Sphinx 2.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka left review comments
@vstinner vstinner vstinner requested changes
@tiran tiran Awaiting requested review from tiran

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting changes docs Documentation in the Doc dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@jakobandersen @the-knights-who-say-ni @bedevere-bot @vstinner @serhiy-storchaka @csabella
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.