Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32k
gh-101100: Clean up Doc/c-api/exceptions.rst and Doc/c-api/sys.rst#114825
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
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.
If you want to specify their declaration, you can simply use typedef-like syntax.
I would move thePy_AuditHookFunction definition directly intoPySys_AddAuditHook definition where it is only used. You need to add.. c:namespace:: NULL before it to avoid adding a prefix.
ThePyOS_sighandler_t definition can be moved closer toPyOS_getsig() orPyOS_setsig() definitions. Duplicated description ofPyOS_sighandler_t can be removed.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Thanks@serhiy-storchaka, I believe I folded in your suggestions in a reasonable way. |
Uh oh!
There was an error while loading.Please reload this page.
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.
It deviated significantly from initial "clean up Doc/c-api/exceptions.rst", isn't? 😉 Maybe split it on several PRs? The auditing part is not trivial.
LGTM after you do something with duplication forPy_AuditHookFunction. For example:
..c:namespace::NULL ..c:type::int (*Py_AuditHookFunction) (constchar *event, PyObject *args, void *userData) The type of the hook function. *event* is (the event argument passed to PySys_Audit() or PySys_AuditTuple()), *args* is (...), *userData* is (the argument passed to PySys_AddAuditHook()). The hook function is always called ... See:pep:`578` for ...
Feature creep is no problem. I'm retired, so have plenty of time to mess around. ;-) Since it seems like I might have to write an actual sentence to flesh out this paragraph, I'm going to spend a bit of time reading PEP 578, but I'll have something later today. |
Doc/c-api/sys.rst Outdated
| .. c:type:: int (*Py_AuditHookFunction) (const char *event, PyObject *args, void *userData) | ||
| The type of the hook function. | ||
| *event* is (the event argument passed to PySys_Audit() or PySys_AuditTuple()), |
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.
Wait, I used parentheses to denote that it is a preliminary text, and that you should rewrite it in good English. Perhaps it is even better to describe parameters not in terms of PySys_Audit() or PySys_AuditTuple(), but in more general terms. Look atsys.audit(). Perhaps "event is a C string identifying the event" is enough.
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.
Ah, I recognized "(...)" as a placeholder, not the more elaborate parenthesized phrasing. Will get back to it.
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.
Perhaps "event is a C string identifying the event" is enough.
I thought about that, but am not familiar enough with the C API documentation to know that descriptions of function prototypes don't simply rely on the actual prototype.
Uh oh!
There was an error while loading.Please reload this page.
Doc/c-api/sys.rst Outdated
| .. c:namespace:: NULL | ||
| .. c:type:: int (*Py_AuditHookFunction) (const char *event, PyObject *args, void *userData) |
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.
If it is not nested,c:namespace is not needed. I would prefer to make it nested, but it is only my personal preference.
Doc/c-api/sys.rst Outdated
| If the interpreter is initialized, this function raisesan auditing event | ||
| ``sys.addaudithook`` with no arguments. If any existing hooks raise an | ||
| exception derived from :class:`Exception`, the new hook will not be | ||
| added and the exception is cleared. As a result, callers cannot assume | ||
| that their hook has been added unless they control all existing hooks. |
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.
Why is it dedented? It belongs to theaudit-event directive.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Sorry, I clearly don't understand the functional implications of the indentation. See if this latest version is more correct. |
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.
LGTM.
Thanks@smontanaro for the PR, and@serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12. |
…rst (pythonGH-114825)(cherry picked from commite1552fd)Co-authored-by: Skip Montanaro <skip.montanaro@gmail.com>
Sorry,@smontanaro and@serhiy-storchaka, I could not cleanly backport this to |
GH-115308 is a backport of this pull request to the3.12 branch. |
@serhiy-storchaka I'll take care of the 3.11 conflict (I need the practice). |
…rst (pythonGH-114825)(cherry picked from commite1552fd)
GH-115311 is a backport of this pull request to the3.11 branch. |
Uh oh!
There was an error while loading.Please reload this page.
This PR cleans up several bits in
Doc/c-api/exceptions.rst:warnings.WarningMessage. It's not actually documented and isn't exposed bywarnings.__all__. It's just a basic exception carrying around a message.__module__attribute. The good stuff is in the containing paragraph.USE_STACKCHECK. The good stuff is in the documentation ofPyOS_CheckStack, which is linked from the containing paragraph.PyExc_OSError.Since I was hunting down
USE_STACKCHECK, I went ahead and suppressed the reference inDoc/c-api/sys.rst, in the documentation forPyOS_CheckStack.I updated
Doc/tools/.nitignoreto removeDoc/c-api/exceptions.rst, but left an entry forDoc/c-api/sys.rst, as I didn't make an attempt to address any other problems it might have.📚 Documentation preview 📚:https://cpython-previews--114825.org.readthedocs.build/