Re: vc-find-revision-no-save?

[Dmitry Gutov]

Hi John,

On 31/01/2023 13:57, John Yates wrote:
> Towards the end of Oct 2022, each of you contributed to the above named
> emacs-devel email thread.
>
> With issue #61071 on debbugs.gnu.org, I have attempted to address some
> of the points mentioned in the email thread via:
>
>      [PATCH 1/3] Refactor and document vc-find-revision caching
>
> I append the cover letter and commit message below.
>
> This is my very first attempt to contribute to Emacs.  So far I have
> received no feedback.  If I am doing something wrong, please let me know.

You've done everything correctly regarding the submission, but it seems 
like nobody highly interested in this particular addition has found the 
time to review it so far.

Myself, I just saw the title (vc-timemachine) and skipped it. It wasn't 
obvious that some existing VC code was altered and needed reviewing. Sorry.

We're also close to releasing Emacs 29, so it seemed like higher priority.

Otherwise, though, it can be a good idea to ping a bug report a week or 
two after receiving to response. If nobody else does, the head 
maintainer usually ends up responding.

I will also use this opportunity to complain about the lack of code 
review tools here. Some of the reviewers here might be able to quote 
attached patches inline, but I don't use Gnus, so that's not a real 
option. Just a lot of opening the patches externally and copying and 
pasting. We could move the review to EMBA, though: 
https://emba.gnu.org/emacs/emacs/-/merge_requests

Now, to go back to the original thread you referred to, I mentioned 
vc-annotate, and you agreed that it has similar features but misses some 
stuff. Such as syntax highlighting. And editing support (is that 
necessary?).

Overall, I think it might be better to add features to vc-annotate than 
add a very similar but different feature. Especially since it has unique 
features of its own, such as showing and being able to jump to a 
revision that last modified a given line. Or the one before it, etc. 
IME, that's usually more useful than going through a file history 
linearly. But that's my opinion.

If that idea holds your interest, Stefan might also give a couple of 
recommendations, since as I recall he suggested something like that in 
the past. E.g. one of the ideas was to remove the sidebar text from the 
buffer text when processing, and instead store it in overlays or some 
such. Then put the buffer into a corresponding major mode which will 
apply font-lock and etc. But keep the (hidable) sidebar using overlays, 
for example. Or put it in the margin (also using overlays). And keep the 
navigation commands. The latter conflicts with having the buffer 
editable (if we're going to keep the one-char bindings). I haven't 
looked yet how it's solved with timemachine. The bindings might depend 
on whether the buffer is in read-only mode or not, though.

Finally, some nits about the first patch:
- It moves from the cache-by-default behavior to dont-cache-by-default.
- It removes an existing user option without a deprecation period.
- It adds a timemachine-related variable to vc.el (vc-tm--revision ?). 
Timemachine will be a separate package, right?

The overall idea seems sound. But if we choose the route of improving 
vc-annotate, a revision cache will probably not help because we would be 
caching the 'git annotate' output instead. Thus making it specific to 
that feature only.