Skip to content
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

gh-113317: Don't use global clinic instance in bad_argument() #114330

Merged
merged 2 commits into from
Jan 23, 2024

Conversation

erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented Jan 19, 2024

Make it possible for a converter to have multiple includes.
Also make the 'includes' an instance attribute. This implies
some converter includes are added to the converters during
template generation (parse_arg()), so we have to query the
converters about their includes at the end of the template
generation.

Make it possible for a converter to have multiple includes, make the
'includes' an instance attribute. This implies converter includes are
added during template generation, so we have to add them to the clinic
instance at the end of the template generation instead of in the
beginning.
@erlend-aasland
Copy link
Contributor Author

Competing PR to #114324 😎⚔️

@erlend-aasland
Copy link
Contributor Author

Post this, the global clinic instance is only used by warn/fail. Poke, poke Argument-Clinic#35.

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Tools/clinic/clinic.py Outdated Show resolved Hide resolved
@erlend-aasland
Copy link
Contributor Author

Thanks for the review, Victor. I'll wait a bit for Alex to chime in before landing.

@erlend-aasland
Copy link
Contributor Author

I'm going to land this tomorrow.

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks! Sorry for the delay!

@erlend-aasland erlend-aasland merged commit e14930f into python:main Jan 23, 2024
33 checks passed
@erlend-aasland erlend-aasland deleted the clinic/converter-includes branch January 23, 2024 10:07
@erlend-aasland
Copy link
Contributor Author

Thanks, both!

@vstinner
Copy link
Member

One less usage of globals!

aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
…ython#114330)

Make it possible for a converter to have multiple includes, by collecting
them in a list on the converter instance. This implies converter includes
are added during template generation, so we have to add them to the
clinic instance at the end of the template generation instead of in the
beginning.
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
…ython#114330)

Make it possible for a converter to have multiple includes, by collecting
them in a list on the converter instance. This implies converter includes
are added during template generation, so we have to add them to the
clinic instance at the end of the template generation instead of in the
beginning.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants