Re: [PATCH 5/8] tag: Implicitly supply --list given the -n option

From: Jeff King <peff@peff.net>
Date: Mar 20, 2017, 04:02 UTC
Message-ID: <20170320040250.7kb3sc25fjilhsta@sigill.intra.peff.net>
In-Reply-To: <20170318103256.27141-6-avarab@gmail.com>

On Sat, Mar 18, 2017 at 10:32:53AM +0000, Ævar Arnfjörð Bjarmason wrote:

> Most of the work for this was done in my preceding "tag: Implicitly
> supply --list given another list-like option", but I've split off this
> patch since it's more contentious. Now invocations these invocations
> will be synonymous:

s/invocations these/these/

Show 13 quoted lines

>     git tag -n 100
>     git tag -n --list 100
> 
> Whereas before the former would die. This doesn't technically
> introduce any more ambiguity than change to the 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".

I think:

  git tag --list -n 100

is similarly confusing now. It's really the optional-argument thing that is misleading, though I suppose silently converting the extra argument into a filter does add an extra helping of confusion (most other programs would complain "100 is not a rev" or something, but git-tag just produces no output).

As I said in the other patch, I could take or leave this one for now, but I think I'd lean towards "leave".

>  Documentation/git-tag.txt |  9 +++++----
>  builtin/tag.c             |  2 +-
>  t/t7004-tag.sh            | 17 ++++++++++++++++-
>  3 files changed, 22 insertions(+), 6 deletions(-)

The patch itself looks fine.

-Peff