Re: [PATCH v2] completion: use builtin completion for format-patch
- Date: Sat, 03 Nov 2018 19:20:38 +0900
- From: Junio C Hamano <gitster@xxxxxxxxx>
- Subject: Re: [PATCH v2] completion: use builtin completion for format-patch
Duy Nguyen <pclouds@xxxxxxxxx> writes:
>> Would it make sense to make send-email's completion helper print these
>> out directly? That way, if someone were to modify send-email in the
>> future, they'd only have to look through one file instead of both
>> send-email and the completions script.
> I did think about that and decided not to do it (in fact the first
> revision of this patch did exactly that).
> If we have to maintain this list manually, we might as well leave to
> the place that matters: the completion script. I don't think the
> person who updates send-email.perl would be always interested in
> completion support. And the one that is interested usually only looks
> at the completion script. Putting this list in send-email.perl just
> makes it harder to find.
I do not necessarily disagree with the conclusion, but I am not sure
if I agree with the last paragraph. If the definition used to list
completable options was in the send-email command, it is more likely
that a patch to send-email.perl that would add/modify an option
without making a matching change to the definition in the same file
gets noticed, whether the developer who is doing the feature is or
is not interested in maintaining the completion script working, no?
Similarly, if one notices that an option the command supports that
ought to get completed does not get completed, and gets motivated
enough to try finding where in the completion script other existing
options that do get completed are handled, wouldn't that lead one to
the right solution, i.e. discovery of the definition in the
Quite honestly, I would expect that our developers and user base are
much more competent than one who
- wants to add completion of the option Y to the command A, which
has known-to-be-working completion of the option X, and yet
- fails to imagine that it could be a possible good first step to
figure out how the option X is completed, so that a new support
for the option Y might be able to emulate it.
Now, once we start going in the direction of having both the
implementation of options *and* the definition of the list of
completable options in send-email.perl script, I would agree with
your initial assessment in a message much earlier in the thread. It
would be very tempting to use the data we feed Getopt::Long as the
source of the list of completable options to reduce the longer-term
maintenance load, which would mean it will involve more work. And
in order to avoid having to invest more work upfront (which I do not
think is necessarily a bad thing), having the definition in the
completion script might be easier to manage---it is closer to the
status quo, especially the state before you taught parse-options API
to give the list of completable options.