Re: [PATCH v2] completion: use builtin completion for format-patch

From: Junio C Hamano <gitster@pobox.com>
To: Duy Nguyen <pclouds@gmail.com>
Cc: "Denton Liu" <liu.denton@gmail.com>,
	"SZEDER Gábor" <szeder.dev@gmail.com>,
	"Git Mailing List" <git@vger.kernel.org>,
	"Anmol Mago" <anmolmago@gmail.com>,
	briankyho@gmail.com, david.lu97@outlook.com,
	shirui.wang@hotmail.com
Subject: Re: [PATCH v2] completion: use builtin completion for format-patch
Date: Fri, 02 Nov 2018 08:52:30 +0900	[thread overview]
Message-ID: <xmqqzhuss6dd.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <CACsJy8AowaAW91wGMYbC8aTOB1NoqQN-5NGx=qUCHp0i6zQRDA@mail.gmail.com> (Duy Nguyen's message of "Thu, 1 Nov 2018 16:40:54 +0100")

Duy Nguyen <pclouds@gmail.com> writes:

>> > I have no comment about this. In an ideal world, sendemail.perl could
>> > be taught to support --git-completion-helper but I don't think my
>> > little remaining Perl knowledge (or time) is enough to do it. Perhaps
>> > this will do. I don't know.
>>
>> So "all", "attach", etc. are added to this list while these similar
>> options are lost from the other variable?  Is this a good trade-off?
>
> Not sure if I understand you correctly, but it looks to me that the
> options in git-send-email.perl are well organized, so we could...

Yes, but I wasn't commenting on your "sendemail should also be able
to help completion by supporting --completion-helper option" (I think
that is a sensible approach).  My comment was about Denton's patch,
which reduced the hard-coded list of format-patch options (i.e. the
first hunk) but had to add back many of them to send-email's
completion (i.e. the last hunk)---overall, it did not help reducing
the number of options hardcoded in the script.

If it makes sense to complete all options to format-patch to
send-email, then as you outlined, grabbing them out of format-patch
with the --completion-helper option at runtime, and using them to
complete both format-patch and send-email would be a good idea.  And
that should be doable even before send-email learns how to list its
supported options to help the completion.

> --git-completon-helper in that script to print all send-email specific
> options, then call "git format-patch --git-completion-helper" to add a
> bunch more. The options that are handled by setup_revisions() will
> have to be maintained manually here like $__git_format_patch_options
> and added on top in both _git_send_email () and _git_format_patch ().
>
> So, nothing option is lost and by the time setup_revisions() supports
> -git-completion-helper, we can get rid of the manual shell variable
> too. The downside is, lots of work, probably.