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 06/05/2018 14:10, Johannes Schindelin wrote:
> 
> > > > > This builtin does not do a whole lot so far, apart from showing a
> > > > > usage that is oddly similar to that of `git tbdiff`. And for a
> > > > > good reason: the next commits will turn `branch-diff` into a
> > > > > full-blown replacement for `tbdiff`.
> > > >
> > > > One minor point about the name: will it become annoying as a tab
> > > > completion conflict with git-branch?
> > >
> > > I did mention this in the commit message of 18/18:
> > >
> > >     Without this patch, we would only complete the `branch-diff` part but
> > >     not the options and other arguments.
> > >
> > >     This of itself may already be slightly disruptive for well-trained
> > >     fingers that assume that `git bra<TAB>ori<TAB>mas<TAB>` would expand to
> > >     `git branch origin/master`, as we now no longer automatically append a
> > >     space after completing `git branch`: this is now ambiguous.
> > >
> > > > It feels really petty complaining about the name, but I just want
> > > > to raise the point, since it will never be easier to change than
> > > > right now.
> > >
> > > I do hear you. Especially since I hate `git cherry` every single
> > > time that I try to tab-complete `git cherry-pick`.
> > >
> > > > (And no, I don't really have another name in mind; I'm just
> > > > wondering if "subset" names like this might be a mild annoyance in
> > > > the long run).
> > >
> > > They totally are, and if you can come up with a better name, I am
> > > really interested in changing it before this hits `next`, even.
> >
> > I gave this just a quick glance so might be I`m missing something 
> > obvious or otherwise well-known here, bur why not `diff-branch` instead?
> 
> I think that is just turning the problem from `branch` to `diff`.
> 
> Of course, we have precedent with diff-index and diff-files. Except that
> they don't auto-complete (because they are low-level commands) and I
> *would* like the subcommand discussed in this here patch series to
> auto-complete.

Yeah, I did suspect it might be something like this (those other ones 
not auto-completing, where we do want it here), thanks for elaborating.

> 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.

What about that side thought you left out from my original message, 
making it `git diff --branch` instead?

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

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