Re: [PATCH] bash completion: use read -r everywhere
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Dec 21, 2011, 18:59 UTC
- Message-ID
- <7vipl9hht4.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <4502a0248bb843018335e9b5cdf70736c096ebe3.1324482693.git.trast@student.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
Show 21 quoted lines
> POSIX specifies
>
> The read utility shall read a single line from standard input.
> By default, unless the -r option is specified, backslash ('\')
> shall act as an escape character...
>
> Our omission of -r breaks the loop reading refnames from
> git-for-each-ref in __git_refs() if there are refnames such as
> "foo'bar", in which case for-each-ref helpfully quotes them as in
>
> $ git update-ref "refs/remotes/test/foo'bar" HEAD
> $ git for-each-ref --shell --format="ref=%(refname:short)" "refs/remotes"
> ref='test/foo'\''bar'
>
> Interpolating the \' here will read "ref='test/foo'''bar'" instead,
> and eval then chokes on the unbalanced quotes.
>
> However, since none of the read loops _want_ to have backslashes
> interpolated, it's much safer to use read -r everywhere.
>
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>Thanks.
As this script is specific to bash, it is secondary importance what POSIX says. The "-r" option is important only because "bash" happens to follow POSIX in this case. I'd like to see the early part of the message reworded perhaps like this:
At various points in the script, we use "read" utility without
giving it the "-r" option that prevents a backslash ('\')
character to act as an escape character. This breaks e.g. reading
refnames from ...Does this regress for zsh users in some ways, by the way?
Show 62 quoted lines
> ---
> contrib/completion/git-completion.bash | 12 ++++++------
> 1 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 78257ae..e7a39ef 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -111,7 +111,7 @@ __git_ps1_show_upstream ()
>
> # get some config options from git-config
> local output="$(git config -z --get-regexp '^(svn-remote\..*\.url|bash\.showupstream)$' 2>/dev/null | tr '\0\n' '\n ')"
> - while read key value; do
> + while read -r key value; do
> case "$key" in
> bash.showupstream)
> GIT_PS1_SHOWUPSTREAM="$value"
> @@ -589,7 +589,7 @@ __git_refs ()
> local ref entry
> git --git-dir="$dir" for-each-ref --shell --format="ref=%(refname:short)" \
> "refs/remotes/" | \
> - while read entry; do
> + while read -r entry; do
> eval "$entry"
> ref="${ref#*/}"
> if [[ "$ref" == "$cur"* ]]; then
> @@ -602,7 +602,7 @@ __git_refs ()
> case "$cur" in
> refs|refs/*)
> git ls-remote "$dir" "$cur*" 2>/dev/null | \
> - while read hash i; do
> + while read -r hash i; do
> case "$i" in
> *^{}) ;;
> *) echo "$i" ;;
> @@ -611,7 +611,7 @@ __git_refs ()
> ;;
> *)
> git ls-remote "$dir" HEAD ORIG_HEAD 'refs/tags/*' 'refs/heads/*' 'refs/remotes/*' 2>/dev/null | \
> - while read hash i; do
> + while read -r hash i; do
> case "$i" in
> *^{}) ;;
> refs/*) echo "${i#refs/*/}" ;;
> @@ -636,7 +636,7 @@ __git_refs_remotes ()
> {
> local i hash
> git ls-remote "$1" 'refs/heads/*' 2>/dev/null | \
> - while read hash i; do
> + while read -r hash i; do
> echo "$i:refs/remotes/$1/${i#refs/heads/}"
> done
> }
> @@ -1863,7 +1863,7 @@ __git_config_get_set_variables ()
> done
>
> git --git-dir="$(__gitdir)" config $config_file --list 2>/dev/null |
> - while read line
> + while read -r line
> do
> case "$line" in
> *.*=*)