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

[Igor Djordjevic <igor.d.djordjevic@gmail.com>]

Hi Stefan,

On 08/05/2018 00:24, Stefan Beller wrote:
> 
> > 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.
> 
> If we go with "branch --diff", because it has the CRUD operations already
> there for branches, I might ask for "remote --diff" to diff two remotes. ;)
> (That command "remote --diff" would not make any sense, would it?)

I`m not sure if this is a reply to me or in general, and whether you 
support what I sad, or argue against it...? Because what you`re 
saying was (or at least should have been) my exact point there :)

> > Basically, what you (conceptually) call "two versions of the same
> > branch", I simply call "two branches" (from usage standpoint).
> 
> If I diff 2 (topic) branches, which are based on a different version
> from upstream, then I see changes from commits that I don't care
> about, but this tool explicitly excludes them. Instead it includes
> the ordering of the commits as well as its commit messages to
> the diff.

Here, I was merely pointing out that you still need to provide two 
branch heads - which might be expected to resemble "two versions of 
the same topic", but they are still (just) "two branches" in Git world.

> > 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).
> 
> I used just that, but narrowed down the comparison to one file
> instead of the whole tree.

Again, not sure if this should support the argument, or argue against 
it? :) My point was that there might be other use cases (as you seem 
to have supported now), and as "diff" is pretty forgiving, might be 
"diff branch" should be as well.

> > With something like `git diff --branch <commit1>...<commit2>` you
> > would get yet another "diff look", useful for use case in question
> > here.
> 
> Personally I think this patch series should neither extend git-diff
> nor git-branch.
> 
> It should not extend git-diff, because currently git-diff can diff
> tree-ishs (and does that very well) and comparing to
> worktree/index.

Hmm, are you saying that `git diff` actually has a too generic name 
for its (more specific) purpose?

> It should also not extend git-branch, as that command is for
> CRUD operations that you hinted at earlier (Earlier I proposed
> git-remote --diff for diffing two remote, which makes no sense,
> another one might be git-worktree, which also just does CRUD
> for worktrees. It would be a bad idea to have "git worktree --diff")

Agreed here.

> Hence I propose "git range-diff", similar to topic-diff, that
> was proposed earlier.

I find it strange that we already have both "diff" and "diff-something" 
commands, and yet you still propose "something-diff" naming pattern 
instead (but I guess it`s mainly because of auto-complete concerns).

Please forgive my lack of code base familiarity, but from what I`ve 
seen so far, and at least from end-user perspective, I may rather expect 
`git diff-range` as low level implementation, and possibly exposed 
through `git diff --range` (with a nice single letter abbreviation?).

> * it "diffs ranges" of commits.

Thus "diff-range", as your description says itself :) ("range-diff" 
might sound like it "ranges diffs"...?)

> * it can also deal with out-of-git things like patch series,
>   but that is a mere by product and may not be desired.
>   Just like git-diff can also compare two files outside a git
>   repo, that would not be a good use case.

Hmm, so still follows `git diff` in general... `git diff --range`? :D

> * it autocompletes well.

Only here I`m not sure if something like `git diff --range` (with 
accompanying single letter option) would be considered "auto-complete 
friendly", or not?

Regards, Buga