Jonathan Nieder <jrnieder@xxxxxxxxx> writes:
> I imagine it would have been enough to say something along the lines of
> "The __gitcomp and __gitcomp_nl functions are unnecessarily verbose.
> __gitcomp_nl sets IFS to " \t\n" unnecessarily before setting it to "\n"
> by mistake. Both functions use 'if' statements to read parameters
> with defaults, where the ${parameter:-default} idiom would be just as
> clear. By fixing these, we can make each function almost a one-liner."
>
> By the way, the subject ("clean up __gitcomp*") tells me almost as
> little as something like "fix __gitcomp*". A person reading the
> shortlog would like to know _how_ you are fixing it, or what the
> impact of the change will be --- e.g., something like "simplify
> __gitcomp and __gitcomp_nl" would be clearer.
I love both of the above two paragraphs. Thanks.
> [...]
>> --- a/contrib/completion/git-completion.bash
>> +++ b/contrib/completion/git-completion.bash
> [...]
>> @@ -524,18 +520,8 @@ __gitcomp ()
>> # appended.
>> __gitcomp_nl ()
>> {
>> - local s=$'\n' IFS=' '$'\t'$'\n'
>> - local cur_="$cur" suffix=" "
>> -
>> - if [ $# -gt 2 ]; then
>> - cur_="$3"
>> - if [ $# -gt 3 ]; then
>> - suffix="$4"
>> - fi
>> - fi
>> -
>> - IFS=$s
>> - COMPREPLY=($(compgen -P "${2-}" -S "$suffix" -W "$1" -- "$cur_"))
>> + local IFS=$'\n'
>> + COMPREPLY=($(compgen -P "${2-}" -S "${4:- }" -W "$1" -- "${3:-$cur}"))
>
> This loses the nice name $suffix for the -S argument. Not a problem,
> just noticing.
The patch looks good, including the localness that is kept for IFS.
--
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