Re: [PATCH 3/6] completion: return the index of found word from __git_find_on_cmdline()

From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: Eric Sunshine <sunshine@sunshineco.com>
Cc: Junio C Hamano <gitster@pobox.com>, Git List <git@vger.kernel.org>
Subject: Re: [PATCH 3/6] completion: return the index of found word from __git_find_on_cmdline()
Date: Thu, 19 Dec 2019 15:39:27 +0100	[thread overview]
Message-ID: <20191219143927.GD8609@szeder.dev> (raw)
In-Reply-To: <CAPig+cRSGfaRggDhauSvJyrO1Zu7ZFSG+gfF134z8UV1ovSuEw@mail.gmail.com>

On Fri, Oct 18, 2019 at 05:01:42PM -0400, Eric Sunshine wrote:
> On Fri, Oct 18, 2019 at 10:37 AM SZEDER Gábor <szeder.dev@gmail.com> wrote:
> > On Thu, Oct 17, 2019 at 01:52:27PM -0400, Eric Sunshine wrote:
> > > > +               case "$1" in
> > > > +               --show-idx)     show_idx=y ;;
> > > > +               *)              return 1 ;;
> > >
> > > Should this emit an error message to aid a person debugging a test
> > > which fails on a call to __git_find_on_cmdline()? [...]
> >
> > And printing anything to standard error during completion is
> > inherently bad: it disrupts the command line, can't be deleted [...]
> > Remaining silent about the unrecognized option
> > is in my opinion better, because then the completion script usually
> > does nothing, and Bash falls back to filename completion.  Yeah,
> > that's not ideal, but at least the user can easily correct it and
> > finish entering the command.
> 
> I had tunnel-vision and was thinking about this only in the context of
> the tests. However, while I agree that spewing errors during
> completion is not ideal, aren't there two different classes of errors
> to consider? Some errors can crop up via normal usage of Git commands
> in Real World situations; those errors should be suppressed since they
> are expected and can be tolerated. However, the second class of error
> (such as passing a bogus option to this internal function) is an
> outright programming mistake by a maintainer of the completion script
> itself, and it would be helpful to let the programmer know as early as
> possible about the mistake.
> 
> Or, are there backward-compatibility or other concerns which would
> make emitting error messages undesirable even for outright programmer
> mistakes?

It's not necessarily an outright programming mistake, and that error
could be triggered by ordinary users as well.

Let's suppose that a user has a custom 'git-foo' command in $PATH with
a custom '_git_foo' completion function in '~/.my-git-completions',
which the helper function '__git_bar --option' from our completion
script.  Let's also suppose that the user sources this completion
function from '~/.bashrc', but otherwise uses the system-wide git
completion script, and that $HOME is shared across multiple computers.

In this (arguably somewhat convoluted) scenario it might happen that
on a not quote up-to-date computer the system-wide git completion
script already has the '__git_bar' helper function, but it doesn't yet
support '--option'.  If '__git_bar' then prints an error to stderr,
then the command line will get badly messed up, and the user will have
to ctrl-C and start over.

However, if '__git_bar' silently ignores the unknown option, then the
worst that can happen is that completion doesn't work, and e.g. it
falls back to Bash's filename completion or offers something
nonsensical.  In either case, after a brief "Huh?!" moment the user
can correct it by hitting backspace a couple of times and then enter
the rest of the command by hand.