bug#62940: 29.0.60; vc: no easy way to get diff of all outgoing changes

bug#62940: 29.0.60; vc: no easy way to get diff of all outgoing changes

[Spencer Baugh]

Reproduction:
1. Make some local changes and commits
2. C-x v O to see the list of local commits that will be pushed
3. But there's no quick way to get a diff combining all those changes,
which is useful if you've made multiple changes to the same function
over the course of multiple commits, or if you want to review your
changes overall before sending them out for review, or any other things
like that.

C-u C-x v = lets you specify two commits to diff, and the second commit
even defaults to the working revision, but there's no easy way to get
the "base" commit that is before all your local changes.  If you're
using git, you can try to remember your upstream branch and type that
in, but that's just a matter of memory.  For other VCSs (e.g. hg)
there's no easy way.

I'm working on adding an easy way to do this in vc, but there are
several ways to approach it and I wanted to check first.

A. (my preferred way) Support a M-1 prefix argument for C-x v =, which
will do a diff against the "base" revision, whatever that is.

B. Add a new keybinding in vc-read-revision which will insert the "base"
revision, so you can just use C-u C-x v = and then insert the base
revision at the first prompt.  Seems hard to make compatible with
non-default completion frameworks.

C. Add new equivalents to C-x v = and C-x v D which diff against the
base revision.  Seems like ugly duplication of keybindings.

D. Make some way to go from a log-view like C-x v O to a diff which
combines all those commits.  Could be nice, but seems a little
unintuitive, and also doesn't support including uncommitted changes in
the diff.

Thoughts, suggestions?

bug#62940: 29.0.60; vc: no easy way to get diff of all outgoing changes

[Filipp Gunbin]

On 18/04/2023 15:12 -0400, Spencer Baugh wrote:

> Reproduction:
> 1. Make some local changes and commits
> 2. C-x v O to see the list of local commits that will be pushed
> 3. But there's no quick way to get a diff combining all those changes,
> which is useful if you've made multiple changes to the same function
> over the course of multiple commits, or if you want to review your
> changes overall before sending them out for review, or any other things
> like that.
>
> C-u C-x v = lets you specify two commits to diff, and the second commit
> even defaults to the working revision, but there's no easy way to get
> the "base" commit that is before all your local changes.  If you're
> using git, you can try to remember your upstream branch and type that
> in, but that's just a matter of memory.  For other VCSs (e.g. hg)
> there's no easy way.
>

[...]

> Thoughts, suggestions?

Usually I just do C-x v L, select commits that interest me (via region),
and do M-x log-view-diff.

Have you tried that?  What do you miss there?

bug#62940: 29.0.60; vc: no easy way to get diff of all outgoing changes

[Dmitry Gutov]

On 18/04/2023 23:36, Filipp Gunbin wrote:
> Usually I just do C-x v L, select commits that interest me (via region),
> and do M-x log-view-diff.

Just in case: 'log-view-diff' is bound to '='.

That's what I wanted to suggest, too. Or use 'vc-log-outgoing':

   C-x v O C-x h =

Which would be D. in the original message. It doesn't support 
uncommitted changes, but I would be surprised if it did.

But one can also try this:

   C-u C-x v D origin/master ^J

Where origin/master is the name of the upstream branch ref. This should 
include uncommitted changes.

bug#62940: 29.0.60; vc: no easy way to get diff of all outgoing changes

[Spencer Baugh]

Filipp Gunbin <fgunbin@fastmail.fm> writes:
> Usually I just do C-x v L, select commits that interest me (via region),
> and do M-x log-view-diff.
>
> Have you tried that?  What do you miss there?

Wow, amazing, I did not think to try that!  After some further
improvements to my vc backend, that is perfect!

bug#62940: 29.0.60; vc: no easy way to get diff of all outgoing changes

[Juri Linkov]

>   C-x v O C-x h =
>
> Which would be D. in the original message. It doesn't support uncommitted
> changes, but I would be surprised if it did.

It's supported only for vc that inserts a text header in logs like RCS.
I proposed to insert extra line that represents uncommitted git changes,
and I'm using this feature all the time.  But the problem is that
it's inserted even when 'C-x v b l' lists another branch
where such line makes no sense.

bug#62940: 29.0.60; vc: no easy way to get diff of all outgoing changes

[sbaugh]

Spencer Baugh <sbaugh@janestreet.com> writes:
> Filipp Gunbin <fgunbin@fastmail.fm> writes:
>> Usually I just do C-x v L, select commits that interest me (via region),
>> and do M-x log-view-diff.
>>
>> Have you tried that?  What do you miss there?
>
> Wow, amazing, I did not think to try that!  After some further
> improvements to my vc backend, that is perfect!

After using it for a bit, one issue is that C-x v O shows changes in all
files; it would be nice if there was a way to get outgoing
changes/commits only for the current fileset.

bug#62940: 29.0.60; vc: no easy way to get diff of all outgoing changes

[Eli Zaretskii]

> Cc: 62940@debbugs.gnu.org, Filipp Gunbin <fgunbin@fastmail.fm>
> From: sbaugh@catern.com
> Date: Fri, 21 Apr 2023 18:36:01 +0000 (UTC)
> 
> After using it for a bit, one issue is that C-x v O shows changes in all
> files; it would be nice if there was a way to get outgoing
> changes/commits only for the current fileset.

What is the current fileset for "C-x v O"? isn't that "all files"
anyway?

I could understand if you were talking about a VC command invoked from
a vc-dir buffer or a Dired buffer, where you can mark files to be
considered as the current fileset.  That would mean your comment is
about the vc-dir command 'O' or the equivalent in Dired.  But
otherwise, I'm not sure I understand the UI you have in mind.

And then there's the issue of whether the backend supports what you
want, of course.

bug#62940: 29.0.60; vc: no easy way to get diff of all outgoing changes

[sbaugh]

Eli Zaretskii <eliz@gnu.org> writes:
>> Cc: 62940@debbugs.gnu.org, Filipp Gunbin <fgunbin@fastmail.fm>
>> From: sbaugh@catern.com
>> Date: Fri, 21 Apr 2023 18:36:01 +0000 (UTC)
>> 
>> After using it for a bit, one issue is that C-x v O shows changes in all
>> files; it would be nice if there was a way to get outgoing
>> changes/commits only for the current fileset.
>
> What is the current fileset for "C-x v O"? isn't that "all files"
> anyway?

Well, yes, that's the issue.  When my current fileset is not "all files"
(for example when I am in an individual file without marking files in a
vc-dir buffer) I would like to be able to see outgoing changes on just
those files.  C-x v O overrides the fileset to be "all files" instead.

> I could understand if you were talking about a VC command invoked from
> a vc-dir buffer or a Dired buffer, where you can mark files to be
> considered as the current fileset.  That would mean your comment is
> about the vc-dir command 'O' or the equivalent in Dired.  But
> otherwise, I'm not sure I understand the UI you have in mind.

1. View a file buffer
2. Hit C-x v O
3. See outgoing changes only for that file

> And then there's the issue of whether the backend supports what you
> want, of course.

bug#62940: 29.0.60; vc: no easy way to get diff of all outgoing changes

[Eli Zaretskii]

> From: sbaugh@catern.com
> Date: Sat, 22 Apr 2023 13:00:48 +0000 (UTC)
> Cc: sbaugh@janestreet.com, 62940@debbugs.gnu.org, fgunbin@fastmail.fm
> 
> 1. View a file buffer
> 2. Hit C-x v O
> 3. See outgoing changes only for that file

I don't think it is TRT to change the behavior of "C-x v O" in the
above scenario.  Because it will make it much harder to see the
changes for all the files, since in Emacs you are almost always "in
some file-visiting buffer".

So the above can only be optional behavior, not the default.

bug#62940: 29.0.60; vc: no easy way to get diff of all outgoing changes

[Dmitry Gutov]

On 22/04/2023 16:17, Eli Zaretskii wrote:
>> From:sbaugh@catern.com
>> Date: Sat, 22 Apr 2023 13:00:48 +0000 (UTC)
>> Cc:sbaugh@janestreet.com,62940@debbugs.gnu.org,fgunbin@fastmail.fm
>>
>> 1. View a file buffer
>> 2. Hit C-x v O
>> 3. See outgoing changes only for that file
> I don't think it is TRT to change the behavior of "C-x v O" in the
> above scenario.  Because it will make it much harder to see the
> changes for all the files, since in Emacs you are almost always "in
> some file-visiting buffer".
> 
> So the above can only be optional behavior, not the default.
> 
> 
> 

We could add a couple of new commands, with -fileset- in their names. 
Couple - for incoming and outgoing.

bug#62940: 29.0.60; vc: no easy way to get diff of all outgoing changes

[Eli Zaretskii]

> Date: Sat, 22 Apr 2023 21:33:35 +0300
> Cc: sbaugh@janestreet.com, 62940@debbugs.gnu.org, fgunbin@fastmail.fm
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> On 22/04/2023 16:17, Eli Zaretskii wrote:
> > I don't think it is TRT to change the behavior of "C-x v O" in the
> > above scenario.  Because it will make it much harder to see the
> > changes for all the files, since in Emacs you are almost always "in
> > some file-visiting buffer".
> > 
> > So the above can only be optional behavior, not the default.
> 
> We could add a couple of new commands, with -fileset- in their names. 
> Couple - for incoming and outgoing.

That would work, of course.

bug#62940: 29.0.60; vc: no easy way to get diff of all outgoing changes

[Spencer Baugh]

Juri Linkov <juri@linkov.net> writes:
>>   C-x v O C-x h =
>>
>> Which would be D. in the original message. It doesn't support uncommitted
>> changes, but I would be surprised if it did.
>
> It's supported only for vc that inserts a text header in logs like RCS.
> I proposed to insert extra line that represents uncommitted git changes,
> and I'm using this feature all the time.

Is your patch for this available somewhere?  I find myself wanting the
same thing for hg.

> But the problem is that it's inserted even when 'C-x v b l' lists
> another branch where such line makes no sense.

Maybe we could only insert this line if there are actually uncommitted
changes?

bug#62940: 29.0.60; vc: no easy way to get diff of all outgoing changes

[Juri Linkov]

>>>   C-x v O C-x h =
>>>
>>> Which would be D. in the original message. It doesn't support uncommitted
>>> changes, but I would be surprised if it did.
>>
>> It's supported only for vc that inserts a text header in logs like RCS.
>> I proposed to insert extra line that represents uncommitted git changes,
>> and I'm using this feature all the time.
>
> Is your patch for this available somewhere?  I find myself wanting the
> same thing for hg.

Searching the right version in the archives would take much time,
so it's easier just to send the version that I use every day.
Here it is, just 3 new lines:

diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index a7763360795..37dc619da5e 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -1419,6 +1419,9 @@ vc-git-print-log
     ;; read-only.
     (let ((inhibit-read-only t))
       (with-current-buffer buffer
+        (when (memq vc-log-view-type '(long short))
+          (insert (propertize "(Type 'd' here to show diffs with working version)\n"
+                              'font-lock-face 'shadow)))
 	(apply #'vc-git-command buffer
 	       'async files
 	       (append

>> But the problem is that it's inserted even when 'C-x v b l' lists
>> another branch where such line makes no sense.
>
> Maybe we could only insert this line if there are actually uncommitted
> changes?

Good idea.  Also this line should not be shown in the output of
'vc-print-branch-log'.

PS: Sorry that answering your other requests about the project features
will take a little more time.

bug#62940: 29.0.60; vc: no easy way to get diff of all outgoing changes

[Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 903 bytes --]

Dmitry Gutov <dmitry@gutov.dev> writes:
> On 18/04/2023 23:36, Filipp Gunbin wrote:
>> Usually I just do C-x v L, select commits that interest me (via region),
>> and do M-x log-view-diff.
>
> Just in case: 'log-view-diff' is bound to '='.
>
> That's what I wanted to suggest, too. Or use 'vc-log-outgoing':
>
>   C-x v O C-x h =
>
> Which would be D. in the original message. It doesn't support
> uncommitted changes, but I would be surprised if it did.
>
> But one can also try this:
>
>   C-u C-x v D origin/master ^J
>
> Where origin/master is the name of the upstream branch ref. This
> should include uncommitted changes.

This works great, but is a bit tricky to use because you need to type
the upstream branch ref.  I think it would be good to provide that as an
additional default in the prompt for the old revision.  The attached
patch does that.  (and probably is enough to close this bug)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-a-last-pushed-revision-default-in-the-vc-diff-pr.patch --]
[-- Type: text/x-patch, Size: 3823 bytes --]

From 0a48d2973363361211c2e91c4f6c7bb75f16a853 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@janestreet.com>
Date: Fri, 13 Sep 2024 16:35:19 -0400
Subject: [PATCH] Add a "last pushed revision" default in the vc-diff prompt

C-u M-x vc-root-diff will prompt for the old revision to use for the
diff.  It includes the current working revision as a default.

Now it also includes the last pushed revision as a default, through a
new pushed-revision vc backend function.

* lisp/vc/vc-git.el (vc-git-pushed-revision): Add.
(vc-git-log-outgoing): Use vc-git-pushed-revision.
* lisp/vc/vc-hg.el (vc-hg-pushed-revision): Add.
* lisp/vc/vc.el (vc-default-pushed-revision): Add new backend function
with default implementation returning nil.
(vc-diff-build-argument-list-internal): Add the last pushed revision
as a default. (bug#62940)
---
 lisp/vc/vc-git.el | 10 +++++++---
 lisp/vc/vc-hg.el  |  6 ++++++
 lisp/vc/vc.el     |  7 +++++++
 3 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index 2a7c8ae5fc4..4afbb978a3f 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -1512,6 +1512,12 @@ vc-git-print-log
                   (list "-p"))
 		'("--")))))))
 
+(defun vc-git-pushed-revision (_file &optional remote-location)
+  "Return the ref for REMOTE-LOCATION."
+  (if (and remote-location (not (string-empty-p remote-location)))
+      remote-location
+    "@{upstream}"))
+
 (defun vc-git-log-outgoing (buffer remote-location)
   (vc-setup-buffer buffer)
   (apply #'vc-git-command buffer 'async nil
@@ -1520,9 +1526,7 @@ vc-git-log-outgoing
            ,(format "--pretty=tformat:%s" (car vc-git-root-log-format))
            "--abbrev-commit"
            ,@(ensure-list vc-git-shortlog-switches)
-           ,(concat (if (string= remote-location "")
-	                "@{upstream}"
-	              remote-location)
+           ,(concat (vc-git-pushed-revision nil remote-location)
 	            "..HEAD"))))
 
 (defun vc-git-log-incoming (buffer remote-location)
diff --git a/lisp/vc/vc-hg.el b/lisp/vc/vc-hg.el
index 876d86dc24f..c0afb225871 100644
--- a/lisp/vc/vc-hg.el
+++ b/lisp/vc/vc-hg.el
@@ -1422,6 +1422,12 @@ vc-hg-log-incoming
   (vc-hg-command buffer 1 nil "incoming" "-n" (unless (string= remote-location "")
 						remote-location)))
 
+(defun vc-hg-pushed-revision (_file &optional remote-location)
+  "Return a revspec for the last commit not outgoing to REMOTE-LOCATION."
+  (unless remote-location
+    (setq remote-location ""))
+  (format "last(. and not outgoing(%s))" remote-location))
+
 (defun vc-hg-log-outgoing (buffer remote-location)
   (vc-setup-buffer buffer)
   (vc-hg-command buffer 1 nil "outgoing" "-n" (unless (string= remote-location "")
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 597a1622f5a..0125924cc51 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -2061,6 +2061,11 @@ vc-read-multiple-revisions
 INITIAL-INPUT are passed on to `vc-read-revision' directly."
   (vc-read-revision prompt files backend default initial-input t))
 
+(defun vc-default-pushed-revision (_file &optional _remote-location)
+  "Return the last pushed revision of FILE.
+The default is to return nil always."
+  nil)
+
 (defun vc-diff-build-argument-list-internal (&optional fileset)
   "Build argument list for calling internal diff functions."
   (let* ((vc-fileset (or fileset (vc-deduce-fileset t))) ;FIXME: why t?  --Stef
@@ -2069,6 +2074,8 @@ vc-diff-build-argument-list-internal
          (first (car files))
          (rev1-default nil)
          ) ;; (rev2-default nil)
+    (when-let ((pushed-revision (vc-call-backend backend 'pushed-revision first)))
+      (push pushed-revision rev1-default))
     (cond
      ;; someday we may be able to do revision completion on non-singleton
      ;; filesets, but not yet.
-- 
2.39.3

bug#62940: 29.0.60; vc: no easy way to get diff of all outgoing changes

[Dmitry Gutov]

Hi!

On 13/09/2024 23:54, Spencer Baugh wrote:
> This works great, but is a bit tricky to use because you need to type
> the upstream branch ref.  I think it would be good to provide that as an
> additional default in the prompt for the old revision.  The attached
> patch does that.  (and probably is enough to close this bug)C-u M-x vc-root-diff will prompt for the old revision to use for the
> diff.  It includes the current working revision as a default.

I like the idea. See comments below.

> Now it also includes the last pushed revision as a default, through a
> new pushed-revision vc backend function.* lisp/vc/vc-git.el (vc-git-pushed-revision): Add.
> (vc-git-log-outgoing): Use vc-git-pushed-revision.
> * lisp/vc/vc-hg.el (vc-hg-pushed-revision): Add.
> * lisp/vc/vc.el (vc-default-pushed-revision): Add new backend function
> with default implementation returning nil.

Am I too used to Git, or should this be called upstream-revision or 
origin-revision?

The user might have just recently cloned the repository and not pushed 
anything yet. Anyway, matter of taste.

(defun vc-hg-pushed-revision (_file &optional remote-location)
> +  "Return a revspec for the last commit not outgoing to REMOTE-LOCATION."
> +  (unless remote-location
> +    (setq remote-location ""))
> +  (format "last(. and not outgoing(%s))" remote-location))

Is the second argument even non-nil in the Hg implementation?

(defun vc-diff-build-argument-list-internal (&optional fileset)
>     "Build argument list for calling internal diff functions."
>     (let* ((vc-fileset (or fileset (vc-deduce-fileset t))) ;FIXME: why t?  --Stef
> @@ -2069,6 +2074,8 @@ vc-diff-build-argument-list-internal
>            (first (car files))
>            (rev1-default nil)
>            ) ;; (rev2-default nil)
> +    (when-let ((pushed-revision (vc-call-backend backend 'pushed-revision first)))
> +      (push pushed-revision rev1-default))

Maybe we would prefer to keep the current suggestion as the first 
default, and have the upstream as the second suggestion (on M-n)? For 
backward compatibility anyway. I'm not 100% sure - the upstream ref 
seems like a more useful default in a lot of cases (and harder to type 
out when you do want it), but it's also workflow-specific.

bug#62940: 29.0.60; vc: no easy way to get diff of all outgoing changes

[Eli Zaretskii]

> Cc: 62940@debbugs.gnu.org, Filipp Gunbin <fgunbin@fastmail.fm>
> Date: Sat, 14 Sep 2024 05:11:48 +0300
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> > (vc-git-log-outgoing): Use vc-git-pushed-revision.
> > * lisp/vc/vc-hg.el (vc-hg-pushed-revision): Add.
> > * lisp/vc/vc.el (vc-default-pushed-revision): Add new backend function
> > with default implementation returning nil.
> 
> Am I too used to Git, or should this be called upstream-revision or 
> origin-revision?

upstream-revision is definitely better than
vc-default-pushed-revision, since some VCSes don't even have the
"push" operation.

More importantly, this change must be accompanied with a suitable
update of the user manual, where we should explain what commit is
suggested as the default.  "Last pushed revision" is somewhat vague
and inaccurate, because the user could switch branches or remotes, or
do something else.  We should find a more accurate description.  Also,
the doc string of vc-root-diff needs to be updated with this
information.

> Maybe we would prefer to keep the current suggestion as the first 
> default, and have the upstream as the second suggestion (on M-n)? For 
> backward compatibility anyway.

Sounds like a good idea to me.

bug#62940: 29.0.60; vc: no easy way to get diff of all outgoing changes

[Dmitry Gutov]

Hi Eli,

On 14/09/2024 10:12, Eli Zaretskii wrote:
> More importantly, this change must be accompanied with a suitable
> update of the user manual, where we should explain what commit is
> suggested as the default.  "Last pushed revision" is somewhat vague
> and inaccurate, because the user could switch branches or remotes, or
> do something else.  We should find a more accurate description.  Also,
> the doc string of vc-root-diff needs to be updated with this
> information.

I wonder how you'd like to see these changes described.

For example, here's how I could document the just-pushed change 
(related, but for a different bug), rev a295d7de9e8.

If we also add the story about the second default being the upstream 
revision, with a description of how such is determined, it might 
overload the text. Maybe for no good reason if most people don't use 
'C-u' with 'C-x v =' anyway, even if for some it's handy.

Should this be a whole separate node, "Reading Revisions for Diff With 
Completion"?

diff --git a/doc/emacs/maintaining.texi b/doc/emacs/maintaining.texi
index 99219b7f5d7..ba60bfddcd1 100644
--- a/doc/emacs/maintaining.texi
+++ b/doc/emacs/maintaining.texi
@@ -883,11 +883,12 @@ Old Revisions
    To compare two arbitrary revisions of the current VC fileset, call
  @code{vc-diff} with a prefix argument: @kbd{C-u C-x v =}.  This
  prompts for two revision IDs (@pxref{VCS Concepts}), and displays a
-diff between those versions of the fileset.  This will not work
+diff between those versions of the fileset.  The first one defaults to
+the previous revision when the fileset changed, and the second defaults
+to nil, which means the contents of the work tree.  This will not work
  reliably for multi-file VC filesets, if the version control system is
-file-based rather than changeset-based (e.g., CVS), since then
-revision IDs for different files would not be related in any
-meaningful way.
+file-based rather than changeset-based (e.g., CVS), since then revision
+IDs for different files would not be related in any meaningful way.

    Instead of the revision ID, some version control systems let you
  specify revisions in other formats.  For instance, under Bazaar you
@@ -920,7 +921,9 @@ Old Revisions
  @code{vc-root-diff} with a prefix argument: @kbd{C-u C-x v D}.  This
  prompts for two revision IDs (@pxref{VCS Concepts}), and displays a
  diff between those versions of the entire version-controlled directory
-trees (RCS, SCCS, CVS, and SRC do not support this feature).
+trees (RCS, SCCS, CVS, and SRC do not support this feature).  The first
+one defaults to the previous revision when the fileset changed, and the
+second defaults to nil, which means the contents of the work tree.

  @vindex vc-diff-switches
    You can customize the @command{diff} options that @kbd{C-x v =} and

bug#62940: 29.0.60; vc: no easy way to get diff of all outgoing changes

[Eli Zaretskii]

> Date: Thu, 10 Oct 2024 03:26:23 +0300
> Cc: sbaugh@janestreet.com, 62940@debbugs.gnu.org, fgunbin@fastmail.fm
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> On 14/09/2024 10:12, Eli Zaretskii wrote:
> > More importantly, this change must be accompanied with a suitable
> > update of the user manual, where we should explain what commit is
> > suggested as the default.  "Last pushed revision" is somewhat vague
> > and inaccurate, because the user could switch branches or remotes, or
> > do something else.  We should find a more accurate description.  Also,
> > the doc string of vc-root-diff needs to be updated with this
> > information.
> 
> I wonder how you'd like to see these changes described.

What I had in mind was to explain what we mean by "last pushed
revision".  AFAICT, you use "the previous revision when the fileset
changed" instead.  IMO, this terminology has the same problem: it
doesn't account for changing branches or remotes, for example.  We
should somehow qualify the description by those situations (which I
agree are somewhat exceptional, but definitely not rare enough to be
ignored).  Moreover, the patch to which I posted the comments uses
"last pushed revision" all over the place, so if we want to use your
proposed terminology instead, we had better modified the doc strings
to use it as well.

> If we also add the story about the second default being the upstream 
> revision, with a description of how such is determined, it might 
> overload the text. Maybe for no good reason if most people don't use 
> 'C-u' with 'C-x v =' anyway, even if for some it's handy.

I don't see a reason why explaining that should take more than a
couple of sentences.

> Should this be a whole separate node, "Reading Revisions for Diff With 
> Completion"?

I don't think that is needed.  If we think some parts of the
description are "too much detail", we could have them in footnotes.

bug#62940: 29.0.60; vc: no easy way to get diff of all outgoing changes

[Dmitry Gutov]

On 10/10/2024 08:27, Eli Zaretskii wrote:
>> Date: Thu, 10 Oct 2024 03:26:23 +0300
>> Cc: sbaugh@janestreet.com, 62940@debbugs.gnu.org, fgunbin@fastmail.fm
>> From: Dmitry Gutov <dmitry@gutov.dev>
>>
>> On 14/09/2024 10:12, Eli Zaretskii wrote:
>>> More importantly, this change must be accompanied with a suitable
>>> update of the user manual, where we should explain what commit is
>>> suggested as the default.  "Last pushed revision" is somewhat vague
>>> and inaccurate, because the user could switch branches or remotes, or
>>> do something else.  We should find a more accurate description.  Also,
>>> the doc string of vc-root-diff needs to be updated with this
>>> information.
>>
>> I wonder how you'd like to see these changes described.
> 
> What I had in mind was to explain what we mean by "last pushed
> revision".  AFAICT, you use "the previous revision when the fileset
> changed" instead.  IMO, this terminology has the same problem: it
> doesn't account for changing branches or remotes, for example.  We
> should somehow qualify the description by those situations (which I
> agree are somewhat exceptional, but definitely not rare enough to be
> ignored).

Not sure what you mean by changing branches, given the revision default 
that is used is determined by the tip of the local branch.

> Moreover, the patch to which I posted the comments uses
> "last pushed revision" all over the place, so if we want to use your
> proposed terminology instead, we had better modified the doc strings
> to use it as well.

It's another term, we'll actually need both (possibly rephrased).

>> If we also add the story about the second default being the upstream
>> revision, with a description of how such is determined, it might
>> overload the text. Maybe for no good reason if most people don't use
>> 'C-u' with 'C-x v =' anyway, even if for some it's handy.
> 
> I don't see a reason why explaining that should take more than a
> couple of sentences.
> 
>> Should this be a whole separate node, "Reading Revisions for Diff With
>> Completion"?
> 
> I don't think that is needed.  If we think some parts of the
> description are "too much detail", we could have them in footnotes.

Okay, check out this attempt. It might be considered too cluttered. 
Perhaps you will have suggestions for improvement.

diff --git a/doc/emacs/maintaining.texi b/doc/emacs/maintaining.texi
index 99219b7f5d7..d50219f0688 100644
--- a/doc/emacs/maintaining.texi
+++ b/doc/emacs/maintaining.texi
@@ -881,13 +881,16 @@ Old Revisions

  @kindex C-u C-x v =
    To compare two arbitrary revisions of the current VC fileset, call
-@code{vc-diff} with a prefix argument: @kbd{C-u C-x v =}.  This
-prompts for two revision IDs (@pxref{VCS Concepts}), and displays a
-diff between those versions of the fileset.  This will not work
-reliably for multi-file VC filesets, if the version control system is
-file-based rather than changeset-based (e.g., CVS), since then
-revision IDs for different files would not be related in any
-meaningful way.
+@code{vc-diff} with a prefix argument: @kbd{C-u C-x v =}.  This prompts
+for two revision IDs (@pxref{VCS Concepts}), and displays a diff between
+those versions of the fileset.  The first one has several default
+values: the revision before the last one when the fileset changed, and
+the last revision of the current branch's upstream.
+The second defaults to nil, which means the contents of
+the work tree.  This will not work reliably for multi-file VC filesets,
+if the version control system is file-based rather than changeset-based
+(e.g., CVS), since then revision IDs for different files would not be
+related in any meaningful way.

    Instead of the revision ID, some version control systems let you
  specify revisions in other formats.  For instance, under Bazaar you
@@ -921,6 +924,9 @@ Old Revisions
  prompts for two revision IDs (@pxref{VCS Concepts}), and displays a
  diff between those versions of the entire version-controlled directory
  trees (RCS, SCCS, CVS, and SRC do not support this feature).
+The first one has several default values: the revision before the last
+one when the fileset changed, and the last revision of the current
+branch's upstream.

  @vindex vc-diff-switches
    You can customize the @command{diff} options that @kbd{C-x v =} and

bug#62940: 29.0.60; vc: no easy way to get diff of all outgoing changes

[Dmitry Gutov]

BTW:

On 13/09/2024 23:54, Spencer Baugh wrote:
> +(defun vc-git-pushed-revision (_file &optional remote-location)
> +  "Return the ref for REMOTE-LOCATION."
> +  (if (and remote-location (not (string-empty-p remote-location)))
> +      remote-location
> +    "@{upstream}"))

The function above and the function below

+(defun vc-hg-pushed-revision (_file &optional remote-location)
> +  "Return a revspec for the last commit not outgoing to REMOTE-LOCATION."
> +  (unless remote-location
> +    (setq remote-location ""))
> +  (format "last(. and not outgoing(%s))" remote-location))

...seem to be describing different kinds of refs: I think the Hg version 
refers to the last commit from the current branch that's present in the 
remote (the merge base), whereas the Git version points to the tip of 
the remote branch.

Which will be different if the remote has some newer changes added 
(perhaps someone else has pushed to it). vc-git-log-outgoing won't 
notice the difference because it uses the ".." range, but vc-git-diff 
compares two versions directly, so e.g. anything new added to the remote 
will be shown as removed in such diff.

We probably want "merge base with remote" for Git as well (git 
merge-base @{upstream} HEAD), but it doesn't have a similar short ref 
syntax.

Could use "merge-base(@{upstream})" or "@{upstream}.." as a special 
value, maybe.

bug#62940: 29.0.60; vc: no easy way to get diff of all outgoing changes

[Eli Zaretskii]

> Date: Fri, 11 Oct 2024 04:13:24 +0300
> Cc: sbaugh@janestreet.com, 62940@debbugs.gnu.org, fgunbin@fastmail.fm
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> Okay, check out this attempt. It might be considered too cluttered. 
> Perhaps you will have suggestions for improvement.

This LGTM, thanks.  One minor nit:

> +@code{vc-diff} with a prefix argument: @kbd{C-u C-x v =}.  This prompts
> +for two revision IDs (@pxref{VCS Concepts}), and displays a diff between
> +those versions of the fileset.  The first one has several default
> +values: the revision before the last one when the fileset changed, and
> +the last revision of the current branch's upstream.
> +The second defaults to nil, which means the contents of
   ^^^^^^^^^^
"Second" is ambiguous here (because of how the previous sentence
ends), so I suggest to say "The second revision ID" instead to
disambiguate it.

bug#62940: 29.0.60; vc: no easy way to get diff of all outgoing changes

[Dmitry Gutov]

On 11/10/2024 04:34, Dmitry Gutov wrote:
> BTW:
> 
> On 13/09/2024 23:54, Spencer Baugh wrote:
>> +(defun vc-git-pushed-revision (_file &optional remote-location)
>> +  "Return the ref for REMOTE-LOCATION."
>> +  (if (and remote-location (not (string-empty-p remote-location)))
>> +      remote-location
>> +    "@{upstream}"))
> 
> The function above and the function below
> 
> +(defun vc-hg-pushed-revision (_file &optional remote-location)
>> +  "Return a revspec for the last commit not outgoing to REMOTE- 
>> LOCATION."
>> +  (unless remote-location
>> +    (setq remote-location ""))
>> +  (format "last(. and not outgoing(%s))" remote-location))
> 
> ...seem to be describing different kinds of refs: I think the Hg version 
> refers to the last commit from the current branch that's present in the 
> remote (the merge base), whereas the Git version points to the tip of 
> the remote branch.
> 
> Which will be different if the remote has some newer changes added 
> (perhaps someone else has pushed to it). vc-git-log-outgoing won't 
> notice the difference because it uses the ".." range, but vc-git-diff 
> compares two versions directly, so e.g. anything new added to the remote 
> will be shown as removed in such diff.
> 
> We probably want "merge base with remote" for Git as well (git merge- 
> base @{upstream} HEAD), but it doesn't have a similar short ref syntax.
> 
> Could use "merge-base(@{upstream})" or "@{upstream}.." as a special 
> value, maybe.

Or alternatively if we would prefer to err on the side of warning the 
user that the upstream has diverged, I suppose it's the Hg 
implementation that would need to change. That can result a simpler 
mental model and documentation as well. What would be Hg's corresponding 
selector for "tip of upstream branch"?

bug#62940: 29.0.60; vc: no easy way to get diff of all outgoing changes

[Dmitry Gutov]

On 11/10/2024 09:07, Eli Zaretskii wrote:
>> +@code{vc-diff} with a prefix argument: @kbd{C-u C-x v =}.  This prompts
>> +for two revision IDs (@pxref{VCS Concepts}), and displays a diff between
>> +those versions of the fileset.  The first one has several default
>> +values: the revision before the last one when the fileset changed, and
>> +the last revision of the current branch's upstream.
>> +The second defaults to nil, which means the contents of
>     ^^^^^^^^^^
> "Second" is ambiguous here (because of how the previous sentence
> ends), so I suggest to say "The second revision ID" instead to
> disambiguate it.

Okay, that works.

bug#62940: 29.0.60; vc: no easy way to get diff of all outgoing changes

[Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 2642 bytes --]

On Fri, Oct 11, 2024, 10:39 AM Dmitry Gutov <dmitry@gutov.dev> wrote:

> On 11/10/2024 04:34, Dmitry Gutov wrote:
> > BTW:
> >
> > On 13/09/2024 23:54, Spencer Baugh wrote:
> >> +(defun vc-git-pushed-revision (_file &optional remote-location)
> >> +  "Return the ref for REMOTE-LOCATION."
> >> +  (if (and remote-location (not (string-empty-p remote-location)))
> >> +      remote-location
> >> +    "@{upstream}"))
> >
> > The function above and the function below
> >
> > +(defun vc-hg-pushed-revision (_file &optional remote-location)
> >> +  "Return a revspec for the last commit not outgoing to REMOTE-
> >> LOCATION."
> >> +  (unless remote-location
> >> +    (setq remote-location ""))
> >> +  (format "last(. and not outgoing(%s))" remote-location))
> >
> > ...seem to be describing different kinds of refs: I think the Hg version
> > refers to the last commit from the current branch that's present in the
> > remote (the merge base), whereas the Git version points to the tip of
> > the remote branch.
> >
> > Which will be different if the remote has some newer changes added
> > (perhaps someone else has pushed to it). vc-git-log-outgoing won't
> > notice the difference because it uses the ".." range, but vc-git-diff
> > compares two versions directly, so e.g. anything new added to the remote
> > will be shown as removed in such diff.
> >
> > We probably want "merge base with remote" for Git as well (git merge-
> > base @{upstream} HEAD), but it doesn't have a similar short ref syntax.
> >
> > Could use "merge-base(@{upstream})" or "@{upstream}.." as a special
> > value, maybe.
>
> Or alternatively if we would prefer to err on the side of warning the
> user that the upstream has diverged, I suppose it's the Hg
> implementation that would need to change. That can result a simpler
> mental model and documentation as well. What would be Hg's corresponding
> selector for "tip of upstream branch"?
>

I'm not sure, I don't think there's no way to specify that with an hg
revset.  Though like most hg users, I don't actually use branches, I use
bookmarks; there's a way to do it with bookmarks, I think. I'll investigate.

I think it's useful to think of these two options relative to vc-outgoing
and vc-incoming.  Comparing $1 against $2 is equivalent to getting the diff
from $3:
- working-tree merge-base = vc-outgoing
- merge-base remote-tip = vc-incoming
- working-tree remote-tip = vc-outgoing+vc-incoming

The first two are more useful than the third, I think, since the third
mixes together two sorts of diffs, making it ambiguous which is which.

>

[-- Attachment #2: Type: text/html, Size: 3664 bytes --]

bug#62940: 29.0.60; vc: no easy way to get diff of all outgoing changes

[Dmitry Gutov]

On 11/10/2024 18:10, Spencer Baugh wrote:

>     Or alternatively if we would prefer to err on the side of warning the
>     user that the upstream has diverged, I suppose it's the Hg
>     implementation that would need to change. That can result a simpler
>     mental model and documentation as well. What would be Hg's
>     corresponding
>     selector for "tip of upstream branch"?
> 
> 
> I'm not sure, I don't think there's no way to specify that with an hg 
> revset.  Though like most hg users, I don't actually use branches, I use 
> bookmarks; there's a way to do it with bookmarks, I think. I'll investigate.

Thanks. Or if it doesn't make sense for Hg (for example, if the remote 
tip will always be present locally too), that would also be good, then 
the current implementation is just right.

> I think it's useful to think of these two options relative to vc- 
> outgoing and vc-incoming.  Comparing $1 against $2 is equivalent to 
> getting the diff from $3:
> - working-tree merge-base = vc-outgoing
> - merge-base remote-tip = vc-incoming
> - working-tree remote-tip = vc-outgoing+vc-incoming
> 
> The first two are more useful than the third, I think, since the third 
> mixes together two sorts of diffs, making it ambiguous which is which.

Makes sense.

I think #2 might be more conveniently accessed from vc-log-incoming 
(which is the only way to view the separate commits and messages).

Anyway, it seems instead of vc-root-diff we can use vc-diff-mergebase, 
it has the same completion for revisions. With REV1=@{upstream} and 
REV2="", we'll get #1, with reverse - #2. The full key sequence:

   C-u C-x v M D @{upstream} RET C-j

Note that vc-*-mergebase is only implemented for Git. But 
vc-hg-log-incoming is defined -- that seems to indicate that 'mergebase' 
would be useful to support for it too.

bug#62940: 29.0.60; vc: no easy way to get diff of all outgoing changes

[Dmitry Gutov]

[-- Attachment #1: Type: text/plain, Size: 506 bytes --]

On 11/10/2024 23:28, Dmitry Gutov wrote:
> 
> Anyway, it seems instead of vc-root-diff we can use vc-diff-mergebase, 
> it has the same completion for revisions. With REV1=@{upstream} and 
> REV2="", we'll get #1, with reverse - #2. The full key sequence:
> 
>    C-u C-x v M D @{upstream} RET C-j
> 
> Note that vc-*-mergebase is only implemented for Git. But vc-hg-log- 
> incoming is defined -- that seems to indicate that 'mergebase' would be 
> useful to support for it too.

Here's my current patch.

[-- Attachment #2: vc-upstream-revision.diff --]
[-- Type: text/x-patch, Size: 6012 bytes --]

diff --git a/doc/emacs/maintaining.texi b/doc/emacs/maintaining.texi
index 99219b7f5d7..55803de6274 100644
--- a/doc/emacs/maintaining.texi
+++ b/doc/emacs/maintaining.texi
@@ -881,13 +881,16 @@ Old Revisions
 
 @kindex C-u C-x v =
   To compare two arbitrary revisions of the current VC fileset, call
-@code{vc-diff} with a prefix argument: @kbd{C-u C-x v =}.  This
-prompts for two revision IDs (@pxref{VCS Concepts}), and displays a
-diff between those versions of the fileset.  This will not work
-reliably for multi-file VC filesets, if the version control system is
-file-based rather than changeset-based (e.g., CVS), since then
-revision IDs for different files would not be related in any
-meaningful way.
+@code{vc-diff} with a prefix argument: @kbd{C-u C-x v =}.  This prompts
+for two revision IDs (@pxref{VCS Concepts}), and displays a diff between
+those versions of the fileset.  The first one has several default
+values: the revision before the last one when the fileset changed, and
+the last revision of the current branch's upstream.
+The second revision ID defaults to nil, which means the contents of
+the work tree.  This will not work reliably for multi-file VC filesets,
+if the version control system is file-based rather than changeset-based
+(e.g., CVS), since then revision IDs for different files would not be
+related in any meaningful way.
 
   Instead of the revision ID, some version control systems let you
 specify revisions in other formats.  For instance, under Bazaar you
@@ -921,6 +924,10 @@ Old Revisions
 prompts for two revision IDs (@pxref{VCS Concepts}), and displays a
 diff between those versions of the entire version-controlled directory
 trees (RCS, SCCS, CVS, and SRC do not support this feature).
+The first one has several default values: the revision before the last
+one when the fileset changed, and the last revision of the current
+branch's upstream.  The second revision ID defaults to nil, which means
+the contents of the work tree.
 
 @vindex vc-diff-switches
   You can customize the @command{diff} options that @kbd{C-x v =} and
diff --git a/etc/NEWS b/etc/NEWS
index 2cd3b5893a1..4e8081f870a 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -441,6 +441,7 @@ overlap.
 They suggest the previous revision as the default for REV1, not the last
 one as before.  This makes them different from 'vc-diff' and
 'vc-root-diff' when those are called without a prefix argument.
+This change also affects 'vc-diff-mergebase'.
 
 ** php-ts-mode
 
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index 5a7ffeffc9d..8727accf0ec 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -1522,6 +1522,12 @@ vc-git-print-log
                   (list "-p"))
 		'("--")))))))
 
+(defun vc-git-upstream-revision (_file &optional remote-location)
+  "Return the ref for REMOTE-LOCATION."
+  (if (and remote-location (not (string-empty-p remote-location)))
+      remote-location
+    "@{upstream}"))
+
 (defun vc-git-log-outgoing (buffer remote-location)
   (vc-setup-buffer buffer)
   (apply #'vc-git-command buffer 'async nil
@@ -1530,9 +1536,7 @@ vc-git-log-outgoing
            ,(format "--pretty=tformat:%s" (car vc-git-root-log-format))
            "--abbrev-commit"
            ,@(ensure-list vc-git-shortlog-switches)
-           ,(concat (if (string= remote-location "")
-	                "@{upstream}"
-	              remote-location)
+           ,(concat (vc-git-upstream-revision nil remote-location)
 	            "..HEAD"))))
 
 (defun vc-git-log-incoming (buffer remote-location)
diff --git a/lisp/vc/vc-hg.el b/lisp/vc/vc-hg.el
index 876d86dc24f..ff09c260f60 100644
--- a/lisp/vc/vc-hg.el
+++ b/lisp/vc/vc-hg.el
@@ -1422,6 +1422,12 @@ vc-hg-log-incoming
   (vc-hg-command buffer 1 nil "incoming" "-n" (unless (string= remote-location "")
 						remote-location)))
 
+(defun vc-hg-upstream-revision (_file &optional remote-location)
+  "Return a revspec for the last commit not outgoing to REMOTE-LOCATION."
+  (unless remote-location
+    (setq remote-location ""))
+  (format "last(. and not outgoing(%s))" remote-location))
+
 (defun vc-hg-log-outgoing (buffer remote-location)
   (vc-setup-buffer buffer)
   (vc-hg-command buffer 1 nil "outgoing" "-n" (unless (string= remote-location "")
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index a30ba06aec3..f716f652f6e 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -2061,6 +2061,11 @@ vc-read-multiple-revisions
 INITIAL-INPUT are passed on to `vc-read-revision' directly."
   (vc-read-revision prompt files backend default initial-input t))
 
+(defun vc-default-upstream-revision (_file &optional _remote-location)
+  "Return the last upstream revision of FILE.
+The default is to return nil always."
+  nil)
+
 (defun vc-diff-build-argument-list-internal (&optional fileset)
   "Build argument list for calling internal diff functions."
   (let* ((vc-fileset (or fileset (vc-deduce-fileset t))) ;FIXME: why t?  --Stef
@@ -2069,6 +2074,8 @@ vc-diff-build-argument-list-internal
          (first (car files))
          (rev1-default nil)
          ) ;; (rev2-default nil)
+    (when-let ((upstream-revision (vc-call-backend backend 'upstream-revision first)))
+      (push upstream-revision rev1-default))
     (cond
      ;; someday we may be able to do revision completion on non-singleton
      ;; filesets, but not yet.
@@ -2169,9 +2176,8 @@ vc-diff-mergebase
 The merge base is a common ancestor between REV1 and REV2 revisions."
   (interactive
    (vc-diff-build-argument-list-internal
-    (or (ignore-errors (vc-deduce-fileset t))
-        (let ((backend (or (vc-deduce-backend) (vc-responsible-backend default-directory))))
-          (list backend (list (vc-call-backend backend 'root default-directory)))))))
+    (let ((backend (or (vc-deduce-backend) (vc-responsible-backend default-directory))))
+      (list backend (list (vc-call-backend backend 'root default-directory))))))
   (when (and (not rev1) rev2)
     (error "Not a valid revision range"))
   (let ((backend (vc-deduce-backend))

bug#62940: 29.0.60; vc: no easy way to get diff of all outgoing changes

[Eli Zaretskii]

> Cc: 62940@debbugs.gnu.org, Filipp Gunbin <fgunbin@fastmail.fm>
> Date: Sat, 12 Oct 2024 01:38:38 +0300
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> --- a/doc/emacs/maintaining.texi
> +++ b/doc/emacs/maintaining.texi
> @@ -881,13 +881,16 @@ Old Revisions
>  
>  @kindex C-u C-x v =
>    To compare two arbitrary revisions of the current VC fileset, call
> -@code{vc-diff} with a prefix argument: @kbd{C-u C-x v =}.  This
> -prompts for two revision IDs (@pxref{VCS Concepts}), and displays a
> -diff between those versions of the fileset.  This will not work
> -reliably for multi-file VC filesets, if the version control system is
> -file-based rather than changeset-based (e.g., CVS), since then
> -revision IDs for different files would not be related in any
> -meaningful way.
> +@code{vc-diff} with a prefix argument: @kbd{C-u C-x v =}.  This prompts
> +for two revision IDs (@pxref{VCS Concepts}), and displays a diff between
> +those versions of the fileset.  The first one has several default
> +values: the revision before the last one when the fileset changed, and
> +the last revision of the current branch's upstream.
> +The second revision ID defaults to nil, which means the contents of
> +the work tree.  This will not work reliably for multi-file VC filesets,
> +if the version control system is file-based rather than changeset-based
> +(e.g., CVS), since then revision IDs for different files would not be
> +related in any meaningful way.
>  
>    Instead of the revision ID, some version control systems let you
>  specify revisions in other formats.  For instance, under Bazaar you
> @@ -921,6 +924,10 @@ Old Revisions
>  prompts for two revision IDs (@pxref{VCS Concepts}), and displays a
>  diff between those versions of the entire version-controlled directory
>  trees (RCS, SCCS, CVS, and SRC do not support this feature).
> +The first one has several default values: the revision before the last
> +one when the fileset changed, and the last revision of the current
> +branch's upstream.  The second revision ID defaults to nil, which means
> +the contents of the work tree.

Nitpicking: please use @code{nil} in Texinfo documentation, not a
literal "nil".