Re: [PATCH] tag: Implicitly supply --list given another list-like option
- Date: Sun, 12 Mar 2017 07:19:52 -0400
- From: Jeff King <peff@xxxxxxxx>
- Subject: Re: [PATCH] tag: Implicitly supply --list given another list-like option
On Sat, Mar 11, 2017 at 12:08:55PM +0000, Ævar Arnfjörð Bjarmason wrote:
> Jeff King pointed out in
> <20170310124247.jvrmmcz2pbv4qf3o@xxxxxxxxxxxxxxxxxxxxx> in reply to
> The difference between "branch" and "tag" here is that "branch
> --contains" implies "--list" (and the argument becomes a pattern).
> Whereas "tag --contains" just detects the situation and complains.
> If anything, I'd think we would consider teaching "tag" to behave
> more like "branch".
> I agree. This change does that, the only tests that broke as a result
> of this were tests that were explicitly checking that this
> "branch-like" usage wasn't permitted, i.e. no actual breakages
> occurred, and I can't imagine an invocation that would negatively
> impact backwards compatibility, i.e. these invocations all just
> errored out before.
Trying to think of counter-arguments, the best I could come up with is
that the situation is potentially ambiguous, and some user could be
confused by us doing the wrong thing. I don't find that particularly
compelling, especially as the "wrong thing" is to list tags, which is
not a destructive operation.
So I think this is an improvement. As for the patch itself:
> + /* We implicitly supply --list with --contains, --points-at,
> + --merged and --no-merged, just like git-branch */
> + if (filter.with_commit || filter.points_at.nr || filter.merge_commit)
> + cmdmode = 'l';
I was about to complain that this needs "!cmdmode", but I just looked
forward in the thread and saw that Junio already covered that in more
detail. I concur.
Thanks for working on this.