Re: [PATCH v2 15/16] tag: implicitly supply --list given the -n option

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Mar 21, 2017, 19:11 UTC
Message-ID: <CACBZZX7yRRTQpcFZ9eO2_+HdB979p6URE+jsXJakxw7jpnOvmA@mail.gmail.com>
In-Reply-To: <xmqq37e6piae.fsf@gitster.mtv.corp.google.com>

On Tue, Mar 21, 2017 at 7:59 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 52 quoted lines

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> Change the "tag" command to treat the "-n" invocation as a list-like
>> option in addition to --contains, --points-at etc.
>>
>> Most of the work for this was done in my earlier "tag: Implicitly
>> supply --list given another list-like option" commit, but I've split
>> off this patch since it's more contentious. Now these will be
>> synonymous:
>>
>>     git tag -n 100
>>     git tag -n --list 100
>
> Hmph.  I would understand if these meant the same thing:
>
>     git tag -l -n 100
>     git tag -l -n=100
>     git tag -l -n100
>
> with or without "-l".  And accepting any of the above three without "-l"
> instead of rejecting is a very good change, I would think.
>
> I however do not understand how accepting this:
>
>     git tag -n --list 100
>
> would be a good thing, as "100" an optional parameter to the "-n"
> option.
>
>> Whereas before the former would die. This doesn't technically
>> introduce any more ambiguity than the aforementioned change applied to
>> th other list-like options, but it does introduce the possibility for
>> more confusion, since instead of the latter of these dying:
>>
>>     git tag -n100
>>     git tag -n 100
>>
>> It now works entirely differently, i.e. invokes list mode with a
>> filter for "100" as a pattern. I.e. it's synonymous with:
>>
>>     git tag -n --list 100
>
> Ahhh, yuck.  OK, so in "git tag -n --list 100", 100 does not have
> anything to do with the -n option.  It is a pattern and -n specifies
> "just one line" by default.
>
> Oh, boy, that is confusing.  While it is very logical.
>
> Still I think it is OK as I can see why people who wanted to have
> '-n' in the first place may want
>
>     git tag -n -l <pattern>

Yeah I see now that this is rather badly explained. I'll fix this up for v3. All of this worked already:

    $ ./git tag 100
    $ ./git tag -n -l 100
    100             tag: add tests for --with and --without
    $ ./git tag -l -n 100
    100             tag: add tests for --with and --without

So actually thinking about it again it doesn't add any more ambiguity than we had before. The change is just strictly getting rid of the need for -l for consistency with --contains, --points-at etc.

I see now that the whole thing that led me down this golden path was that I was removing the failing "git tag -n 100" test, so while I was still wrapping my mind around this I thought I was introducing some *more* confusion, but really that test was just testing that it didn't work, as opposed to "git tag -l -n 100".

I'm just going to squash this into the "tag: implicitly supply --list given another list-like option" patch for v3 unless you have objections, I think there's no reason to split this off any more than splitting off e.g. --points-at etc.