bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov <dgutov@yandex.ru>]

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

On 22.11.2019 9:20, Eli Zaretskii wrote:

>> Somewhat better, but all backends would have to be updated anyway, for
>> this to work.
> 
> It's new functionality, so that goes without saying.  Or did you mean
> something specific when you said "updated"?

Not every new functionality needs explicit support from backends.

>> So the change in VC backend API is comparable to adding a new
>> action.
> 
> We don't necessarily need a change in the API: vc-print-log-internal
> has enough arguments to pass this new meaning to it and to the
> backends.  But even if there's a change in the API, it isn't a
> catastrophe from my POV.

The backend API. It also has certain backward compatibility expectations.

Anyway, I'm saying the change you are proposing is roughly the same 
magnitude in complexity as adding a new backend action.

>> I don't mind this too much (asking vc-git-print-log to include the diffs
>> makes sense, at least), but doing it this way loses out on the
>> opportunity to support all backends in one fell swoop.
> 
> I don't understand why would we lose that opportunity.  We will have
> to write new code for each backend in any alternative,

Not necessarily. See the attached patch (it's a modification of Juri's).

Since we don't have a way to combine async process invocation, there is 
some complexity there with accept-process-output. But from what I see, 
the diff operation is considerably more resource-intensive, at least for 
big Hg repos.

There is a catch, however: it requires an implementation of 
region-history-mode. But, as you remarked, it can be extracted to be 
more backend-independent.

Consequently, for now this "default" implementation only adds 
print-revision support to Hg.

> VC used to
> be simple and elegant, and this proliferation of too many high-level
> commands makes it more and more complex,

I hardly see any complexity in the presence of a command. There's more 
more of it in tiny details of implementation of the main ones.

 > and inevitably causes us
 > tweak the user level and UI to this or that particular VCS, which is
 > wrong in the long run.

When was the last time we did that? And how adding a new command would 
cause it in this case?

[-- Attachment #2: vc-print-revision-with-default.diff --]
[-- Type: text/x-patch, Size: 6090 bytes --]

diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index ca4c66a06d..729d98f143 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -80,6 +80,7 @@
 ;; - log-search (buffer pattern)                   OK
 ;; - log-view-mode ()                              OK
 ;; - show-log-entry (revision)                     OK
+;; - print-revision (revision)                     OK
 ;; - comment-history (file)                        ??
 ;; - update-changelog (files)                      COULD BE SUPPORTED
 ;; * diff (file &optional rev1 rev2 buffer async)  OK
@@ -1163,6 +1164,22 @@ vc-git-print-log
                     (list start-revision)))
 		'("--")))))))
 
+(defun vc-git-print-revision (buffer revision)
+  "Show the details of REVISION with output in BUFFER.
+With a prefix argument, ask for a command to run that will output
+the revision information."
+  (let ((args `("show" "--no-color" ,(or revision ""))))
+    (when current-prefix-arg
+      (setq args (cdr (split-string
+		       (read-shell-command
+                        "Show revision with command: "
+                        (format "%s %s" vc-git-program
+                                (mapconcat 'identity args " "))
+                        'vc-git-history)
+		       " " t))))
+    (vc-setup-buffer buffer)
+    (apply 'vc-git-command buffer 'async nil args)))
+
 (defun vc-git-log-outgoing (buffer remote-location)
   (vc-setup-buffer buffer)
   (vc-git-command
@@ -1226,7 +1243,7 @@ vc-git-log-view-mode
   (set (make-local-variable 'log-view-file-re) regexp-unmatchable)
   (set (make-local-variable 'log-view-per-file-logs) nil)
   (set (make-local-variable 'log-view-message-re)
-       (if (not (memq vc-log-view-type '(long log-search)))
+       (if (not (memq vc-log-view-type '(long log-search print-revision)))
 	   (cadr vc-git-root-log-format)
 	 "^commit *\\([0-9a-z]+\\)"))
   ;; Allow expanding short log entries.
@@ -1235,7 +1252,7 @@ vc-git-log-view-mode
     (set (make-local-variable 'log-view-expanded-log-entry-function)
 	 'vc-git-expanded-log-entry))
   (set (make-local-variable 'log-view-font-lock-keywords)
-       (if (not (memq vc-log-view-type '(long log-search)))
+       (if (not (memq vc-log-view-type '(long log-search print-revision)))
 	   (list (cons (nth 1 vc-git-root-log-format)
 		       (nth 2 vc-git-root-log-format)))
 	 (append
diff --git a/lisp/vc/vc-hg.el b/lisp/vc/vc-hg.el
index 17d38fa400..ae0c93bf9d 100644
--- a/lisp/vc/vc-hg.el
+++ b/lisp/vc/vc-hg.el
@@ -564,7 +564,8 @@ vc-hg-expanded-log-entry
 (defun vc-hg-revision-table (files)
   (let ((default-directory (file-name-directory (car files))))
     (with-temp-buffer
-      (vc-hg-command t nil files "log" "--template" "{rev} ")
+      (vc-hg-command t nil nil "branches" "--template" "{branch}\n")
+      (vc-hg-command t nil nil "tags" "--template" "{tag}\n")
       (split-string
        (buffer-substring-no-properties (point-min) (point-max))))))
 
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 0d29c80d02..be4ed95fe3 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -359,6 +359,10 @@
 ;;   and make sure it is displayed in the buffer's window.  The default
 ;;   implementation of this function works for RCS-style logs.
 ;;
+;; - print-revision (revision)
+;;
+;;   Show details of REVISION.
+;;
 ;; - comment-history (file)
 ;;
 ;;   Return a string containing all log entries that were made for FILE.
@@ -2516,6 +2520,31 @@ vc-print-branch-log
                          (list default-directory) branch t
                          (when (> vc-log-show-limit 0) vc-log-show-limit)))
 
+;;;###autoload
+(defun vc-print-revision (revision)
+  "Show the details of the revision REVISION."
+  (interactive (list (unless current-prefix-arg
+                       (let ((default (thing-at-point 'word)))
+                         (vc-read-revision
+                          (if default
+                              (format "Revision to show (default %s): " default)
+                            "Revision to show: ")
+                          nil nil default)))))
+  (when (equal revision "")
+    (error "No revision specified"))
+  (let ((backend (vc-deduce-backend))
+        rootdir)
+    (if backend
+        (setq rootdir (vc-call-backend backend 'root default-directory))
+      (setq rootdir (read-directory-name "Directory for VC print-revision: "))
+      (setq backend (vc-responsible-backend rootdir))
+      (unless backend
+        (error "Directory is not version controlled")))
+    (setq default-directory rootdir)
+    (vc-incoming-outgoing-internal backend revision
+                                   "*vc-revision*" 'print-revision)
+    (vc-call-backend backend 'region-history-mode)))
+
 ;;;###autoload
 (defun vc-log-incoming (&optional remote-location)
   "Show a log of changes that will be received with a pull operation from REMOTE-LOCATION.
@@ -3091,6 +3120,30 @@ vc-default-dir-status-files
   (funcall update-function
            (mapcar (lambda (file) (list file 'up-to-date)) files)))
 
+(defun vc-default-print-revision (backend buffer revision)
+  (let* ((buffer-name " *vc-revision-log*")
+         (buf (get-buffer-create buffer-name))
+         proc)
+    (vc-call-backend backend 'print-log (list default-directory) buf nil revision 1)
+    (unwind-protect
+        (progn
+          (setq proc (get-buffer-process buf))
+          (while (process-live-p proc)
+            (accept-process-output proc 0.1))
+          (save-current-buffer
+            (vc-setup-buffer buffer)
+            (let ((inhibit-read-only t))
+              (insert-buffer-substring buf)
+              (insert "\n"))
+            (vc-call-backend backend 'diff (list default-directory)
+                             (vc-call-backend backend 'previous-revision nil revision)
+                             revision
+                             buffer
+                             t)
+            (vc-run-delayed
+              (goto-char (point-min)))))
+      (kill-buffer buf))))
+
 (defun vc-check-headers ()
   "Check if the current file has any headers in it."
   (interactive)