-
Notifications
You must be signed in to change notification settings - Fork 391
fix(bash_completion): use _comp_split
and new _comp_compgen
for safer splitting
#919
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
Conversation
fa65c68
to
af57fd5
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.
_tilde
and _known_hosts_real
fixes cherry-picked, will revisit the main content of this later. (As a first impression, I like the idea.)
7f2168f
to
9c92730
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.
Oh yes, this is great, thanks! Left some nitty comments, pre-approved with them addressed/not as you like.
Co-authored-by: Ville Skyttä <ville.skytta@iki.fi>
cdeda74
to
eaeb53e
Compare
Co-authored-by: Ville Skyttä <ville.skytta@iki.fi>
Co-authored-by: Ville Skyttä <ville.skytta@iki.fi>
We allow positional parameters ($1...) in the current word specified as `-- CUR` at the end of the argument list.
We needed to adjust IFS for splitting before, but we do not need it now because we switched to `_comp_split`. IFS is anyway adjusted by callers `_parse_help` and `_part_usage`. Co-authored-by: Ville Skyttä <ville.skytta@iki.fi>
Added https://github.com/scop/bash-completion/projects/2#card-89052206 for the completions/* work, perhaps that's something we could try to do for 2.12. |
Now we have a utility function
_comp_split
to safely split strings (including the output of thecompgen
builtin and other commands) without being affected by pathname expansions, unexpected word splitting, etc. I would like to switch from the repeatedlocal IFS=... reset=$(shopt -po noglob); set -o noglob; ...; IFS=$' \t\n'; $reset
to_comp_split
._comp_split
to return whether nonzero elements are produced in the exit status. This is because((${#arr[@]}))
is tested in many cases after splitting strings._comp_split -l arr "$(compgen ARGS)"
, so it is useful to define a function_comp_compgen arr ARGS
(in particular to reduce nested quoting and command substitutions)._comp_compgen
, etc.The changes suggested in this PR are actually the ones that I mentioned in the following PRs before.
compgen -W '${arr[@]}'
#887 (comment)The changes to
completions/*
are left for later PRs.