On Sat, Mar 11, 2017 at 12:08:55PM +0000, Ævar Arnfjörð Bjarmason wrote:
> Jeff King pointed out in > <20170310124247.jvrmmcz2pbv4q...@sigill.intra.peff.net> 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
