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

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 67062 in the body.
You can then email your comments to 67062 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: Jim Porter <jporterbugs <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 30.0.50; [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)
Date: Fri, 10 Nov 2023 18:49:59 -0800

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

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

[0001-Abbreviate-the-VC-revision-in-vc-annotate-s-buffer-n.patch (text/plain, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 67062 <at> debbugs.gnu.org
Subject: Re: bug#67062: 30.0.50;
 [PATCH] Abbreviate the revision in 'vc-annotate' (for Git)
Date: Sat, 11 Nov 2023 09:41:53 +0200

> Date: Fri, 10 Nov 2023 18:49:59 -0800
> From: Jim Porter <jporterbugs <at> 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.)

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 67062 <at> debbugs.gnu.org
Subject: Re: bug#67062: 30.0.50; [PATCH] Abbreviate the revision in
 'vc-annotate' (for Git)
Date: Sat, 11 Nov 2023 13:31:28 -0800

On 11/10/2023 11:41 PM, Eli Zaretskii wrote:
>> Date: Fri, 10 Nov 2023 18:49:59 -0800
>> From: Jim Porter <jporterbugs <at> 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).

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Eli Zaretskii <eliz <at> gnu.org>, Jim Porter <jporterbugs <at> gmail.com>
Cc: 67062 <at> debbugs.gnu.org
Subject: Re: bug#67062: 30.0.50; [PATCH] Abbreviate the revision in
 'vc-annotate' (for Git)
Date: Sun, 12 Nov 2023 00:00:13 +0200

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.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 67062 <at> debbugs.gnu.org
Subject: Re: bug#67062: 30.0.50; [PATCH] Abbreviate the revision in
 'vc-annotate' (for Git)
Date: Sat, 11 Nov 2023 16:31:43 -0800

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

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

[0001-Abbreviate-the-VC-revision-in-vc-annotate-s-buffer-n.patch (text/plain, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: jporterbugs <at> gmail.com, 67062 <at> debbugs.gnu.org
Subject: Re: bug#67062: 30.0.50; [PATCH] Abbreviate the revision in
 'vc-annotate' (for Git)
Date: Sun, 12 Nov 2023 08:03:44 +0200

> Date: Sun, 12 Nov 2023 00:00:13 +0200
> Cc: 67062 <at> debbugs.gnu.org
> From: Dmitry Gutov <dmitry <at> 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.

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: jporterbugs <at> gmail.com, 67062 <at> debbugs.gnu.org
Subject: Re: bug#67062: 30.0.50; [PATCH] Abbreviate the revision in
 'vc-annotate' (for Git)
Date: Sun, 12 Nov 2023 12:58:06 +0200

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.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: jporterbugs <at> gmail.com, 67062 <at> debbugs.gnu.org
Subject: Re: bug#67062: 30.0.50; [PATCH] Abbreviate the revision in
 'vc-annotate' (for Git)
Date: Sun, 12 Nov 2023 13:15:45 +0200

> Date: Sun, 12 Nov 2023 12:58:06 +0200
> Cc: jporterbugs <at> gmail.com, 67062 <at> debbugs.gnu.org
> From: Dmitry Gutov <dmitry <at> 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.

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: jporterbugs <at> gmail.com, 67062 <at> debbugs.gnu.org
Subject: Re: bug#67062: 30.0.50; [PATCH] Abbreviate the revision in
 'vc-annotate' (for Git)
Date: Sun, 12 Nov 2023 13:21:29 +0200

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.

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

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: jporterbugs <at> gmail.com, Eli Zaretskii <eliz <at> gnu.org>, 67062 <at> debbugs.gnu.org
Subject: Re: bug#67062: 30.0.50; [PATCH] Abbreviate the revision in
 'vc-annotate' (for Git)
Date: Sun, 12 Nov 2023 20:48:07 +0200

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

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>, Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 67062 <at> debbugs.gnu.org
Subject: Re: bug#67062: 30.0.50; [PATCH] Abbreviate the revision in
 'vc-annotate' (for Git)
Date: Sun, 12 Nov 2023 11:07:38 -0800

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

On 11/11/2023 10:03 PM, Eli Zaretskii wrote:
>> Date: Sun, 12 Nov 2023 00:00:13 +0200
>> Cc: 67062 <at> debbugs.gnu.org
>> From: Dmitry Gutov <dmitry <at> 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

[0001-Abbreviate-the-VC-revision-in-vc-annotate-s-buffer-n.patch (text/plain, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: dmitry <at> gutov.dev, 67062 <at> debbugs.gnu.org
Subject: Re: bug#67062: 30.0.50; [PATCH] Abbreviate the revision in
 'vc-annotate' (for Git)
Date: Sun, 12 Nov 2023 21:11:05 +0200

> Date: Sun, 12 Nov 2023 11:07:38 -0800
> Cc: 67062 <at> debbugs.gnu.org
> From: Jim Porter <jporterbugs <at> 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.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: dmitry <at> gutov.dev, 67062 <at> debbugs.gnu.org
Subject: Re: bug#67062: 30.0.50; [PATCH] Abbreviate the revision in
 'vc-annotate' (for Git)
Date: Sun, 12 Nov 2023 11:33:33 -0800

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

On 11/12/2023 11:11 AM, Eli Zaretskii wrote:
>> Date: Sun, 12 Nov 2023 11:07:38 -0800
>> Cc: 67062 <at> debbugs.gnu.org
>> From: Jim Porter <jporterbugs <at> 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".)

[0001-Abbreviate-the-VC-revision-in-vc-annotate-s-buffer-n.patch (text/plain, attachment)]

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Jim Porter <jporterbugs <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 67062 <at> debbugs.gnu.org
Subject: Re: bug#67062: 30.0.50; [PATCH] Abbreviate the revision in
 'vc-annotate' (for Git)
Date: Sun, 12 Nov 2023 22:35:15 +0200

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.

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Juri Linkov <juri <at> linkov.net>
Cc: jporterbugs <at> gmail.com, Eli Zaretskii <eliz <at> gnu.org>, 67062 <at> debbugs.gnu.org
Subject: Re: bug#67062: 30.0.50; [PATCH] Abbreviate the revision in
 'vc-annotate' (for Git)
Date: Sun, 12 Nov 2023 22:37:42 +0200

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.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>, Juri Linkov <juri <at> linkov.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 67062 <at> debbugs.gnu.org
Subject: Re: bug#67062: 30.0.50; [PATCH] Abbreviate the revision in
 'vc-annotate' (for Git)
Date: Sun, 12 Nov 2023 14:13:58 -0800

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.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 67062 <at> debbugs.gnu.org
Subject: Re: bug#67062: 30.0.50; [PATCH] Abbreviate the revision in
 'vc-annotate' (for Git)
Date: Sun, 12 Nov 2023 14:15:20 -0800

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.

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

From: Juri Linkov <juri <at> linkov.net>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: Dmitry Gutov <dmitry <at> gutov.dev>, Eli Zaretskii <eliz <at> gnu.org>,
 67062 <at> debbugs.gnu.org
Subject: Re: bug#67062: 30.0.50; [PATCH] Abbreviate the revision in
 'vc-annotate' (for Git)
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 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.

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Juri Linkov <juri <at> linkov.net>, Jim Porter <jporterbugs <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 67062 <at> debbugs.gnu.org
Subject: Re: bug#67062: 30.0.50; [PATCH] Abbreviate the revision in
 'vc-annotate' (for Git)
Date: Mon, 13 Nov 2023 16:04:51 +0200

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.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: jporterbugs <at> gmail.com, 67062 <at> debbugs.gnu.org, dmitry <at> gutov.dev
Subject: Re: bug#67062: 30.0.50; [PATCH] Abbreviate the revision in
 'vc-annotate' (for Git)
Date: Mon, 13 Nov 2023 16:19:47 +0200

> From: Juri Linkov <juri <at> linkov.net>
> Cc: Dmitry Gutov <dmitry <at> gutov.dev>,  Eli Zaretskii <eliz <at> gnu.org>,
>   67062 <at> 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.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>, Juri Linkov <juri <at> linkov.net>
Cc: dmitry <at> gutov.dev, 67062 <at> debbugs.gnu.org
Subject: Re: bug#67062: 30.0.50; [PATCH] Abbreviate the revision in
 'vc-annotate' (for Git)
Date: Thu, 14 Dec 2023 11:42:45 -0800

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

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

[0001-Abbreviate-the-VC-revision-in-vc-annotate-s-buffer-n.patch (text/plain, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: dmitry <at> gutov.dev, 67062 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#67062: 30.0.50; [PATCH] Abbreviate the revision in
 'vc-annotate' (for Git)
Date: Thu, 14 Dec 2023 21:51:36 +0200

> Date: Thu, 14 Dec 2023 11:42:45 -0800
> Cc: 67062 <at> debbugs.gnu.org, dmitry <at> gutov.dev
> From: Jim Porter <jporterbugs <at> 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.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: dmitry <at> gutov.dev, 67062 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#67062: 30.0.50; [PATCH] Abbreviate the revision in
 'vc-annotate' (for Git)
Date: Thu, 14 Dec 2023 12:34:28 -0800

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

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

[0001-Abbreviate-the-VC-revision-in-vc-annotate-s-buffer-n.patch (text/plain, attachment)]

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Jim Porter <jporterbugs <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 67062 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#67062: 30.0.50; [PATCH] Abbreviate the revision in
 'vc-annotate' (for Git)
Date: Thu, 14 Dec 2023 23:58:18 +0200

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.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 67062 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#67062: 30.0.50; [PATCH] Abbreviate the revision in
 'vc-annotate' (for Git)
Date: Thu, 14 Dec 2023 16:06:03 -0800

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.

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Jim Porter <jporterbugs <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 67062 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#67062: 30.0.50; [PATCH] Abbreviate the revision in
 'vc-annotate' (for Git)
Date: Fri, 15 Dec 2023 02:15:16 +0200

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.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 67062 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#67062: 30.0.50; [PATCH] Abbreviate the revision in
 'vc-annotate' (for Git)
Date: Thu, 14 Dec 2023 16:23:22 -0800

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

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

[0001-Abbreviate-the-VC-revision-in-vc-annotate-s-buffer-n.patch (text/plain, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: dmitry <at> gutov.dev, 67062 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#67062: 30.0.50; [PATCH] Abbreviate the revision in
 'vc-annotate' (for Git)
Date: Fri, 15 Dec 2023 08:32:23 +0200

> Date: Thu, 14 Dec 2023 12:34:28 -0800
> Cc: dmitry <at> gutov.dev, 67062 <at> debbugs.gnu.org, juri <at> linkov.net
> From: Jim Porter <jporterbugs <at> 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.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: dmitry <at> gutov.dev, 67062 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#67062: 30.0.50; [PATCH] Abbreviate the revision in
 'vc-annotate' (for Git)
Date: Thu, 14 Dec 2023 22:36:36 -0800

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"?

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: dmitry <at> gutov.dev, 67062 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#67062: 30.0.50; [PATCH] Abbreviate the revision in
 'vc-annotate' (for Git)
Date: Fri, 15 Dec 2023 10:50:00 +0200

> Date: Thu, 14 Dec 2023 22:36:36 -0800
> Cc: dmitry <at> gutov.dev, 67062 <at> debbugs.gnu.org, juri <at> linkov.net
> From: Jim Porter <jporterbugs <at> 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.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: dmitry <at> gutov.dev, 67062 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#67062: 30.0.50; [PATCH] Abbreviate the revision in
 'vc-annotate' (for Git)
Date: Sat, 23 Dec 2023 11:37:24 -0800

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

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

[0001-Abbreviate-the-VC-revision-in-vc-annotate-s-buffer-n.patch (text/plain, attachment)]

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Jim Porter <jporterbugs <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 67062 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#67062: 30.0.50; [PATCH] Abbreviate the revision in
 'vc-annotate' (for Git)
Date: Sat, 23 Dec 2023 21:43:29 +0200

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!

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: dmitry <at> gutov.dev, 67062 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#67062: 30.0.50; [PATCH] Abbreviate the revision in
 'vc-annotate' (for Git)
Date: Sat, 23 Dec 2023 21:49:45 +0200

> Date: Sat, 23 Dec 2023 11:37:24 -0800
> Cc: dmitry <at> gutov.dev, 67062 <at> debbugs.gnu.org, juri <at> linkov.net
> From: Jim Porter <jporterbugs <at> 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.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: dmitry <at> gutov.dev, 67062-done <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#67062: 30.0.50; [PATCH] Abbreviate the revision in
 'vc-annotate' (for Git)
Date: Wed, 27 Dec 2023 14:26:37 -0800

On 12/23/2023 11:49 AM, Eli Zaretskii wrote:
>> Date: Sat, 23 Dec 2023 11:37:24 -0800
>> Cc: dmitry <at> gutov.dev, 67062 <at> debbugs.gnu.org, juri <at> linkov.net
>> From: Jim Porter <jporterbugs <at> 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).

Previous Next

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