-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
gh-125843: indicate which C function caused a curses.error
#125844
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
base: main
Are you sure you want to change the base?
Conversation
…/error-type-125843
curses.error
curses.error
curses.error
curses.error
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.
Actually, there are some funcname
that should be corrected but I need to see how they should really be corrected.
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.
Thanks! This should add some clarity.
#define CURSES_ERROR_FORMAT "%s() returned ERR" | ||
#define CURSES_ERROR_VERBOSE_FORMAT "%s() (called by %s()) returned ERR" | ||
#define CURSES_ERROR_NULL_FORMAT "%s() returned NULL" | ||
#define CURSES_ERROR_NULL_VERBOSE_FORMAT "%s() (called by %s()) returned NULL" | ||
#define CURSES_ERROR_MUST_CALL_FORMAT "must call %s() first" |
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 don't think the names are too useful, given that most are used in the consolidated function, so they're meant be used only once.
* A NULL 'python_funcname' falls back to 'curses_funcname' and vice-versa. | ||
* If both names are NULL, the error message is 'catchall_ERR'. |
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.
Is it valid for python_funcname
to be NUL but curses_funcname
to be set?
I see only these cases:
- the function names are the same, or
PyCursesCheckERR
was called by the user (onlypython_funcname
is non-NULL) - they names are different (both are non-NULL)
PyCursesCheckERR
called by user with NULL (so both are NULL)
Is it worth an assert?
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.
Mmh. Semantically, yes. In practice no. Sorry, it was a relic of the previous implementatiom. So either python and curses names are the different, or only python function name is given (and matches that of curses).
I won't be able to put anything before the beta release but I don't mind delaying this one for 3.15. However if you find time, I'd be happy if you make the change!
static void | ||
PyCursesSetError_ForWin_From(PyCursesWindowObject *win, |
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.
Ideally, don't use Py
, the prefix for public API, for new static functions.
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.
Maybe I should just have a curses_set_error_*
prefix? I don't remember whether I unfortunately introduced this naming in this or in a previous PR :(
@@ -3408,7 +3564,7 @@ _curses_initscr_impl(PyObject *module) | |||
WINDOW *win; | |||
|
|||
if (curses_initscr_called) { | |||
wrefresh(stdscr); | |||
(void)wrefresh(stdscr); // TODO(picnixz): should we report an error? |
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.
Please open issues for TODOs.
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.
Sorry, I'll do so when I'm back (in 10 days)
curses
by indicating failed C function #125843📚 Documentation preview 📚: https://cpython-previews--125844.org.readthedocs.build/