Re: [PATCH v2 02/18] Add a new builtin: branch-diff

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: May 21, 2018, 10:33 UTC
Message-ID: <nycvar.QRO.7.76.6.1805211153370.77@tvgsbejvaqbjf.bet>
In-Reply-To: <3cefc6b3-3dbd-9cb1-20d0-193116191726@gmail.com>

Hi Buga,

On Mon, 7 May 2018, Igor Djordjevic wrote:

Show 56 quoted lines

> On 07/05/2018 09:48, Jeff King wrote:
> > 
> > > > Let's, please, not fall into the trap of polluting git-branch with
> > > > utterly unrelated functionality, as has happened a few times with
> > > > other Git commands. Let's especially not do so merely for the sake of
> > > > tab-completion. git-branch is for branch management; it's not for
> > > > diff'ing.
> > >
> > > I totally disagree. `git branch` is *the* command to work with branches.
> > > Yes, you can manage branches. But you can also list them. And now you can
> > > also compare them.
> > 
> > One of the things I don't like about "git branch --diff" is that this
> > feature is not _just_ about branches at all. E.g., I could do:
> > 
> >   git tbdiff HEAD~10 HEAD~5 foo
> > 
> > Or even:
> > 
> >   git tbdiff v2.16.0 v2.17.0 my-rewritten-v2.17.0
> > 
> > Those arguments really are just commitishes, not necessarily branches.
> > One of the current interface rules for "git branch" is that the branch
> > names we hand it are interpreted _exactly_ as branch names. You cannot
> > "git branch -m v2.16.0", and there is no ambiguity in "git branch -d
> > foo" if "foo" is both a tag and a branch.
> > 
> > But this new mode does not fit the pattern at all.
> > 
> > If we were to attach this to an existing command, I think it has more to
> > do with "diff" than "branch". But I'm not sure we want to overload
> > "diff" either (which has traditionally been about two endpoints, and
> > does not really traverse at all, though arguably "foo...bar" is a bit of
> > a cheat :) ).
> > 
> > > > Of the suggestions thus far, Junio's git-topic-diff seems the least
> > > > worse, and doesn't suffer from tab-completion problems.
> > >
> > > Except that this is too limited a view.
> > 
> > Right, I agree with you. Topic branches are the intended use, but that's
> > not what it _does_, and obviously it can be applied in other cases. So
> > since "branch" is too specific, I think "topic branch" is even more so.
> > 
> > It's really "diff-history" or something, I think. That's not very
> > catchy, but I think the best name would imply that it was diffing a set
> > of commits (so even "diff-commit" would not be right, because that again
> > sounds like endpoints).
> 
> This is exactly what I feel as well, thanks for concise and 
> to-the-point spelling out.
> 
> From user interface perspective, I would expect something like this 
> to be possible (and natural):
> 
> (1) git diff topic-v1...topic-v2

No, we cannot. The `git diff topic-v1...topic-v2` invocation has worked for a looooong time, and does something very different.

We should not even allow ourselves to think of such a breakage.

> (2) git diff --branch topic-v1...topic-v2

From my point of view, `git diff --branch` indicates that I diff *branches*. Which is not really something that makes sense, and definitely not what this command is about.

We are not comparing branches.

We are comparing versions of the same branch.

Show 9 quoted lines

> (1) is what we are all familiar with, providing a diff between two 
> revisions with focus on file changes, where (2) shifts focus to 
> history changes.
> 
> It`s all still a comparison between two revisions (pointed to by 
> "topic-v1" and "topic-v2" branch heads in this specific example), but 
> it differs in what we are comparing - (1) set of files contained in 
> endpoints, or (2) set of revisions contained in (or "leading to") 
> endpoints.

It is very much not about comparing *two* revisions. It is very much about comparing two *ranges of* revisions, and not just any ranges, no. Those ranges need to be so related as to contain mostly identical changes. Otherwise, `git branch --diff` will spend a ton of time, just to come back with a series of `-` lines followed by a series of `+` lines (figuratively, not literally). Which would be stupid, to spend that much time on something that `git rev-list --left-right topic1...topic2` would have computed a lot faster.

> Hmm... what about `git diff --history`? :/ It does seem more "true" 
> to what it does, though I still like `git diff --branch` more 
> (catchier, indeed).

It certainly is catchier. But also a ton more puzzling.

I do not want to compare histories, after all. That would be like saying: okay, topic1 and topic2 ended up at the same stage, but *how* did they get there?

What I *want* to ask via the command implemented by this patch series is the question: there was a set of patches previously, and now I have a set of revised patches, what changed?

Most fellow German software engineers (who seem to have a knack for idiotically long variable/function names) would now probably suggest:

	git compare-patch-series-with-revised-patch-series

I hope you agree that that is better *and* worse than your suggestions, depending from what angle you look at it: it is better because it describes what the command is *actually* doing. But it is much worse at the same time because it is too long.

Ciao, Dscho