From: Felipe Contreras Date: Sun, 15 Apr 2012 04:45:43 GMT Subject: Re: [PATCH v3 1/5] completion: simplify __gitcomp_1 Message-ID: In-Reply-To: On Sun, Apr 15, 2012 at 1:54 AM, Felipe Contreras wrote: > On Sun, Apr 15, 2012 at 1:36 AM, Thomas Rast wrote: >> Felipe Contreras 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. Actually, after a private discussion with Thomas, I realized that's on zsh, on bash it doesn't work as I expected, plus, in both 'echo "-n"' does the unexpected thing (to me). So, it should be, maybe 'print "%s\n" "$c$2$s"', or 'print "%s$s\n" "$c$2"'. However, I would like to simplify it even more: __gitcomp_1 () { local c w s IFS=' '$'\t'$'\n' for c in $1; do w="$c$2" case "$w" in --*=*|*.) s="" ;; *) s=" " ;; esac printf "%s$s\n" "$w" done } -- Felipe Contreras