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.

Sign up for GitHub

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

Jump to bottom

fix: disable shell completion if double dash is included in arguments (v3) #1933

Merged
merged 3 commits into from
Jun 29, 2024
Merged

fix: disable shell completion if double dash is included in arguments (v3) #1933

merged 3 commits into from
Jun 29, 2024
+74 −0

Conversation

suzuki-shunsuke
Copy link
Contributor

What type of PR is this?

(REQUIRED)

  • bug

What this PR does / why we need it:

(REQUIRED)

This pull request disables shell completion if double dash -- is included in arguments.
Arguments after double dash -- are not flags but positional arguments.

https://unix.stackexchange.com/a/11382

Which issue(s) this PR fixes:

(REQUIRED)

Fixes #1932

Special notes for your reviewer:

(fill-in or delete this section)

Testing

(fill-in or delete this section)

Release Notes

(REQUIRED)

@suzuki-shunsuke
fix: disable shell completion if double dash is included in arguments
1b2854e 
Arguments after double dash `--` are not flags but positional arguments.

https://unix.stackexchange.com/a/11382
@suzuki-shunsuke suzuki-shunsuke mentioned this pull request Jun 27, 2024
Closed
3 tasks
@dearchap
Copy link
Contributor

@suzuki-shunsuke Did you want this for v2 or v3 ? If you want it for v2 you need to base off v2-maint branch. Also I would suggest you add some test cases.

@suzuki-shunsuke
Copy link
Contributor Author

Thank you for your review.

suzuki-shunsuke Did you want this for v2 or v3 ? If you want it for v2 you need to base off v2-maint branch.

I think both v2 and v3 have this issue, so we should fix both v2 and v3.
I'll send a pull request to v2-maint too.

Also I would suggest you add some test cases.

Sure. I'll do it.

@dearchap
Copy link
Contributor

v2 is in maint mode. No new features

@suzuki-shunsuke
Copy link
Contributor Author

I think this is a bug fix rather than a new feature.
Are you not willing to accept this change to v2?

I'm using v2 because v3 is still alpha, but if v3 is stable I will upgrade v2 to v3.

@suzuki-shunsuke
Copy link
Contributor Author

suzuki-shunsuke commented Jun 29, 2024 •
edited
Loading

Also I would suggest you add some test cases.

a832345 I added tests.

The base branch is main, which means this pull request is for v3.

✅ I'll send another pull request for v2.

@suzuki-shunsuke suzuki-shunsuke marked this pull request as ready for review June 29, 2024 06:44
@suzuki-shunsuke suzuki-shunsuke requested a review from a team as a code owner June 29, 2024 06:44
This was referenced Jun 29, 2024
Merged
Open
@suzuki-shunsuke suzuki-shunsuke changed the title fix: disable shell completion if double dash is included in arguments fix: disable shell completion if double dash is included in arguments (v3) Jun 29, 2024
@dearchap dearchap merged commit fd760fc into urfave:main Jun 29, 2024
13 checks passed
@suzuki-shunsuke suzuki-shunsuke deleted the fix-shell-completion-with-double-dash branch June 29, 2024 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dearchap dearchap dearchap approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Shell completion works wrongly even if double dash -- are included in arguments
2 participants
@suzuki-shunsuke @dearchap