Re: [PATCH v2 7/7] completion: recognize more long-options

From: Cornelius Weig <cornelius.weig@tngtech.com>
Date: Feb 1, 2017, 16:49 UTC
Message-ID: <2841d2de-32ad-eae8-6039-9251a40bb00e@tngtech.com>
In-Reply-To: <CAM0VKj=Ein4yrKG2aZnN7JU80ctZBQromcR6BEu-TyMLenLFCg@mail.gmail.com>

Hi Gabor,

 thanks for taking a look at these commits.

On 01/31/2017 11:17 PM, SZEDER Gábor wrote:

Show 14 quoted lines

> On Fri, Jan 27, 2017 at 10:17 PM,  <cornelius.weig@tngtech.com> wrote:
>> From: Cornelius Weig <cornelius.weig@tngtech.com>
>>
>> Recognize several new long-options for bash completion in the following
>> commands:
> 
> Adding more long options that git commands learn along the way is
> always an improvement.  However, seeing "_several_ new long options"
> (or "some long options" in one of the other patches in the series)
> makes the reader wonder: are these the only new long options missing
> or are there more?  If there are more, why only these are added?  If
> there aren't any more missing long options left, then please say so,
> e.g. "Add all missing long options, except the potentially
> desctructive ones, for the following commands: ...."

Personally, I agree with you that

> Adding more long options that git commands learn along the way is
> always an improvement.

However, people may start complaining that their terminal becomes too cluttered when doing a double-Tab. In my cover letter, I go to length about this. My assumption was that all options that are mentioned in the introduction of the command man-page should be important enough to have them in the completion list. I'll change my commit message accordingly.

>>  - rm: --force
> 
> '--force' is a potentially destructive option, too.

Thanks for spotting this.

Btw, I haven't found that non-destructive options should not be eligible for completion. To avoid confusion about this in the future, I suggest to also change the documentation:

index 933bb6e..96f1c7f 100644

--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -13,7 +13,7 @@
 #    *) git email aliases for git-send-email
 #    *) tree paths within 'ref:path/to/file' expressions
 #    *) file paths within current working directory and index
-#    *) common --long-options
+#    *) common non-destructive --long-options
 #
 # To use these routines:
 #


I take it you have also looked at the code itself? Then I would gladly
mention you as reviewer in my sign-off.