-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Add test script to compare rg --help
output to zsh completion function
#540
Conversation
39b36e6
to
8eb2289
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks pretty good! And yeah, I don't think I care too much about korn shell compatibility. :-) (Note that I am not the originator of these shell scripts.) If you wanted to make this #!/bin/bash
to be more precise, then that's fine!
To add this to CI, I think it needs to go in ci/script.sh
, which is run by Travis. Once you do that, it should be testable by pushing to this PR.
set -e | ||
|
||
main() { | ||
local rg="target/${TARGET}/release/rg" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably can't do this exact thing. This would at, the very least, depend on ripgrep being built before these tests are run. I think we could probably do it by using a debug build, but right now, a release build is not built in CI. It might be easier to just use grep
. :P (I think you could set rg=grep
and it will just work.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh hmm I see you're using the --replace
flag. If you just change rg
to target/${TARGET}/debug/rg
, then I think that should be OK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, i could easily have replicated most of the find/replace stuff with grep
and/or sed
, but i figured that since i needed rg
for the --help
output anyway i might as well just use it for everything
8eb2289
to
ea7e22c
Compare
Updated the Would you like me to add completion for |
Whichever is most convenient for you! I'm find smushing it into this one via a separate commit. |
2a1f00b
to
6142174
Compare
Done, seems like everything's passing now! |
rg --help
output to zsh completion functionrg --help
output to zsh completion function
@okdana You're awesome. :-) Thanks! |
Per our discussion in #536: Here's something to hopefully prevent the zsh completion function from getting out-of-synch with the actual app.
I used a slightly different strategy from the one i mentioned before, but it's the same basic idea — pull all of the options out of the
--help
output, pull all of the options out of the completion function, compare the two. I tried to address your concerns about false positives/negatives as best i could; i've documented the assumptions i make in the script, and i think it should be OK as long as nobody does anything bizarre.I've tested this in
dash
,bash
, andzsh
. It also works inksh93
if you makelocal
an alias fortypeset
... but you uselocal
in your other scripts so i assumeksh
compatibility isn't a big concern.Anyway, it turns out actually that the
-f
option is missing from the completion function, which is fortuitous i guess because it seems to demonstrate that the script behaves as expected! Here's the output:Unless you have concerns with it i think i'm happy with the script itself. I'm not sure how to go about integrating it with your other tests though. Should i just add it to
appveyor.yml
or does it belong somewhere else?