Skip to content

fix: avoid executing actions when completing -- in v2#2379

Open
happysnaker wants to merge 2 commits into
urfave:v2-maintfrom
happysnaker:fix-1993-v2-double-dash-completion
Open

fix: avoid executing actions when completing -- in v2#2379
happysnaker wants to merge 2 commits into
urfave:v2-maintfrom
happysnaker:fix-1993-v2-double-dash-completion

Conversation

@happysnaker

Copy link
Copy Markdown

What type of PR is this?

  • bug

What this PR does / why we need it:

Prevents --generate-bash-completion from falling back to normal command execution when the user is completing after -- in v2.

The change keeps the existing v2 behavior that disables completion after a positional separator that already appeared earlier, but trims the completion flag before normal dispatch so the command action is not executed accidentally.

It also preserves completing -- itself, which allows the shell script to request long-flag suggestions without running the action.

Which issue(s) this PR fixes:

Fixes #1993

Special notes for your reviewer:

I kept this intentionally minimal for v2-maint:

  • no shell script changes
  • no behavior change for arguments after an earlier --
  • added regression coverage for both the helper and App.Run

Testing

Added regression tests for:

  • checkShellCompleteFlag when -- is the token being completed
  • checkShellCompleteFlag when -- already appeared before completion
  • App.Run to ensure double-dash completion does not execute the action

Release Notes

Prevent v2 shell completion from executing command actions when completing after `--`.

@happysnaker happysnaker requested a review from a team as a code owner June 30, 2026 10:13
@happysnaker

Copy link
Copy Markdown
Author

Quick follow-up after checking the latest PR runs: the behavior change itself is covered by the new regression tests and the substantive remaining failure is the existing binary-size gate on the v2 test workflow (2.3MB vs current 2.2MB max). The separate lint failure appears to come from the runner/toolchain mismatch (file requires newer Go version go1.26 (application built with go1.24)), which matches what I saw on the previous revision as well.

If you would prefer, I can try a follow-up commit that trims this patch further or moves the guard/test shape around to see whether it avoids the binary-size bump.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant