GNU bug report logs - #73232
[PATCH] Allow vc-diff to suggest a default revision in vc-dir

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 73232 in the body.
You can then email your comments to 73232 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: bug-gnu-emacs <at> gnu.org
Cc: Juri Linkov <juri <at> linkov.net>
Subject: [PATCH] Allow vc-diff to suggest a default revision in vc-dir
Date: Fri, 13 Sep 2024 11:51:47 -0400

[Message part 1 (text/plain, inline)]

Tags: patch


C-u M-x vc-root-diff will prompt for the old revision to use for the
diff.  The prompt will have a default calculated by
vc-diff-build-argument-list-internal.  The default is either the
working revision of the current fileset or the revision before that.

vc-diff-build-argument-list-internal contained a check (added in
c0d66cb21bac57f5ec0378e8a04aac8f35c3eb5c) which explicitly avoided
setting a default if the current fileset was a directory.  This check
was added in 1997 when vc only worked for single files.

This prevents a backend from choosing to return a non-nil value from
'working-revision when passed a directory.  (The vc-hg and vc-git
backends, at least, will do this)

Allow this by moving the file-directory-p check, so that we try
calling 'working-revision when the fileset is a single directory.  The
call is in inside ignore-errors, so if a backend errors when passed a
directory, we'll just get no default, as before.  (Most backends will
just return nil for a directory, rather than erroring)

Also, while we're here, explicitly pass the backend to
vc-working-revision rather than having vc-working-revision recompute
it.

Concretely this has the effect that for the vc-git and vc-hg backends,
running C-u M-x vc-root-diff in vc-dir will have the same behavior as
running C-u M-x vc-root-diff in a clean file: The "Previous revision:"
prompt's default will be the revision before HEAD.

* lisp/vc/vc.el (vc-diff-build-argument-list-internal): Move
file-directory-p check.

In GNU Emacs 29.2.50 (build 17, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.15.12, Xaw scroll bars) of 2024-09-06 built on
 igm-qws-u22796a
Repository revision: e6d04c06a7eb6ce932b52a346368d02b7a811a00
Repository branch: emacs-29
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: Rocky Linux 8.10 (Green Obsidian)

Configured using:
 'configure --with-x-toolkit=lucid --without-gpm --without-gconf
 --without-selinux --without-imagemagick --with-modules --with-gif=no
 --with-cairo --with-rsvg --without-compress-install
 --with-native-compilation=aot --with-tree-sitter
 PKG_CONFIG_PATH=/usr/local/home/garnish/libtree-sitter/0.22.6-1/lib/pkgconfig/'

[0001-Allow-vc-diff-to-suggest-a-default-revision-in-vc-di.patch (text/patch, attachment)]

Message #8 received at 73232 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: 73232 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#73232: [PATCH] Allow vc-diff to suggest a default revision in
 vc-dir
Date: Fri, 13 Sep 2024 19:10:52 +0300

> Cc: Juri Linkov <juri <at> linkov.net>
> Date: Fri, 13 Sep 2024 11:51:47 -0400
> From:  Spencer Baugh via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> -     ;; if it's a directory, don't supply any revision default
> -     ((file-directory-p first)
> -      nil)
>       ;; if the file is not up-to-date, use working revision as older revision
> -     ((not (vc-up-to-date-p first))
> -      (setq rev1-default (vc-working-revision first)))
> +     ((not (and (file-directory-p first) (vc-up-to-date-p first)))
> +      (setq rev1-default (vc-working-revision first backend)))

Doesn't this change the conditions under which we use
vc-working-revision for regular files?  Did you perhaps mean

   ((and (not (file-directory-p first)) (vc-up-to-date-p first))

instead?

Message #11 received at 73232 <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 73232 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#73232: [PATCH] Allow vc-diff to suggest a default revision
 in vc-dir
Date: Fri, 13 Sep 2024 12:25:10 -0400

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:
>> Cc: Juri Linkov <juri <at> linkov.net>
>> Date: Fri, 13 Sep 2024 11:51:47 -0400
>> From:  Spencer Baugh via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
>> 
>> -     ;; if it's a directory, don't supply any revision default
>> -     ((file-directory-p first)
>> -      nil)
>>       ;; if the file is not up-to-date, use working revision as older revision
>> -     ((not (vc-up-to-date-p first))
>> -      (setq rev1-default (vc-working-revision first)))
>> +     ((not (and (file-directory-p first) (vc-up-to-date-p first)))
>> +      (setq rev1-default (vc-working-revision first backend)))
>
> Doesn't this change the conditions under which we use
> vc-working-revision for regular files?  Did you perhaps mean
>
>    ((and (not (file-directory-p first)) (vc-up-to-date-p first))
>
> instead?

Oops, yes, fixed.

[0001-Allow-vc-diff-to-suggest-a-default-revision-in-vc-di.patch (text/x-patch, inline)]

From f87b92ce5775d0558b7d9dd502687d1f460bcd2a Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh <at> janestreet.com>
Date: Fri, 13 Sep 2024 11:34:55 -0400
Subject: [PATCH] Allow vc-diff to suggest a default revision in vc-dir

C-u M-x vc-root-diff will prompt for the old revision to use for the
diff.  The prompt will have a default calculated by
vc-diff-build-argument-list-internal.  The default is either the
working revision of the current fileset or the revision before that.

vc-diff-build-argument-list-internal contained a check (added in
c0d66cb21bac57f5ec0378e8a04aac8f35c3eb5c) which explicitly avoided
setting a default if the current fileset was a single directory.  This
check was added in 1997 when vc only worked for single files.

This prevents a backend from choosing to return a non-nil value from
'working-revision when passed a directory.  (The vc-hg and vc-git
backends, at least, will do this)

Allow this by moving the file-directory-p check, so that we try
calling 'working-revision when the fileset is a single directory.  The
call is in inside ignore-errors, so if a backend errors when passed a
directory, we'll just get no default, as before.  (Most backends will
just return nil for a directory, rather than erroring)

Also, while we're here, explicitly pass the backend to
vc-working-revision rather than having vc-working-revision recompute
it.

Concretely this has the effect that for the vc-git and vc-hg backends,
running C-u M-x vc-root-diff in vc-dir will have the same behavior as
running C-u M-x vc-root-diff in a clean file: The "Previous revision:"
prompt's default will be the revision before HEAD.

* lisp/vc/vc.el (vc-diff-build-argument-list-internal): Move
file-directory-p check. (bug#73232)
---
 lisp/vc/vc.el | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 597a1622f5a..7b1133565f9 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -2074,17 +2074,14 @@ vc-diff-build-argument-list-internal
      ;; filesets, but not yet.
      ((/= (length files) 1)
       nil)
-     ;; if it's a directory, don't supply any revision default
-     ((file-directory-p first)
-      nil)
      ;; if the file is not up-to-date, use working revision as older revision
-     ((not (vc-up-to-date-p first))
-      (setq rev1-default (vc-working-revision first)))
+     ((and (not (file-directory-p first)) (not (vc-up-to-date-p first)))
+      (setq rev1-default (vc-working-revision first backend)))
      ;; if the file is not locked, use last revision and current source as defaults
      (t
       (setq rev1-default (ignore-errors ;If `previous-revision' doesn't work.
                            (vc-call-backend backend 'previous-revision first
-                                            (vc-working-revision first))))
+                                            (vc-working-revision first backend))))
       (when (string= rev1-default "") (setq rev1-default nil))))
     ;; construct argument list
     (let* ((rev1-prompt (format-prompt "Older revision" rev1-default))
-- 
2.39.3

Message #14 received at 73232 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Spencer Baugh <sbaugh <at> janestreet.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 73232 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#73232: [PATCH] Allow vc-diff to suggest a default revision in
 vc-dir
Date: Sat, 14 Sep 2024 04:45:42 +0300

Hi Spencer,

On 13/09/2024 19:25, Spencer Baugh via Bug reports for GNU Emacs, the 
Swiss army knife of text editors wrote:
> Concretely this has the effect that for the vc-git and vc-hg backends,
> running C-u M-x vc-root-diff in vc-dir will have the same behavior as
> running C-u M-x vc-root-diff in a clean file: The "Previous revision:"
> prompt's default will be the revision before HEAD.

Is this consistent with the current behavior with files?

I mean, if there are any uncommitted changes in a file, we suggest the 
current revision as the one to diff against.

But with a directory we can't so easily determine whether are 
uncommitted changes, but in all likelihood, most of the time when you're 
working on a new feature, there would be. So statistically speaking, 
shouldn't we default to the "file with changes" behavior, suggesting the 
HEAD revision?

I can see where you're coming from though -- that default isn't very 
useful, one might as well not press C-u.

Maybe we should switch to suggesting the previous revision in the prompt 
even when file has changes?

> * lisp/vc/vc.el (vc-diff-build-argument-list-internal): Move
> file-directory-p check. (bug#73232)

Maybe this should mention reusing the value of backend too, for 
completeness.

Also I wonder if it's okay to have a multi-paragraph description in the 
commit message. CONTRIBUTE seems to suggest one paragraph (if any) 
between the summary and the ChangeLog-style contents, but it doesn't 
suggest moving any parts of the description to the bug tracker. But IDK, 
for those who still read the changelog files (instead of using 
vc-print-root-log) this might be too much for a short change. The 
detailed description is great, though, so thanks.

Eli and others, what do you think?

Message #17 received at 73232 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: sbaugh <at> janestreet.com, 73232 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#73232: [PATCH] Allow vc-diff to suggest a default revision in
 vc-dir
Date: Sat, 14 Sep 2024 10:04:27 +0300

> Date: Sat, 14 Sep 2024 04:45:42 +0300
> Cc: 73232 <at> debbugs.gnu.org, juri <at> linkov.net
> From: Dmitry Gutov <dmitry <at> gutov.dev>
> 
> Hi Spencer,
> 
> On 13/09/2024 19:25, Spencer Baugh via Bug reports for GNU Emacs, the 
> Swiss army knife of text editors wrote:
> > Concretely this has the effect that for the vc-git and vc-hg backends,
> > running C-u M-x vc-root-diff in vc-dir will have the same behavior as
> > running C-u M-x vc-root-diff in a clean file: The "Previous revision:"
> > prompt's default will be the revision before HEAD.
> 
> Is this consistent with the current behavior with files?
> 
> I mean, if there are any uncommitted changes in a file, we suggest the 
> current revision as the one to diff against.
> 
> But with a directory we can't so easily determine whether are 
> uncommitted changes, but in all likelihood, most of the time when you're 
> working on a new feature, there would be. So statistically speaking, 
> shouldn't we default to the "file with changes" behavior, suggesting the 
> HEAD revision?
> 
> I can see where you're coming from though -- that default isn't very 
> useful, one might as well not press C-u.
> 
> Maybe we should switch to suggesting the previous revision in the prompt 
> even when file has changes?
> 
> > * lisp/vc/vc.el (vc-diff-build-argument-list-internal): Move
> > file-directory-p check. (bug#73232)
> 
> Maybe this should mention reusing the value of backend too, for 
> completeness.
> 
> Also I wonder if it's okay to have a multi-paragraph description in the 
> commit message. CONTRIBUTE seems to suggest one paragraph (if any) 
> between the summary and the ChangeLog-style contents, but it doesn't 
> suggest moving any parts of the description to the bug tracker. But IDK, 
> for those who still read the changelog files (instead of using 
> vc-print-root-log) this might be too much for a short change. The 
> detailed description is great, though, so thanks.

I've modified CONTRIBUTE to not insist on "a paragraph".  (This is
exactly why I hate to codify stuff that is usually a no-brainer:
people tend to interpret each word and letter literally, as if they
were in a legally-binding document, and there's no end to complaints
and discussions about these unimportant details.)

My POV is that the description should be as clear and concise as
possible.  In most cases, there should be a bug report for the issue,
or some on-list discussion of it, and the gory details should be
described there, with only a reference left in the log message.  And,
of course, the important stuff that explains how the code works and
why it does something non-trivial should be in comments in the code.

IOW, I consider a long and very detailed description in the log
message a clear sign that something went wrong in the process: either
we didn't have the issue discussed in a place that can be referenced,
or the code needs to be cleaned-up or commented, or something else.
Git log is NOT the best place to document code changes.

> Eli and others, what do you think?

If you are asking about this bug report, then frankly I don't
understand the use case: when would a user want to invoke this command
with point on a directory?  Directories are never important items in
VC-related activities; files are.  When I'm looking at a tree with
changes, I'm never interested with directories, only with files.  It
is not a coincidence that "git status" will never say anything about
directories; e.g., if you create a directory, you will not see it in
the status, unlike a new file.

So I think I'd need to understand better the use case before I make up
my mind about this proposal.

Message #20 received at 73232 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: sbaugh <at> janestreet.com, 73232 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#73232: [PATCH] Allow vc-diff to suggest a default revision in
 vc-dir
Date: Sat, 14 Sep 2024 19:13:40 +0300

On 14/09/2024 10:04, Eli Zaretskii wrote:

>> Also I wonder if it's okay to have a multi-paragraph description in the
>> commit message. CONTRIBUTE seems to suggest one paragraph (if any)
>> between the summary and the ChangeLog-style contents, but it doesn't
>> suggest moving any parts of the description to the bug tracker. But IDK,
>> for those who still read the changelog files (instead of using
>> vc-print-root-log) this might be too much for a short change. The
>> detailed description is great, though, so thanks.
> 
> I've modified CONTRIBUTE to not insist on "a paragraph".  (This is
> exactly why I hate to codify stuff that is usually a no-brainer:
> people tend to interpret each word and letter literally, as if they
> were in a legally-binding document, and there's no end to complaints
> and discussions about these unimportant details.)
> 
> My POV is that the description should be as clear and concise as
> possible.  In most cases, there should be a bug report for the issue,
> or some on-list discussion of it, and the gory details should be
> described there, with only a reference left in the log message.  And,
> of course, the important stuff that explains how the code works and
> why it does something non-trivial should be in comments in the code.
> 
> IOW, I consider a long and very detailed description in the log
> message a clear sign that something went wrong in the process: either
> we didn't have the issue discussed in a place that can be referenced,
> or the code needs to be cleaned-up or commented, or something else.
> Git log is NOT the best place to document code changes.

Thanks. So if I understand this right, we're allowed to have multiple 
paragraphs in the commit message's description, but we'd rather not.

Perhaps we'd opt for this approach more often when there is no existing 
bug report.

>> Eli and others, what do you think?
> 
> If you are asking about this bug report, then frankly I don't
> understand the use case: when would a user want to invoke this command
> with point on a directory?

I was asking primarily about the documentation standard, so it's okay if 
you don't have a strong position on this. Thanks anyway.

> Directories are never important items in
> VC-related activities; files are.

IME it's pretty common to invoke vc-root-diff from the vc-dir buffer.

And we also added the capability to call vc-diff from Dired recently.

I don't do this myself often, TBH, but if someone is working in large 
repo, and the project is split into directories along subsystem lines, 
then one might want to make commits limited to subdirectories. Then it 
should be useful to call vc-diff on such a directory first.

Message #23 received at 73232 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: sbaugh <at> janestreet.com, 73232 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#73232: [PATCH] Allow vc-diff to suggest a default revision in
 vc-dir
Date: Sat, 14 Sep 2024 19:49:41 +0300

> Date: Sat, 14 Sep 2024 19:13:40 +0300
> Cc: sbaugh <at> janestreet.com, 73232 <at> debbugs.gnu.org, juri <at> linkov.net
> From: Dmitry Gutov <dmitry <at> gutov.dev>
> 
> On 14/09/2024 10:04, Eli Zaretskii wrote:
> 
> > Directories are never important items in
> > VC-related activities; files are.
> 
> IME it's pretty common to invoke vc-root-diff from the vc-dir buffer.

Yes, but vc-dir shows files.  Directories are shown, but when there
are files with "unusual" status, they dominate the display.

> I don't do this myself often, TBH, but if someone is working in large 
> repo, and the project is split into directories along subsystem lines, 
> then one might want to make commits limited to subdirectories. Then it 
> should be useful to call vc-diff on such a directory first.

Does anyone really do such things nowadays?

Message #26 received at 73232 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: sbaugh <at> janestreet.com, Eli Zaretskii <eliz <at> gnu.org>, 73232 <at> debbugs.gnu.org
Subject: Re: bug#73232: [PATCH] Allow vc-diff to suggest a default revision
 in vc-dir
Date: Tue, 17 Sep 2024 09:59:06 +0300

> And we also added the capability to call vc-diff from Dired recently.

Most of the time I'm using 'C-u C-x v =' on a file or directory
to select another branch as an "Older revision" to compare branches.
But no useful default is possible in such use cases.

Message #29 received at 73232 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Spencer Baugh <sbaugh <at> janestreet.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 73232 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#73232: [PATCH] Allow vc-diff to suggest a default revision in
 vc-dir
Date: Sat, 28 Sep 2024 00:49:53 +0300

Hi all,

On 14/09/2024 04:45, Dmitry Gutov wrote:
> 
> I can see where you're coming from though -- that default isn't very 
> useful, one might as well not press C-u.
> 
> Maybe we should switch to suggesting the previous revision in the prompt 
> even when file has changes?

Here's what seems to me an overall improvement, based on the original 
change. And more consistent as well.

* No special case for when FIRST is a directory OR it's not up-to-date.
* Make REV1-DEFAULT a list value.
* In 'vc-root-version-diff', don't try calling 'vc-deduce-fileset' and 
construct a (BACKEND DEFAULT-DIR) fileset right away.

As a result, 'C-u C-x v d' consistently provides completion and diff 
relating to the whole repository, not for files as point (if any). 
Previously, it used the revision that last touched the corresponding 
file, or nil, if the file was untracked (e.g. in Dired).

Further, don't offer the working revision as REV1-DEFAULT. Except for 
historical reasons and some idea of consistency, I can't see a scenario 
where that would be useful, which would not be covered by calling 'C-x v 
d' without a prefix. Someone please correct me here.

And combined with Spencer's patch from https://debbugs.gnu.org/62940#46, 
we get this:

* First default is HEAD^ (the last revision before the latest).
* Second default is @{upstream}.
* Then the elements from vc-revision-history.

WDYT?

Message #32 received at 73232 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Spencer Baugh <sbaugh <at> janestreet.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 73232 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#73232: [PATCH] Allow vc-diff to suggest a default revision in
 vc-dir
Date: Sat, 28 Sep 2024 00:55:23 +0300

[Message part 1 (text/plain, inline)]

And here's the diff.

On 28/09/2024 00:49, Dmitry Gutov wrote:
> Hi all,
> 
> On 14/09/2024 04:45, Dmitry Gutov wrote:
>>
>> I can see where you're coming from though -- that default isn't very 
>> useful, one might as well not press C-u.
>>
>> Maybe we should switch to suggesting the previous revision in the 
>> prompt even when file has changes?
> 
> Here's what seems to me an overall improvement, based on the original 
> change. And more consistent as well.
> 
> * No special case for when FIRST is a directory OR it's not up-to-date.
> * Make REV1-DEFAULT a list value.
> * In 'vc-root-version-diff', don't try calling 'vc-deduce-fileset' and 
> construct a (BACKEND DEFAULT-DIR) fileset right away.
> 
> As a result, 'C-u C-x v d' consistently provides completion and diff 
> relating to the whole repository, not for files as point (if any). 
> Previously, it used the revision that last touched the corresponding 
> file, or nil, if the file was untracked (e.g. in Dired).
> 
> Further, don't offer the working revision as REV1-DEFAULT. Except for 
> historical reasons and some idea of consistency, I can't see a scenario 
> where that would be useful, which would not be covered by calling 'C-x v 
> d' without a prefix. Someone please correct me here.
> 
> And combined with Spencer's patch from https://debbugs.gnu.org/62940#46, 
> we get this:
> 
> * First default is HEAD^ (the last revision before the latest).
> * Second default is @{upstream}.
> * Then the elements from vc-revision-history.
> 
> WDYT?

[vc-diff-build-argument-list-internal-for-root.diff (text/x-patch, attachment)]

Message #35 received at 73232 <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 73232 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#73232: [PATCH] Allow vc-diff to suggest a default revision
 in vc-dir
Date: Fri, 04 Oct 2024 07:38:55 -0400

Dmitry Gutov <dmitry <at> gutov.dev> writes:
> And here's the diff.
>
> On 28/09/2024 00:49, Dmitry Gutov wrote:
>> Hi all,
>> On 14/09/2024 04:45, Dmitry Gutov wrote:
>>>
>>> I can see where you're coming from though -- that default isn't
>>> very useful, one might as well not press C-u.
>>>
>>> Maybe we should switch to suggesting the previous revision in the
>>> prompt even when file has changes?
>> Here's what seems to me an overall improvement, based on the
>> original change. And more consistent as well.
>> * No special case for when FIRST is a directory OR it's not
>> up-to-date.
>> * Make REV1-DEFAULT a list value.
>> * In 'vc-root-version-diff', don't try calling 'vc-deduce-fileset'
>> and construct a (BACKEND DEFAULT-DIR) fileset right away.
>> As a result, 'C-u C-x v d' consistently provides completion and diff
>> relating to the whole repository, not for files as point (if
>> any). Previously, it used the revision that last touched the
>> corresponding file, or nil, if the file was untracked (e.g. in
>> Dired).
>> Further, don't offer the working revision as REV1-DEFAULT. Except
>> for historical reasons and some idea of consistency, I can't see a
>> scenario where that would be useful, which would not be covered by
>> calling 'C-x v d' without a prefix. Someone please correct me here.
>> And combined with Spencer's patch from
>> https://debbugs.gnu.org/62940#46, we get this:
>> * First default is HEAD^ (the last revision before the latest).
>> * Second default is @{upstream}.
>> * Then the elements from vc-revision-history.
>> WDYT?

This seems reasonable to me.  Then we have the following reliable
behaviors:

- [some diff command] RET
   diffs the working tree against HEAD
- C-u [some diff command] RET RET
   diffs the working tree against HEAD^
- C-u [some diff command] M-n RET RET
   diffs the working tree against @{upstream}

This seems like a big improvement, since previously would need to
actually read the prompt in the C-u case to figure out whether the
default was correct/what you wanted.

Message #40 received at 73232-done <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 73232-done <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#73232: [PATCH] Allow vc-diff to suggest a default revision in
 vc-dir
Date: Thu, 10 Oct 2024 02:36:54 +0300

On 04/10/2024 14:38, Spencer Baugh wrote:
>>> Here's what seems to me an overall improvement, based on the
>>> original change. And more consistent as well.
>>> * No special case for when FIRST is a directory OR it's not
>>> up-to-date.
>>> * Make REV1-DEFAULT a list value.
>>> * In 'vc-root-version-diff', don't try calling 'vc-deduce-fileset'
>>> and construct a (BACKEND DEFAULT-DIR) fileset right away.
>>> As a result, 'C-u C-x v d' consistently provides completion and diff
>>> relating to the whole repository, not for files as point (if
>>> any). Previously, it used the revision that last touched the
>>> corresponding file, or nil, if the file was untracked (e.g. in
>>> Dired).
>>> Further, don't offer the working revision as REV1-DEFAULT. Except
>>> for historical reasons and some idea of consistency, I can't see a
>>> scenario where that would be useful, which would not be covered by
>>> calling 'C-x v d' without a prefix. Someone please correct me here.
>>> And combined with Spencer's patch from
>>> https://debbugs.gnu.org/62940#46, we get this:
>>> * First default is HEAD^ (the last revision before the latest).
>>> * Second default is @{upstream}.
>>> * Then the elements from vc-revision-history.
>>> WDYT?
> This seems reasonable to me.  Then we have the following reliable
> behaviors:
> 
> - [some diff command] RET
>     diffs the working tree against HEAD
> - C-u [some diff command] RET RET
>     diffs the working tree against HEAD^
> - C-u [some diff command] M-n RET RET
>     diffs the working tree against @{upstream}
> 
> This seems like a big improvement, since previously would need to
> actually read the prompt in the C-u case to figure out whether the
> default was correct/what you wanted.

Yep, I think predictability is key.

Since there seem to be no objections, I've pushed the proposed change to 
master (just step number 2, the 3rd one is in another bug#). If 
anybody's having problems as a result, feedback welcome.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.