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

Hi Stefan,

On 08/05/2018 00:24, Stefan Beller wrote:

quoted

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 :)

quoted

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.

quoted

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.

quoted

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