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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Dscho,

On 07/05/2018 03:34, Johannes Schindelin wrote:
> 
> > > I think Todd's idea to shift it from a full-blown builtin to a cmdmode
> > > of `branch` makes tons of sense.
> >
> > I don`t know, I still find it a bit strange that in order to "diff
> > something", you go to "something" and tell it to "diff itself" - not
> > because it`s a weird concept (OOP, anyone? :]), but because we already
> > have "diff" command that can accept different things, thus just teaching
> > it to accept additional "something" (branch, in this case), seems more
> > natural (to me) - "branch diff" being just another "diff" mode of
> > operation.
> 
> You also have to call `git branch` to list branches. And to rename
> branches. And to delete them. So why not also compare them at the same
> time?

Maybe because we already have a command that specifically does 
comparison? :)

List, rename, delete -- all these seem more as basic CRUD operations, 
where comparison is a more complex one. And not to get me wrong - I 
could see "branch diff" being part of "branch", but not really when 
"diff" already exists as a separate thing, already doing quite some 
(but still diff related, and configurable) stuff.

> > What about that side thought you left out from my original message,
> > making it `git diff --branch` instead?
> 
> I really did not like this, as all of the `git diff` options really are
> about comparing two revisions, not two *sets* of revisions.

I see what you mean, but I would argue this being a deliberate user 
choice here, like picking a diff "strategy" - I`d say it still utterly 
does compare two revisions (branch tips, in this case), just putting 
focus on comparing revisions that lead to them (branch history), 
instead of just files found in them (branch files).

> Further, if I put my unsuspecting user hat on, I would ask myself how you
> can compare branches with one another? That is what I would expect `git
> diff --branch` to do, not to compare two versions of *the same* branch.

I totally agree with you here, and thus I have a question - what 
determines "two versions of *the same* branch"? :) Do you still 
explicitly provide both "old" and "new" version branch tips?

I see "multiple versions of the same branch" more as a conceptual 
model, and not something Git is aware of (I think?) - BUT, even if it 
was, I don`t see why this should be a(n artificial) restriction?

Basically, what you (conceptually) call "two versions of the same 
branch", I simply call "two branches" (from usage standpoint).

And you may have a branch that got split, or more of them that got 
unified, so defining "previous branch version" may not be that 
straightforward - it`s really just "two commit ranges" (as man page 
defines it in general), with "two versions of a patch series" only 
being the most common/expected use case of the former.

Finally, if user picks two totally unrelated "branches" to compare, 
he won`t get a really useful diff - but it`s the same as if he would 
compare two totally unrelated commits (where tree state massively 
changed in between, or having unrelated histories, even).

Besides, while I might still not be much into the matter, but isn`t 
"branch" in Git just a pointer to revision? Being so, there is really 
no such thing as "branch" in terms of being a specific (sub)set of 
revisions (commits), other then "everything from branch head/pointer 
to root commit" (in general).

Yes, we do perceive "a branch" being a specific set of topic related 
commits, but which *exact* commits we are interested in ("branch" lower 
bounds) may differ in regards to what we aim for - how far do we consider 
one branch to reach in the past depends solely on the use case.

> So `git diff --branch` does not at all convey the same to me as `git
> branch --diff`, and I find that the latter does match better what this
> patch series tries to achieve.

I agree with the first part, but it seems to me your finding is 
biased due to your (expected) use case.

> > But if "branch diff" is considered to be too special-cased mode of
> > "diff" so that supporting it from `diff` itself would make it feel
> > awkward in both usage and maintenance (in terms of many other regular
> > `diff` specific options being unsupported), I guess I would understand
> > having it outside `diff` altogether (and implemented as proposed `git
> > branch --diff`, or something)... for the time being, at least :)
> 
> The branch diff is not even a special-cased mode of diff. It is *way* more
> complicated than that. It tries to find 1:1 correspondences between *sets*
> of commits, and then only outputs a "sort" of a diff between the commits
> that correspond with each other. I say "sort" of a diff because that diff
> does not look like `git diff <commit1> <commit2>` at all!

But there is not only one `git diff <commit1> <commit2>` looks, it 
depends on other options (like --name-status, for example), which is 
my point exactly :)

With something like `git diff --branch <commit1>...<commit2>` you 
would get yet another "diff look", useful for use case in question 
here.

Regards, Buga



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux