Subject: Re: [PATCH v2 01/41] parse-options: support --git-completion-helper

From: SZEDER Gábor <[email-masked]>
Date: Thu, 8 Feb 2018 00:41:52 +0100

> > OK how about some thing like this fixup patch? __gitcomp_builtin now
> > allows to add extra options as well as remove some.
> >
> > -- 8< --
> >  __gitcomp_builtin ()
> >  {
> > +       local incl="$2"
> > +       local excl="$3"
> > +               options="$(__git ${cmd/_/ } --git-completion-helper) $incl "
> > +               for i in $excl; do
> > +                       options="${options/$i /}"
> 
> Is 'options' guaranteed to end with a space?

It is, note the space before the closing double quote in:

  options="$(__git ${cmd/_/ } --git-completion-helper) $incl "

> If not, then this
> expulsion will fail for the very last option. I'd think you can get by
> fine with just "${options/$i}".

I would prefer a space both at the beginning and at the end of the
pattern.  Please excuse the contrived corner case, but it could still
fail if the option to be excluded is a suffix of an other option:

  $ o="$(echo --foo--bar --baz --bar) "
  $ echo "'${o/--bar /}'"       # exclude '--bar'
  '--foo--baz --bar '

Maybe we'll never have --opt--ions with a doubledash in them[1], but
still...

  $ o=" $(echo --foo--bar --baz --bar) "
  $ echo "'${o/ --bar / }'"
  ' --foo--bar --baz '

[1] - Interestingly, grep shows that the German translation does
      contain a '--reset--author'.