bug#55871: Acknowledgement (27.1; vc-git.el log view 'a', 'f', 'd' do not work when following renames)

[Dmitry Gutov <dgutov@yandex.ru>]

Hi Nicholas,

On 10.06.2022 20:31, Nicolás Ojeda Bär wrote:
> Dear all,
> 
> Attached is a patch that solves some of the problems in this issue (not all).
> 
> Its main feature is that it should not modify anything if you are not
> using `vc-git-print-log-follow`.
> 
> If you are, the patch makes it possible to use `a`, `f` and `d` from
> inside the `*vc-change-log*` buffer.
> 
> The patch itself is very much based on the one proposed in
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=8756#53, with some
> changes to make sure that nothing changes if `vc-git-print-log-follow`
> is not being used. Additionally, a horrible HACK (see commit message)
> is used to allow `vc-git.el` to discriminate between `d` and `D` in
> the `*vc-change-log*` buffer, as these two functions require different
> Git commands to be executed when following renames (this is also
> discussed in the just linked bug report).

Thanks for your help with updating this patch. I've tried it out, and it 
seems functional, which is great.

First of all, how do you feel about signing copyright assignment for 
Emacs? AFAICT you haven't done so yet, and this patch borders on our 
allowed limit without assignment (if I take only the lines that changed 
substantially from Dmitry Dzhus's version). It would probably be easier 
to get the assignment going anyway, so you continue to be able to 
contribute.

There are also a few of improvements I'm eyeing here (not requirements 
for acceptance -- I can do them myself, if you like):

1) vc-git--shalist-raw doesn't seem to serve much of a purpose. It is 
there only to delay the conversion from the raw string into a list?

2) It would be better to produce shalist entirely lazily. I'm not sure 
what's stopping us from doing that, given that vc-git--shalist knows how 
to find the log buffer. Finding the values for all the arguments might 
be a problem, so we can save the args (start-revision, limit) to a new var.

3) git-log-view-diff-whole-changeset might be unnecessary. vc-git-diff 
could look up two file names, for both revisions (the current and the 
one corresponding to the previous revision in the sha list), or 
log-view-diff-common could pass explicitly the previous revision from 
the already-printed list, rather than fetch it using (vc-call-backend 
log-view-vc-backend 'previous-revision nil fr) -- I'm not sure why it's 
doing it this way.

At first I just changed vc-git-diff to do this:

     (let ((file1 (vc-git--rev-to-filename rev1))
           (file2 (vc-git--rev-to-filename rev2)))
       (when (or file1 file2)
         ;; Run diff from the repository root because our file names are
         ;; relative to it
         (setq default-directory (vc-git-root default-directory))
         (when file1 (push file1 files))
         (when file2 (push file2 files))))

...and reverted the change to vc-git-previous-revision, and that almost 
worked, except a revision that did rename only showed one side 
(addition, not deletion).

WDYT?

I experimented with --follow myself in the past, and it is annoying in 
that it skips commits, some of which are visible in the log when you 
don't use --follow, details here: 
https://stackoverflow.com/questions/46487476/git-log-follow-graph-skips-commits

So I figured the approach in (3) has something to do with it. But it 
seems not to be the case.