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

[Dmitry Gutov <dgutov@yandex.ru>]

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

On 13/12/2022 03:23, Dmitry Gutov wrote:
>> I read your message, and I think immitating GitHub's UI is a great
>> idea, as it liberates us from having to do any kind of "pretreatment"
>> of the git log (which can be expensive for large repos).
> 
> Indeed. I suppose we'll lose out in some more complex cases (e.g. if 
> --follow tracks copies, it might track the cases when several files had 
> been copied into one, including when that action spanned several 
> commits; and thus --follow could show the history of each such file), 
> but we should win in the most common cases (single-file copies and 
> renames) OOtB, which we don't have any good support for still.
> 
>> We should focus in this direction to fix this issue. If I get some
>> spare time over the holidays I will try to take a look (sadly, I
>> cannot promise that will be the case...).
> 
> We won't be able to put the improvement into Emacs 29 anyway (the 
> release branch has been cut, it's now bugfix-only), so there is no hurry.
> 
> Let's see who gets to this first. If you wanted to finish up your patch 
> instead, I'm not going to say no either. But GitHub's approach seems 
> like it should require less (and less complex) code.

Attached is the implementation for this alternative approach.

It's Git-only, but otherwise seems to function well (with potential for 
future additions). The look of the message and the button could use some 
work, but this is the best I came up thus far.

As far as testing, it allowed following all files in Emacs's repo (that 
I have tried) to their original creation. Including etc/NEWS.29, and 
most other NEWS.*, with NEWS.28 being an exception (apparently because 
on that occasion NEWS was truncated before NEWS.28 was created).

Also important: set vc-git-print-log-follow to nil. Otherwise logs don't 
end on renames, and this feature doesn't get a chance to work.

Cheers,
Dmitry.

[-- Attachment #2: vc-git-file-name-changes.diff --]
[-- Type: text/x-patch, Size: 7123 bytes --]

diff --git a/etc/NEWS b/etc/NEWS
index 1ff2f8a149f..4dd11c99927 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.
 
+*** Support for viewing file change history across renames.
+When a fileset's VC change history ends at a rename, we now print the
+old name(s) and a button which jumps to their history.  Only supported
+with Git at the moment.
+
+*** New option 'vc-git-file-name-changes-switches'.
+It allows tweaking the thresholds for rename and copy detection.
+
 ** Diff mode
 
 +++
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index 2e057ecfaa7..e2c2ed5c79c 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -89,6 +89,7 @@
 ;; - make-version-backups-p (file)                 NOT NEEDED
 ;; - previous-revision (file rev)                  OK
 ;; - next-revision (file rev)                      OK
+;; - file-name-changes (rev)                       OK
 ;; - check-headers ()                              COULD BE SUPPORTED
 ;; - delete-file (file)                            OK
 ;; - rename-file (old new)                         OK
@@ -152,6 +153,20 @@ vc-git-shortlog-switches
                  (repeat :tag "Argument List" :value ("") string))
   :version "30.1")
 
+;; XXX: (setq vc-git-log-switches '("--simplify-merges")) can also
+;; create fuller history when using this feature.  Not sure why.
+(defcustom vc-git-file-name-changes-switches '("-M" "-C")
+  "String or list of string to pass to Git when finding previous names.
+
+This option should usually at least contain '-M'.  You can adjust
+the flags to change the similarity thresholds (default 50%).  Or
+add `--find-copies-harder' (slower in large projects, since it
+uses a full scan)."
+  :type '(choice (const :tag "None" nil)
+                 (string :tag "Argument String")
+                 (repeat :tag "Argument List" :value ("") string))
+  :version "30.1")
+
 (defcustom vc-git-resolve-conflicts t
   "When non-nil, mark conflicted file as resolved upon saving.
 That is performed after all conflict markers in it have been
@@ -1239,6 +1254,30 @@ vc-git-find-revision
      nil
      "cat-file" "blob" (concat (if rev rev "HEAD") ":" fullname))))
 
+(defun vc-git-file-name-changes (rev)
+  (with-temp-buffer
+    (let ((root (vc-git-root default-directory)))
+      (apply #'vc-git-command (current-buffer) t nil
+             "diff"
+             "--name-status"
+             "--diff-filter=ADCR"
+             (concat rev "^") rev
+             (vc-switches 'git 'file-name-changes))
+      (let (res)
+        (goto-char (point-min))
+        (while (re-search-forward "^\\([CMR]\\)[0-9]*\t\\([^\n\t]+\\)\\(?:\t\\([^\n\t]+\\)\\)?" nil t)
+          (pcase (match-string 1)
+            ("A" (push (cons nil (match-string 2)) res))
+            ("D" (push (cons (match-string 2) nil) res))
+            ((or "C" "R") (push (cons (match-string 2) (match-string 3)) res))
+            ;; ("M" (push (cons (match-string 1) (match-string 1)) res))
+            ))
+        (mapc (lambda (c)
+                (if (car c) (setcar c (expand-file-name (car c) root)))
+                (if (cdr c) (setcdr c (expand-file-name (cdr c) root))))
+                res)
+        (nreverse res)))))
+
 (defun vc-git-find-ignore-file (file)
   "Return the git ignore file that controls FILE."
   (expand-file-name ".gitignore"
@@ -1416,7 +1455,15 @@ vc-git-clone
 ;; Long explanation here:
 ;; https://stackoverflow.com/questions/46487476/git-log-follow-graph-skips-commits
 (defcustom vc-git-print-log-follow nil
-  "If true, follow renames in Git logs for a single file."
+  "If true, use the flag `--follow' when producing single file logs.
+
+It will make the printed log automatically follow the renames.
+The downsides is that the log produced this way may omit
+certain (merge) commits, and that `log-view-diff' fails on
+commits that used the previous name, in that log buffer.
+
+When this variable is nil, and the log ends with a rename, we
+print a button that shows the log for the previous name."
   :type 'boolean
   :version "26.1")
 
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 958929fe4c6..e626d72d59a 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -517,6 +517,13 @@
 ;;   Return the revision number that precedes REV for FILE, or nil if no such
 ;;   revision exists.
 ;;
+;; - file-name-changes (rev)
+;;
+;;   Return the list of pairs with changes in file names in REV.  When
+;;   a file was added, it should be a cons with nil car.  When
+;;   deleted, a cons with nil cdr.  When copied or renamed, a cons
+;;   with the source name as car and destination name as cdr.
+;;
 ;; - next-revision (file rev)
 ;;
 ;;   Return the revision number that follows REV for FILE, or nil if no such
@@ -2695,9 +2702,42 @@ vc-print-log-setup-buttons
       (goto-char (point-min))
       (while (re-search-forward log-view-message-re nil t)
         (cl-incf entries))
-      ;; If we got fewer entries than we asked for, then displaying
-      ;; the "more" buttons isn't useful.
-      (when (>= entries limit)
+      (if (< entries limit)
+          ;; The log has been printed in full.  Perhaps it started
+          ;; with a copy or rename?
+          (let* ((last-revision (log-view-current-tag (point-max)))
+                 ;; Could skip this when vc-git-print-log-follow = t.
+                 (name-changes
+                  (condition-case nil
+                      (vc-call-backend log-view-vc-backend
+                                       'file-name-changes last-revision)
+                    (vc-not-supported nil)))
+                 (matching-changes
+                  (cl-delete-if-not (lambda (f) (member f log-view-vc-fileset))
+                                    name-changes :key #'cdr))
+                 (old-names (mapcar #'car matching-changes))
+                 (relatives (mapcar #'file-relative-name old-names)))
+            (when old-names
+              (goto-char (point-max))
+              (insert "\n")
+              (insert
+               (format
+                "Renamed from %s"
+                (mapconcat (lambda (s)
+                             (propertize s 'font-lock-face
+                                         'log-view-file))
+                           relatives ", "))
+               " ")
+              ;; TODO: Also print a "Next log" button above the buffer
+              ;; created by this button to be able to go back quickly.
+              (insert-text-button
+               "View log"
+               'action (lambda (&rest _ignore)
+                         (vc-print-log-internal log-view-vc-backend old-names
+                                                last-revision nil limit))
+               'help-echo
+               "Show the log for the file name(s) before the rename")))
+        ;; Perhaps there are more entries in the log.
         (goto-char (point-max))
         (insert "\n")
         (insert-text-button