Simplest update to bash completions to prevent unbounded variable errors
PATCH 1/1 Simplest update to bash completions to prevent unbounded variable errors
Another try at fixing bash completions in "set -u" environments.
Here, I've gone back to changing $# to ${#-}, but only where necessary.
Additionally added some comments and omitted things like Vim modelines.
--- contrib/completion/git-completion.bash | 42 ++++++++++++++++++++++--------- 1 files changed, 30 insertions(+), 12 deletions(-)
contrib/completion/git-completion.bash +30 -12
@@ -1,3 +1,4 @@ +#!bash # # bash completion support for core Git. #
@@ -50,9 +51,11 @@ case "$COMP_WORDBREAKS" in *) COMP_WORDBREAKS="$COMP_WORDBREAKS:" esac +# __gitdir accepts 0 or 1 arguments (i.e., location) +# returns location of .git repo __gitdir () { - if [ -z "$1" ]; then + if [ $# -eq 0 ] || [ -z "$1" ]; then if [ -n "$__git_dir" ]; then echo "$__git_dir" elif [ -d .git ]; then
@@ -67,6 +70,8 @@ __gitdir () fi } +# __git_ps1 accepts 0 or 1 arguments (i.e., format string) +# returns text to add to bash PS1 prompt (includes branch name) __git_ps1 () { local g="$(git rev-parse --git-dir 2>/dev/null)"
@@ -111,7 +116,7 @@ __git_ps1 () fi fi - if [ -n "$1" ]; then + if [ $# -gt 0 ] && [ -n "$1" ]; then printf "$1" "${b##refs/heads/}$r" else printf " (%s)" "${b##refs/heads/}$r"
@@ -119,6 +124,7 @@ __git_ps1 () fi } +# __gitcomp_1 requires 2 arguments __gitcomp_1 () { local c IFS=' '$'\t'$'\n'
@@ -131,6 +137,8 @@ __gitcomp_1 () done } +# __gitcomp accepts 1, 2, 3, or 4 arguments +# generates completion reply with compgen __gitcomp () { local cur="${COMP_WORDS[COMP_CWORD]}"
@@ -143,22 +151,23 @@ __gitcomp () ;; *) local IFS=$'\n' - COMPREPLY=($(compgen -P "$2" \ - -W "$(__gitcomp_1 "$1" "$4")" \ + COMPREPLY=($(compgen -P "${2-}" \ + -W "$(__gitcomp_1 "${1-}" "${4-}")" \ -- "$cur")) ;; esac } +# __git_heads accepts 0 or 1 arguments (to pass to __gitdir) __git_heads () { - local cmd i is_hash=y dir="$(__gitdir "$1")" + local cmd i is_hash=y dir="$(__gitdir "${1-}")" if [ -d "$dir" ]; then git --git-dir="$dir" for-each-ref --format='%(refname:short)' \ refs/heads return fi - for i in $(git ls-remote "$1" 2>/dev/null); do + for i in $(git ls-remote "${1-}" 2>/dev/null); do case "$is_hash,$i" in y,*) is_hash=n ;; n,*^{}) is_hash=y ;;
@@ -168,15 +177,16 @@ __git_heads () done } +# __git_tags accepts 0 or 1 arguments (to pass to __gitdir) __git_tags () { - local cmd i is_hash=y dir="$(__gitdir "$1")" + local cmd i is_hash=y dir="$(__gitdir "${1-}")" if [ -d "$dir" ]; then git --git-dir="$dir" for-each-ref --format='%(refname:short)' \ refs/tags return fi - for i in $(git ls-remote "$1" 2>/dev/null); do + for i in $(git ls-remote "${1-}" 2>/dev/null); do case "$is_hash,$i" in y,*) is_hash=n ;; n,*^{}) is_hash=y ;;
@@ -186,9 +196,10 @@ __git_tags () done } +# __git_refs accepts 0 or 1 arguments (to pass to __gitdir) __git_refs () { - local i is_hash=y dir="$(__gitdir "$1")" + local i is_hash=y dir="$(__gitdir "${1-}")" local cur="${COMP_WORDS[COMP_CWORD]}" format refs if [ -d "$dir" ]; then case "$cur" in
@@ -218,6 +229,7 @@ __git_refs () done } +# __git_refs2 requires 1 argument (to pass to __git_refs) __git_refs2 () { local i
@@ -226,6 +238,7 @@ __git_refs2 () done } +# __git_refs_remotes requires 1 argument (to pass to ls-remote) __git_refs_remotes () { local cmd i is_hash=y
@@ -470,6 +483,7 @@ __git_aliases () done } +# __git_aliased_command requires 1 argument __git_aliased_command () { local word cmdline=$(git --git-dir="$(__gitdir)" \
@@ -482,6 +496,7 @@ __git_aliased_command () done } +# __git_find_subcommand requires 1 argument __git_find_subcommand () { local word subcommand c=1
@@ -1766,13 +1781,16 @@ _gitk () __git_complete_revlist } -complete -o default -o nospace -F _git git -complete -o default -o nospace -F _gitk gitk +complete -o bashdefault -o default -o nospace -F _git git 2>/dev/null \ + || complete -o default -o nospace -F _git git +complete -o bashdefault -o default -o nospace -F _gitk gitk 2>/dev/null \ + || complete -o default -o nospace -F _gitk gitk # The following are necessary only for Cygwin, and only are needed # when the user has tab-completed the executable name and consequently # included the '.exe' suffix. # if [ Cygwin = "$(uname -o 2>/dev/null)" ]; then -complete -o default -o nospace -F _git git.exe +complete -o bashdefault -o default -o nospace -F _git git.exe 2>/dev/null \ + || complete -o default -o nospace -F _git git.exe fi
Signature · 1 line
1.6.1.87.g15624
Quoted 1 line · Ted Pavlic
Another try at fixing bash completions in "set -u" environments.
I agree with Junio; setting -u in your interactive shell is as bad as export CDPATH. Its crazy.
Quoted 1 line
Additionally added some comments and omitted things like Vim modelines.
These are orthogonal to the -u corrections. They should be in a different patch. The comments are wecome. The '#!bash' looks like a good idea. But a vim specific modeline, I don't like, for the reasons Junio has already stated.
contrib/completion/git-completion.bash case "$COMP_WORDBREAKS" in · line 58
Quoted 6 lines
contrib/completion/git-completion.bash +3
-1+# __gitdir accepts 0 or 1 arguments (i.e., location) +# returns location of .git repo __gitdir () { - if [ -z "$1" ]; then + if [ $# -eq 0 ] || [ -z "$1" ]; then
This is one of those places where [ -z "${1-}" ] is likely easier to read then the || usage you have introduced. We don't care if we got no args, or we got one that is the empty string, either way the $1 cannot be a gitdir and we need to guess it.
contrib/completion/git-completion.bash __git_ps1 () · line 120
Quoted 7 lines
contrib/completion/git-completion.bash +1
-1@@ -111,7 +116,7 @@ __git_ps1 () fi fi - if [ -n "$1" ]; then + if [ $# -gt 0 ] && [ -n "$1" ]; then printf "$1" "${b##refs/heads/}$r"
Eh, I'd rather see [ -n "${1-}" ] over the && test.
contrib/completion/git-completion.bash _gitk () · line 1787
Quoted 6 lines
contrib/completion/git-completion.bash +4
-2-complete -o default -o nospace -F _git git -complete -o default -o nospace -F _gitk gitk +complete -o bashdefault -o default -o nospace -F _git git 2>/dev/null \ + || complete -o default -o nospace -F _git git +complete -o bashdefault -o default -o nospace -F _gitk gitk 2>/dev/null \ + || complete -o default -o nospace -F _gitk gitk
Why are we switching to bashdefault? Is this an unrelated change from the -u stuff and should go into its own commit, with its own justification?
Signature · 1 line
Shawn.
Quoted 3 lines
Quoted 1 line
Another try at fixing bash completions in "set -u" environments.
I agree with Junio; setting -u in your interactive shell is as bad as export CDPATH. Its crazy.
This whole series of patches was inspired by a group of workstations at a university that set -u by default for all users.
Additionally, doesn't "set -u" make tcsh users feel more at home in bash? Certainly other shells have this same behavior in their interactive modes.
Quoted 6 lines
Quoted 1 line
Additionally added some comments and omitted things like Vim modelines.
These are orthogonal to the -u corrections. They should be in a different patch. The comments are wecome. The '#!bash' looks like a good idea. But a vim specific modeline, I don't like, for the reasons Junio has already stated.
OK. Can do.
Quoted 4 lines
contrib/completion/git-completion.bash case "$COMP_WORDBREAKS" in · line 58
Quoted 2 lines
contrib/completion/git-completion.bash +1
-1- if [ -z "$1" ]; then + if [ $# -eq 0 ] || [ -z "$1" ]; thenThis is one of those places where [ -z "${1-}" ] is likely easier
That was a mistake. I missed that hunk. I meant to use the ${1-}.
Quoted 3 lines
Review
Quoted 1 line
(quoted diff) +1
-0+ if [ $# -gt 0 ]&& [ -n "$1" ]; thenEh, I'd rather see [ -n "${1-}" ] over the&& test.
Again, my mistake. It was late and I missed it.
Quoted 8 lines
contrib/completion/git-completion.bash _gitk () · line 1787
Quoted 4 lines
contrib/completion/git-completion.bash +4
-0+complete -o bashdefault -o default -o nospace -F _git git 2>/dev/null \ + || complete -o default -o nospace -F _git git +complete -o bashdefault -o default -o nospace -F _gitk gitk 2>/dev/null \ + || complete -o default -o nospace -F _gitk gitkWhy are we switching to bashdefault? Is this an unrelated change from the -u stuff and should go into its own commit, with its own justification?
Ok.
From what I understand, normal bash completion is like setting "-o bashdefault -o default". That is, it tries the bash completions first before going to the filename completion. This change makes it so that git jumps back to bash completion if nothing git-specific is found. If nothing bash-specific is found, it will go back to standard default filename completion.
--Ted
Signature · 5 lines
Ted Pavlic <[email protected]> Please visit my ALS association page: http://web.alsa.org/goto/tedpavlic My family appreciates your support in the fight to defeat ALS.
Quoted 6 lines · Ted Pavlic
Quoted 3 lines
Quoted 1 line
Another try at fixing bash completions in "set -u" environments.
I agree with Junio; setting -u in your interactive shell is as bad as export CDPATH. Its crazy.
This whole series of patches was inspired by a group of workstations at a university that set -u by default for all users.
The changes look less nasty than I originally thought. If you can split the history out and justify the changes in the corresponding commit messages, I think I can ACK the series.
Signature · 1 line
Shawn.