unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Merge-base alias for git vc-diff
@ 2018-12-26 22:32 Juri Linkov
  2018-12-27  1:07 ` Dmitry Gutov
  0 siblings, 1 reply; 27+ messages in thread
From: Juri Linkov @ 2018-12-26 22:32 UTC (permalink / raw)
  To: emacs-devel

I can't find how vc could compare the HEAD of the branch with its merge base.

The documentation recommends using `git diff topic...master' to find
changes that occurred on the master branch since when the topic branch
was started off it.

But it seems that vc-diff uses neither `..' nor `...'

Even `vc-git-log-incoming' and `vc-git-log-outgoing' use only `..'

Since "git diff A...B" is equivalent to "git diff $(git-merge-base A B) B",
doing the same means adding a new alias "MERGE-BASE" to the completions of
`C-u C-x v D' (vc-root-diff) to complement the existing alias "HEAD".

Does this make sense or there is a simpler way to do the same?



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Merge-base alias for git vc-diff
  2018-12-26 22:32 Merge-base alias for git vc-diff Juri Linkov
@ 2018-12-27  1:07 ` Dmitry Gutov
  2018-12-27 20:02   ` Juri Linkov
  0 siblings, 1 reply; 27+ messages in thread
From: Dmitry Gutov @ 2018-12-27  1:07 UTC (permalink / raw)
  To: Juri Linkov, emacs-devel

On 27.12.2018 0:32, Juri Linkov wrote:

> Since "git diff A...B" is equivalent to "git diff $(git-merge-base A B) B",
> doing the same means adding a new alias "MERGE-BASE" to the completions of
> `C-u C-x v D' (vc-root-diff) to complement the existing alias "HEAD".

Do we have aliases like that in VC? HEAD is simply a ref that Git 
recognizes, but MERGE-BASE is not (*). Not sure how to handle it best.

Maybe you'll want to both extend the 'diff' backend command and add a 
new boolean argument to the end.

And also add a new command that would use it, similar to 
vc-root-version-diff (or a new meaning to some value of the prefix 
argument).

(*) Also: merge-base with what? master is not always the base.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Merge-base alias for git vc-diff
  2018-12-27  1:07 ` Dmitry Gutov
@ 2018-12-27 20:02   ` Juri Linkov
  2018-12-27 20:42     ` Dmitry Gutov
                       ` (2 more replies)
  0 siblings, 3 replies; 27+ messages in thread
From: Juri Linkov @ 2018-12-27 20:02 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: emacs-devel

>> Since "git diff A...B" is equivalent to "git diff $(git-merge-base A B) B",
>> doing the same means adding a new alias "MERGE-BASE" to the completions of
>> `C-u C-x v D' (vc-root-diff) to complement the existing alias "HEAD".
>
> Do we have aliases like that in VC? HEAD is simply a ref that Git
> recognizes, but MERGE-BASE is not (*). Not sure how to handle it best.

MERGE-BASE could be a VC alias, not Git alias.  So it could be handled
specially by ‘vc-git-diff’, ‘vc-git-revision-table’, etc.

> Maybe you'll want to both extend the 'diff' backend command and add a new
> boolean argument to the end.

This is the easiest part of the task.

> And also add a new command that would use it, similar to
> vc-root-version-diff (or a new meaning to some value of the prefix
> argument).

Adding a new command like ‘vc-merge-base-diff’ is easy, but first
I'd like to try to extend the existing command because it is bound
to a short key sequence: ‘C-u C-x v D’ then type ‘TAB’ for completion
and select e.g. ‘MERGE-BASE’.

> (*) Also: merge-base with what? master is not always the base.

This is another problem, then we need to add the prefix MERGE-BASE
to all branches listed in the completions: MERGE-BASE:master,
MERGE-BASE:branch-a, MERGE-BASE:branch-b, ...  Too bad.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Merge-base alias for git vc-diff
  2018-12-27 20:02   ` Juri Linkov
@ 2018-12-27 20:42     ` Dmitry Gutov
  2018-12-28 17:53     ` Stefan Monnier
  2019-01-02  0:14     ` Juri Linkov
  2 siblings, 0 replies; 27+ messages in thread
From: Dmitry Gutov @ 2018-12-27 20:42 UTC (permalink / raw)
  To: Juri Linkov; +Cc: emacs-devel

On 27.12.2018 22:02, Juri Linkov wrote:

> This is another problem, then we need to add the prefix MERGE-BASE
> to all branches listed in the completions: MERGE-BASE:master,
> MERGE-BASE:branch-a, MERGE-BASE:branch-b, ...  Too bad.

Yeah, seems non-obvious. Maybe create a whitelist for branch names?



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Merge-base alias for git vc-diff
  2018-12-27 20:02   ` Juri Linkov
  2018-12-27 20:42     ` Dmitry Gutov
@ 2018-12-28 17:53     ` Stefan Monnier
  2018-12-29 21:39       ` Juri Linkov
  2019-01-02  0:14     ` Juri Linkov
  2 siblings, 1 reply; 27+ messages in thread
From: Stefan Monnier @ 2018-12-28 17:53 UTC (permalink / raw)
  To: emacs-devel

> This is another problem, then we need to add the prefix MERGE-BASE
> to all branches listed in the completions: MERGE-BASE:master,
> MERGE-BASE:branch-a, MERGE-BASE:branch-b, ...  Too bad.

Maybe we should allow the "old revision" to be of the form "<REV>..."
and/or the "new revision" to be of the form "...<REV>".


        Stefan




^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Merge-base alias for git vc-diff
  2018-12-28 17:53     ` Stefan Monnier
@ 2018-12-29 21:39       ` Juri Linkov
  0 siblings, 0 replies; 27+ messages in thread
From: Juri Linkov @ 2018-12-29 21:39 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

>> This is another problem, then we need to add the prefix MERGE-BASE
>> to all branches listed in the completions: MERGE-BASE:master,
>> MERGE-BASE:branch-a, MERGE-BASE:branch-b, ...  Too bad.
>
> Maybe we should allow the "old revision" to be of the form "<REV>..."
> and/or the "new revision" to be of the form "...<REV>".

This is the most natural representation, but the only problem is its
discoverability - the users have to learn this syntax.  OTOH, after
typing TAB TAB we can show the virtual revision name MERGE-BASE
in the list of completions, but its drawback is that after selecting
MERGE-BASE we have to show another minibuffer asking for the branch name
where to look for the common ancestor.  Or we could support both ways.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Merge-base alias for git vc-diff
  2018-12-27 20:02   ` Juri Linkov
  2018-12-27 20:42     ` Dmitry Gutov
  2018-12-28 17:53     ` Stefan Monnier
@ 2019-01-02  0:14     ` Juri Linkov
  2019-01-02  1:55       ` Stefan Monnier
  2 siblings, 1 reply; 27+ messages in thread
From: Juri Linkov @ 2019-01-02  0:14 UTC (permalink / raw)
  To: emacs-devel

>> And also add a new command that would use it, similar to
>> vc-root-version-diff (or a new meaning to some value of the prefix
>> argument).
>
> Adding a new command like ‘vc-merge-base-diff’ is easy, but first
> I'd like to try to extend the existing command because it is bound
> to a short key sequence: ‘C-u C-x v D’ then type ‘TAB’ for completion
> and select e.g. ‘MERGE-BASE’.

Since there is no good solution to reuse the existing command,
bug#33950 implements two new commands:

* vc-log-mergebase (bound to ‘C-x v M L’) shows the same output as
  GitLab shows in the “Commits” tab of the merge request.

* vc-diff-mergebase (bound to ‘C-x v M D’) shows the same output as
  GitLab shows in the “Changes” tab of the merge request.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Merge-base alias for git vc-diff
  2019-01-02  0:14     ` Juri Linkov
@ 2019-01-02  1:55       ` Stefan Monnier
  2019-01-02 15:34         ` João Távora
  2019-01-02 21:29         ` Juri Linkov
  0 siblings, 2 replies; 27+ messages in thread
From: Stefan Monnier @ 2019-01-02  1:55 UTC (permalink / raw)
  To: emacs-devel

> Since there is no good solution to reuse the existing command,
> bug#33950 implements two new commands:
>
> * vc-log-mergebase (bound to ‘C-x v M L’) shows the same output as
>   GitLab shows in the “Commits” tab of the merge request.
>
> * vc-diff-mergebase (bound to ‘C-x v M D’) shows the same output as
>   GitLab shows in the “Changes” tab of the merge request.

I must say I find this solution very unsatisfactory.


        Stefan




^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Merge-base alias for git vc-diff
  2019-01-02  1:55       ` Stefan Monnier
@ 2019-01-02 15:34         ` João Távora
  2019-01-02 22:25           ` Dmitry Gutov
  2019-01-02 21:29         ` Juri Linkov
  1 sibling, 1 reply; 27+ messages in thread
From: João Távora @ 2019-01-02 15:34 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

On Wed, Jan 2, 2019 at 2:08 AM Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>
> > Since there is no good solution to reuse the existing command,
> > bug#33950 implements two new commands:
> >
> > * vc-log-mergebase (bound to ‘C-x v M L’) shows the same output as
> >   GitLab shows in the “Commits” tab of the merge request.
> >
> > * vc-diff-mergebase (bound to ‘C-x v M D’) shows the same output as
> >   GitLab shows in the “Changes” tab of the merge request.
>
> I must say I find this solution very unsatisfactory.

Me too.  I don't mind adding those commands, but I think
the "..." syntax should be supported, too.

One way to enhance dicoverability is to add a short help in
the first few lines of the minibuffer.

João


-- 
João Távora



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Merge-base alias for git vc-diff
  2019-01-02  1:55       ` Stefan Monnier
  2019-01-02 15:34         ` João Távora
@ 2019-01-02 21:29         ` Juri Linkov
  2019-01-03  3:40           ` Stefan Monnier
  1 sibling, 1 reply; 27+ messages in thread
From: Juri Linkov @ 2019-01-02 21:29 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

>> Since there is no good solution to reuse the existing command,
>> bug#33950 implements two new commands:
>>
>> * vc-log-mergebase (bound to ‘C-x v M L’) shows the same output as
>>   GitLab shows in the “Commits” tab of the merge request.
>>
>> * vc-diff-mergebase (bound to ‘C-x v M D’) shows the same output as
>>   GitLab shows in the “Changes” tab of the merge request.
>
> I must say I find this solution very unsatisfactory.

The reason I proposed separate commands is because I can't find
a good solution how to combine this with the existing commands.
Unsolved problems:

1. I don't know how to change the *Completions* buffer header from
   “Possible completions are:” to
   “Possible completions are (<REV>... means diff with common ancestor):”

2. How to support inserting a completion from the *Completions* buffer
   into the minibuffer but not exiting the minibuffer after choose-completion.
   Then after selecting a completion the user could add “...” to the end.

3. ‘C-u C-x v L’ asks for the limit only.  Maybe adding a new prefix arg
   ‘C-u C-u C-x v L’ could ask for two revisions: start rev and end rev.

Then ‘C-u C-u C-x v D’ could also interpret its double prefix arg as diff
with the common ancestor.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Merge-base alias for git vc-diff
  2019-01-02 15:34         ` João Távora
@ 2019-01-02 22:25           ` Dmitry Gutov
  2019-01-02 22:48             ` João Távora
  0 siblings, 1 reply; 27+ messages in thread
From: Dmitry Gutov @ 2019-01-02 22:25 UTC (permalink / raw)
  To: João Távora, Stefan Monnier; +Cc: emacs-devel

On 02.01.2019 18:34, João Távora wrote:

> Me too.  I don't mind adding those commands, but I think
> the "..." syntax should be supported, too.

I think it's not terrible. And if we were choosing between new commands 
and new revision syntax, the former is probably easier to discover.

If we used the same revision syntax in more contexts (commands), that 
might be more important.

> One way to enhance dicoverability is to add a short help in
> the first few lines of the minibuffer.

Isn't minibuffer normally just one line tall?



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Merge-base alias for git vc-diff
  2019-01-02 22:25           ` Dmitry Gutov
@ 2019-01-02 22:48             ` João Távora
  2019-01-02 23:07               ` Dmitry Gutov
  2019-01-03  3:22               ` Stefan Monnier
  0 siblings, 2 replies; 27+ messages in thread
From: João Távora @ 2019-01-02 22:48 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Stefan Monnier, emacs-devel

On Wed, Jan 2, 2019 at 10:26 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
>
> On 02.01.2019 18:34, João Távora wrote:
>
> > Me too.  I don't mind adding those commands, but I think
> > the "..." syntax should be supported, too.
>
> I think it's not terrible. And if we were choosing between new commands
> and new revision syntax, the former is probably easier to discover.

That depends on how one uses Emacs.  I personally prefer hints,
completion and C-u modifiers to new commands.

> Isn't minibuffer normally just one line tall?

Don't know. Is there a hard rule against printing more lines?  Or
a real disadvantage?

João



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Merge-base alias for git vc-diff
  2019-01-02 22:48             ` João Távora
@ 2019-01-02 23:07               ` Dmitry Gutov
  2019-01-03  3:22               ` Stefan Monnier
  1 sibling, 0 replies; 27+ messages in thread
From: Dmitry Gutov @ 2019-01-02 23:07 UTC (permalink / raw)
  To: João Távora; +Cc: Stefan Monnier, emacs-devel

On 03.01.2019 1:48, João Távora wrote:

> That depends on how one uses Emacs.  I personally prefer hints,
> completion and C-u modifiers to new commands.

If we can reuse one of modifiers like that, yeah. But there's a limited 
amount of prefix modifiers we can use and, again, it's not very 
discoverable.

That's not to say that you couldn't integrate the new commands to be 
delegated to with certain prefix values, like vc-root-diff does.

To be clear, I'm not wild about adding new commands, but if we add 
functionality, it must go somewhere.

> Don't know. Is there a hard rule against printing more lines?  Or
> a real disadvantage?

Windows are jumping, it can be annoying. And in the end it might 
redistribute heights between windows in a top-bottom split.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Merge-base alias for git vc-diff
  2019-01-02 22:48             ` João Távora
  2019-01-02 23:07               ` Dmitry Gutov
@ 2019-01-03  3:22               ` Stefan Monnier
  2019-01-03 13:30                 ` João Távora
  1 sibling, 1 reply; 27+ messages in thread
From: Stefan Monnier @ 2019-01-03  3:22 UTC (permalink / raw)
  To: João Távora; +Cc: emacs-devel, Dmitry Gutov

> That depends on how one uses Emacs.  I personally prefer hints,
> completion and C-u modifiers to new commands.

So do I.  Especially since we'd then also have to add new
log-view-merge-diff commands and possibly others.

>> Isn't minibuffer normally just one line tall?
> Don't know. Is there a hard rule against printing more lines?
> Or a real disadvantage?

FWIW, my minibuffer has 1 line and can't grow because it's in
a minibuffer-only frame.


        Stefan



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Merge-base alias for git vc-diff
  2019-01-02 21:29         ` Juri Linkov
@ 2019-01-03  3:40           ` Stefan Monnier
  2019-01-10 21:25             ` Juri Linkov
  0 siblings, 1 reply; 27+ messages in thread
From: Stefan Monnier @ 2019-01-03  3:40 UTC (permalink / raw)
  To: Juri Linkov; +Cc: emacs-devel

> The reason I proposed separate commands is because I can't find
> a good solution how to combine this with the existing commands.

You'd have to change the notion of "revision" in vc-git.el to allow
a leading or trailing "...": remember, this is specific to Git so it can
only belong in vc-git.el.

It also means that revision completion in C-x v ~ will also allow those
"..." even tho they don't make any sense there.


        Stefan



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Merge-base alias for git vc-diff
  2019-01-03  3:22               ` Stefan Monnier
@ 2019-01-03 13:30                 ` João Távora
  2019-01-03 20:38                   ` Juri Linkov
  0 siblings, 1 reply; 27+ messages in thread
From: João Távora @ 2019-01-03 13:30 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Juri Linkov, emacs-devel, Dmitry Gutov

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> FWIW, my minibuffer has 1 line and can't grow because it's in
> a minibuffer-only frame.

Kinky :) I don't think a rigid minibuffer is a great idea, but scratch
the multi-line minibuffer approach, just add a C-h shortcut that
displays some contextual help like `save-some-buffers'.

Or add some hint to the beginning of the line, there's plenty of space.

Here's two other ideas:

1. Isn't merge-base suitable for a toggle switch?  Could sth like
   vc-git-prefer-merge-base be useful?  Maybe I'd set that to t, and it
   shows nicely in a checkbox in the VC menu for discoverability.  But I
   like the next idea better.

2. A diff is always a comparison of two revisions, right?  For vc-dir,
   isn't a UI like query-replace's suitable here?  You know, that thing
   where you type the pattern and the replacement interactively in the
   same minibuffer prompt, separated by the arrow.  You can M-p, C-r,
   etc, flawlessly. I really like that UI, it's really intuitive (kudos
   to whoever participated, btw).
    
   So here, instead of pattern and replacement, two revisions.  All one
   would need, apparently, is to turn the arrow into a ".." and make it
   toggleable to "..." with say, C-M-y.  I think it would simplify the
   completion problem: on both sides of the ".." or "..." you just
   complete a revision using whatever system we presumably already have
   in place for that.

João





^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Merge-base alias for git vc-diff
  2019-01-03 13:30                 ` João Távora
@ 2019-01-03 20:38                   ` Juri Linkov
  2019-01-03 21:44                     ` João Távora
  0 siblings, 1 reply; 27+ messages in thread
From: Juri Linkov @ 2019-01-03 20:38 UTC (permalink / raw)
  To: João Távora; +Cc: emacs-devel, Stefan Monnier, Dmitry Gutov

> 2. A diff is always a comparison of two revisions, right?  For vc-dir,
>    isn't a UI like query-replace's suitable here?  You know, that thing
>    where you type the pattern and the replacement interactively in the
>    same minibuffer prompt, separated by the arrow.  You can M-p, C-r,
>    etc, flawlessly. I really like that UI, it's really intuitive (kudos
>    to whoever participated, btw).
>
>    So here, instead of pattern and replacement, two revisions.  All one
>    would need, apparently, is to turn the arrow into a ".." and make it
>    toggleable to "..." with say, C-M-y.  I think it would simplify the
>    completion problem: on both sides of the ".." or "..." you just
>    complete a revision using whatever system we presumably already have
>    in place for that.

Good idea.  This brings the minibuffer input syntax closer to the real
command line syntax.  The only problem is that then we have to use
different amount of dots for vc-root-diff and vc-print-root-log because
in git: ‘git diff A...B’ corresponds to ‘git log A..B’
whereas ‘git diff A..B’ corresponds to ‘git log A...B’



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Merge-base alias for git vc-diff
  2019-01-03 20:38                   ` Juri Linkov
@ 2019-01-03 21:44                     ` João Távora
  2019-01-05 22:28                       ` Juri Linkov
  2019-01-07 22:33                       ` Juri Linkov
  0 siblings, 2 replies; 27+ messages in thread
From: João Távora @ 2019-01-03 21:44 UTC (permalink / raw)
  To: Juri Linkov; +Cc: emacs-devel, Stefan Monnier, Dmitry Gutov

Juri Linkov <juri@linkov.net> writes:

>> 2. A diff is always a comparison of two revisions, right?  For vc-dir,
>>    isn't a UI like query-replace's suitable here?  You know, that thing
>>    where you type the pattern and the replacement interactively in the
>>    same minibuffer prompt, separated by the arrow.  You can M-p, C-r,
>>    etc, flawlessly. I really like that UI, it's really intuitive (kudos
>>    to whoever participated, btw).
>>
>>    So here, instead of pattern and replacement, two revisions.  All one
>>    would need, apparently, is to turn the arrow into a ".." and make it
>>    toggleable to "..." with say, C-M-y.  I think it would simplify the
>>    completion problem: on both sides of the ".." or "..." you just
>>    complete a revision using whatever system we presumably already have
>>    in place for that.
>
> Good idea.

Cool. I'd say go for it! (unless someone seriously objects).

For a first stab at this, most of the UI work would be in
vc-diff-build-argument-list-internal which would, if passed an optional
argument, say CHOOSE-REVISION-RELATIONSHIP, use the new UI and return an
extra element at the end saying the merge-base should be used.  It
should be a question of grabbing this element in vc-version-diff and
passing it this down to vc-<backend>-diff.

The Git backend would know how to handle it (as could presumably other
backends).  The value of CHOOSE-REVISION-RELATIONSHIP passed in
vc-version-diff could be a backend-specific decision.

> The only problem is that then we have to use different amount of dots
> for vc-root-diff and vc-print-root-log because in git: ‘git diff
> A...B’ corresponds to ‘git log A..B’ whereas ‘git diff A..B’
> corresponds to ‘git log A...B’

Really? Ugh, that unfortunate.  Well we could just live with it or use
different symbols to abstract it away.

João



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Merge-base alias for git vc-diff
  2019-01-03 21:44                     ` João Távora
@ 2019-01-05 22:28                       ` Juri Linkov
  2019-01-07 22:33                       ` Juri Linkov
  1 sibling, 0 replies; 27+ messages in thread
From: Juri Linkov @ 2019-01-05 22:28 UTC (permalink / raw)
  To: João Távora; +Cc: emacs-devel, Stefan Monnier, Dmitry Gutov

>> The only problem is that then we have to use different amount of dots
>> for vc-root-diff and vc-print-root-log because in git: ‘git diff
>> A...B’ corresponds to ‘git log A..B’ whereas ‘git diff A..B’
>> corresponds to ‘git log A...B’
>
> Really? Ugh, that unfortunate.  Well we could just live with it or use
> different symbols to abstract it away.

Another question is whether to use the same Git syntax for other dVCSes?
It seems that finding a common ancestor is a common task among them:
https://stackoverflow.com/questions/6743068/merge-base-analog-for-mercurial-and-bzr-to-find-common-ancestors-as-possible-fo



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Merge-base alias for git vc-diff
  2019-01-03 21:44                     ` João Távora
  2019-01-05 22:28                       ` Juri Linkov
@ 2019-01-07 22:33                       ` Juri Linkov
  2019-01-08 14:58                         ` Stefan Monnier
  1 sibling, 1 reply; 27+ messages in thread
From: Juri Linkov @ 2019-01-07 22:33 UTC (permalink / raw)
  To: João Távora; +Cc: emacs-devel, Stefan Monnier, Dmitry Gutov

>> The only problem is that then we have to use different amount of dots
>> for vc-root-diff and vc-print-root-log because in git: ‘git diff
>> A...B’ corresponds to ‘git log A..B’ whereas ‘git diff A..B’
>> corresponds to ‘git log A...B’
>
> Really? Ugh, that unfortunate.  Well we could just live with it or use
> different symbols to abstract it away.

I still have no idea how to read a start/end revision in
‘C-u C-x v L’.  Currently it accepts a limit as a number.
I wonder is it possible to parse its minibuffer's input
and to distinguish such number from the revision?
Maybe using a regexp?



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Merge-base alias for git vc-diff
  2019-01-07 22:33                       ` Juri Linkov
@ 2019-01-08 14:58                         ` Stefan Monnier
  2019-01-09  0:05                           ` Juri Linkov
  0 siblings, 1 reply; 27+ messages in thread
From: Stefan Monnier @ 2019-01-08 14:58 UTC (permalink / raw)
  To: emacs-devel

> I still have no idea how to read a start/end revision in
> ‘C-u C-x v L’.

I think you need to use vc-print-branch-log.  All other VC print-log
commands show their age (they were designed back when branches were
rare, so you'd just "get *the* log" and then search for the revision of
interest).


        Stefan




^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Merge-base alias for git vc-diff
  2019-01-08 14:58                         ` Stefan Monnier
@ 2019-01-09  0:05                           ` Juri Linkov
  0 siblings, 0 replies; 27+ messages in thread
From: Juri Linkov @ 2019-01-09  0:05 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

>> I still have no idea how to read a start/end revision in
>> ‘C-u C-x v L’.
>
> I think you need to use vc-print-branch-log.  All other VC print-log
> commands show their age (they were designed back when branches were
> rare, so you'd just "get *the* log" and then search for the revision of
> interest).

I wonder how it could be used to limit its output to the merge base?
Maybe

  M-x vc-print-branch-log RET master..branch



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Merge-base alias for git vc-diff
  2019-01-03  3:40           ` Stefan Monnier
@ 2019-01-10 21:25             ` Juri Linkov
  2019-01-10 22:14               ` Dmitry Gutov
  2019-01-10 22:41               ` Stefan Monnier
  0 siblings, 2 replies; 27+ messages in thread
From: Juri Linkov @ 2019-01-10 21:25 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

>> The reason I proposed separate commands is because I can't find
>> a good solution how to combine this with the existing commands.
>
> You'd have to change the notion of "revision" in vc-git.el to allow
> a leading or trailing "...": remember, this is specific to Git so it can
> only belong in vc-git.el.
>
> It also means that revision completion in C-x v ~ will also allow those
> "..." even tho they don't make any sense there.

After trying to implement this with

diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index f9ea4def15..8f8e151549 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -1247,4 +1249,10 @@ vc-git-diff
       (setq command "diff-index")
       (unless rev1 (setq rev1 "HEAD")))
+    (when (string-match "\\`\\(.*\\)\\.\\.\\.\\'" rev1)
+      (setq rev1 (string-trim-right (or (vc-git--run-command-string
+                                         nil "merge-base"
+                                         (match-string 1 rev1)
+                                         rev2)
+                                        (match-string 1 rev1)))))
     (if vc-git-diff-switches
         (apply #'vc-git-command (or buffer "*vc-diff*")

and using with `C-u C-x v D master... RET branch RET'
exposed an unexpected problem: vc-diff-internal sets
diff-vc-revisions to the list ("master..." "branch")

But diff-mode fails to use diff-vc-revisions for such features as
jumping to the corresponding source line with diff-goto-source that
signals an error in vc-git-find-revision:

  (error "Failed (status 128): git --no-pager cat-file blob master...:lisp/vc/vc-git.el .")

and leaves the buffer with an error message in it:

  fatal: Not a valid object name master...:lisp/vc/vc-git.el

One way to resolve this is to interpret "master...branch" as a syntax
to denote the merge base, then set diff-vc-revisions instead of

  ("master..." "branch")

to

  ("master...branch" "branch")

This means that this feature should be used only this way:

  C-u C-x v D master...branch RET branch RET



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: Merge-base alias for git vc-diff
  2019-01-10 21:25             ` Juri Linkov
@ 2019-01-10 22:14               ` Dmitry Gutov
  2019-01-10 23:25                 ` Juri Linkov
  2019-01-10 22:41               ` Stefan Monnier
  1 sibling, 1 reply; 27+ messages in thread
From: Dmitry Gutov @ 2019-01-10 22:14 UTC (permalink / raw)
  To: Juri Linkov, Stefan Monnier; +Cc: emacs-devel

On 11.01.2019 00:25, Juri Linkov wrote:
> This means that this feature should be used only this way:
> 
>    C-u C-x v D master...branch RET branch RET

We could use a more special syntax for this, in order for the user not 
to be confused regarding master...branch..branch.

For example:

C-u C-x v D master^branch RET branch RET

Hopefully not many branches have a ^ in their names.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Merge-base alias for git vc-diff
  2019-01-10 21:25             ` Juri Linkov
  2019-01-10 22:14               ` Dmitry Gutov
@ 2019-01-10 22:41               ` Stefan Monnier
  2019-01-10 23:18                 ` Juri Linkov
  1 sibling, 1 reply; 27+ messages in thread
From: Stefan Monnier @ 2019-01-10 22:41 UTC (permalink / raw)
  To: Juri Linkov; +Cc: emacs-devel

>   (error "Failed (status 128): git --no-pager cat-file blob master...:lisp/vc/vc-git.el .")

I think this can/should be solved by making vc-git-find-revision throw
away the "..."


        Stefan



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Merge-base alias for git vc-diff
  2019-01-10 22:41               ` Stefan Monnier
@ 2019-01-10 23:18                 ` Juri Linkov
  0 siblings, 0 replies; 27+ messages in thread
From: Juri Linkov @ 2019-01-10 23:18 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

>>   (error "Failed (status 128): git --no-pager cat-file blob master...:lisp/vc/vc-git.el .")
>
> I think this can/should be solved by making vc-git-find-revision throw
> away the "..."

Sorry, I don't understand.  "git cat-file blob master:lisp/vc/vc-git.el"
will get the revision from the master's HEAD, but what is needed is
a revision of the merge base between master and branch, i.e. what
is returned by "git-merge-base master branch".



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Merge-base alias for git vc-diff
  2019-01-10 22:14               ` Dmitry Gutov
@ 2019-01-10 23:25                 ` Juri Linkov
  0 siblings, 0 replies; 27+ messages in thread
From: Juri Linkov @ 2019-01-10 23:25 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Stefan Monnier, emacs-devel

>> This means that this feature should be used only this way:
>>
>>    C-u C-x v D master...branch RET branch RET
>
> We could use a more special syntax for this, in order for the user not to
> be confused regarding master...branch..branch.
>
> For example:
>
> C-u C-x v D master^branch RET branch RET
>
> Hopefully not many branches have a ^ in their names.

This is too ad-hoc syntax that might conflict with some VCS special syntax
like HEAD^2.

Let me remind that "git diff A...B" is equivalent to
"git diff $(git-merge-base A B) B".

So we have at least 3 possible solutions:

  C-u C-x v D master...branch RET branch RET  -- where master...branch
                                              -- internally is substituted
                                              -- by the merge-base revision
                                              -- found by "git-merge-base master branch"

  C-u C-x v D master...branch RET RET   -- shortcut of the above, but the problem is
                                        -- how to get "branch" from rev1
                                        -- when rev2 is empty

  C-x v M D master RET branch RET  -- special command for merge-base revisions



^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2019-01-10 23:25 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-12-26 22:32 Merge-base alias for git vc-diff Juri Linkov
2018-12-27  1:07 ` Dmitry Gutov
2018-12-27 20:02   ` Juri Linkov
2018-12-27 20:42     ` Dmitry Gutov
2018-12-28 17:53     ` Stefan Monnier
2018-12-29 21:39       ` Juri Linkov
2019-01-02  0:14     ` Juri Linkov
2019-01-02  1:55       ` Stefan Monnier
2019-01-02 15:34         ` João Távora
2019-01-02 22:25           ` Dmitry Gutov
2019-01-02 22:48             ` João Távora
2019-01-02 23:07               ` Dmitry Gutov
2019-01-03  3:22               ` Stefan Monnier
2019-01-03 13:30                 ` João Távora
2019-01-03 20:38                   ` Juri Linkov
2019-01-03 21:44                     ` João Távora
2019-01-05 22:28                       ` Juri Linkov
2019-01-07 22:33                       ` Juri Linkov
2019-01-08 14:58                         ` Stefan Monnier
2019-01-09  0:05                           ` Juri Linkov
2019-01-02 21:29         ` Juri Linkov
2019-01-03  3:40           ` Stefan Monnier
2019-01-10 21:25             ` Juri Linkov
2019-01-10 22:14               ` Dmitry Gutov
2019-01-10 23:25                 ` Juri Linkov
2019-01-10 22:41               ` Stefan Monnier
2019-01-10 23:18                 ` Juri Linkov

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).