gh-115172: Fix explicit index extries for the C API #115173
Conversation
serhiy-storchaka
added
docs
|
@smontanaro, could you please look at it? |
|
@serhiy-storchaka I've added your fork as an(other) upstream and have your Edit: ignore this:
(In retrospect, I think my comment was pretty much worthless. It turns out that strikethrough markup doesn't seem to be supported here...) |
| single: T_STRING | ||
| single: T_STRING_INPLACE | ||
| single: T_OBJECT_EX | ||
| single: T_BYTE (C macro) |
There was a problem hiding this comment.
Shouldn't all of these have a Py_ prefix?
There was a problem hiding this comment.
These are legacy names, which require including <structmembers.h>. There are also aliases with the Py_ prefix.
There was a problem hiding this comment.
Okay. Again, it would be nice to have some way to identify them as legacy/deprecated/discouraged names.
| single: READ_RESTRICTED | ||
| single: WRITE_RESTRICTED | ||
| single: RESTRICTED | ||
| single: READ_RESTRICTED (C macro) |
There was a problem hiding this comment.
Just wondering... would it make sense to indicate in the index that these *_RESTRICTED macros are deprecated? For example:
single: READ_RESTRICTED (deprecated C macro)
single: WRITE_RESTRICTED (deprecated C macro)
single: RESTRICTED (deprecated C macro)
If so, this might well be applied in many other places.
There was a problem hiding this comment.
If we add
.. c:macro:: READ_RESTRICTEDit will add index entry for "READ_RESTRICTED (C macro)". No way to add "deprecated" here (unless we write our own Sphinx extensions). And some of deprecated names are declared in such way, so we will have inconsistency,
Also, I think that an index entry is just an index entry. It contains the name and maybe some qualifier to distinguish it from the same name in other namespaces. But there is no non-deprecated C macro with the same name.
There was a problem hiding this comment.
These index entries were added manually, right? Can't they just be edited right where they sit? That way, the word "deprecated" will be part of the index entry, subtly discouraging people from using them in their (new) code. I wasn't thinking of adding a c:macro directive.
There was a problem hiding this comment.
But if we have two deprecated names, and one of them is declared with with the c:macro directive, the index entries will be:
BAR (C macro)
BAZ (deprecated C macro)
Both names are deprecated, but only one of them has an index entry that says this. Would not it confuse users?
I think that the index is not a right place for this. It is just an index, and you have to click the link to get the information.
There was a problem hiding this comment.
That's fine. Was just tossing the idea out there.
|
|
|
Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12. |
|
Sorry, @serhiy-storchaka, I could not cleanly backport this to |
|
Sorry, @serhiy-storchaka, I could not cleanly backport this to |
…honGH-115173) (cherry picked from commit 573acb3) Co-authored-by: Serhiy Storchaka <[email protected]>
|
GH-115292 is a backport of this pull request to the 3.12 branch. |
…honGH-115173) (cherry picked from commit 573acb3) Co-authored-by: Serhiy Storchaka <[email protected]>
|
GH-115293 is a backport of this pull request to the 3.11 branch. |
📚 Documentation preview 📚: https://cpython-previews--115173.org.readthedocs.build/