Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@xxxxxxxxx> writes:
>> @@ -2181,7 +2181,7 @@ _git ()
>> c=$((++c))
>> done
>>
>> - if [ -z "$command" ]; then
>> + if [ -z "${command-}" ]; then
>> case "${COMP_WORDS[COMP_CWORD]}" in
>> --*) __gitcomp "
>> --paginate
>
> Why not this patch, instead of the above hunk?
My mistake. The original patch was as you suggest, then I changed it
to the above to make it follow the style of surrounding code.
If one makes it a goal to lose the ${foo-} lines:
- In __gitdir, [ -z "${1-}" ] should be [ $# -eq 0 ]
- In __gitdir callers, "${1-}" should be replaced by "$@"
- In _git, __git_dir should be initialized to ""
- In _gitk and __git_ps1, __git_dir should be made local and
initialized (independent bugfix)
- In __git_ps1, the code examining $GIT_PS1_DESCRIBE_STYLE
should be protected by an if [ -n "${GIT_PS1_DESCRIBE_STYLE:+set}" ]
- The checks for GIT_PS1_SHOWDIRTYSTATE and GIT_PS1_SHOWUNTRACKEDFILES
would also be adjusted
- __gitcomp should get some local variables set to "" and then
conditionally set to its arguments
- "${1-}" in __git_heads and __git_tags should be "$dir"
These look like good things to do anyway. Assuming your change
has already been made, the follow-up patch would look something
like this.
-- %< --
Subject: bash-completion: Avoid using uninitialized values
The "${var-}" idiom is obscure and makes it easy to hide uses of
values not initialized by the completion script that could have leaked
from the user’s environment.
Untested. The only intentional change in functionality from this
patch is that __git_dir has been made local to _gitk() and __git_ps1.
Signed-off-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
---
contrib/completion/git-completion.bash | 52 +++++++++++++++++++++-----------
1 files changed, 34 insertions(+), 18 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 2682f52..e291d8d 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -63,8 +63,8 @@ esac
# returns location of .git repo
__gitdir ()
{
- if [ -z "${1-}" ]; then
- if [ -n "${__git_dir-}" ]; then
+ if [ $# -eq 0 ]; then
+ if [ -n "$__git_dir" ]; then
echo "$__git_dir"
elif [ -d .git ]; then
echo .git
@@ -82,6 +82,7 @@ __gitdir ()
# returns text to add to bash PS1 prompt (includes branch name)
__git_ps1 ()
{
+ local __git_dir=""
local g="$(__gitdir)"
if [ -n "$g" ]; then
local r=""
@@ -108,18 +109,20 @@ __git_ps1 ()
fi
b="$(git symbolic-ref HEAD 2>/dev/null)" || {
-
- b="$(
- case "${GIT_PS1_DESCRIBE_STYLE-}" in
+ if [ -z "${GIT_PS1_DESCRIBE_STYLE:+set}" ] ||
+ [ "$GIT_PS1_DESCRIBE_STYLE" = default ]; then
+ b=$(git describe --exact-match HEAD 2>/dev/null)
+ else b=$(case "$GIT_PS1_DESCRIBE_STYLE" in
(contains)
git describe --contains HEAD ;;
(branch)
git describe --contains --all HEAD ;;
(describe)
git describe HEAD ;;
- (* | default)
+ (*)
git describe --exact-match HEAD ;;
- esac 2>/dev/null)" ||
+ esac 2>/dev/null)
+ fi ||
b="$(cut -c1-7 "$g/HEAD" 2>/dev/null)..." ||
b="unknown"
@@ -140,7 +143,7 @@ __git_ps1 ()
b="GIT_DIR!"
fi
elif [ "true" = "$(git rev-parse --is-inside-work-tree 2>/dev/null)" ]; then
- if [ -n "${GIT_PS1_SHOWDIRTYSTATE-}" ]; then
+ if [ -n "${GIT_PS1_SHOWDIRTYSTATE:+set}" ]; then
if [ "$(git config --bool bash.showDirtyState)" != "false" ]; then
git diff --no-ext-diff --quiet --exit-code || w="*"
if git rev-parse --quiet --verify HEAD >/dev/null; then
@@ -150,11 +153,11 @@ __git_ps1 ()
fi
fi
fi
- if [ -n "${GIT_PS1_SHOWSTASHSTATE-}" ]; then
+ if [ -n "${GIT_PS1_SHOWSTASHSTATE:+set}" ]; then
git rev-parse --verify refs/stash >/dev/null 2>&1 && s="$"
fi
- if [ -n "${GIT_PS1_SHOWUNTRACKEDFILES-}" ]; then
+ if [ -n "${GIT_PS1_SHOWUNTRACKEDFILES:+set}" ]; then
if [ -n "$(git ls-files --others --exclude-standard)" ]; then
u="%"
fi
@@ -183,18 +186,30 @@ __gitcomp_1 ()
# generates completion reply with compgen
__gitcomp ()
{
+ local replies=""
+ local pfx=""
local cur="${COMP_WORDS[COMP_CWORD]}"
+ local sfx=""
+ if [ $# -gt 3 ]; then
+ sfx=$4
+ fi
if [ $# -gt 2 ]; then
cur="$3"
fi
+ if [ $# -gt 1 ]; then
+ pfx=$2
+ fi
+ if [ $# -gt 0 ]; then
+ replies=$1
+ fi
case "$cur" in
--*=)
COMPREPLY=()
;;
*)
local IFS=$'\n'
- COMPREPLY=($(compgen -P "${2-}" \
- -W "$(__gitcomp_1 "${1-}" "${4-}")" \
+ COMPREPLY=($(compgen -P "$pfx" \
+ -W "$(__gitcomp_1 "$replies" "$sfx")" \
-- "$cur"))
;;
esac
@@ -203,13 +218,13 @@ __gitcomp ()
# __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 "$@")"
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 "$dir" 2>/dev/null); do
case "$is_hash,$i" in
y,*) is_hash=n ;;
n,*^{}) is_hash=y ;;
@@ -222,13 +237,13 @@ __git_heads ()
# __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 "$@")"
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 "$dir" 2>/dev/null); do
case "$is_hash,$i" in
y,*) is_hash=n ;;
n,*^{}) is_hash=y ;;
@@ -241,7 +256,7 @@ __git_tags ()
# __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 "$@")"
local cur="${COMP_WORDS[COMP_CWORD]}" format refs
if [ -d "$dir" ]; then
case "$cur" in
@@ -2167,7 +2182,7 @@ _git_tag ()
_git ()
{
- local i c=1 command="" __git_dir
+ local i c=1 command="" __git_dir=""
while [ $c -lt $COMP_CWORD ]; do
i="${COMP_WORDS[c]}"
@@ -2265,6 +2280,7 @@ _gitk ()
{
__git_has_doubledash && return
+ local __git_dir=""
local cur="${COMP_WORDS[COMP_CWORD]}"
local g="$(__gitdir)"
local merge=""
--
1.7.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html