From: Dmitry Gutov <dgutov@yandex.ru>
To: John Yates <john@yates-sheets.org>,
Philip Kaludercic <philipk@posteo.net>,
Stefan Monnier <monnier@iro.umontreal.ca>,
Stefan Kangas <stefankangas@gmail.com>,
schwab@linux-m68k.org, Juri Linkov <juri@linkov.net>
Cc: Emacs developers <emacs-devel@gnu.org>
Subject: Re: vc-find-revision-no-save?
Date: Sat, 11 Feb 2023 01:46:59 +0200 [thread overview]
Message-ID: <4423f660-933d-bbcb-1432-76548dfbf08c@yandex.ru> (raw)
In-Reply-To: <CAJnXXoj0BUsgKapEW=bLUWePULXsm-KFHRpcK02pFJZP7gPKwQ@mail.gmail.com>
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.
next prev parent reply other threads:[~2023-02-10 23:46 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-31 11:57 vc-find-revision-no-save? John Yates
2023-02-10 23:46 ` Dmitry Gutov [this message]
2023-03-27 19:38 ` vc-find-revision-no-save? John Yates
2023-03-27 20:50 ` vc-find-revision-no-save? John Yates
-- strict thread matches above, loose matches on Subject: below --
2022-10-24 0:27 vc-find-revision-no-save? John Yates
2022-10-24 9:42 ` vc-find-revision-no-save? Dmitry Gutov
2022-10-24 16:02 ` vc-find-revision-no-save? Philip Kaludercic
2022-10-24 17:14 ` vc-find-revision-no-save? Dmitry Gutov
2022-10-24 21:10 ` vc-find-revision-no-save? Stefan Monnier
2022-10-25 2:22 ` vc-find-revision-no-save? Eli Zaretskii
2022-10-28 21:57 ` vc-find-revision-no-save? Richard Stallman
2022-10-29 6:46 ` vc-find-revision-no-save? Philip Kaludercic
2022-10-29 7:07 ` vc-find-revision-no-save? Stefan Kangas
2022-10-29 9:18 ` vc-find-revision-no-save? Philip Kaludercic
2022-10-29 9:24 ` vc-find-revision-no-save? Andreas Schwab
2022-10-29 9:26 ` vc-find-revision-no-save? Philip Kaludercic
2022-10-29 15:14 ` vc-find-revision-no-save? Stefan Monnier
2022-10-29 15:40 ` vc-find-revision-no-save? Philip Kaludercic
2022-10-24 19:37 ` vc-find-revision-no-save? Juri Linkov
2022-10-24 20:00 ` vc-find-revision-no-save? John Yates
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4423f660-933d-bbcb-1432-76548dfbf08c@yandex.ru \
--to=dgutov@yandex.ru \
--cc=emacs-devel@gnu.org \
--cc=john@yates-sheets.org \
--cc=juri@linkov.net \
--cc=monnier@iro.umontreal.ca \
--cc=philipk@posteo.net \
--cc=schwab@linux-m68k.org \
--cc=stefankangas@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).