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

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 13248 in the body.
You can then email your comments to 13248 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Vitalie Spinu <spinuvit <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 24.2.50;
	select-window moves point (in comint-postoutput-scroll-to-bottom)
Date: Fri, 21 Dec 2012 14:14:31 +0100

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)

Message #8 received at 13248 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Vitalie Spinu <spinuvit <at> gmail.com>
Cc: 13248 <at> debbugs.gnu.org
Subject: Re: bug#13248: 24.2.50;
	select-window moves point (in comint-postoutput-scroll-to-bottom)
Date: Fri, 21 Dec 2012 15:25:07 +0100

> 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

Message #11 received at 13248 <at> debbugs.gnu.org (full text, mbox):

From: Vitalie Spinu <spinuvit <at> gmail.com>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 13248 <at> debbugs.gnu.org
Subject: Re: bug#13248: 24.2.50;
	select-window moves point (in comint-postoutput-scroll-to-bottom)
Date: Fri, 21 Dec 2012 15:38:55 +0100

  >> martin rudalics <rudalics <at> 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

Message #14 received at 13248 <at> debbugs.gnu.org (full text, mbox):

From: Vitalie Spinu <spinuvit <at> gmail.com>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 13248 <at> debbugs.gnu.org
Subject: Re: bug#13248: 24.2.50;
	select-window moves point (in comint-postoutput-scroll-to-bottom)
Date: Fri, 21 Dec 2012 15:48:00 +0100

  >> martin rudalics <rudalics <at> 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

Message #17 received at 13248 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Vitalie Spinu <spinuvit <at> gmail.com>
Cc: 13248 <at> debbugs.gnu.org
Subject: Re: bug#13248: 24.2.50;
	select-window moves point (in comint-postoutput-scroll-to-bottom)
Date: Sat, 22 Dec 2012 11:18:52 +0100

> 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

Message #20 received at 13248 <at> debbugs.gnu.org (full text, mbox):

From: Vitalie Spinu <spinuvit <at> gmail.com>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 13248 <at> debbugs.gnu.org
Subject: Re: [PATCH] bug#13248: 24.2.50;
	select-window moves point (in comint-postoutput-scroll-to-bottom)
Date: Tue, 25 Dec 2012 01:18:56 +0100

[Message part 1 (text/plain, inline)]

  >> martin rudalics <rudalics <at> 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

[comint-fix.patch (text/x-diff, inline)]

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 <at> 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 <at> 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)

Message #23 received at 13248 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Vitalie Spinu <spinuvit <at> gmail.com>
Cc: 13248 <at> debbugs.gnu.org
Subject: Re: [PATCH] bug#13248: 24.2.50;
	select-window moves point (in comint-postoutput-scroll-to-bottom)
Date: Tue, 25 Dec 2012 19:09:38 +0100

> 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

Message #26 received at 13248 <at> debbugs.gnu.org (full text, mbox):

From: Vitalie Spinu <spinuvit <at> gmail.com>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 13248 <at> debbugs.gnu.org
Subject: Re: [PATCH] bug#13248: 24.2.50;
	select-window moves point (in comint-postoutput-scroll-to-bottom)
Date: Tue, 25 Dec 2012 23:28:56 +0100

[Message part 1 (text/plain, inline)]

  >> martin rudalics <rudalics <at> 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

[Message part 2 (text/x-diff, inline)]

commit 1099eb540d2246836007a83d249db4bf2565c753 (refs/heads/comint-fix)
Author: Vitalie Spinu <spinuvit <at> 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 <at> gmail.com>
+
+	* comint.el (comint-postoutput-scroll-to-bottom): Cleanup
+	comint-postoutput-scroll-to-bottom (Bug#13248).
+
 2012-12-24  Dmitry Gutov  <dgutov <at> 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."

Message #29 received at 13248 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Vitalie Spinu <spinuvit <at> gmail.com>
Cc: 13248 <at> debbugs.gnu.org
Subject: Re: [PATCH] bug#13248: 24.2.50;
	select-window moves point (in comint-postoutput-scroll-to-bottom)
Date: Thu, 27 Dec 2012 08:36:48 +0100

> 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

Message #34 received at 13248-done <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Vitalie Spinu <spinuvit <at> gmail.com>
Cc: 13248-done <at> debbugs.gnu.org
Subject: Re: bug#13248: [PATCH] bug#13248: 24.2.50; select-window moves point
	(in comint-postoutput-scroll-to-bottom)
Date: Wed, 02 Jan 2013 09:03:09 +0100

> Installed as revision 111344 on trunk.

Bug closed.

Thanks, martin

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.