bug#13248: [PATCH] bug#13248: 24.2.50; select-window moves point (in comint-postoutput-scroll-to-bottom)

[Vitalie Spinu <spinuvit@gmail.com>]

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

  >> martin rudalics <rudalics@gmx.at>
  >> on Tue, 25 Dec 2012 19:09:38 +0100 wrote:

  >> Here is a patch of the comint-postoutput-scroll-to-bottom to circumvent
  >> resetting the point on select-window.

  > I'm too silly to understand what this is supposed to do.  But the
  > doc-string of `comint-adjust-point' says "Move point in the selected
  > window based on Comint settings." which, together with the fact that you
  > call this in a loop over all windows showing some buffer, indicates to
  > use `set-window-point' rather than `goto-char'.  

It is indeed tricky, I also was bothered by goto-char intricacy, but
decided not to intrude too much.

  > So why can't you write something like

  > (dolist (w (get-buffer-window-list current nil t))
  >   (when (and (< (window-point) (process-mark process))
  > 	     ...)
  >     (set-window-point w ...)))

  > here?

Because of the recentering in this piece: 

╭──────── #2132 ─ /home/vitoshka/TVC/emacs/lisp/comint.el ──
│ 		    (and comint-scroll-show-maximum-output
│ 			 (eobp)
│ 			 (recenter (- -1 scroll-margin))))
╰──────── #2134 ─


And this code:

╭──────── #2140 ─ /home/vitoshka/TVC/emacs/lisp/comint.el ──
│     (and (< (point) (process-mark process))
╰──────── #2140 ─

which implicitly relies on previous reseting of point by select-window.

And this:

╭──────── #2144 ─ /home/vitoshka/TVC/emacs/lisp/comint.el ──
│ 		 (if (eq (selected-window) selected) 'this 'others))
╰──────── #2144 ─

which compares current window with the original one. 


Because of this "implicitness" this piece of code is very tricky. I have
cleaned it up. Note that the internal function comint-adjust-point is no
longer used anywhere in emacs. I left it in the code for backward
compatibility. I think it is quite safe to remove it completely. It is
very unlikely that it has been used outside of this specific context.

    Vitalie


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-diff, Size: 2648 bytes --]

commit 1099eb540d2246836007a83d249db4bf2565c753 (refs/heads/comint-fix)
Author: Vitalie Spinu <spinuvit@gmail.com>
Date:   Tue Dec 25 00:22:55 2012 +0100

    Cleanup comint-postoutput-scroll-to-bottom (Bug#13248)

	Modified   lisp/ChangeLog
diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index 6d5e77d..cf782e9 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,8 @@
+2012-12-24  Vitalie Spinu  <spinuvit@gmail.com>
+
+	* comint.el (comint-postoutput-scroll-to-bottom): Cleanup
+	comint-postoutput-scroll-to-bottom (Bug#13248).
+
 2012-12-24  Dmitry Gutov  <dgutov@yandex.ru>
 
 	* progmodes/ruby-mode.el: Bump the version to 1.2 (Bug#13200).
	Modified   lisp/comint.el
diff --git a/lisp/comint.el b/lisp/comint.el
index cff9afe..7530f24 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -2120,19 +2120,31 @@ This function should be in the list `comint-output-filter-functions'."
 	 ((bound-and-true-p follow-mode)
 	  (follow-comint-scroll-to-bottom))
 	 (t
-	  (let ((selected (selected-window)))
-	    (dolist (w (get-buffer-window-list current nil t))
-	      (select-window w)
-	      (unwind-protect
-		  (progn
-		    (comint-adjust-point selected)
-		    ;; Optionally scroll to the bottom of the window.
-		    (and comint-scroll-show-maximum-output
-			 (eobp)
-			 (recenter (- -1 scroll-margin))))
-		(select-window selected))))))
+          (dolist (w (get-buffer-window-list current nil t))
+            (comint-adjust-window-point w process)
+            ;; Optionally scroll to the bottom of the window.
+            (and comint-scroll-show-maximum-output
+                 (eq (window-point w) (point-max))
+                 (with-selected-window w
+                   (recenter (- -1 scroll-margin)))))))
       (set-buffer current))))
 
+
+(defun comint-adjust-window-point (window process)
+  "Move point in WINDOW based on Comint settings.
+For point adjustment use the process-mark of PROCESS."
+  (and (< (window-point window) (process-mark process))
+       (or (memq comint-move-point-for-output '(t all))
+           ;; Maybe user wants point to jump to end.
+           (eq comint-move-point-for-output
+               (if (eq (selected-window) window) 'this 'others))
+           ;; If point was at the end, keep it at end.
+           (and (marker-position comint-last-output-start)
+                (>= (window-point window) comint-last-output-start)))
+       (set-window-point window (process-mark process))))
+
+
+;; this function is nowhere used
 (defun comint-adjust-point (selected)
   "Move point in the selected window based on Comint settings.
 SELECTED is the window that was originally selected."