bug#21067: 25.0.50; [PATCH] With mercurial, vc-print-log puts point at eob

[Wolfgang Jenkner]

Call vc-dir for some repo under hg version control and then type l in
the resulting *vc-dir* buffer.

As expected, this produces a *vc-change-log* buffer with the most recent
change at the beginning.  However, point is at the end of the buffer.
I would expect point to be put next to the most recent commit, as it is
the case for, e.g., git.  Note that explicitly selecting a revision
(typing C-u l) does work.

The different behaviour is due to the hg log subprocess being run
synchronously while the git subprocess is run asynchronously.

For an async process the following happens:

vc-process-filter inserts each output chunk at the process mark and
updates the latter; vc-run-delayed has advised the sentinel to go to the
process mark and insert additional stuff (buttons) when the process is
finished.  All those insertions happen inside save-excursion forms.
However, the sentinel sets point to the value of vc-sentinel-movepoint
(if non-nil).

For a sync process the following happens:

The output is inserted and additional stuff set up with vc-run-delayed
is inserted at point.  Those insertions don't happen inside
save-excursion forms and vc-sentinel-movepoint is ignored.

Now, while it is true that the name of vc-sentinel-movepoint doesn't
sound like it was meant for synchronous processes anyway, it is also
true that generic functions like vc-print-log and callees don't know
whether the subprocess will be run synchronously or asynchronously.

So I wonder whether it wouldn't be worthwile to make their job easier
(and thereby avoid bugs like the one described above) by handling async
and sync processes alike in this respect.  The condition for this to
work is that backend functions don't expect point to be preserved,
except, of course, if it is actually their purpose to compute point
(like for vc-git-show-log-entry).

Here's a patch.

-- >8 --
Subject: [PATCH] Handle point for sync vc processes like for async ones

* lisp/vc/vc-dispatcher.el (vc--process-sentinel): Cut some code.
(vc-exec-after): Move it here.  Remove comment which has not been
matching the code for a while.
(vc-do-command): Use save-excursion around the sync process call.
---
 lisp/vc/vc-dispatcher.el | 39 ++++++++++++++++++---------------------
 1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/lisp/vc/vc-dispatcher.el b/lisp/vc/vc-dispatcher.el
index ec55867..4f44d35 100644
--- a/lisp/vc/vc-dispatcher.el
+++ b/lisp/vc/vc-dispatcher.el
@@ -199,25 +199,7 @@ Another is that undo information is not kept."
                 ;; Leave mode-line uncluttered, normally.
                 (unless (eq 'exit status)
                   (format " (%s)" status))))
-        (let (vc-sentinel-movepoint
-              (m (process-mark p)))
-          ;; Normally, we want async code such as sentinels to not move point.
-          (save-excursion
-            (goto-char m)
-            ;; Each sentinel may move point and the next one should be run
-            ;; at that new point.  We could get the same result by having
-            ;; each sentinel read&set process-mark, but since `cmd' needs
-            ;; to work both for async and sync processes, this would be
-            ;; difficult to achieve.
-            (vc-exec-after code)
-            (move-marker m (point)))
-          ;; But sometimes the sentinels really want to move point.
-          (when vc-sentinel-movepoint
-           (let ((win (get-buffer-window (current-buffer) 0)))
-             (if (not win)
-                 (goto-char vc-sentinel-movepoint)
-               (with-selected-window win
-                 (goto-char vc-sentinel-movepoint))))))))))
+        (vc-exec-after code)))))
 
 (defun vc-set-mode-line-busy-indicator ()
   (setq mode-line-process
@@ -241,7 +223,20 @@ CODE should be a function of no arguments."
      ((or (null proc) (eq (process-status proc) 'exit))
       ;; Make sure we've read the process's output before going further.
       (when proc (accept-process-output proc))
-      (if (functionp code) (funcall code) (eval code)))
+      (let (vc-sentinel-movepoint)
+        ;; Normally, we want code to not move point.
+        (save-excursion
+          (let ((m (when proc (process-mark proc))))
+            (goto-char (or m (point-max)))
+            (if (functionp code) (funcall code) (eval code))
+            (when m (move-marker m (point)))))
+        ;; But sometimes the sentinels really want to move point.
+        (when vc-sentinel-movepoint
+          (let ((win (get-buffer-window (current-buffer) 0)))
+            (if (not win)
+                (goto-char vc-sentinel-movepoint)
+              (with-selected-window win
+                (goto-char vc-sentinel-movepoint)))))))
      ;; If a process is running, add CODE to the sentinel
      ((eq (process-status proc) 'run)
       (vc-set-mode-line-busy-indicator)
@@ -337,7 +332,9 @@ case, and the process object in the asynchronous case."
            (when vc-command-messages
              (message "Running %s in foreground..." full-command))
            (let ((buffer-undo-list t))
-             (setq status (apply 'process-file command nil t nil squeezed)))
+              ;; Use `save-excursion' like `vc-process-filter' does.
+             (setq status (save-excursion
+                             (apply 'process-file command nil t nil 
squeezed))))
            (when (and (not (eq t okstatus))
                       (or (not (integerp status))
                           (and okstatus (< okstatus status))))
-- 
2.4.5