-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
fix(bash) Less false positives for keywords in arguments ie, --keyword-flag
#2669
Conversation
Yes, nice! Perhaps add an example of one or two of the |
--keyword-flag
I just got to adding such a test, and I was surprised to see that it failed:
became
I'm reading and trying to understand why this is happening, but I'm not sure I'll be able to figure it out. |
Actually, I'm not sure if that's a failure or not? These terms (e.g. |
Hmm... Options (like |
@egor-rogov Creating a new issue so this doesn't get bogged down in that. @sirosen Thanks for looking into. :-) And thanks for contributing! It's truly appreciated. |
@sirosen Can you please update CHANGES.md and then we can get this merged? |
In bash, option, param, and command names may contain builtins as hyphen-delimited components. As in `foo --set-bar`. In order to handle this, just add `-` to the pattern's set of word characters. Includes a new test for bash tokens containing keywords, which checks hyphens and underscores for a few common cases.
3ecaa3c
to
79d5452
Compare
I pushed a commit with the changelog update, but also did a squash-rebase so that it will merge cleanly. |
For future, don't bother we almost always squash commits when merging anyways. |
In bash, option, param, and command names may contain builtins as hyphen-delimited components. As in
foo --set-bar
.In order to handle this, just add
-
to the pattern's set of word characters. Includes a new test for bash tokens containing keywords, which checks hyphens and underscores for a few common cases.Against current
master
, the new test fails, which helps to confirm the fix.resolves #2668
Aside: I really have to compliment you guys on the structure of the testsuite. Even with my very limited knowledge in this domain, adding a new failing test case and verifying it was a breeze. Thanks to everyone who works on this project!