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

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

[Vitalie Spinu]

Hi, 

I am struggling with a-not-so-easily reproducible bug that occurs only
with ESS when sending input from a buffer to a sub-process. 

Whenever I send a string to a subprocess, the point in process buffer
jumps to the middle of the buffer, instead of staying at the
process-mark at eob, where it supposed to be. And this is not ESS
problem.

Here is what I found during my investigation.  It happens with
(select-window w) in the `comint-postoutput-scroll-to-bottom'
(reproduced below). I was monitoring the value of the (point) just
before and after it, and it looks like this:

 before (point):9943
 after  (point):8619

So the point is clearly moved in select-window. Moreover the (point)
equals (window-end) just before select-window is called, so it is
visible. Consequently, the following (comint-adjust-point selected) is
completely screwed because it relies on point *not* being moved!  

Here is a relevant piece of `comint-postoutput-scroll-to-bottom'


╭──────── #2124 ─ /home/vitoshka/TVC/emacs/lisp/comint.el ──
│ 	    (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))))))
│       (set-buffer current))))
╰──────── #2134 ─


The variable `comint-scroll-show-maximum-output' is the default t, this
is why comint runs `comint-postoutput-scroll-to-bottom' in its
`comint-output-filter-functions'.

I can propose a patch for commit to reset the point, but it looks like
there is something much more fundamental going on in here.

    Thanks, 
    Vitalie



====================================================================

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix
  default enable-multibyte-characters: t

Major mode: Emacs-Lisp

Minor modes in effect:
  sr-popviewer-mode: t
  TeX-PDF-mode: t
  rainbow-delimiters-mode: t
  global-auto-complete-mode: t
  auto-complete-mode: t
  diff-auto-refine-mode: t
  helm-match-plugin-mode: t
  shell-dirtrack-mode: t
  eldoc-mode: t
  show-paren-mode: t
  savehist-mode: t
  display-time-mode: t
  ido-everywhere: t
  global-auto-revert-mode: t
  global-subword-mode: t
  subword-mode: t
  tooltip-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t
  hs-minor-mode: t

Recent input:
C-p C-p C-u C-M-x <down-mouse-1> <mouse-1> C-n C-SPC 
C-n C-n C-c C-r c c c C-o C-o M-> C-o <help-echo> <down-mouse-1> 
<mouse-1> <down-mouse-1> <mouse-1> C-M-p C-p C-p C-p 
C-p C-p C-p C-p C-SPC C-n C-n C-c C-r n n n n n n n 
n n n n n n n n n n n n n n n n n n n n n n n n n n 
n n n n n n n n n n n n n n n n n n n n n n n n n n 
n n n n n n n C-p C-p C-SPC C-n C-n C-c C-r n n n n 
n n n n n n n n n n n n n n n n n n n n n n n n n n 
n n n n n n n n n n n n n n n n n n n n n n n n n n 
n n n n n n n n n n n C-a <return> C-p C-p C-p C-e 
C-n C-n C-o C-x C-f ~ / t v <return> e m a c <return> 
<return> c o m i <return> M-o s c r o l <return> M-o 
s c r o C-SPC o u t C-s <return> M-k M-k M-k M-k M-k 
M-k M-k C-n C-n C-n C-n C-n C-n C-n C-SPC C-n C-n C-n 
C-n C-n C-n C-n C-n C-n C-n C-n M-x k i l <return> 
C-x b C-g M-x e s s - v e r <return> M-x v e r s <return> 
M-x b u g - r e <M-backspace> <M-backspace> s e n d 
C-SPC b u g C-g M-x e m a c s C-SPC b u g <return>

Recent messages:
Opening TLS connection to `imap.gmail.com'...
Opening TLS connection with `gnutls-cli --insecure -p 993 imap.gmail.com'...failed
Opening TLS connection with `gnutls-cli --insecure -p 993 imap.gmail.com --protocols ssl3'...failed
Opening TLS connection with `openssl s_client -connect imap.gmail.com:993 -no_ssl2 -ign_eof'...done
Opening TLS connection to `imap.gmail.com'...done
Auto-saving...
Mark set
Quit
ess-version : 12.09-1 [<unknown>]
GNU Emacs 24.2.50.1 (i686-pc-linux-gnu, GTK+ Version 2.24.13) of 2012-11-15 on vitoshka-home
Quit

Load-path shadows:
/home/vitoshka/Dropbox/ELPA/magit-20121030.2025/.dir-locals hides /home/vitoshka/Dropbox/ELPA/sunrise-commander-20121117.2055/.dir-locals
/home/vitoshka/Dropbox/ELPA/magit-20121030.2025/.dir-locals hides ~/VC/gnus/.dir-locals
~/VC/gnus/lisp/lpath hides ~/VC/auctex/lpath
/home/vitoshka/Dropbox/ELPA/popup-20121020.1203/popup hides ~/VC/popup-el/popup
~/VC/org-mode/lisp/org-remember hides /usr/local/share/emacs/24.2.50/lisp/org/org-remember
~/VC/org-mode/lisp/org-protocol hides /usr/local/share/emacs/24.2.50/lisp/org/org-protocol
~/VC/org-mode/lisp/ob-dot hides /usr/local/share/emacs/24.2.50/lisp/org/ob-dot
~/VC/gnus/lisp/nnweb hides /usr/local/share/emacs/24.2.50/lisp/gnus/nnweb
~/VC/gnus/lisp/yenc hides /usr/local/share/emacs/24.2.50/lisp/gnus/yenc
~/VC/gnus/lisp/gnus-srvr hides /usr/local/share/emacs/24.2.50/lisp/gnus/gnus-srvr
/home/vitoshka/Dropbox/ELPA/rebox2-20121113.2100/rebox2 hides /home/vitoshka/Dropbox/.emacs.d/site-lisp/rebox2/rebox2

Features:
(shadow emacsbug edebug essddr helm-elisp helm-eval url-cache nnfolder
bbdb-message sendmail flymake qp shr-color color shr url-http url-auth
url-gw org-wl org-w3m org-vm org-rmail org-mhe org-mew org-irc
org-jsinfo org-infojs org-html org-info org-gnus org-docview org-bibtex
bibtex org-bbdb wdired reftex-auc igrep tex-info texinfo two-column
iso-transl vc-dispatcher vc-svn helm-mode url-handlers smiley gnus-cite
flow-fill mm-archive mail-extr gnus-bcklg gnus-async gnus-ml gnus-topic
utf-7 nndraft nnmh nnimap parse-time utf7 gnus-agent gnus-srvr
gnus-score score-mode nnvirtual gnus-msg gnus-cache bbdb-gnus gnus-art
mm-uu mml2015 epg-config mm-view mml-smime smime dig bbdb-mua bbdb-com
netrc network-stream starttls tls gnus-notify gnus-demon nntp bbdb
timezone supercite regi nnir gnus-sum gnus-group gnus-undo nnmail
mail-source nnoo gnus-start gnus-spec gnus-int gnus-range message rfc822
mml mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047
rfc2045 ietf-drums mailabbrev gmm-utils mailheader gnus-win gnus-load
gnus gnus-ems gnus-compat nnheader mail-utils sunrise-x-popviewer
sunrise-x-checkpoints bookmark sunrise-x-modeline sunrise-x-loop
sunrise-x-tree sunrise-commander term ehelp electric hl-line find-dired
esh-var esh-io esh-cmd esh-opt esh-ext esh-proc esh-arg esh-groups
eshell esh-util esh-module esh-mode disp-table enriched desktop debug
view magithub crm json magit-bisect magit-key-mode magit sort smex
dabbrev skeleton helm-imenu misearch multi-isearch texmathp vc-git
preview prv-emacs tex-buf reftex-dcr zotelo flyspell ispell font-latex
latex tex-style tex dbus latexenc mule-util help-mode rainbow-delimiters
helm-misc helm-files image-dired dired-x dired-aux helm-tags
helm-bookmark helm-adaptative helm-info helm-net xml url url-proxy
url-privacy url-expand url-methods url-history url-cookie url-domsuf
url-util mailcap helm-plugin helm-locate helm-help helm-external
helm-buffers helm-grep helm-regexp grep helm-elscreen helm-utils ffap
url-parse url-vars ob-latex ob-R appt diary-lib diary-loaddefs org-clock
org-exp ob-exp org-agenda org ob-tangle ob-ref ob-lob ob-table
org-footnote org-src ob-comint ob-keys org-pcomplete org-list org-faces
org-entities noutline outline org-version ob-emacs-lisp ob org-compat
org-macs ob-eval org-loaddefs find-func cal-menu calendar cal-loaddefs
iimage pos-tip ac-octave octave-inf octave-mod ac-math
auto-complete-config auto-complete popup saveplace doc-view jka-compr
image-mode reftex reftex-vars reftex-cite preview-latex tex-site
auto-loads info-look psvn log-edit pcvs-util add-log diff-mode elp
ediff-merg ediff-diff ediff-wind ediff-mult ediff-help ediff-init
ediff-util dired xquery-mode generic rng-nxml rng-valid rng-loc rng-uri
rng-parse nxml-parse rng-match rng-dt rng-util rng-pttrn nxml-ns
nxml-mode nxml-outln nxml-rap nxml-util nxml-glyph nxml-enc xmltok
smart-operator rx rebox2 warnings slime-media slime-presentations
slime-scratch slime-asdf slime-repl slime derived pp hyperspec menu-bar+
helm-descbinds helm-match-plugin helm helm-config mic-paren eldoc-eval
adaptive-wrap-autoloads bbdb-autoloads bm-autoloads bookmark+-autoloads
edit-server-autoloads eldoc-eval-autoloads esk-autoloads
flex-isearch-autoloads fuzzy-autoloads fuzzy-match-autoloads
git-blame-autoloads helm-autoloads helm-descbinds-autoloads
htmlize-autoloads ido-load-library-autoloads ido-ubiquitous-autoloads
ido-yes-or-no-autoloads igrep-autoloads jabber-autoloads
js2-mode-autoloads lacarte-autoloads magit-gh-pulls-autoloads
gh-autoloads logito-autoloads magit-push-remote-autoloads
magithub-autoloads magit-autoloads markdown-mode+-autoloads
markdown-mode-autoloads memory-usage-autoloads mic-paren-autoloads
minimap-autoloads oauth2-autoloads persistent-soft-autoloads
list-utils-autoloads pcache-autoloads finder-inf popup-autoloads
rainbow-delimiters-autoloads rainbow-mode-autoloads rebox2-autoloads
smex-autoloads stem-autoloads sunrise-commander-autoloads
synonyms-autoloads w3m-autoloads zotelo-autoloads package tramp
tramp-compat auth-source eieio byte-opt bytecomp byte-compile cconv
assoc gnus-util mm-util mail-prsvr password-cache tramp-loaddefs
format-spec ess-toolbar ess-mouse mouseme thingatpt browse-url ess-menu
ess-swv ess-noweb ess-noweb-font-lock-mode ess-bugs-l essd-els ess-sas-d
ess-sas-l ess-sas-a shell pcomplete ess-arc-d ess-vst-d ess-xls-d
ess-lsp-l ess-sta-d ess-sta-l cc-vars cc-defs make-regexp ess-sp6-d
ess-sp5-d ess-sp3-d ess-julia ess-r-d ess-tracebug compile ess-roxy
easy-mmode hideshow ess-help info reporter ess-developer ess-r-args
eldoc ess-s-l speedbar sb-image ezimage dframe ess ess-inf comint
ansi-color ring ess-mode ess-noweb-mode edmacro kmacro ess-utils
ess-custom ess-compat ess-site ibuf-ext ibuffer recentf tree-widget
wid-edit easymenu uniquify paren savehist time cus-start cus-load
solarized-dark-theme solarized imenu-anywhere cl-macs gv imenu iflipb
ido sh-script smie executable advice help-fns advice-preload autorevert
subword server cl cl-lib time-date tooltip ediff-hook vc-hooks
lisp-float-type mwheel x-win x-dnd tool-bar dnd fontset image regexp-opt
fringe tabulated-list newcomment lisp-mode register page menu-bar
rfn-eshadow timer select scroll-bar mouse jit-lock font-lock syntax
facemenu font-core frame cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese hebrew greek romanian slovak
czech european ethiopic indian cyrillic chinese case-table epa-hook
jka-cmpr-hook help simple abbrev minibuffer loaddefs button faces
cus-face macroexp files text-properties overlay sha1 md5 base64 format
env code-pages mule custom widget hashtable-print-readable backquote
make-network-process dbusbind dynamic-setting system-font-setting
font-render-setting move-toolbar gtk x-toolkit x multi-tty emacs)

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

[martin rudalics]

 > Here is what I found during my investigation.  It happens with
 > (select-window w) in the `comint-postoutput-scroll-to-bottom'
 > (reproduced below). I was monitoring the value of the (point) just
 > before and after it, and it looks like this:
 >
 >  before (point):9943
 >  after  (point):8619
 >
 > So the point is clearly moved in select-window. Moreover the (point)
 > equals (window-end) just before select-window is called, so it is
 > visible. Consequently, the following (comint-adjust-point selected) is
 > completely screwed because it relies on point *not* being moved!
 >
 > Here is a relevant piece of `comint-postoutput-scroll-to-bottom'
 >
 >
 > ╭──────── #2124 ─ /home/vitoshka/TVC/emacs/lisp/comint.el ──
 > │ 	    (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))))))
 > │       (set-buffer current))))
 > ╰──────── #2134 ─
 >
 >
 > The variable `comint-scroll-show-maximum-output' is the default t, this
 > is why comint runs `comint-postoutput-scroll-to-bottom' in its
 > `comint-output-filter-functions'.
 >
 > I can propose a patch for commit to reset the point, but it looks like
 > there is something much more fundamental going on in here.

select_window (in window.c) has the following comment

   /* Go to the point recorded in the window.
      This is important when the buffer is in more
      than one window.  It also matters when
      redisplay_window has altered point after scrolling,
      because it makes the change only in the window.  */

Is it this behavior that bothers you?

IIUC what you want in `comint-postoutput-scroll-to-bottom' is to
`set-window-point' of the respective window.  If you really want to move
`point' in a buffer _and_ show the effect in a window, do it with that
window selected.

martin

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

[Vitalie Spinu]

  >> martin rudalics <rudalics@gmx.at>
  >> on Fri, 21 Dec 2012 15:25:07 +0100 wrote:

  >> Here is what I found during my investigation.  It happens with
  >> (select-window w) in the `comint-postoutput-scroll-to-bottom'
  >> (reproduced below). I was monitoring the value of the (point) just
  >> before and after it, and it looks like this:
  >> 
  >> before (point):9943
  >> after  (point):8619
  >> 
  >> So the point is clearly moved in select-window. Moreover the (point)
  >> equals (window-end) just before select-window is called, so it is
  >> visible. Consequently, the following (comint-adjust-point selected) is
  >> completely screwed because it relies on point *not* being moved!
  >> 

  > select_window (in window.c) has the following comment

  >   /* Go to the point recorded in the window.
  >      This is important when the buffer is in more
  >      than one window.  It also matters when
  >      redisplay_window has altered point after scrolling,
  >      because it makes the change only in the window.  */

  > Is it this behavior that bothers you?

Yes, indeed this is precisely what happening. Thanks, it clarifies the
problem.

  > IIUC what you want in `comint-postoutput-scroll-to-bottom' is to
  > `set-window-point' of the respective window.  If you really want to move
  > `point' in a buffer _and_ show the effect in a window, do it with that
  > window selected.

And this is what comint-postoutput-scroll-to-bottom fails to do.

    Vitalie

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

[Vitalie Spinu]

  >> martin rudalics <rudalics@gmx.at>
  >> on Fri, 21 Dec 2012 15:25:07 +0100 wrote:

[...]


  > select_window (in window.c) has the following comment

  >   /* Go to the point recorded in the window.
  >      This is important when the buffer is in more
  >      than one window.  It also matters when
  >      redisplay_window has altered point after scrolling,
  >      because it makes the change only in the window.  */

Is "recorded point" the same as window-point? Why is this not in the
docstring of select-window?

    Vitalie

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

[martin rudalics]

> Is "recorded point" the same as window-point? Why is this not in the
> docstring of select-window?

I have updated both the doc-string and the documentation of
`select-window' in revision 111058 of the release-branch.
Kindly have a look.

Thank you, martin

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

[Vitalie Spinu]

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

  >> martin rudalics <rudalics@gmx.at>
  >> on Fri, 21 Dec 2012 15:25:07 +0100 wrote:

[...]

  >> So the point is clearly moved in select-window. Moreover the (point)
  >> equals (window-end) just before select-window is called, so it is
  >> visible. Consequently, the following (comint-adjust-point selected) is
  >> completely screwed because it relies on point *not* being moved!

[...]

  >> 
  >> The variable `comint-scroll-show-maximum-output' is the default t, this
  >> is why comint runs `comint-postoutput-scroll-to-bottom' in its
  >> `comint-output-filter-functions'.
  >> 
  >> I can propose a patch for commit to reset the point, 

[...]

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

    Thanks, 
    Vitalie


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

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index 6d5e77d..dc5a4be 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): Don't reset
+	buffer point on select-window (Bug#13248).
+
 2012-12-24  Dmitry Gutov  <dgutov@yandex.ru>
 
 	* progmodes/ruby-mode.el: Bump the version to 1.2 (Bug#13200).
diff --git a/lisp/comint.el b/lisp/comint.el
index cff9afe..fa3764d 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -2120,12 +2120,14 @@ 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)))
+	  (let ((selected (selected-window))
+                ;; select-window resets point; thus, save
+                (old-point (point)))
 	    (dolist (w (get-buffer-window-list current nil t))
 	      (select-window w)
 	      (unwind-protect
 		  (progn
-		    (comint-adjust-point selected)
+		    (comint-adjust-point selected old-point)
 		    ;; Optionally scroll to the bottom of the window.
 		    (and comint-scroll-show-maximum-output
 			 (eobp)
@@ -2133,9 +2135,12 @@ This function should be in the list `comint-output-filter-functions'."
 		(select-window selected))))))
       (set-buffer current))))
 
-(defun comint-adjust-point (selected)
+(defun comint-adjust-point (selected &optional saved-point)
   "Move point in the selected window based on Comint settings.
-SELECTED is the window that was originally selected."
+SELECTED is the window that was originally selected.
+
+If SAVED-POINT is given, use it as reference instead of the
+current point."
   (let ((process (get-buffer-process (current-buffer))))
     (and (< (point) (process-mark process))
 	 (or (memq comint-move-point-for-output '(t all))
@@ -2144,7 +2149,8 @@ SELECTED is the window that was originally selected."
 		 (if (eq (selected-window) selected) 'this 'others))
 	     ;; If point was at the end, keep it at end.
 	     (and (marker-position comint-last-output-start)
-		  (>= (point) comint-last-output-start)))
+		  (>= (or saved-point (point))
+                      comint-last-output-start)))
 	 (goto-char (process-mark process)))))
 
 (defun comint-truncate-buffer (&optional _string)

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

[martin rudalics]

 > 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'.  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?

martin

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

[Vitalie Spinu]

[-- 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."

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

[martin rudalics]

> 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.

Installed as revision 111344 on trunk.

Thank you, martin

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

[martin rudalics]

> Installed as revision 111344 on trunk.

Bug closed.

Thanks, martin