Repository navigation
gh-145868: Suggest names on typos in __future__ imports - #154347
Conversation
| FUTURE_GENERATOR_STOP, | ||
| FUTURE_ANNOTATIONS, | ||
| FUTURE_BRACES); | ||
| if ( |
There was a problem hiding this comment.
Nit: Maybe we can hoist the call out of the condition:
PyObject *suggestion = NULL;
PyObject *future_features = Py_BuildValue(...);
if (future_features != NULL) {
suggestion = _Py_CalculateSuggestions(future_features, name->name);
Py_DECREF(future_features);
}
if (suggestion != NULL) {
...
}This also lets us drop the Py_XDECREF at the end.
| #define FUTURE_BARRY_AS_BDFL "barry_as_FLUFL" | ||
| #define FUTURE_GENERATOR_STOP "generator_stop" | ||
| #define FUTURE_ANNOTATIONS "annotations" | ||
| #define FUTURE_BRACES "braces" |
There was a problem hiding this comment.
Do we need to expose FUTURE_BRACES in the public header? It is only used in Python/future.c and braces is an easter egg, not a real feature, so maybe we can keep the define local to future.c instead of growing the C API surface.
| FUTURE_BARRY_AS_BDFL, | ||
| FUTURE_GENERATOR_STOP, | ||
| FUTURE_ANNOTATIONS, | ||
| FUTURE_BRACES); |
There was a problem hiding this comment.
I am not sure we want to suggest braces here: it is not in __future__.all_feature_names, so following the suggestion just gives not a chance. I am fine keeping it as part of the joke, but wanted to double check this is intentional.
| @@ -0,0 +1,2 @@ | |||
| Calculate suggestions during :exc:`SyntaxError` on typos when importing | |||
There was a problem hiding this comment.
Small nit: the wording reads a bit odd. Maybe something like "Suggest the closest valid feature name in the :exc:SyntaxError message when an unknown name is imported from :mod:__future__."
| PyErr_Format(PyExc_SyntaxError, | ||
| UNDEFINED_FUTURE_FEATURE, feature); | ||
| PyObject *suggestion; | ||
| PyObject *future_features = Py_BuildValue("[sssssssssss]", |
There was a problem hiding this comment.
Nit: this list duplicates the strcmp chain above, so whoever adds a new future feature can easily forget to update it and the new name will silently get no suggestion. Can you add a small comment saying both need to be kept in sync?
|
These suggestions are a nice touch, @sobolevn. Thank you very much! |
|
Thank you for the help on getting this merged! I am comletely overwhelmed by other work 🫶 |
from __future__ import#145868