On Sun, Apr 15, 2012 at 1:36 AM, Thomas Rast <trast@xxxxxxxxxxx> wrote:
> Felipe Contreras <felipe.contreras@xxxxxxxxx> writes:
>
>> # __gitcomp_1 requires 2 arguments
>> __gitcomp_1 ()
>> {
>> - local c IFS=' '$'\t'$'\n'
>> + local c s IFS=' '$'\t'$'\n'
>> for c in $1; do
>> case "$c$2" in
>> - --*=*) printf %s$'\n' "$c$2" ;;
>> - *.) printf %s$'\n' "$c$2" ;;
>> - *) printf %s$'\n' "$c$2 " ;;
>> + --*=* | *.) s="" ;;
>> + *) s=" " ;;
>> esac
>> + echo "$c$2$s"
>> done
>> }
>
> Sorry for not noticing earlier, but...
>
> I did a double take at the change to 'echo'. I'm guessing from the
> patch that $c$2$s is never just '-e' or some other option taken by the
> bash version[1] of echo. But can you be sure? Do you know off hand
> whether '-nbogus' complains, treats the -n as usual and prints 'bogus',
> or echoes '-nbogus'[2]? Are you sure future bash versions won't break
> this?
That doesn't make any sense to me. If you want that, you should do
'eval "echo $foo"', and even if you do 'eval "echo \"$foo\""', that
would be avoided.
> Also, I can't help but complain about your commit messages (again).
Well, we can start a discussion about how "simplify __gitcomp_1" does
not explain sufficiently that this is indeed a simplification of
__gitcomp_1, but from previous experiences you don't really want to
discuss, you just want to be right, and me to follow orders.
Personally, I don't see why every modified line needs an ode.
> 1. the refactoring of the partial command: printf %s$'\n' "$c$2
> 2. the change to echo
Or
1. simplifying __gitcomp_1
> Footnotes:
> [1] POSIX states that echo "shall not support any options" and "shall
> not recognize the -- argument", but we have printfs all over the code
> base because option support is extremely inconsistent
This is a *bash* completion script.
If you *need* an essay for a commit message for a patch that
essentially does nothing but simplify some code, I'll just drop it;
it's not worth my effort.
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