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 completions for ninja build system #3415
Conversation
@@ -0,0 +1,5 @@ | |||
function __fish_print_ninja_targets | |||
if [ -f build.ninja ] | |||
ninja -t targets | sed 's/:.*//' |
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.
What happens here when you call ninja without that file?
If it's just some error message, it's probably easier to suppress it so this can be used even if there's some other filename that ninja can use (now or in the future).
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.
Also, we try to use our string
tool in our completions - string replace -r ':.*' ''
.
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.
You mean something like
...
ninja -t targets | string replace -r ':.*' '' 2> /dev/null
?
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.
With the redirection after the ninja
, not the string
call. Otherwise yes.
@@ -0,0 +1,4 @@ | |||
function __fish_print_ninja_tools | |||
echo list | |||
ninja -t list | grep -v ':' | sed -Ee 's/[[:space:]]+([^[:space:]]+).*/\1/' |
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.
As above, string
is your friend here - string match -v ':' | string replace -r '\s+(\S+).*' '$1'
(untested).
The "-E" option to sed is problematic as it's completely undocumented for GNU sed (though it appears to 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.
fixed, I've squashed it back to 1 commit
f3821b1
to
a545cd2
Compare
Merged, thanks! |
Description
https://ninja-build.org/
Tested on ubuntu 16.{04,10} & OS X 10.11