-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
[SEVERE] Autocomplete after double dash (--) executing command action. #1993
Comments
@AaronLieb Yes this is similar to #1916 . I have a fix for this in v3. Let me see if I can backport to v2. Thanks for reporting |
No the fix is in PR #1919 . |
@AaronLieb Would you be able to test the PR in #1919 to see if it fixes your issue ? |
@AaronLieb I looked at v2 and its more complicated to fix there. Not sure if I have the bandwidth to do it for v2. If you end up doing a PR I will be glad to review. |
@dearchap I'll see if I have some time today to test the v3 fix and push out a v2 PR. My experience with this library and go experience in general is pretty limited, so if I push out a PR, feel free to give plenty of criticisms. |
@dearchap the latest v3 alpha version with the v3 fix has an unverified commit and was incorrectly added to go.pkg.dev. The creation date is incorrect, isn't marked as the latest version, and fails the checksum when running I'm working on the v2 fix |
@AaronLieb fixed the 3.0.0-alpha10 release issue. Please check |
@dearchap appears the checksum is still failing when running |
@meatballhat can you take a look ? |
If you want to use the latest import "github.com/urfave/cli/v3" and then Please let us know if this isn't working for you, or if you're unable to use this solution 🙇🏼 |
@AaronLieb I've removed v3.0.0-alpha10. Try this release v3.0.0-alpha9.2 |
@dearchap I was able to get alpha9.2 working Here are the behaviors I observed:
|
@AaronLieb Thanks for the feedback, I'll look into these issues asap. |
@AaronLieb Can you share your test code ? I have tests for exactly the scenarios that you described and everything is supposed to work. https://github.com/urfave/cli/blob/v3.0.0-alpha9.2/completion_test.go#L59 |
Hey, I just spend some time investigating this problem (completion not working for flags if I type
I think I disagree with PR #1938 – it essentially broke shell completion for flags. |
My urfave/cli version is
v2.27.5
Checklist
Dependency Management
Describe the bug
If Bash completion is enabled, and the user attempts to tab complete after a double dash (--), it will execute the normal command action.
To reproduce
Describe the steps or code required to reproduce the behavior
main.go
using the v2 zsh_autocomplete
Sourcing it as such in my ~/.zshrc
mkdir /tmp/cli_test/ && ls /tmp/cli_test
Dir is empty
Try autocomplete
Check dir
TestFile.out is present
Observed behavior
Due to the double dash (--), the
--generate-bash-completion
flag was treated as an argument, and the command was executed normally. This behavior is INCREDIBLY DANGEROUS as it can cause premature execution of a command, without even pressing enter!Expected behavior
There are several ways this could be handled.
The ideal solution would be autocompleting the list of long flags, instead of executing the normal command action
Another solution would be preventing the command from being executed and not autocompleting.
Additional context
This problem was introduced by this PR
#1938
I personally disagree with this PR being marked as a bug instead of a feature, and think that this behavior should be toggleable instead of enabled for all cli applications.
This PR did disable bash autocomplete after a double dash, but introduced a much worse issue, which is executing the command normally...
Want to fix this yourself?
If I find time, I may be willing to fix this myself, but I wanted to report this as soon as possible since this bug could cause severe damage in the worse case scenario.
Run
go version
and paste its output hereRun
go env
and paste its output hereThe text was updated successfully, but these errors were encountered: