* [PATCH 1/3] bash-completion: Support running when set -u is enabled
@ 2009-01-14 20:02 ted
2009-01-14 20:02 ` [PATCH 2/3] bash-completion: Try bash completions before simple filetype ted
2009-01-15 15:34 ` [PATCH 1/3] bash-completion: Support running when set -u is enabled Shawn O. Pearce
0 siblings, 2 replies; 6+ messages in thread
From: ted @ 2009-01-14 20:02 UTC (permalink / raw)
To: spearce; +Cc: git, gitster, Ted Pavlic
From: Ted Pavlic <ted@tedpavlic.com>
Under "set -u" semantics, it is an error to access undefined
variables. Some user environments may enable this setting in the
interactive shell.
In any context where the completion functions access an undefined
variable, accessing a default empty string (aka "${1-}" instead of
"$1") is a reasonable way to code the function, as it silences
the undefined variable error while still supplying an empty string.
In this patch, functions that should always take an argument still use
$1. Functions that have optional arguments use ${1-}.
Signed-off-by: Ted Pavlic <ted@tedpavlic.com>
Acked-by: Shawn O. Pearce <spearce@spearce.org>
---
contrib/completion/git-completion.bash | 18 +++++++++---------
1 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 7b074d7..5d1515c 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -52,7 +52,7 @@ esac
__gitdir ()
{
- if [ -z "$1" ]; then
+ if [ -z "${1-}" ]; then
if [ -n "$__git_dir" ]; then
echo "$__git_dir"
elif [ -d .git ]; then
@@ -111,7 +111,7 @@ __git_ps1 ()
fi
fi
- if [ -n "$1" ]; then
+ if [ -n "${1-}" ]; then
printf "$1" "${b##refs/heads/}$r"
else
printf " (%s)" "${b##refs/heads/}$r"
@@ -143,8 +143,8 @@ __gitcomp ()
;;
*)
local IFS=$'\n'
- COMPREPLY=($(compgen -P "$2" \
- -W "$(__gitcomp_1 "$1" "$4")" \
+ COMPREPLY=($(compgen -P "${2-}" \
+ -W "$(__gitcomp_1 "${1-}" "${4-}")" \
-- "$cur"))
;;
esac
@@ -152,13 +152,13 @@ __gitcomp ()
__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 ;;
@@ -170,13 +170,13 @@ __git_heads ()
__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 ;;
@@ -188,7 +188,7 @@ __git_tags ()
__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
--
1.6.1.87.g15624
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/3] bash-completion: Try bash completions before simple filetype
2009-01-14 20:02 [PATCH 1/3] bash-completion: Support running when set -u is enabled ted
@ 2009-01-14 20:02 ` ted
2009-01-14 20:02 ` [PATCH 3/3] bash-completion: Added comments to remind about required arguments ted
2009-01-15 15:34 ` [PATCH 1/3] bash-completion: Support running when set -u is enabled Shawn O. Pearce
1 sibling, 1 reply; 6+ messages in thread
From: ted @ 2009-01-14 20:02 UTC (permalink / raw)
To: spearce; +Cc: git, gitster, Ted Pavlic
From: Ted Pavlic <ted@tedpavlic.com>
When a git completion is not found, a bash shell should try bash-type
completions first before going to standard filetype completions. This
patch /adds/ "-o bashdefault" to the completion line. If that option
is not available, it uses the old method.
This behavior was inspired by Mercurial's bash completion script.
Signed-off-by: Ted Pavlic <ted@tedpavlic.com>
Acked-by: Shawn O. Pearce <spearce@spearce.org>
---
contrib/completion/git-completion.bash | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 5d1515c..201f9a6 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1766,13 +1766,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
--
1.6.1.87.g15624
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] bash-completion: Added comments to remind about required arguments
2009-01-14 20:02 ` [PATCH 2/3] bash-completion: Try bash completions before simple filetype ted
@ 2009-01-14 20:02 ` ted
2009-01-15 15:35 ` Shawn O. Pearce
0 siblings, 1 reply; 6+ messages in thread
From: ted @ 2009-01-14 20:02 UTC (permalink / raw)
To: spearce; +Cc: git, gitster, Ted Pavlic
From: Ted Pavlic <ted@tedpavlic.com>
Adds a few simple comments above commands that take arguments. These
comments are meant to remind developers of potential problems that can
occur when the script is sourced on systems with "set -u." Any
function which "requires" arguments really ought to be called with
explicit arguments given.
Also adds a #!bash to the top of bash completions so that editing
software can always identify that the file is of sh type.
Signed-off-by: Ted Pavlic <ted@tedpavlic.com>
Acked-by: Shawn O. Pearce <spearce@spearce.org>
---
contrib/completion/git-completion.bash | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 201f9a6..f8b845a 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1,3 +1,4 @@
+#!bash
#
# bash completion support for core Git.
#
@@ -50,6 +51,8 @@ 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
@@ -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)"
@@ -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]}"
@@ -150,6 +158,7 @@ __gitcomp ()
esac
}
+# __git_heads accepts 0 or 1 arguments (to pass to __gitdir)
__git_heads ()
{
local cmd i is_hash=y dir="$(__gitdir "${1-}")"
@@ -168,6 +177,7 @@ __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-}")"
@@ -186,6 +196,7 @@ __git_tags ()
done
}
+# __git_refs accepts 0 or 1 arguments (to pass to __gitdir)
__git_refs ()
{
local i is_hash=y dir="$(__gitdir "${1-}")"
@@ -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
--
1.6.1.87.g15624
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 3/3] bash-completion: Added comments to remind about required arguments
2009-01-14 20:02 ` [PATCH 3/3] bash-completion: Added comments to remind about required arguments ted
@ 2009-01-15 15:35 ` Shawn O. Pearce
2009-01-15 15:47 ` Ted Pavlic
0 siblings, 1 reply; 6+ messages in thread
From: Shawn O. Pearce @ 2009-01-15 15:35 UTC (permalink / raw)
To: ted; +Cc: git, gitster
ted@tedpavlic.com wrote:
> From: Ted Pavlic <ted@tedpavlic.com>
>
> Adds a few simple comments above commands that take arguments. These
> comments are meant to remind developers of potential problems that can
> occur when the script is sourced on systems with "set -u." Any
> function which "requires" arguments really ought to be called with
> explicit arguments given.
>
> Also adds a #!bash to the top of bash completions so that editing
> software can always identify that the file is of sh type.
>
> Signed-off-by: Ted Pavlic <ted@tedpavlic.com>
> Acked-by: Shawn O. Pearce <spearce@spearce.org>
Yup, this and also 2/3 look fine. But again, the message, it
shouldn't be indented.
> ---
> contrib/completion/git-completion.bash | 15 +++++++++++++++
> 1 files changed, 15 insertions(+), 0 deletions(-)
--
Shawn.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] bash-completion: Support running when set -u is enabled
2009-01-14 20:02 [PATCH 1/3] bash-completion: Support running when set -u is enabled ted
2009-01-14 20:02 ` [PATCH 2/3] bash-completion: Try bash completions before simple filetype ted
@ 2009-01-15 15:34 ` Shawn O. Pearce
1 sibling, 0 replies; 6+ messages in thread
From: Shawn O. Pearce @ 2009-01-15 15:34 UTC (permalink / raw)
To: ted; +Cc: git, gitster
ted@tedpavlic.com wrote:
> From: Ted Pavlic <ted@tedpavlic.com>
>
> Under "set -u" semantics, it is an error to access undefined
> variables. Some user environments may enable this setting in the
> interactive shell.
>
> In any context where the completion functions access an undefined
> variable, accessing a default empty string (aka "${1-}" instead of
> "$1") is a reasonable way to code the function, as it silences
> the undefined variable error while still supplying an empty string.
>
> In this patch, functions that should always take an argument still use
> $1. Functions that have optional arguments use ${1-}.
>
> Signed-off-by: Ted Pavlic <ted@tedpavlic.com>
> Acked-by: Shawn O. Pearce <spearce@spearce.org>
Yup. This patch isn't as bad as everyone made it out to be.
I think we should go ahead and include it.
But usually we don't indent the commit message like you did
above; instead the message should be aligned on the left margin at
column 0. git log will indent the message during display, hence
any identation you add in the patch email just makes the message
that much harder to read from git log.
> ---
> contrib/completion/git-completion.bash | 18 +++++++++---------
> 1 files changed, 9 insertions(+), 9 deletions(-)
--
Shawn.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-01-15 15:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-01-14 20:02 [PATCH 1/3] bash-completion: Support running when set -u is enabled ted
2009-01-14 20:02 ` [PATCH 2/3] bash-completion: Try bash completions before simple filetype ted
2009-01-14 20:02 ` [PATCH 3/3] bash-completion: Added comments to remind about required arguments ted
2009-01-15 15:35 ` Shawn O. Pearce
2009-01-15 15:47 ` Ted Pavlic
2009-01-15 15:34 ` [PATCH 1/3] bash-completion: Support running when set -u is enabled Shawn O. Pearce
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).