Skip to content

Fix parsing when a parameter is named help#3678

Open
kdeldycke wants to merge 1 commit into
pallets:mainfrom
kdeldycke:fix-help-name-collision
Open

Fix parsing when a parameter is named help#3678
kdeldycke wants to merge 1 commit into
pallets:mainfrom
kdeldycke:fix-help-name-collision

Conversation

@kdeldycke

Copy link
Copy Markdown
Collaborator

This PR fixes #2819 by having ou pre-configured --help parameter automatically change its internal name used to store its value, in the case of a user providing its own argument or option conflicting with ours.

Note that I arbitrarily used a convention of adding underscores _ to that internal name until no other parameter conflict with it. I'm ready to change that strategy if it is does not look good.

@kdeldycke kdeldycke added this to the 8.5.0 milestone Jul 8, 2026
@kdeldycke kdeldycke added the f:parameters feature: input parameter types label Jul 8, 2026
@kdeldycke kdeldycke requested review from Rowlando13 and davidism July 8, 2026 12:19
@kdeldycke kdeldycke added the bug label Jul 8, 2026
@kdeldycke kdeldycke force-pushed the fix-help-name-collision branch from 44f9b05 to bb8244c Compare July 8, 2026 12:22
@kdeldycke kdeldycke added f:help feature: help text and removed f:parameters feature: input parameter types labels Jul 8, 2026
@kdeldycke kdeldycke force-pushed the fix-help-name-collision branch 2 times, most recently from 775a1cf to ee8a410 Compare July 8, 2026 16:18
@davidism

davidism commented Jul 8, 2026

Copy link
Copy Markdown
Member

Perhaps we could just always force an internal name, like _click_default_help?

@Rowlando13

Copy link
Copy Markdown
Member

I think that is probably best.

@kdeldycke

Copy link
Copy Markdown
Collaborator Author

Perhaps we could just always force an internal name, like _click_default_help?

Yeah why not! Need to check if there is a nasty case in which a user can create its own argument or option that get assigned this internal ID. 😈

Wait. Hold on to that PR. I might have found that this issue not only affects --help but any parameter... 🤔 Let me verify that...

@kdeldycke kdeldycke marked this pull request as draft July 9, 2026 09:18
Fix parsing when a parameter is named `help`, closing pallets#2819
@kdeldycke kdeldycke force-pushed the fix-help-name-collision branch from ee8a410 to 3495fba Compare July 9, 2026 12:05
@kdeldycke

Copy link
Copy Markdown
Collaborator Author

I just changed the strategy to always naming Click's own --help with the _click_default_help constant. And also make the name collision detection more generic and systematic: it now warns at the parameters fetching level. This greatly simplifies the tests and code.

The only compromise: the public to_info_dict() of --help now returns unconditionally the _click_default_help storage ID instead of help. I can restore part of my strategy of only adopting _click_default_help dynamically for --help if a colliding argument is detected.

@kdeldycke kdeldycke marked this pull request as ready for review July 9, 2026 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug f:help feature: help text

Projects

None yet

Development

Successfully merging this pull request may close these issues.

help not resolving automatically

3 participants