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

From: [email-masked] (GNU bug Tracking System)
Date: Mon, 10 May 2021 11:58:01 +0000

Your bug report

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

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
21067: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=21067
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#21067: 25.0.50; [PATCH] With mercurial, vc-print-log puts point at eob
From: Dmitry Gutov <[email-masked]>
To: Lars Ingebrigtsen <[email-masked]>
Cc: Wolfgang Jenkner <[email-masked]>, [email-masked]
Date: Mon, 10 May 2021 14:57:31 +0300
List-Id: <debbugs-submit.debbugs.gnu.org>

Version: 26.2

On 10.05.2021 14:28, Lars Ingebrigtsen wrote:
> This was five years ago -- skimming this thread, I'm not quite sure what
> the conclusion here was.  Is this something that should still be worked
> on, or has this problem gone away in the meantime?

Thanks for the reminder.

Looks like we've fixed this in Emacs 26.2. Commit fcd66d059, bug#31764.

Closing, reopen if you still can reproduce.

Subject: 25.0.50; [PATCH] With mercurial, vc-print-log puts point at eob
From: Wolfgang Jenkner <[email-masked]>
To: [email-masked]
Date: Wed, 15 Jul 2015 13:09:22 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

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