Web lists-archives.com

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
> that::
> 
>     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.

-Peff