On Thu, Feb 2, 2012 at 10:48 AM, Jonathan Nieder <jrnieder@xxxxxxxxx> wrote:
> Felipe Contreras wrote:
>
>> Felipe Contreras (4):
>> completion: be nicer with zsh
>
> Since I can't find this patch in the mail archive, I'll reply here.
> Luckily the most important bit is above already.
>
> I think I mentioned before that this subject line is what will appear
> in the shortlog and the shortlog is all that some people will see of
> the changelog, so it should include a self-contained description of
> the impact of the patch.
Exactly, and "completion: avoid default value assignment on : true
command" tells *nothing* to most people. Why is this patch needed? Do
I care about it?
OTOH "completion: be nicer with zsh" explains the purpose of the patch
and people that don't care about zsh can happily ignore it if they
want, and the ones that care about zsh might want to back port it, or
whatever.
Also, if you are looking at a shortlog and looking for patches related
to zsh, you would easily see this.
"completion: avoid default value assignment on : true command" almost
ensures that the people reading that summary would need to read
further to see *why*.
> -- >8 --
> From: Felipe Contreras <felipe.contreras@xxxxxxxxx>
> Date: Thu, 2 Feb 2012 03:15:17 +0200
> Subject: completion: avoid default value assignment on : true command
No useful information provided yet. Should I care about this? I guess
I need to open the mail and see.
> 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
Ok, a bug in zsh, I can skip this. Or if I care about zsh, then I
still have to read further, because I still don't know what's the
problem.
And BTW, this is extremely detailed. Where's the evidence? Is there a
bug report? How can I follow this to find out when it's fixed. (hint:
missing link)
> 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.
Ok, still reading.
> So avoid the construct, and use an explicit 'test -n "$foo" ||
> foo=$(bar)' instead.
Yeah...
> 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.
Aha! Finally we arrive to the important part.
I guess we have different styles of writing. Personally, I don't see
the point of forcing the readers to go through the whole thing.
Compare this to mine:
| Subject: [PATCH v3 1/4] completion: be nicer with zsh
At this point most people can skip this.
| And yet another bug in zsh[1] causes a mismatch; zsh seems to have
| problem emulating wordspliting, but only on the command substitution.
|
| Let's avoid it.
A bug in zsh in certain situations... got it.
| I found this issue because __git_compute_porcelain_commands listed all
| commands (not only porcelain).
At this point it might have been better to mention "git <TAB><TAB>"
instead, as you did. But not a big deal, the problem is clear; all
commands are listed.
Almost all people would stop at this point; either they don't care
about zsh, or they care, and they want the patch.
| This is because in zsh the following code:
|
| for i in $__git_all_commands
|
| would evaluate $__git_all_commands as a single word (with spaces),
| ${=__git_all_commands} should be used to do word splitting expansion
| (unless SH_WORD_SPLIT is used).
|
| sh emulation should take care of that, but the command expantion is
| messing up with that.
And more details for the skeptics, or the ones that want to learn more.
| tl;dr: $__git_porcelain_commands = $__git_all_commands
Wrapping it up, to make clear what happens.
| [1] http://article.gmane.org/gmane.comp.shells.zsh.devel/24296
And then a link to further details, discussion, and possible fixes.
Cheers.
--
Felipe Contreras
--
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