Felipe Contreras <felipe.contreras@xxxxxxxxx> writes:
> On Thu, Feb 2, 2012 at 9:27 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> Jonathan Nieder <jrnieder@xxxxxxxxx> writes:
>>> However, clearly I did not say it clearly enough. :) I guess it's
>>> better to take a cue from storytellers and show rather than tell.
>>
>> Very big thanks for this ;-)
>
> Not a single comment regarding what I said?
What entitles you to force me to refraining from commenting at all until I
read everything in my mailbox and after waiting for a while to make sure
there is no more to come to the thread?
In any case, "be nicer with zsh" conveys no more meaningful information
than "this is some patch about zsh". Let's try to avoid warm and fuzzy
words that imply "goodness", e.g. "improve" and "be nicer with" because
nobody sends a patch to purposefully make Git worse and expects it to be
applied.
I found Jonathan's alternative "avoid default value assignment on : true
command" at least a bit better for the purpose of jogging the short-term
memory in the "'git shortlog v1.7.9.. contrib/completion/' tells us that
we have applied several patches, and I remember that : ${var=word} one!"
sense. It is not super-useful for the longer term, though.
Here is what I ended up in preparation for queuing the series. I still
haven't seen any version of 4/4, but please check $gmane/189683 and see if
that matches what you intended. Also I am assuming $gmane/189606 relayed
by Jonathan is a squash between your 2 and 3 (which didn't reach me), so
please advise if that does not match what you want to have.
Thanks.
-- >8 --
From: Felipe Contreras <felipe.contreras@xxxxxxxxx>
Subject: [PATCH] completion: work around zsh option propagation bug
zsh versions from 4.3.0 to present (4.3.15) do not correctly propagate the
SH_WORD_SPLIT option into the subshell in ${foo:=$(bar)} expressions. For
example, after running
emulate sh
fn () {
var='one two'
printf '%s\n' $var
}
x=$(fn)
: ${y=$(fn)}
printing "$x" results in two lines as expected, but printing "$y" results
in a single line because $var is expanded as a single word when evaluating
fn to compute y.
So avoid the construct, and use an explicit 'test -n "$foo" || foo=$(bar)'
instead. This fixes a bug tht caused all commands to be treated as
porcelain and show up in "git <TAB><TAB>" completion, because the list of
all commands was treated as a single word in __git_list_porcelain_commands
and did not match any of the patterns that would usually cause plumbing to
be excluded.
[jn: clarified commit message, indentation style fix]
Signed-off-by: Felipe Contreras <felipe.contreras@xxxxxxxxx>
Signed-off-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---
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 1496c6d..c636166 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -676,7 +676,8 @@ __git_merge_strategies=
# is needed.
__git_compute_merge_strategies ()
{
- : ${__git_merge_strategies:=$(__git_list_merge_strategies)}
+ test -n "$__git_merge_strategies" ||
+ __git_merge_strategies=$(__git_list_merge_strategies)
}
__git_complete_revlist_file ()
@@ -854,7 +855,8 @@ __git_list_all_commands ()
__git_all_commands=
__git_compute_all_commands ()
{
- : ${__git_all_commands:=$(__git_list_all_commands)}
+ test -n "$__git_all_commands" ||
+ __git_all_commands=$(__git_list_all_commands)
}
__git_list_porcelain_commands ()
@@ -947,7 +949,8 @@ __git_porcelain_commands=
__git_compute_porcelain_commands ()
{
__git_compute_all_commands
- : ${__git_porcelain_commands:=$(__git_list_porcelain_commands)}
+ test -n "$__git_porcelain_commands" ||
+ __git_porcelain_commands=$(__git_list_porcelain_commands)
}
__git_pretty_aliases ()
--
1.7.9.172.ge26ae
--
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