bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

[Jim Porter]

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

Currently, when running 'vc-annotate' for a Git repo, the buffer name is 
very long: it has the form "*Annotate FILE (rev REVISION)*", and for 
Git, REVISION is 40(?) characters long. Seeing the full Git SHA isn't 
(in my opinion) useful, especially not in a space-limited area like the 
mode line. At the default width of 80 columns, this pushes much of the 
mode line information off-screen.

Attached is a patch to add a 'short-revision' function for VC backends, 
and a Git implementation for it.

Does this make sense? Should there be an option to restore the previous 
behavior? (I'm not sure why anyone would *want* the old behavior, but 
I'm not opposed to adding an option.)

[-- Attachment #2: 0001-Abbreviate-the-VC-revision-in-vc-annotate-s-buffer-n.patch --]
[-- Type: text/plain, Size: 3308 bytes --]

From 9103aa64b64d0a79835c072086d040734ddf1c2a Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Fri, 10 Nov 2023 18:42:29 -0800
Subject: [PATCH] Abbreviate the VC revision in vc-annotate's buffer name

* lisp/vc/vc-annotate.el (vc-annotate): Call 'short-revision'

* lisp/vc/vc-hooks.el (vc-default-short-revision): New function.

* lisp/vc/vc-git.el (vc-git-short-revision): New function.
(vc-git--rev-parse): New optional argument SHORT.
---
 lisp/vc/vc-annotate.el |  4 +++-
 lisp/vc/vc-git.el      | 14 +++++++++++---
 lisp/vc/vc-hooks.el    |  5 +++++
 3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/lisp/vc/vc-annotate.el b/lisp/vc/vc-annotate.el
index de6c3adbbdb..85161347cbf 100644
--- a/lisp/vc/vc-annotate.el
+++ b/lisp/vc/vc-annotate.el
@@ -409,7 +409,9 @@ vc-annotate
 				  nil nil "20")))))))
   (vc-ensure-vc-buffer)
   (setq vc-annotate-display-mode display-mode) ;Not sure why.  --Stef
-  (let* ((temp-buffer-name (format "*Annotate %s (rev %s)*" (buffer-name) rev))
+  (let* ((temp-buffer-name (format "*Annotate %s (rev %s)*" (buffer-name)
+                                   (vc-call-backend (vc-backend file)
+                                                    'short-revision rev)))
          (temp-buffer-show-function 'vc-annotate-display-select)
          ;; If BUF is specified, we presume the caller maintains current line,
          ;; so we don't need to do it here.  This implementation may give
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index 707fc7cfc07..2ff6f5564ed 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -403,6 +403,11 @@ vc-git-working-revision
   (let (process-file-side-effects)
     (vc-git--rev-parse "HEAD")))
 
+(defun vc-git-short-revision (rev)
+  "Git-specific version of `vc-short-revision'."
+  (let (process-file-side-effects)
+    (vc-git--rev-parse rev 'short)))
+
 (defun vc-git--symbolic-ref (file)
   (or
    (vc-file-getprop file 'vc-git-symbolic-ref)
@@ -1830,11 +1835,14 @@ vc-git-previous-revision
     ;; does not (and cannot) quote.
     (vc-git--rev-parse (concat rev "~1"))))
 
-(defun vc-git--rev-parse (rev)
+(defun vc-git--rev-parse (rev &optional short)
   (with-temp-buffer
     (and
-     (vc-git--out-ok "rev-parse" rev)
-     (buffer-substring-no-properties (point-min) (+ (point-min) 40)))))
+     (apply #'vc-git--out-ok "rev-parse"
+            (append (when short '("--short"))
+                    (list rev)))
+     (goto-char (point-min))
+     (buffer-substring-no-properties (point) (pos-eol)))))
 
 (defun vc-git-next-revision (file rev)
   "Git-specific version of `vc-next-revision'."
diff --git a/lisp/vc/vc-hooks.el b/lisp/vc/vc-hooks.el
index c16fb63b2ff..38c84a0ceea 100644
--- a/lisp/vc/vc-hooks.el
+++ b/lisp/vc/vc-hooks.el
@@ -502,6 +502,11 @@ vc-working-revision
                            (vc-call-backend
                             backend 'working-revision file))))))
 
+(defun vc-default-short-revision (_backend rev)
+  "Return a \"shortened\" version of the revision REV.
+This default implementation simply returns REV unchanged."
+  rev)
+
 (defun vc-default-registered (backend file)
   "Check if FILE is registered in BACKEND using vc-BACKEND-master-templates."
   (let ((sym (vc-make-backend-sym backend 'master-templates)))
-- 
2.25.1

bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

[Eli Zaretskii]

> Date: Fri, 10 Nov 2023 18:49:59 -0800
> From: Jim Porter <jporterbugs@gmail.com>
> 
> Attached is a patch to add a 'short-revision' function for VC backends, 
> and a Git implementation for it.

If this is a Git-only issue, perhaps it would be better to have a
Git-only option, instead of defining a whole new VC method?

In any case, please document whatever is eventually accepted, both in
NEWS and in the manual.  (Frankly, I don't understand why patches are
not submitted with documentation to match to begin with.)

bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

[Jim Porter]

On 11/10/2023 11:41 PM, Eli Zaretskii wrote:
>> Date: Fri, 10 Nov 2023 18:49:59 -0800
>> From: Jim Porter <jporterbugs@gmail.com>
>>
>> Attached is a patch to add a 'short-revision' function for VC backends,
>> and a Git implementation for it.
> 
> If this is a Git-only issue, perhaps it would be better to have a
> Git-only option, instead of defining a whole new VC method?

Perhaps, though I'm not sure the best way to do that. I'll also take a 
look at some other VC backends to see if they could benefit. I usually 
use Git these days, so I haven't tried vc-annotate using a different 
backend.

> In any case, please document whatever is eventually accepted, both in
> NEWS and in the manual.

Definitely. This was just a sketch of a patch to make sure the idea 
makes sense and to get feedback on whether I should do this in a totally 
different way (which would likely require totally different 
documentation too).

bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

[Dmitry Gutov]

On 11/11/2023 09:41, Eli Zaretskii wrote:
> If this is a Git-only issue, perhaps it would be better to have a
> Git-only option, instead of defining a whole new VC method?

Our general approach is to prefer global options and dynamic dispatch on 
backends, resorting to using per-backend options when it's much easier 
to do.

In this case it might actually be more difficult to go the second route 
since the intention is to only use the short hash in this particular 
place. vc-annotate is common code and it will need to indicate that 
intention to the backend somehow.

bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

[Jim Porter]

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

On 11/11/2023 2:00 PM, Dmitry Gutov wrote:
> On 11/11/2023 09:41, Eli Zaretskii wrote:
>> If this is a Git-only issue, perhaps it would be better to have a
>> Git-only option, instead of defining a whole new VC method?
> 
> Our general approach is to prefer global options and dynamic dispatch on 
> backends, resorting to using per-backend options when it's much easier 
> to do.
> 
> In this case it might actually be more difficult to go the second route 
> since the intention is to only use the short hash in this particular 
> place. vc-annotate is common code and it will need to indicate that 
> intention to the backend somehow.

Thanks for taking a look. It sounds like the strategy I went with is at 
least approximately right, so here's an updated patch with a NEWS entry.

I looked through the manuals and didn't see anywhere to add a mention of 
this though. There's a section about 'vc-annotate', but it's written for 
an Emacs user, rather than an Elisp programmer, and I think trying to 
explain "short revisions" in that section would just add unnecessary 
detail. If we still want to add some mention of this to a manual, I 
guess it would make the most sense in some section about how to use the 
VC package as an Elisp programmer. I didn't see much about that though...

[-- Attachment #2: 0001-Abbreviate-the-VC-revision-in-vc-annotate-s-buffer-n.patch --]
[-- Type: text/plain, Size: 4017 bytes --]

From 3dd9434161b36f01397c3269ca839bca3db0d59e Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Fri, 10 Nov 2023 18:42:29 -0800
Subject: [PATCH] Abbreviate the VC revision in vc-annotate's buffer name

* lisp/vc/vc-annotate.el (vc-annotate): Call 'short-revision'

* lisp/vc/vc-hooks.el (vc-default-short-revision): New function.

* lisp/vc/vc-git.el (vc-git-short-revision): New function.
(vc-git--rev-parse): New optional argument SHORT.

* etc/NEWS: Announce this change (bug#67062).
---
 etc/NEWS               |  6 ++++++
 lisp/vc/vc-annotate.el |  4 +++-
 lisp/vc/vc-git.el      | 14 +++++++++++---
 lisp/vc/vc-hooks.el    |  5 +++++
 4 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 767e4c27b43..0632001648c 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -396,6 +396,12 @@ switches for shortlogs, such as the one produced by 'C-x v L'.
 *** Obsolete command 'vc-switch-backend' re-added as 'vc-change-backend'.
 The command was previously obsoleted and unbound in Emacs 28.
 
+---
+*** 'vc-annotate' can now abbreviate the revision in the buffer name.
+VC backends with a 'vc-BACKEND-short-revision' functions can convert a
+revision to a shorter form, and 'vc-annotate' will use this form in
+its buffer name.  Currently, the Git backend supports this.
+
 ** Diff mode
 
 +++
diff --git a/lisp/vc/vc-annotate.el b/lisp/vc/vc-annotate.el
index de6c3adbbdb..85161347cbf 100644
--- a/lisp/vc/vc-annotate.el
+++ b/lisp/vc/vc-annotate.el
@@ -409,7 +409,9 @@ vc-annotate
 				  nil nil "20")))))))
   (vc-ensure-vc-buffer)
   (setq vc-annotate-display-mode display-mode) ;Not sure why.  --Stef
-  (let* ((temp-buffer-name (format "*Annotate %s (rev %s)*" (buffer-name) rev))
+  (let* ((temp-buffer-name (format "*Annotate %s (rev %s)*" (buffer-name)
+                                   (vc-call-backend (vc-backend file)
+                                                    'short-revision rev)))
          (temp-buffer-show-function 'vc-annotate-display-select)
          ;; If BUF is specified, we presume the caller maintains current line,
          ;; so we don't need to do it here.  This implementation may give
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index 707fc7cfc07..2ff6f5564ed 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -403,6 +403,11 @@ vc-git-working-revision
   (let (process-file-side-effects)
     (vc-git--rev-parse "HEAD")))
 
+(defun vc-git-short-revision (rev)
+  "Git-specific version of `vc-short-revision'."
+  (let (process-file-side-effects)
+    (vc-git--rev-parse rev 'short)))
+
 (defun vc-git--symbolic-ref (file)
   (or
    (vc-file-getprop file 'vc-git-symbolic-ref)
@@ -1830,11 +1835,14 @@ vc-git-previous-revision
     ;; does not (and cannot) quote.
     (vc-git--rev-parse (concat rev "~1"))))
 
-(defun vc-git--rev-parse (rev)
+(defun vc-git--rev-parse (rev &optional short)
   (with-temp-buffer
     (and
-     (vc-git--out-ok "rev-parse" rev)
-     (buffer-substring-no-properties (point-min) (+ (point-min) 40)))))
+     (apply #'vc-git--out-ok "rev-parse"
+            (append (when short '("--short"))
+                    (list rev)))
+     (goto-char (point-min))
+     (buffer-substring-no-properties (point) (pos-eol)))))
 
 (defun vc-git-next-revision (file rev)
   "Git-specific version of `vc-next-revision'."
diff --git a/lisp/vc/vc-hooks.el b/lisp/vc/vc-hooks.el
index c16fb63b2ff..38c84a0ceea 100644
--- a/lisp/vc/vc-hooks.el
+++ b/lisp/vc/vc-hooks.el
@@ -502,6 +502,11 @@ vc-working-revision
                            (vc-call-backend
                             backend 'working-revision file))))))
 
+(defun vc-default-short-revision (_backend rev)
+  "Return a \"shortened\" version of the revision REV.
+This default implementation simply returns REV unchanged."
+  rev)
+
 (defun vc-default-registered (backend file)
   "Check if FILE is registered in BACKEND using vc-BACKEND-master-templates."
   (let ((sym (vc-make-backend-sym backend 'master-templates)))
-- 
2.25.1

bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

[Eli Zaretskii]

> Date: Sun, 12 Nov 2023 00:00:13 +0200
> Cc: 67062@debbugs.gnu.org
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> On 11/11/2023 09:41, Eli Zaretskii wrote:
> > If this is a Git-only issue, perhaps it would be better to have a
> > Git-only option, instead of defining a whole new VC method?
> 
> Our general approach is to prefer global options and dynamic dispatch on 
> backends, resorting to using per-backend options when it's much easier 
> to do.

Which I think is the case here.  What other VC backend has such long
revision strings?  I couldn't think of any.  And for Git, there could
be the choice of either shortening the SHA1 signature or using what
"git describe" returns.  Which is why I suggested an option specific
to vc-git.

> In this case it might actually be more difficult to go the second route 
> since the intention is to only use the short hash in this particular 
> place. vc-annotate is common code and it will need to indicate that 
> intention to the backend somehow.

I'm not sure I follow.  All we need is a new function to call instead
of vc-working-revision, that's all.  That new function will indicate
the intention to the backend.  Sounds easy enough.

IOW, if Git is a special case, there's IMO nothing wrong with having
code that is specific to Git.  Inventing a VC method that will do
nothing in every VCS but Git sounds un-economical and not very elegant
to me, FWIW.

bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

[Dmitry Gutov]

On 12/11/2023 08:03, Eli Zaretskii wrote:
> I'm not sure I follow.  All we need is a new function to call instead
> of vc-working-revision, that's all.  That new function will indicate
> the intention to the backend.  Sounds easy enough.

vc-annotate will check (if (eq backend 'Git)) and call a different 
function in such case? Rather dirty, but I guess that'll also work.

bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

[Eli Zaretskii]

> Date: Sun, 12 Nov 2023 12:58:06 +0200
> Cc: jporterbugs@gmail.com, 67062@debbugs.gnu.org
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> On 12/11/2023 08:03, Eli Zaretskii wrote:
> > I'm not sure I follow.  All we need is a new function to call instead
> > of vc-working-revision, that's all.  That new function will indicate
> > the intention to the backend.  Sounds easy enough.
> 
> vc-annotate will check (if (eq backend 'Git)) and call a different 
> function in such case? Rather dirty, but I guess that'll also work.

no, vc-annotate will call a new function, which will have a special
code for Git alone.

bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

[Dmitry Gutov]

On 12/11/2023 13:15, Eli Zaretskii wrote:
> no, vc-annotate will call a new function, which will have a special
> code for Git alone.

That's one extra indirection, but conceptually the same. We couldn't 
modify vc-working-revision for this, so it'll have to be a new function.

And the new function (name pending) will need to check (eq backend 'Git) 
and call vc-git-short-revision anyway.

bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

[Juri Linkov]

>> no, vc-annotate will call a new function, which will have a special
>> code for Git alone.
>
> That's one extra indirection, but conceptually the same. We couldn't modify
> vc-working-revision for this, so it'll have to be a new function.

But maybe could add a new optional arg to vc-working-revision?

bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

[Jim Porter]

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

On 11/11/2023 10:03 PM, Eli Zaretskii wrote:
>> Date: Sun, 12 Nov 2023 00:00:13 +0200
>> Cc: 67062@debbugs.gnu.org
>> From: Dmitry Gutov <dmitry@gutov.dev>
>>
>> Our general approach is to prefer global options and dynamic dispatch on
>> backends, resorting to using per-backend options when it's much easier
>> to do.
> 
> Which I think is the case here.  What other VC backend has such long
> revision strings?  I couldn't think of any.

Game of Trees[1] is one, though you could argue that that's cheating 
because it uses the Git repository format. It does have a GNU ELPA 
package though, so the author would probably want to add a 
'vc-got-short-revision' function. (Or something similar depending on 
what this patch looks like if/when it merges.) Looking at some GoT 
repositories, they *do* still use the long SHA-1 hashes for revision 
identifiers.

In fact, there are at least a couple Git-compatible VCSes now. Facebook 
wrote one called "Sapling", though I haven't used it. Based on some 
screenshots at least, it looks like Sapling also uses SHA-1 hashes for 
revision IDs.

In any case, we don't necessarily need to provide a default 
implementation for the 'short-revision' function. What about something 
like this? I'm not sure it's better, but it does let us avoid defining a 
no-op implementation for the "default backend".

[1] https://gameoftrees.org/index.html

[2] https://github.com/facebook/sapling

[-- Attachment #2: 0001-Abbreviate-the-VC-revision-in-vc-annotate-s-buffer-n.patch --]
[-- Type: text/plain, Size: 3351 bytes --]

From 0d55915e1984cb002b0f7b9a9e22d03c8b9431a7 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Fri, 10 Nov 2023 18:42:29 -0800
Subject: [PATCH] Abbreviate the VC revision in vc-annotate's buffer name

* lisp/vc/vc-annotate.el (vc-annotate): Try to call 'short-revision'.

* lisp/vc/vc-git.el (vc-git-short-revision): New function.
(vc-git--rev-parse): New optional argument SHORT.

* etc/NEWS: Announce this change (bug#67062).
---
 etc/NEWS               |  6 ++++++
 lisp/vc/vc-annotate.el |  6 +++++-
 lisp/vc/vc-git.el      | 14 +++++++++++---
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 767e4c27b43..0632001648c 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -396,6 +396,12 @@ switches for shortlogs, such as the one produced by 'C-x v L'.
 *** Obsolete command 'vc-switch-backend' re-added as 'vc-change-backend'.
 The command was previously obsoleted and unbound in Emacs 28.
 
+---
+*** 'vc-annotate' can now abbreviate the revision in the buffer name.
+VC backends with a 'vc-BACKEND-short-revision' functions can convert a
+revision to a shorter form, and 'vc-annotate' will use this form in
+its buffer name.  Currently, the Git backend supports this.
+
 ** Diff mode
 
 +++
diff --git a/lisp/vc/vc-annotate.el b/lisp/vc/vc-annotate.el
index de6c3adbbdb..bee8cf23872 100644
--- a/lisp/vc/vc-annotate.el
+++ b/lisp/vc/vc-annotate.el
@@ -409,7 +409,11 @@ vc-annotate
 				  nil nil "20")))))))
   (vc-ensure-vc-buffer)
   (setq vc-annotate-display-mode display-mode) ;Not sure why.  --Stef
-  (let* ((temp-buffer-name (format "*Annotate %s (rev %s)*" (buffer-name) rev))
+  (let* ((short-rev (or (ignore-errors (vc-call-backend (vc-backend file)
+                                                        'short-revision rev))
+                        rev))
+         (temp-buffer-name (format "*Annotate %s (rev %s)*" (buffer-name)
+                                   short-rev))
          (temp-buffer-show-function 'vc-annotate-display-select)
          ;; If BUF is specified, we presume the caller maintains current line,
          ;; so we don't need to do it here.  This implementation may give
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index 707fc7cfc07..2ff6f5564ed 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -403,6 +403,11 @@ vc-git-working-revision
   (let (process-file-side-effects)
     (vc-git--rev-parse "HEAD")))
 
+(defun vc-git-short-revision (rev)
+  "Git-specific version of `vc-short-revision'."
+  (let (process-file-side-effects)
+    (vc-git--rev-parse rev 'short)))
+
 (defun vc-git--symbolic-ref (file)
   (or
    (vc-file-getprop file 'vc-git-symbolic-ref)
@@ -1830,11 +1835,14 @@ vc-git-previous-revision
     ;; does not (and cannot) quote.
     (vc-git--rev-parse (concat rev "~1"))))
 
-(defun vc-git--rev-parse (rev)
+(defun vc-git--rev-parse (rev &optional short)
   (with-temp-buffer
     (and
-     (vc-git--out-ok "rev-parse" rev)
-     (buffer-substring-no-properties (point-min) (+ (point-min) 40)))))
+     (apply #'vc-git--out-ok "rev-parse"
+            (append (when short '("--short"))
+                    (list rev)))
+     (goto-char (point-min))
+     (buffer-substring-no-properties (point) (pos-eol)))))
 
 (defun vc-git-next-revision (file rev)
   "Git-specific version of `vc-next-revision'."
-- 
2.25.1

bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

[Eli Zaretskii]

> Date: Sun, 12 Nov 2023 11:07:38 -0800
> Cc: 67062@debbugs.gnu.org
> From: Jim Porter <jporterbugs@gmail.com>
> 
> In any case, we don't necessarily need to provide a default 
> implementation for the 'short-revision' function. What about something 
> like this? I'm not sure it's better, but it does let us avoid defining a 
> no-op implementation for the "default backend".

Fine by me, except that I think this should be optional behavior.

bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

[Jim Porter]

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

On 11/12/2023 11:11 AM, Eli Zaretskii wrote:
>> Date: Sun, 12 Nov 2023 11:07:38 -0800
>> Cc: 67062@debbugs.gnu.org
>> From: Jim Porter <jporterbugs@gmail.com>
>>
>> In any case, we don't necessarily need to provide a default
>> implementation for the 'short-revision' function. What about something
>> like this? I'm not sure it's better, but it does let us avoid defining a
>> no-op implementation for the "default backend".
> 
> Fine by me, except that I think this should be optional behavior.

Ok, how about this? If you think I should add a bit about this to the 
manual, let me know. However, it seems like a fairly minor thing to me, 
and I don't want to distract the manual reader from the more-useful parts.

(I also welcome a suggestion on a shorter name than 
'vc-annotate-abbreviate-revision-in-buffer-name', but I wanted to be 
careful not to give the impression that this would apply to the revision 
IDs that you see in the VC-Annotate buffer's *contents*. I guess I could 
abbreviate some of the existing words though, like "revision" -> "rev".)

[-- Attachment #2: 0001-Abbreviate-the-VC-revision-in-vc-annotate-s-buffer-n.patch --]
[-- Type: text/plain, Size: 3958 bytes --]

From 9726e1acb70d7d02648665cc259e174670ca839f Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Fri, 10 Nov 2023 18:42:29 -0800
Subject: [PATCH] Abbreviate the VC revision in vc-annotate's buffer name

* lisp/vc/vc-annotate.el
(vc-annotate-abbreviate-revision-in-buffer-name): New option...
(vc-annotate): ... use it, and try to call 'short-revision' when
requested.

* lisp/vc/vc-git.el (vc-git-short-revision): New function.
(vc-git--rev-parse): New optional argument SHORT.

* etc/NEWS: Announce this change (bug#67062).
---
 etc/NEWS               |  7 +++++++
 lisp/vc/vc-annotate.el | 13 ++++++++++++-
 lisp/vc/vc-git.el      | 14 +++++++++++---
 3 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 767e4c27b43..df11757e18d 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -396,6 +396,13 @@ switches for shortlogs, such as the one produced by 'C-x v L'.
 *** Obsolete command 'vc-switch-backend' re-added as 'vc-change-backend'.
 The command was previously obsoleted and unbound in Emacs 28.
 
+---
+*** 'vc-annotate' can now abbreviate the revision in the buffer name.
+When the option 'vc-annotate-abbreviate-revision-in-buffer-name' is
+non-nil (the default), 'vc-annotate' will use an abbreviated revision
+in its buffer name if the VC backend has a 'vc-BACKEND-short-revision'
+function.
+
 ** Diff mode
 
 +++
diff --git a/lisp/vc/vc-annotate.el b/lisp/vc/vc-annotate.el
index de6c3adbbdb..cacf05db6d9 100644
--- a/lisp/vc/vc-annotate.el
+++ b/lisp/vc/vc-annotate.el
@@ -162,6 +162,11 @@ vc-annotate-menu-elements
   :type '(repeat number)
   :group 'vc)
 
+(defcustom vc-annotate-abbreviate-revision-in-buffer-name t
+  "If non-nil, \\[vc-annotate] will use short revisions in its buffer name."
+  :type 'boolean
+  :group 'vc)
+
 (defvar-keymap vc-annotate-mode-map
   :doc "Local keymap used for VC-Annotate mode."
   "a"   #'vc-annotate-revision-previous-to-line
@@ -409,7 +414,13 @@ vc-annotate
 				  nil nil "20")))))))
   (vc-ensure-vc-buffer)
   (setq vc-annotate-display-mode display-mode) ;Not sure why.  --Stef
-  (let* ((temp-buffer-name (format "*Annotate %s (rev %s)*" (buffer-name) rev))
+  (let* ((displayed-rev
+          (or (and vc-annotate-abbreviate-revision-in-buffer-name
+                   (ignore-errors (vc-call-backend (vc-backend file)
+                                                   'short-revision rev)))
+              rev))
+         (temp-buffer-name (format "*Annotate %s (rev %s)*" (buffer-name)
+                                   displayed-rev))
          (temp-buffer-show-function 'vc-annotate-display-select)
          ;; If BUF is specified, we presume the caller maintains current line,
          ;; so we don't need to do it here.  This implementation may give
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index 707fc7cfc07..05216701fa9 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -403,6 +403,11 @@ vc-git-working-revision
   (let (process-file-side-effects)
     (vc-git--rev-parse "HEAD")))
 
+(defun vc-git-short-revision (rev)
+  "Return an abbreviated version of the revision REV."
+  (let (process-file-side-effects)
+    (vc-git--rev-parse rev 'short)))
+
 (defun vc-git--symbolic-ref (file)
   (or
    (vc-file-getprop file 'vc-git-symbolic-ref)
@@ -1830,11 +1835,14 @@ vc-git-previous-revision
     ;; does not (and cannot) quote.
     (vc-git--rev-parse (concat rev "~1"))))
 
-(defun vc-git--rev-parse (rev)
+(defun vc-git--rev-parse (rev &optional short)
   (with-temp-buffer
     (and
-     (vc-git--out-ok "rev-parse" rev)
-     (buffer-substring-no-properties (point-min) (+ (point-min) 40)))))
+     (apply #'vc-git--out-ok "rev-parse"
+            (append (when short '("--short"))
+                    (list rev)))
+     (goto-char (point-min))
+     (buffer-substring-no-properties (point) (pos-eol)))))
 
 (defun vc-git-next-revision (file rev)
   "Git-specific version of `vc-next-revision'."
-- 
2.25.1

bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

[Dmitry Gutov]

On 12/11/2023 21:07, Jim Porter wrote:
> In any case, we don't necessarily need to provide a default 
> implementation for the 'short-revision' function. What about something 
> like this? I'm not sure it's better, but it does let us avoid defining a 
> no-op implementation for the "default backend".

Looks worse to me than having the default "identity" implementation.

Sorry.

Though it might go in anyway, since Eli likes it.

bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

[Dmitry Gutov]

On 12/11/2023 20:48, Juri Linkov wrote:
>>> no, vc-annotate will call a new function, which will have a special
>>> code for Git alone.
>> That's one extra indirection, but conceptually the same. We couldn't modify
>> vc-working-revision for this, so it'll have to be a new function.
> But maybe could add a new optional arg to vc-working-revision?

That's worse: we'll end up with a number-of-args migration, having to 
use a condition-case form for a number of years. All for a new argument 
that isn't used by the majority of the backends.

bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

[Jim Porter]

On 11/12/2023 12:37 PM, Dmitry Gutov wrote:
> On 12/11/2023 20:48, Juri Linkov wrote:
>> But maybe could add a new optional arg to vc-working-revision?
> 
> That's worse: we'll end up with a number-of-args migration, having to 
> use a condition-case form for a number of years. All for a new argument 
> that isn't used by the majority of the backends.

Also, it wouldn't work for what we want to do: REV in 'vc-annotate' 
isn't necessarily the current revision; it could be any revision. We 
just want to take that revision and turn it into a friendlier (read: 
shorter) form for the buffer name.

bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

[Jim Porter]

On 11/12/2023 12:35 PM, Dmitry Gutov wrote:
> On 12/11/2023 21:07, Jim Porter wrote:
>> In any case, we don't necessarily need to provide a default 
>> implementation for the 'short-revision' function. What about something 
>> like this? I'm not sure it's better, but it does let us avoid defining 
>> a no-op implementation for the "default backend".
> 
> Looks worse to me than having the default "identity" implementation.
> 
> Sorry.
> 
> Though it might go in anyway, since Eli likes it.

I'll do whichever gets the patch merged. While I personally find the 
no-op implementation for the default backend to be cleaner, doing it the 
other way doesn't bother me enough that I'll fight for it.

bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

[Juri Linkov]

> We just want to take that revision and turn it into a friendlier
> (read: shorter) form for the buffer name.

Then the best solution is to introduce a new variable
'vc-short-revision' that vc-annotate should either set as a
buffer-local value or let-bind around the vc backend API call.

Then the git backend could use it optionally depending
on the value of a new user option 'vc-git-short-revision'.

This is much better that adding a new API call.  We don't add
new API calls lightly since any change in API requires updating
the documentation at the top of vc.el and at the top of vc backend
implementation files.

bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

[Dmitry Gutov]

On 13/11/2023 09:02, Juri Linkov wrote:
> Then the best solution is to introduce a new variable
> 'vc-short-revision' that vc-annotate should either set as a
> buffer-local value or let-bind around the vc backend API call.

That would also work for me.

bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Dmitry Gutov <dmitry@gutov.dev>,  Eli Zaretskii <eliz@gnu.org>,
>   67062@debbugs.gnu.org
> Date: Mon, 13 Nov 2023 09:02:35 +0200
> 
> > We just want to take that revision and turn it into a friendlier
> > (read: shorter) form for the buffer name.
> 
> Then the best solution is to introduce a new variable
> 'vc-short-revision' that vc-annotate should either set as a
> buffer-local value or let-bind around the vc backend API call.
> 
> Then the git backend could use it optionally depending
> on the value of a new user option 'vc-git-short-revision'.

This solution is also fine by me.

Thanks.

bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

[Jim Porter]

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

On 11/13/2023 6:19 AM, Eli Zaretskii wrote:
> This solution is also fine by me.
> 
> Thanks.

Ok, having tried out a few different implementation options, how about 
something like this? This adds a new function, 'vc-short-revision', that 
calls 'vc-working-revision' for most backends, but first checks if 
'vc-BACKEND-short-revision' exists; if so, it calls that instead. Then 
the Git backend defines this function to do the necessary legwork. 
Hopefully this strikes the right balance between not requiring any extra 
code for other backends (including the default "backend"), while also 
not hardcoding "(if (eq backend 'Git)" in a generic function.

In practice, this should work, though I should mention one caveat: when 
you pass a particular REV to 'vc-annotate', it will use that REV as-is 
in the buffer name. In my other patches, 'vc-annotate' would normalize 
the REV (so, for example, you could pass in the full 40-character Git 
SHA to 'vc-annotate' and the buffer name would only show the shortened 
form). I'm not sure this is a big deal though, since the Git revisions 
that you see on each line for 'vc-annotate' are the short revs by 
default; that means that when you call 
'vc-annotate-revision-previous-to-line', it'll pass the *short* rev at 
that line to 'vc-annotate'.

[-- Attachment #2: 0001-Abbreviate-the-VC-revision-in-vc-annotate-s-buffer-n.patch --]
[-- Type: text/plain, Size: 4362 bytes --]

From 67084d2292d93f582370503df74c68c0e95bd209 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 14 Dec 2023 11:31:27 -0800
Subject: [PATCH] Abbreviate the VC revision in vc-annotate's buffer name

* lisp/vc/vc-hooks.el (vc-short-revision): New function...

* lisp/vc/vc-annotate.el (vc-annotate): ... call it.

* lisp/vc/vc-git.el (vc-git-use-short-revisions): New option.
(vc-git-short-revision): New function.
(vc-git--rev-parse): Add SHORT argument.

* etc/NEWS: Announce this change (bug#67062).
---
 etc/NEWS               |  6 ++++++
 lisp/vc/vc-annotate.el |  2 +-
 lisp/vc/vc-git.el      | 20 +++++++++++++++++---
 lisp/vc/vc-hooks.el    |  8 ++++++++
 4 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 1ff2f8a149f..a2dcdfc901a 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -457,6 +457,12 @@ With this value only the revision number is displayed on the mode-line.
 *** Obsolete command 'vc-switch-backend' re-added as 'vc-change-backend'.
 The command was previously obsoleted and unbound in Emacs 28.
 
+---
+*** 'vc-annotate' can now abbreviate the revision in the buffer name.
+VC backends with a 'vc-BACKEND-short-revision' function can show the
+revision in a shorter form, and 'vc-annotate' will use this form in
+its buffer name.  Currently, the Git backend supports this.
+
 ** Diff mode
 
 +++
diff --git a/lisp/vc/vc-annotate.el b/lisp/vc/vc-annotate.el
index de6c3adbbdb..a63b8fe3ec2 100644
--- a/lisp/vc/vc-annotate.el
+++ b/lisp/vc/vc-annotate.el
@@ -397,7 +397,7 @@ vc-annotate
    (save-current-buffer
      (vc-ensure-vc-buffer)
      (list buffer-file-name
-	   (let ((def (vc-working-revision buffer-file-name)))
+	   (let ((def (vc-short-revision buffer-file-name)))
 	     (if (null current-prefix-arg) def
 	       (vc-read-revision
 		(format-prompt "Annotate from revision" def)
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index 2e057ecfaa7..f8675620568 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -111,6 +111,12 @@ vc-git
   :version "24.1"
   :group 'vc)
 
+(defcustom vc-git-use-short-revisions t
+  "If non-nil, use short Git revisions when requested.
+If nil, always use full Git revisions."
+  :type 'boolean
+  :version "30.1")
+
 (defcustom vc-git-diff-switches t
   "String or list of strings specifying switches for Git diff under VC.
 If nil, use the value of `vc-diff-switches'.  If t, use no switches."
@@ -403,6 +409,11 @@ vc-git-working-revision
   (let (process-file-side-effects)
     (vc-git--rev-parse "HEAD")))
 
+(defun vc-git-short-revision (_file)
+  "Return the working revision in abbreviated form."
+  (let (process-file-side-effects)
+    (vc-git--rev-parse "HEAD" 'short)))
+
 (defun vc-git--symbolic-ref (file)
   (or
    (vc-file-getprop file 'vc-git-symbolic-ref)
@@ -1832,11 +1843,14 @@ vc-git-previous-revision
     ;; does not (and cannot) quote.
     (vc-git--rev-parse (concat rev "~1"))))
 
-(defun vc-git--rev-parse (rev)
+(defun vc-git--rev-parse (rev &optional short)
   (with-temp-buffer
     (and
-     (vc-git--out-ok "rev-parse" rev)
-     (buffer-substring-no-properties (point-min) (+ (point-min) 40)))))
+     (apply #'vc-git--out-ok "rev-parse"
+            (append (when short '("--short")) ;; FIXME
+                    (list rev)))
+     (goto-char (point-min))
+     (buffer-substring-no-properties (point) (pos-eol)))))
 
 (defun vc-git-next-revision (file rev)
   "Git-specific version of `vc-next-revision'."
diff --git a/lisp/vc/vc-hooks.el b/lisp/vc/vc-hooks.el
index 8451128286b..0f36e2bc7ae 100644
--- a/lisp/vc/vc-hooks.el
+++ b/lisp/vc/vc-hooks.el
@@ -506,6 +506,14 @@ vc-working-revision
                            (vc-call-backend
                             backend 'working-revision file))))))
 
+(defun vc-short-revision (file &optional backend)
+  "Return the repository version for FILE in human-readable form.
+If FILE is not registered, this function always returns nil."
+  (setq backend (or backend (vc-backend file)))
+  (if (vc-find-backend-function backend 'short-revision)
+      (vc-call-backend backend 'short-revision file)
+    (vc-working-revision file backend)))
+
 (defun vc-default-registered (backend file)
   "Check if FILE is registered in BACKEND using vc-BACKEND-master-templates."
   (let ((sym (vc-make-backend-sym backend 'master-templates)))
-- 
2.25.1

bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

[Eli Zaretskii]

> Date: Thu, 14 Dec 2023 11:42:45 -0800
> Cc: 67062@debbugs.gnu.org, dmitry@gutov.dev
> From: Jim Porter <jporterbugs@gmail.com>
> 
> On 11/13/2023 6:19 AM, Eli Zaretskii wrote:
> > This solution is also fine by me.
> > 
> > Thanks.

Please in the future try retaining more of the context, so that what
"this solution" was will be clear even without the need to look up the
previous messages.  This is especially important when the previous
message happened long enough for people to forget the point where we
left it, like in this case.

> +*** 'vc-annotate' can now abbreviate the revision in the buffer name.
> +VC backends with a 'vc-BACKEND-short-revision' function can show the
> +revision in a shorter form, and 'vc-annotate' will use this form in
> +its buffer name.  Currently, the Git backend supports this.

This should mention the new defcustom as well.

Also, "only Git supports this" sounds like the other backends are in
some kind of disadvantage, whereas the truth is that only Git needs
this, since its usual full revision IDs are so long.

> +(defcustom vc-git-use-short-revisions t
> +  "If non-nil, use short Git revisions when requested.

The "when requested" part is too vague to leave it at that.  Please
elaborate in the doc string: when will it "be requested"?  One way is
to mention the commands affected by that option.

bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

[Jim Porter]

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

On 12/14/2023 11:51 AM, Eli Zaretskii wrote:
> Please in the future try retaining more of the context, so that what
> "this solution" was will be clear even without the need to look up the
> previous messages.  This is especially important when the previous
> message happened long enough for people to forget the point where we
> left it, like in this case.

Will do. (I use threading in my mail client, so I see all the context 
and didn't think about this.) The most-relevant message in the previous 
discussion is probably this one[1]:

On 11/11/2023 10:03 PM, Eli Zaretskii wrote:
 > I'm not sure I follow.  All we need is a new function to call instead
 > of vc-working-revision, that's all.  That new function will indicate
 > the intention to the backend.  Sounds easy enough.


>> +*** 'vc-annotate' can now abbreviate the revision in the buffer name.
>> +VC backends with a 'vc-BACKEND-short-revision' function can show the
>> +revision in a shorter form, and 'vc-annotate' will use this form in
>> +its buffer name.  Currently, the Git backend supports this.
> 
> This should mention the new defcustom as well.

Oops, I forgot to attach the latest version of my patch where I 
mentioned this already. Now fixed.

> Also, "only Git supports this" sounds like the other backends are in
> some kind of disadvantage, whereas the truth is that only Git needs
> this, since its usual full revision IDs are so long.

I changed this to: "Currently, this only applies to the Git backend."

>> +(defcustom vc-git-use-short-revisions t
>> +  "If non-nil, use short Git revisions when requested.
> 
> The "when requested" part is too vague to leave it at that.  Please
> elaborate in the doc string: when will it "be requested"?  One way is
> to mention the commands affected by that option.

Updated to mention 'vc-short-revision' and 'vc-annotate' specifically.

[1] https://lists.gnu.org/archive/html/bug-gnu-emacs/2023-11/msg00615.html

[-- Attachment #2: 0001-Abbreviate-the-VC-revision-in-vc-annotate-s-buffer-n.patch --]
[-- Type: text/plain, Size: 4550 bytes --]

From 1dba6e2acc90662069a2bf512dfcbf9a3cf5f0bb Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 14 Dec 2023 11:31:27 -0800
Subject: [PATCH] Abbreviate the VC revision in vc-annotate's buffer name

* lisp/vc/vc-hooks.el (vc-short-revision): New function...

* lisp/vc/vc-annotate.el (vc-annotate): ... call it.

* lisp/vc/vc-git.el (vc-git-use-short-revisions): New option.
(vc-git-short-revision): New function.
(vc-git--rev-parse): Add SHORT argument.

* etc/NEWS: Announce this change (bug#67062).
---
 etc/NEWS               |  8 ++++++++
 lisp/vc/vc-annotate.el |  2 +-
 lisp/vc/vc-git.el      | 23 ++++++++++++++++++++---
 lisp/vc/vc-hooks.el    |  8 ++++++++
 4 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 1ff2f8a149f..6d2cc8fd15e 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -457,6 +457,14 @@ With this value only the revision number is displayed on the mode-line.
 *** Obsolete command 'vc-switch-backend' re-added as 'vc-change-backend'.
 The command was previously obsoleted and unbound in Emacs 28.
 
+---
+*** 'vc-annotate' can now abbreviate the revision in the buffer name.
+VC backends with a 'vc-BACKEND-short-revision' function can show the
+revision in a shorter form, and 'vc-annotate' will use this form in
+its buffer name.  Currently, this only applies to the Git backend.  To
+restore the previous behavior, set 'vc-git-use-short-revisions' to
+nil.
+
 ** Diff mode
 
 +++
diff --git a/lisp/vc/vc-annotate.el b/lisp/vc/vc-annotate.el
index de6c3adbbdb..a63b8fe3ec2 100644
--- a/lisp/vc/vc-annotate.el
+++ b/lisp/vc/vc-annotate.el
@@ -397,7 +397,7 @@ vc-annotate
    (save-current-buffer
      (vc-ensure-vc-buffer)
      (list buffer-file-name
-	   (let ((def (vc-working-revision buffer-file-name)))
+	   (let ((def (vc-short-revision buffer-file-name)))
 	     (if (null current-prefix-arg) def
 	       (vc-read-revision
 		(format-prompt "Annotate from revision" def)
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index 2e057ecfaa7..572b8b9291c 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -111,6 +111,15 @@ vc-git
   :version "24.1"
   :group 'vc)
 
+(defcustom vc-git-use-short-revisions t
+  "If non-nil, return short Git revisions when calling `vc-short-revision'.
+If nil, always return full Git revisions.
+
+This affects the revision shown in the buffer name for
+`vc-annotate'."
+  :type 'boolean
+  :version "30.1")
+
 (defcustom vc-git-diff-switches t
   "String or list of strings specifying switches for Git diff under VC.
 If nil, use the value of `vc-diff-switches'.  If t, use no switches."
@@ -403,6 +412,11 @@ vc-git-working-revision
   (let (process-file-side-effects)
     (vc-git--rev-parse "HEAD")))
 
+(defun vc-git-short-revision (_file)
+  "Return the working revision in abbreviated form."
+  (let (process-file-side-effects)
+    (vc-git--rev-parse "HEAD" 'short)))
+
 (defun vc-git--symbolic-ref (file)
   (or
    (vc-file-getprop file 'vc-git-symbolic-ref)
@@ -1832,11 +1846,14 @@ vc-git-previous-revision
     ;; does not (and cannot) quote.
     (vc-git--rev-parse (concat rev "~1"))))
 
-(defun vc-git--rev-parse (rev)
+(defun vc-git--rev-parse (rev &optional short)
   (with-temp-buffer
     (and
-     (vc-git--out-ok "rev-parse" rev)
-     (buffer-substring-no-properties (point-min) (+ (point-min) 40)))))
+     (apply #'vc-git--out-ok "rev-parse"
+            (append (when short '("--short")) ;; FIXME
+                    (list rev)))
+     (goto-char (point-min))
+     (buffer-substring-no-properties (point) (pos-eol)))))
 
 (defun vc-git-next-revision (file rev)
   "Git-specific version of `vc-next-revision'."
diff --git a/lisp/vc/vc-hooks.el b/lisp/vc/vc-hooks.el
index 8451128286b..0f36e2bc7ae 100644
--- a/lisp/vc/vc-hooks.el
+++ b/lisp/vc/vc-hooks.el
@@ -506,6 +506,14 @@ vc-working-revision
                            (vc-call-backend
                             backend 'working-revision file))))))
 
+(defun vc-short-revision (file &optional backend)
+  "Return the repository version for FILE in human-readable form.
+If FILE is not registered, this function always returns nil."
+  (setq backend (or backend (vc-backend file)))
+  (if (vc-find-backend-function backend 'short-revision)
+      (vc-call-backend backend 'short-revision file)
+    (vc-working-revision file backend)))
+
 (defun vc-default-registered (backend file)
   "Check if FILE is registered in BACKEND using vc-BACKEND-master-templates."
   (let ((sym (vc-make-backend-sym backend 'master-templates)))
-- 
2.25.1

bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

[Dmitry Gutov]

On 14/12/2023 22:34, Jim Porter wrote:
> +(defcustom vc-git-use-short-revisions t
> +  "If non-nil, return short Git revisions when calling `vc-short-revision'.
> +If nil, always return full Git revisions.
> +
> +This affects the revision shown in the buffer name for
> +`vc-annotate'."
> +  :type 'boolean
> +  :version "30.1")

Do we need this option? I'm probably missing something, but IIUC a new 
variable to control the behavior was suggested as an alternative to 
using a new backend action. But here we add a new backend action anyway.

I don't mind either way, though.

bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

[Jim Porter]

On 12/14/2023 1:58 PM, Dmitry Gutov wrote:
> Do we need this option? I'm probably missing something, but IIUC a new 
> variable to control the behavior was suggested as an alternative to 
> using a new backend action. But here we add a new backend action anyway.

I had originally tried to do this purely with adding some variables and 
then using the existing backend functions. However, that would have 
resulted in 'vc-working-revision' returning either the full Git SHA or 
the short form depending on those options.

I didn't think that would be a safe idea, since a) other callers of 
'vc-working-revision' might not be prepared to handle short Git SHAs, 
and b) 'vc-working-revision' caches the revision, so it's not as easy as 
just setting a variable that controls the behavior: you'd need some 
special logic for invalidating the cache, or perhaps ignoring the cache 
when 'vc-use-short-revisions' is non-nil or something. All of those 
issues scared me enough that I wanted to avoid altering the behavior of 
'vc-working-revision' in any way.

While this patch does add a new backend action, it's purely optional and 
no other backend needs to be aware of it at all. I suppose I could 
hardcode the Git behavior in 'vc-short-revision', but that didn't seem 
to me to have any benefits over this version.

bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

[Dmitry Gutov]

On 15/12/2023 02:06, Jim Porter wrote:
> b) 'vc-working-revision' caches the revision, so it's not as easy as 
> just setting a variable that controls the behavior: you'd need some 
> special logic for invalidating the cache, or perhaps ignoring the cache 
> when 'vc-use-short-revisions' is non-nil or something.

That makes sense indeed. But you could add a new function 
vc-short-revision and a variable vc-use-show-revisions, where the latter 
would be obeyed by vc-git-working-revision only.

vc-short-revision could call (vc-call-backend backend 'working-revision 
file) directly (with vc-use-show-revisions bound to t), since it doesn't 
seem to really need caching, at least in the current usage. Or it could 
create its own cache.

> While this patch 
> does add a new backend action, it's purely optional and no other backend 
> needs to be aware of it at all. I suppose I could hardcode the Git 
> behavior in 'vc-short-revision', but that didn't seem to me to have any 
> benefits over this version.

I agree that the new backend action 'short-revision' (unique to Git) is 
not a problem.

I'm just saying that 'vc-git-use-short-revisions' might be unnecessary.

It's possible, I suppose, that some users might prefer long revisions 
instead of short ones (in the buffer name for vc-annotate), but we could 
wait for such requests to show up first.

bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

[Jim Porter]

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

On 12/14/2023 4:06 PM, Jim Porter wrote:
> While this patch does add a new backend action, it's purely optional and 
> no other backend needs to be aware of it at all. I suppose I could 
> hardcode the Git behavior in 'vc-short-revision', but that didn't seem 
> to me to have any benefits over this version.

Actually, now that I think about it, I could add the 'vc-short-revision' 
function and then use *that* to set a special variable, like so (see 
attached). I'm not sure how much I like this, but it does completely 
avoid adding any new backend functions (not even optional ones).

[-- Attachment #2: 0001-Abbreviate-the-VC-revision-in-vc-annotate-s-buffer-n.patch --]
[-- Type: text/plain, Size: 4063 bytes --]

From a5a564a90166933a1664e49243a31aa7b5996edf Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 14 Dec 2023 11:31:27 -0800
Subject: [PATCH] Abbreviate the VC revision in vc-annotate's buffer name

* lisp/vc/vc-hooks.el (vc-use-short-revision): New variable.
(vc-short-revision): New function.

* lisp/vc/vc-annotate.el (vc-annotate-use-short-revision): New
option...
(vc-annotate): ... use it.

* lisp/vc/vc-git.el (vc-git--rev-parse): Consult
'vc-use-short-revision'.

* etc/NEWS: Announce this change (bug#67062).
---
 etc/NEWS               |  8 ++++++++
 lisp/vc/vc-annotate.el | 10 +++++++++-
 lisp/vc/vc-git.el      |  7 +++++--
 lisp/vc/vc-hooks.el    |  9 +++++++++
 4 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 1ff2f8a149f..d8e7fd1d67f 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -457,6 +457,14 @@ With this value only the revision number is displayed on the mode-line.
 *** Obsolete command 'vc-switch-backend' re-added as 'vc-change-backend'.
 The command was previously obsoleted and unbound in Emacs 28.
 
+---
+*** 'vc-annotate' can now abbreviate the revision in the buffer name.
+VC backends with a 'vc-BACKEND-short-revision' function can show the
+revision in a shorter form, and 'vc-annotate' will use this form in
+its buffer name.  Currently, this only applies to the Git backend.  To
+restore the previous behavior, set 'vc-annotate-use-short-revision' to
+nil.
+
 ** Diff mode
 
 +++
diff --git a/lisp/vc/vc-annotate.el b/lisp/vc/vc-annotate.el
index de6c3adbbdb..cfca7cbfac0 100644
--- a/lisp/vc/vc-annotate.el
+++ b/lisp/vc/vc-annotate.el
@@ -162,6 +162,11 @@ vc-annotate-menu-elements
   :type '(repeat number)
   :group 'vc)
 
+(defcustom vc-annotate-use-short-revision t
+  "If non-nil, \\[vc-annotate] will use short revisions in its buffer name."
+  :type 'boolean
+  :group 'vc)
+
 (defvar-keymap vc-annotate-mode-map
   :doc "Local keymap used for VC-Annotate mode."
   "a"   #'vc-annotate-revision-previous-to-line
@@ -397,7 +402,10 @@ vc-annotate
    (save-current-buffer
      (vc-ensure-vc-buffer)
      (list buffer-file-name
-	   (let ((def (vc-working-revision buffer-file-name)))
+	   (let ((def (funcall (if vc-annotate-use-short-revision
+                                   #'vc-short-revision
+                                 #'vc-working-revision)
+                               buffer-file-name)))
 	     (if (null current-prefix-arg) def
 	       (vc-read-revision
 		(format-prompt "Annotate from revision" def)
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index 2e057ecfaa7..cc96ae8510a 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -1835,8 +1835,11 @@ vc-git-previous-revision
 (defun vc-git--rev-parse (rev)
   (with-temp-buffer
     (and
-     (vc-git--out-ok "rev-parse" rev)
-     (buffer-substring-no-properties (point-min) (+ (point-min) 40)))))
+     (apply #'vc-git--out-ok "rev-parse"
+            (append (when vc-use-short-revision '("--short"))
+                    (list rev)))
+     (goto-char (point-min))
+     (buffer-substring-no-properties (point) (pos-eol)))))
 
 (defun vc-git-next-revision (file rev)
   "Git-specific version of `vc-next-revision'."
diff --git a/lisp/vc/vc-hooks.el b/lisp/vc/vc-hooks.el
index 8451128286b..a2d3a2616f6 100644
--- a/lisp/vc/vc-hooks.el
+++ b/lisp/vc/vc-hooks.el
@@ -506,6 +506,15 @@ vc-working-revision
                            (vc-call-backend
                             backend 'working-revision file))))))
 
+(defvar vc-use-short-revision nil)
+
+(defun vc-short-revision (file &optional backend)
+  "Return the repository version for FILE in a shortened form.
+If FILE is not registered, this function always returns nil."
+  (let ((vc-use-short-revision t))
+    (vc-call-backend (or backend (vc-backend file))
+                     'working-revision file)))
+
 (defun vc-default-registered (backend file)
   "Check if FILE is registered in BACKEND using vc-BACKEND-master-templates."
   (let ((sym (vc-make-backend-sym backend 'master-templates)))
-- 
2.25.1

bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

[Eli Zaretskii]

> Date: Thu, 14 Dec 2023 12:34:28 -0800
> Cc: dmitry@gutov.dev, 67062@debbugs.gnu.org, juri@linkov.net
> From: Jim Porter <jporterbugs@gmail.com>
> 
> On 12/14/2023 11:51 AM, Eli Zaretskii wrote:
> > Please in the future try retaining more of the context, so that what
> > "this solution" was will be clear even without the need to look up the
> > previous messages.  This is especially important when the previous
> > message happened long enough for people to forget the point where we
> > left it, like in this case.
> 
> Will do. (I use threading in my mail client, so I see all the context 
> and didn't think about this.) The most-relevant message in the previous 
> discussion is probably this one[1]:

Granted, I use threading as well, but I don't keep messages
indefinitely, nor archive everything I get.  A month-old message is
likely to be expunged, and I then need to look for it on the
bug-tracker or in the list archives.

> > Also, "only Git supports this" sounds like the other backends are in
> > some kind of disadvantage, whereas the truth is that only Git needs
> > this, since its usual full revision IDs are so long.
> 
> I changed this to: "Currently, this only applies to the Git backend."

This still sounds like other backends are less functional.  I think
the fact that other backends don't really need this should be stated
explicitly.

Thanks.

bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

[Jim Porter]

On 12/14/2023 10:32 PM, Eli Zaretskii wrote:
>>> Also, "only Git supports this" sounds like the other backends are in
>>> some kind of disadvantage, whereas the truth is that only Git needs
>>> this, since its usual full revision IDs are so long.
>>
>> I changed this to: "Currently, this only applies to the Git backend."
> 
> This still sounds like other backends are less functional.  I think
> the fact that other backends don't really need this should be stated
> explicitly.

How about, "This is only relevant for the Git backend"?

bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

[Eli Zaretskii]

> Date: Thu, 14 Dec 2023 22:36:36 -0800
> Cc: dmitry@gutov.dev, 67062@debbugs.gnu.org, juri@linkov.net
> From: Jim Porter <jporterbugs@gmail.com>
> 
> On 12/14/2023 10:32 PM, Eli Zaretskii wrote:
> >>> Also, "only Git supports this" sounds like the other backends are in
> >>> some kind of disadvantage, whereas the truth is that only Git needs
> >>> this, since its usual full revision IDs are so long.
> >>
> >> I changed this to: "Currently, this only applies to the Git backend."
> > 
> > This still sounds like other backends are less functional.  I think
> > the fact that other backends don't really need this should be stated
> > explicitly.
> 
> How about, "This is only relevant for the Git backend"?

Too vague, IMO.

But I don't want to argue about this tiny detail for too long, I can
always change the wording later if I think it's needed.

bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

[Jim Porter]

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

On 12/15/2023 12:50 AM, Eli Zaretskii wrote:
> Too vague, IMO.
> 
> But I don't want to argue about this tiny detail for too long, I can
> always change the wording later if I think it's needed.

I rewrote the NEWS entry and added some more detail to the docstrings. 
If no one has any objections, I'll merge this a bit after the new year 
(so that people don't have to do code review during the holidays).

[-- Attachment #2: 0001-Abbreviate-the-VC-revision-in-vc-annotate-s-buffer-n.patch --]
[-- Type: text/plain, Size: 4239 bytes --]

From 706da4638a802660c9332a3bdaa980522ab7be44 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 14 Dec 2023 11:31:27 -0800
Subject: [PATCH] Abbreviate the VC revision in vc-annotate's buffer name

* lisp/vc/vc-hooks.el (vc-use-short-revision): New variable.
(vc-short-revision): New function.

* lisp/vc/vc-annotate.el (vc-annotate-use-short-revision): New
option...
(vc-annotate): ... use it.

* lisp/vc/vc-git.el (vc-git--rev-parse): Consult
'vc-use-short-revision'.

* etc/NEWS: Announce this change (bug#67062).
---
 etc/NEWS               |  6 ++++++
 lisp/vc/vc-annotate.el | 10 +++++++++-
 lisp/vc/vc-git.el      |  7 +++++--
 lisp/vc/vc-hooks.el    | 12 ++++++++++++
 4 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 6df17aa3f0a..7c21f04a0f7 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -492,6 +492,12 @@ switch is used, commands to see the diff of the old revision ('d'),
 check out an old file version ('f') or annotate it right away ('a'),
 also work on revisions which precede renames.
 
+---
+*** 'vc-annotate' now abbreviates the Git revision in the buffer name.
+When using the Git backend, 'vc-annotate' will use an abbreviated
+revision identifier in its buffer name.  To restore the previous
+behavior, set 'vc-annotate-use-short-revision' to nil.
+
 *** New option 'vc-git-file-name-changes-switches'.
 It allows tweaking the thresholds for rename and copy detection.
 
diff --git a/lisp/vc/vc-annotate.el b/lisp/vc/vc-annotate.el
index de6c3adbbdb..cfca7cbfac0 100644
--- a/lisp/vc/vc-annotate.el
+++ b/lisp/vc/vc-annotate.el
@@ -162,6 +162,11 @@ vc-annotate-menu-elements
   :type '(repeat number)
   :group 'vc)
 
+(defcustom vc-annotate-use-short-revision t
+  "If non-nil, \\[vc-annotate] will use short revisions in its buffer name."
+  :type 'boolean
+  :group 'vc)
+
 (defvar-keymap vc-annotate-mode-map
   :doc "Local keymap used for VC-Annotate mode."
   "a"   #'vc-annotate-revision-previous-to-line
@@ -397,7 +402,10 @@ vc-annotate
    (save-current-buffer
      (vc-ensure-vc-buffer)
      (list buffer-file-name
-	   (let ((def (vc-working-revision buffer-file-name)))
+	   (let ((def (funcall (if vc-annotate-use-short-revision
+                                   #'vc-short-revision
+                                 #'vc-working-revision)
+                               buffer-file-name)))
 	     (if (null current-prefix-arg) def
 	       (vc-read-revision
 		(format-prompt "Annotate from revision" def)
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index 24469f04f7c..bd74e2a6a44 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -1857,8 +1857,11 @@ vc-git-previous-revision
 (defun vc-git--rev-parse (rev)
   (with-temp-buffer
     (and
-     (vc-git--out-ok "rev-parse" rev)
-     (buffer-substring-no-properties (point-min) (+ (point-min) 40)))))
+     (apply #'vc-git--out-ok "rev-parse"
+            (append (when vc-use-short-revision '("--short"))
+                    (list rev)))
+     (goto-char (point-min))
+     (buffer-substring-no-properties (point) (pos-eol)))))
 
 (defun vc-git-next-revision (file rev)
   "Git-specific version of `vc-next-revision'."
diff --git a/lisp/vc/vc-hooks.el b/lisp/vc/vc-hooks.el
index 8451128286b..e84cdffe2dd 100644
--- a/lisp/vc/vc-hooks.el
+++ b/lisp/vc/vc-hooks.el
@@ -506,6 +506,18 @@ vc-working-revision
                            (vc-call-backend
                             backend 'working-revision file))))))
 
+(defvar vc-use-short-revision nil
+  "If non-nil, VC backend functions should return short revisions if possible.
+This is set to t when calling `vc-short-revision', which will
+then call the \\=`working-revision' backend function.")
+
+(defun vc-short-revision (file &optional backend)
+  "Return the repository version for FILE in a shortened form.
+If FILE is not registered, this function always returns nil."
+  (let ((vc-use-short-revision t))
+    (vc-call-backend (or backend (vc-backend file))
+                     'working-revision file)))
+
 (defun vc-default-registered (backend file)
   "Check if FILE is registered in BACKEND using vc-BACKEND-master-templates."
   (let ((sym (vc-make-backend-sym backend 'master-templates)))
-- 
2.25.1

bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

[Dmitry Gutov]

On 23/12/2023 21:37, Jim Porter wrote:
> On 12/15/2023 12:50 AM, Eli Zaretskii wrote:
>> Too vague, IMO.
>>
>> But I don't want to argue about this tiny detail for too long, I can
>> always change the wording later if I think it's needed.
> 
> I rewrote the NEWS entry and added some more detail to the docstrings. 
> If no one has any objections, I'll merge this a bit after the new year 
> (so that people don't have to do code review during the holidays).

Looks okay to me.

And happy holidays to everyone!

bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

[Eli Zaretskii]

> Date: Sat, 23 Dec 2023 11:37:24 -0800
> Cc: dmitry@gutov.dev, 67062@debbugs.gnu.org, juri@linkov.net
> From: Jim Porter <jporterbugs@gmail.com>
> 
> On 12/15/2023 12:50 AM, Eli Zaretskii wrote:
> > Too vague, IMO.
> > 
> > But I don't want to argue about this tiny detail for too long, I can
> > always change the wording later if I think it's needed.
> 
> I rewrote the NEWS entry and added some more detail to the docstrings. 
> If no one has any objections, I'll merge this a bit after the new year 
> (so that people don't have to do code review during the holidays).

Thanks, this version LGTM.

bug#67062: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)

[Jim Porter]

On 12/23/2023 11:49 AM, Eli Zaretskii wrote:
>> Date: Sat, 23 Dec 2023 11:37:24 -0800
>> Cc: dmitry@gutov.dev, 67062@debbugs.gnu.org, juri@linkov.net
>> From: Jim Porter <jporterbugs@gmail.com>
>>
>> I rewrote the NEWS entry and added some more detail to the docstrings.
>> If no one has any objections, I'll merge this a bit after the new year
>> (so that people don't have to do code review during the holidays).
> 
> Thanks, this version LGTM.

Since everyone is ok with this, I've now merged it to the master branch 
as ea4cbb3aae3. Closing this now (though if there are any further 
concerns, just let me know and I'll address them).