Skip to content

gh-113317: Pass clinic to AC parse_arg() methods - #114319

Closed
vstinner wants to merge 1 commit into
python:mainfrom
vstinner:ac_pass_clinic
Closed

gh-113317: Pass clinic to AC parse_arg() methods#114319
vstinner wants to merge 1 commit into
python:mainfrom
vstinner:ac_pass_clinic

Conversation

@vstinner

@vstinnervstinner commented Jan 19, 2024

Copy link
Copy Markdown
Member

Add 'clinic' parameter to Argument Clinic parse_arg() and bad_argument() methods. Remove usage of the 'clinic' global variable in these methods.

Add 'clinic' parameter to Argument Clinic parse_arg() and
bad_argument() methods. Remove usage of the 'clinic' global variable
in these methods.
@vstinner

Copy link
Copy Markdown
MemberAuthor

Currently, it is possible to get limited_capi from clinic.limited_capi. But I'm not sure how the code is going to be refactored later, so I prefer to keep the two parameters for now: limited_capi and clinic.

@erlend-aasland

Copy link
Copy Markdown
Contributor

Sorry, I disagree with this approach 🙂 It would be better to teach CConverter.add_include to handle multiple includes per converter, and then in output_templates() fetch all includes from all the converters and add them via clinic.add_include(). If we teach CConverter.add_include that trick, we won't need clinic in bad_argument. Also, that will result in a much smaller diff! What do you think?

@vstinner

Copy link
Copy Markdown
MemberAuthor

It would be better to teach CConverter.add_include to handle multiple includes per converter, and then in output_templates() fetch all includes from all the converters and add them via clinic.add_include().

Hum, that's strange: I already wrote an add_include() method in CConverter. In fact, bad_argument() can just reuse it.

I wrote PR #114324 which is way shorter.

@vstinner

Copy link
Copy Markdown
MemberAuthor

I wrote PR #114324 which is way shorter.

Let's focus on this approach instead. I close this PR.

@vstinner
vstinner deleted the ac_pass_clinic branch January 19, 2024 17:24
Sign up for freeto 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.

2 participants

@vstinner@erlend-aasland