bug#14829: 24.3; split-window-keep-point breaks shell tab completion

bug#14829: 24.3; split-window-keep-point breaks shell tab completion

[Barry Warsaw]

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


Start `emacs -Q`
(make the window full screen height, 80 columns wide)
M-x shell RET
(in the shell buffer)
$ cd /tmp
$ mkdir foo
$ cd foo
$ touch aa.txt ab.txt cc.txt dd.txt ee.txt ff.txt gg.txt hh.txt ii.txt jj.txt kk.txt ll.txt mm.txt nn.txt oo.txt pp.txt qq.txt rr.txt ss.txt tt.txt uu.txt vv.txt ww.txt xx.txt yy.txt zz.txt
$ ls -l
$ ls -l
(this is just to fill the buffer with output.  now your shell prompt
should be the last line at the bottom of the frame, if not, hit C-l a
few times until it is.)
$ cat a<TAB>

With `(setq split-window-keep-point t)` you will see the window split,
with the top window scrolled to leave point just after the `a` in the
middle of the window.  The bottom window will have the completions for
aa.txt and ab.txt

With `(setq split-window-keep-point nil)` you will not get any window
split, but point will jump someplace higher up in the window (e.g. for
me it jumps to just after the gg.txt line).


In GNU Emacs 24.3.1 (x86_64-pc-linux-gnu, GTK+ Version 3.8.2)
 of 2013-06-14 on komainu, modified by Debian
Windowing system distributor `The X.Org Foundation', version 11.0.11303000
System Description:	Ubuntu Saucy Salamander (development branch)

Configured using:
 `configure '--build' 'x86_64-linux-gnu' '--build' 'x86_64-linux-gnu'
 '--prefix=/usr' '--sharedstatedir=/var/lib' '--libexecdir=/usr/lib'
 '--localstatedir=/var/lib' '--infodir=/usr/share/info'
 '--mandir=/usr/share/man' '--with-pop=yes'
 '--enable-locallisppath=/etc/emacs24:/etc/emacs:/usr/local/share/emacs/24.3/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/24.3/site-lisp:/usr/share/emacs/site-lisp'
 '--with-crt-dir=/usr/lib/x86_64-linux-gnu' '--with-x=yes'
 '--with-x-toolkit=gtk3' '--with-toolkit-scroll-bars'
 'build_alias=x86_64-linux-gnu' 'CFLAGS=-g -O2 -fstack-protector
 --param=ssp-buffer-size=4 -Wformat -Werror=format-security -Wall'
 'LDFLAGS=-Wl,-Bsymbolic-functions -Wl,-z,relro'
 'CPPFLAGS=-D_FORTIFY_SOURCE=2''

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

Major mode: Shell

Minor modes in effect:
  erc-notifications-mode: t
  delete-selection-mode: t
  icomplete-mode: t
  shell-dirtrack-mode: t
  erc-spelling-mode: t
  erc-services-mode: t
  erc-ring-mode: t
  erc-networks-mode: t
  erc-netsplit-mode: t
  erc-menu-mode: t
  erc-list-mode: t
  erc-pcomplete-mode: t
  erc-button-mode: t
  erc-fill-mode: t
  erc-stamp-mode: t
  erc-autojoin-mode: t
  show-paren-mode: t
  savehist-mode: t
  rcirc-track-minor-mode: t
  global-whitespace-mode: t
  erc-track-mode: t
  erc-track-minor-mode: t
  erc-match-mode: t
  erc-irccontrols-mode: t
  erc-noncommands-mode: t
  erc-move-to-prompt-mode: t
  erc-readonly-mode: t
  desktop-save-mode: t
  tooltip-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-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


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

bug#14829: (no subject)

[Barry Warsaw]

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

This fixes it for me.

;; Work around a bug in split-window-keep-point and completion.
(defadvice completion-at-point (around completion-at-point-around)
  "Point safe completion"
  (let ((split-window-keep-point t))
    ad-do-it))
(ad-activate 'completion-at-point)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

bug#14829: 24.3; split-window-keep-point breaks shell tab completion

[martin rudalics]

 > With `(setq split-window-keep-point t)` you will see the window split,
 > with the top window scrolled to leave point just after the `a` in the
 > middle of the window.  The bottom window will have the completions for
 > aa.txt and ab.txt
 >
 > With `(setq split-window-keep-point nil)` you will not get any window
 > split, but point will jump someplace higher up in the window (e.g. for
 > me it jumps to just after the gg.txt line).

`split-window-below' here does

     (setq new-window (split-window nil size))
     (unless split-window-keep-point

so I don't understand how setting `split-window-keep-point' can affect
the decisison whether to split the window.

Does anyone know where this behavior comes from?

martin

bug#14829: 24.3; split-window-keep-point breaks shell tab completion

[Eli Zaretskii]

> Date: Fri, 12 Jul 2013 10:20:22 +0200
> From: martin rudalics <rudalics@gmx.at>
> Cc: 14829@debbugs.gnu.org
> 
>  > With `(setq split-window-keep-point t)` you will see the window split,
>  > with the top window scrolled to leave point just after the `a` in the
>  > middle of the window.  The bottom window will have the completions for
>  > aa.txt and ab.txt
>  >
>  > With `(setq split-window-keep-point nil)` you will not get any window
>  > split, but point will jump someplace higher up in the window (e.g. for
>  > me it jumps to just after the gg.txt line).
> 
> `split-window-below' here does
> 
>      (setq new-window (split-window nil size))
>      (unless split-window-keep-point
> 
> so I don't understand how setting `split-window-keep-point' can affect
> the decisison whether to split the window.

Maybe it is split, and then deleted, before we have a chance of
displaying it?

bug#14829: 24.3; split-window-keep-point breaks shell tab completion

[martin rudalics]

 > Maybe it is split, and then deleted, before we have a chance of
 > displaying it?

Maybe it is not split at all for other reasons.  But that decision
should never be affected by `split-window-keep-point'.  Barry, can you
check whether and/or why not a window splitting occurs?

martin

bug#14829: 24.3; split-window-keep-point breaks shell tab completion

[Barry Warsaw]

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

On Jul 12, 2013, at 12:26 PM, Eli Zaretskii wrote:

>Maybe it is split, and then deleted, before we have a chance of
>displaying it?

I think that's exactly what's happening.  I did a long session of edebug
tracing and observed that behavior (split, then deleted).  I'm sorry that I
don't remember the details - it was quite a long spelunk through the Elisp.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

bug#14829: 24.3; split-window-keep-point breaks shell tab completion

[Barry Warsaw]

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

On Jul 12, 2013, at 12:12 PM, martin rudalics wrote:

> > Maybe it is split, and then deleted, before we have a chance of
> > displaying it?
>
>Maybe it is not split at all for other reasons.  But that decision
>should never be affected by `split-window-keep-point'.  Barry, can you
>check whether and/or why not a window splitting occurs?

With this workaround:

(defadvice completion-at-point (around completion-at-point-around)
  "Point safe completion"
  (let ((split-window-keep-point t))
    ad-do-it))
(ad-activate 'completion-at-point)

and the setup I mentioned earlier (e.g. shell buffer filled with output,
prompt on bottom line of window)...

If you deactivate the advice, what happens when you hit TAB is that you don't
see the split, but you also don't see the completion buffer.  Point just jumps
up to the end-of-line on a line about half-way up the window.

With the advice activated, the window gets split, with the completion buffer
on bottom, and the shell buffer scrolled up so that point remains on the
prompt line which is now approximately in the middle of its buffer (looks to
be a few lines lower, but that's really immaterial).

I notice the exact same behavior in an ERC window, so it's clearly related to
completion-at-point.  Hence the advice in the workaround.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

bug#14829: (no subject)

[Stefan Monnier]

> This fixes it for me.
> ;; Work around a bug in split-window-keep-point and completion.
> (defadvice completion-at-point (around completion-at-point-around)
>   "Point safe completion"
>   (let ((split-window-keep-point t))
>     ad-do-it))
> (ad-activate 'completion-at-point)

Hmm... looking at it further I see that:
- split-window-keep-point defaults to t.
- the problem could affect other cases than completion.
Basically, I'm leaning towards forcing split-window-keep-point to t in
display-buffer, since AFAICT a value of nil only makes sense when you
split a window into two so that they both show the same buffer, whereas
in the case of display-buffer, you always want the current buffer's
point to stay put.

Why do you (and other people) set split-window-keep-point to nil?
What is the expected behavior from it?

The docstring seems to indicate it's only meant to affect C-x 2 rather
than all cases where we create a new window.  Martin?


        Stefan

bug#14829: (no subject)

[martin rudalics]

 > Hmm... looking at it further I see that:
 > - split-window-keep-point defaults to t.
 > - the problem could affect other cases than completion.
 > Basically, I'm leaning towards forcing split-window-keep-point to t in
 > display-buffer, since AFAICT a value of nil only makes sense when you
 > split a window into two so that they both show the same buffer, whereas
 > in the case of display-buffer, you always want the current buffer's
 > point to stay put.

With `inhibit-same-window' t one might want to show the same buffer in the
new window.

 > The docstring seems to indicate it's only meant to affect C-x 2 rather
 > than all cases where we create a new window.  Martin?

It happens because \\[split-window-below] gets substituted by C-x 2.

Why not declare `split-window-keep-point' obsolete and always keep
point?

martin

bug#14829: (no subject)

[Barry Warsaw]

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

On Jul 22, 2013, at 01:18 PM, Stefan Monnier wrote:

>> This fixes it for me.
>> ;; Work around a bug in split-window-keep-point and completion.
>> (defadvice completion-at-point (around completion-at-point-around)
>>   "Point safe completion"
>>   (let ((split-window-keep-point t))
>>     ad-do-it))
>> (ad-activate 'completion-at-point)
>
>Hmm... looking at it further I see that:
>- split-window-keep-point defaults to t.
>- the problem could affect other cases than completion.
>Basically, I'm leaning towards forcing split-window-keep-point to t in
>display-buffer, since AFAICT a value of nil only makes sense when you
>split a window into two so that they both show the same buffer, whereas
>in the case of display-buffer, you always want the current buffer's
>point to stay put.

This seems reasonable to me, as long as it doesn't affect C-x 2.

>Why do you (and other people) set split-window-keep-point to nil?
>What is the expected behavior from it?
>
>The docstring seems to indicate it's only meant to affect C-x 2 rather
>than all cases where we create a new window.  Martin?

C-x 2 is the only reason I set it to nil.  There used to be an old XEmacs hack
(might have been some of my personal elisp) that did what I think was called
"slow-splitting" although it was not actually slow.  It did exactly what C-x 2
with s-w-k-p set to nil does.  I love this for tall windows since in most
cases, nothing changes except you get a modeline in the middle of the window
where the split occurs.

So I'm all in favor of keeping the behavior for C-x 2 and inhibiting it for
all other purposes.  I have definitely encountered other situations that the
defadvice above does not handle.  One example is pdb-track in python-mode.
When you hit the breakpoint, the shell buffer is split with the code shown
below, but the split does not happen correct.  Point is left at the same bogus
place as described in the original bug report.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

bug#14829: (no subject)

[Barry Warsaw]

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

On Jul 22, 2013, at 07:37 PM, martin rudalics wrote:

>It happens because \\[split-window-below] gets substituted by C-x 2.
>
>Why not declare `split-window-keep-point' obsolete and always keep
>point?

Please no!  C-x 2 with split-window-keep-point nil is a *really* important
feature, to me at least.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

bug#14829: (no subject)

[Stefan Monnier]

>> Hmm... looking at it further I see that:
>> - split-window-keep-point defaults to t.
>> - the problem could affect other cases than completion.
>> Basically, I'm leaning towards forcing split-window-keep-point to t in
>> display-buffer, since AFAICT a value of nil only makes sense when you
>> split a window into two so that they both show the same buffer, whereas
>> in the case of display-buffer, you always want the current buffer's
>> point to stay put.
> With `inhibit-same-window' t one might want to show the same buffer in the
> new window.

I can live with that "problem".


        Stefan

bug#14829: (no subject)

[martin rudalics]

>> Why not declare `split-window-keep-point' obsolete and always keep
>> point?
> 
> Please no!  C-x 2 with split-window-keep-point nil is a *really* important
> feature, to me at least.

OK.  Don't worry ;-)

martin

bug#14829: (no subject)

[martin rudalics]

 > I can live with that "problem".

So what do we prefer?  Handle this option in `split-window-sensibly' or
in `split-window-below'?

martin

bug#14829: (no subject)

[Stefan Monnier]

>> I can live with that "problem".
> So what do we prefer?  Handle this option in `split-window-sensibly' or
> in `split-window-below'?

I think we can start by let-binding split-window-keep-point to t in
display-buffer.


        Stefan

bug#14829: (no subject)

[Barry Warsaw]

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

On Jul 23, 2013, at 09:09 AM, Stefan Monnier wrote:

>>> I can live with that "problem".
>> So what do we prefer?  Handle this option in `split-window-sensibly' or
>> in `split-window-below'?
>
>I think we can start by let-binding split-window-keep-point to t in
>display-buffer.

I'm happy to test out a patch if it can be applied to 24.3.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

bug#14829: (no subject)

[martin rudalics]

> I think we can start by let-binding split-window-keep-point to t in
> display-buffer.

Done.

martin

bug#14829: (no subject)

[martin rudalics]

> I'm happy to test out a patch if it can be applied to 24.3.

It's trivial, try the one below.

Thanks, martin

--- lisp/window.el	2013-07-06 14:24:54 +0000
+++ lisp/window.el	2013-07-25 09:32:11 +0000
@@ -5470,6 +5470,9 @@
    (let ((buffer (if (bufferp buffer-or-name)
  		    buffer-or-name
  		  (get-buffer buffer-or-name)))
+	;; Make sure that when we split windows the old window keeps
+	;; point, bug#14829.
+	(split-window-keep-point t)
  	;; Handle the old form of the first argument.
  	(inhibit-same-window (and action (not (listp action)))))
      (unless (listp action) (setq action nil))

bug#14829: (no subject)

[Barry Warsaw]

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

On Jul 25, 2013, at 12:05 PM, martin rudalics wrote:

>> I'm happy to test out a patch if it can be applied to 24.3.
>
>It's trivial, try the one below.
>
>Thanks, martin
>
>--- lisp/window.el	2013-07-06 14:24:54 +0000
>+++ lisp/window.el	2013-07-25 09:32:11 +0000
>@@ -5470,6 +5470,9 @@
>    (let ((buffer (if (bufferp buffer-or-name)
>  		    buffer-or-name
>  		  (get-buffer buffer-or-name)))
>+	;; Make sure that when we split windows the old window keeps
>+	;; point, bug#14829.
>+	(split-window-keep-point t)
>  	;; Handle the old form of the first argument.
>  	(inhibit-same-window (and action (not (listp action)))))
>      (unless (listp action) (setq action nil))

Works great, thanks.  I've applied this to Ubuntu 13.10's version of Emacs
24.3.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

bug#14829: (no subject)

[martin rudalics]

> Works great, thanks.  I've applied this to Ubuntu 13.10's version of Emacs
> 24.3.

If you feel that no more should be done, please close this bug.

Thanks, martin

bug#14829: (no subject)

[martin rudalics]

> Works great, thanks.  I've applied this to Ubuntu 13.10's version of Emacs
> 24.3.

Checked in as revision 113542.  Bug closed.

Thanks, martin