bug#16909: 24.3; scrolling *Completions* window with tab sometimes chooses a wrong window

bug#16909: 24.3; scrolling *Completions* window with tab sometimes chooses a wrong window

[Lukasz Pawelczyk]

[-- Attachment #1.1: Type: text/plain, Size: 4647 bytes --]

Hi,

Steps to reproduce (this should be reproducible even on 80x25):
1. Use the attached emacs config file
2. C-x C-f: open a long file (lets say a few screens long)
3. C-x 3: split window right
4. C-x o: move to the right window
5. C-x b *scratch*: display the scratch buffer in the new window
6. C-x 2: split the new window below
7. C-x o: move to the lower-right window (it should display scratch buffer
as well)
8. Type "(g" (without quote ofc) and hit TAB, the auto-complete window
should appear on the lower left
9. Hit tab few more times, the upper left window will be scrolled, not the
*Completions* one.

I've attached a file help.txt describing this graphically.

All of this happens because a window that's chosen for displaying a window
for completion is created with 'display-buffer', while the window that is
scrolled with TAB during completion is choosen with a 'other-window'
('scroll-other-window') function. And as shown here those windows are not
always the same.

Exactly the same thing can happen with semantic completion. The
'display-buffer' and 'scroll-other-window' are also used there to the same
effect in this situation.

The workaround is shown in the emacs.el file, force the other window
scrolling to a *Completions* buffer. But this effectively breaks
other-window scrolling for anything else.


In GNU Emacs 24.3.1 (x86_64-redhat-linux-gnu, GTK+ Version 3.9.10)
 of 2013-08-14 on buildvm-17.phx2.fedoraproject.org
Configured using:
 `configure '--build=x86_64-redhat-linux-gnu'
 '--host=x86_64-redhat-linux-gnu' '--program-prefix='
 '--disable-dependency-tracking' '--prefix=/usr' '--exec-prefix=/usr'
 '--bindir=/usr/bin' '--sbindir=/usr/sbin' '--sysconfdir=/etc'
 '--datadir=/usr/share' '--includedir=/usr/include'
 '--libdir=/usr/lib64' '--libexecdir=/usr/libexec'
 '--localstatedir=/var' '--sharedstatedir=/var/lib'
 '--mandir=/usr/share/man' '--infodir=/usr/share/info' '--with-dbus'
 '--with-gif' '--with-jpeg' '--with-png' '--with-rsvg' '--with-tiff'
 '--with-xft' '--with-xpm' '--with-x-toolkit=gtk3' '--with-gpm=no'
 'build_alias=x86_64-redhat-linux-gnu'
 'host_alias=x86_64-redhat-linux-gnu' 'CFLAGS=-DMAIL_USE_LOCKF -O2 -g
 -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
 -fstack-protector-strong --param=ssp-buffer-size=4
 -grecord-gcc-switches -m64 -mtune=generic' 'LDFLAGS=-Wl,-z,relro ''

Important settings:
  value of $LC_MONETARY: pl_PL.utf8
  value of $LC_NUMERIC: pl_PL.utf8
  value of $LC_TIME: pl_PL.utf8
  value of $LANG: en_US.utf8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix
  default enable-multibyte-characters: t

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-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
  line-number-mode: t
  transient-mark-mode: t

Recent input:
ESC [ > 1 ; 3 4 0 9 ; 0 c ESC x r e p o TAB r TAB
RET

Recent messages:
("emacs")
Loading /usr/share/emacs/site-lisp/site-start.d/cmake-init.el
(source)...done
Loading /usr/share/emacs/site-lisp/site-start.d/desktop-entry-mode-init.el
(source)...done
For information about GNU Emacs and the GNU system, type C-h C-a.
Making completion list...

Load-path shadows:
None found.

Features:
(shadow sort gnus-util mail-extr emacsbug message format-spec rfc822 mml
mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev
gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util
mail-prsvr mail-utils help-mode easymenu time-date buffer-move-autoloads
color-theme-actress-autoloads color-theme-solarized-autoloads
color-theme-autoloads idle-highlight-mode-autoloads point-undo-autoloads
redo+-autoloads rpm-spec-mode-autoloads smart-tabs-mode-autoloads
windresize-autoloads package 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)


-- 
Regards
Havner

[-- Attachment #1.2: Type: text/html, Size: 6259 bytes --]

[-- Attachment #2: help.txt --]
[-- Type: text/plain, Size: 1761 bytes --]

+-------------------+---------------------+
|                   |                     |
|                   |                     |
|                   |                     |
|    long file      |      scratch        |
|                   |                     |
|                   |                     |
|                   |                     |
|                   |                     |
+                   +---------------------+
|                   |                     |
|                   |                     |
|                   |                     |
|                   |       scratch       |
|                   |                     |
|                   |       (g TAB        |
|                   |                     |
|                   |                     |
|                   |                     |
+-------------------+---------------------+

+-------------------+---------------------+
|                   |                     |
|                   |                     |
|                   |                     |
|    long file      |      scratch        |
|  (gets scrolled)  |                     |
|                   |                     |
|                   |                     |
|                   |                     |
+-------------------+---------------------+
|                   |                     |
|                   |                     |
|                   |                     |
|   *Completions*   |       scratch       |
|     (should be    |                     |
|      scrolled)    |       (g TAB        |
|                   |                     |
|                   |                     |
|                   |                     |
+-------------------+---------------------+

[-- Attachment #3: emacs.el --]
[-- Type: text/x-emacs-lisp, Size: 230 bytes --]

(custom-set-variables
 '(inhibit-startup-screen t)
 '(split-height-threshold 10)
 '(split-width-threshold 10)
 '(tab-always-indent (quote complete)))

; A workaround for the bug:
;(setq other-window-scroll-buffer "*Completions*")

bug#16909: 24.3; scrolling *Completions* window with tab sometimes chooses a wrong window

[martin rudalics]

 > The workaround is shown in the emacs.el file, force the other window
 > scrolling to a *Completions* buffer. But this effectively breaks
 > other-window scrolling for anything else.

You mean to set `other-window-for-scrolling' to the *Completions*
buffer?  I think that's what the completions code should do.  But why do
you say that it "effectively breaks other-window scrolling for anything
else"?  IIUC a similar approach is used by `save-some-buffers' and seems
to work without problems.

martin

bug#16909: 24.3; scrolling *Completions* window with tab sometimes chooses a wrong window

[Lukasz Pawelczyk]

On 28 Feb 2014, at 19:24, martin rudalics <rudalics@gmx.at> wrote:

> > The workaround is shown in the emacs.el file, force the other window
> > scrolling to a *Completions* buffer. But this effectively breaks
> > other-window scrolling for anything else.
> 
> You mean to set `other-window-for-scrolling' to the *Completions*
> buffer?  I think that's what the completions code should do.  But why do
> you say that it "effectively breaks other-window scrolling for anything
> else"?  IIUC a similar approach is used by `save-some-buffers' and seems
> to work without problems.

I’m saying that the Tab should be able to scroll the *Completions* buffer automatically without setting anything.
And in the use case I showed above it doesn’t.

I just posted this setq line as a workaround. And why does it break? Because if you set it then C-M-v will always try to scroll *Completions* and not regular other-window buffer which is useful sometimes (e.g. while using help, an approach mentioned even in tutorial).

Just forget about the workaround I mentioned. The use-case shows a bug. That’s all.


-- 
Regards,
Havner

bug#16909: 24.3; scrolling *Completions* window with tab sometimes chooses a wrong window

[martin rudalics]

 > I just posted this setq line as a workaround. And why does it break? Because if you set it then C-M-v will always try to scroll *Completions* and not regular other-window buffer which is useful sometimes (e.g. while using help, an approach mentioned even in tutorial).

So it seems there are two ways to tackle this problem:

(1) Kill the *Completions* buffer when we're done so it won't be
     inadvertently displayed by `other-window-for-scrolling'.

(2) Bind `other-window-scroll-buffer' temporarily only as long as the
     *Completions* buffer is shown.

martin

bug#16909: 24.3; scrolling *Completions* window with tab sometimes chooses a wrong window

[Lukasz Pawelczyk]

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

On Saturday, March 1, 2014, martin rudalics <rudalics@gmx.at> wrote:

> > I just posted this setq line as a workaround. And why does it break?
> Because if you set it then C-M-v will always try to scroll *Completions*
> and not regular other-window buffer which is useful sometimes (e.g. while
> using help, an approach mentioned even in tutorial).
>
> So it seems there are two ways to tackle this problem:
>
> (1) Kill the *Completions* buffer when we're done so it won't be
>     inadvertently displayed by `other-window-for-scrolling'.


How is this supposed to work?
The problem is _during_ the Completions buffer is shown, when we're hitting
Tab, not when we're done. Besides its window is usually closed
automatically. Killing the buffer won't make the scroll-other-window work
the usuall way.


> (2) Bind `other-window-scroll-buffer' temporarily only as long as the
>     *Completions* buffer is shown.


This would work I imagine. But don't do this in general when the
Completions buffer is shown, but only when we use autocomplete
functionality with Tab _and_ the buffer is shown.


>
> martin
>


-- 
Regards
Havner

[-- Attachment #2: Type: text/html, Size: 1684 bytes --]

bug#16909: 24.3; scrolling *Completions* window with tab sometimes chooses a wrong window

[martin rudalics]

 >> (1) Kill the *Completions* buffer when we're done so it won't be
 >>     inadvertently displayed by `other-window-for-scrolling'.
 >
 >
 > How is this supposed to work?
 > The problem is _during_ the Completions buffer is shown, when we're hitting
 > Tab, not when we're done. Besides its window is usually closed
 > automatically.

We have to detect the moment when the window is closed automatically
anyway.  At that time we can either kill the buffer or reset
`other-window-scroll-buffer'.

 > Killing the buffer won't make the scroll-other-window work
 > the usuall way.

How comes?  Ahh... I see.  If `other-window-scroll-buffer' is non-nil
and the buffer was killed in the meantime `display-buffer' throws an
arg out of range error.  That's silly ... should be fixed now.

 >> (2) Bind `other-window-scroll-buffer' temporarily only as long as the
 >>     *Completions* buffer is shown.
 >
 >
 > This would work I imagine. But don't do this in general when the
 > Completions buffer is shown, but only when we use autocomplete
 > functionality with Tab _and_ the buffer is shown.

Could you try doing that?  I have no idea where to start.

martin

bug#16909: 24.3; scrolling *Completions* window with tab sometimes chooses a wrong window

[Lukasz Pawelczyk]

On 1 Mar 2014, at 20:18, martin rudalics <rudalics@gmx.at> wrote:

> > How is this supposed to work?
> > The problem is _during_ the Completions buffer is shown, when we're hitting
> > Tab, not when we're done. Besides its window is usually closed
> > automatically.
> 
> We have to detect the moment when the window is closed automatically
> anyway.  At that time we can either kill the buffer or reset
> `other-window-scroll-buffer’.

I’m still not sure that even using this variable is a wise idea. It might be used
by a user to configure his things. A simple custom function that will always scroll
*Completions* window seems like a better choice. You can always scroll
by buffer name. No need to force the usage of scroll-other-window function.

> > Killing the buffer won't make the scroll-other-window work
> > the usuall way.
> 
> How comes?  Ahh... I see.  If `other-window-scroll-buffer' is non-nil
> and the buffer was killed in the meantime `display-buffer' throws an
> arg out of range error.  That's silly ... should be fixed now.

I meant that if you only kill the buffer (after we used autocomplete) and
then user will scroll-other-window (for whatever reason) it won’t behave as
if its value is nil. We’d have to make it nil as well (as you mentioned now).

> > This would work I imagine. But don't do this in general when the
> > Completions buffer is shown, but only when we use autocomplete
> > functionality with Tab _and_ the buffer is shown.
> 
> Could you try doing that?  I have no idea where to start.

If I had an idea where to start I would have sent a patch instead of reporting a bug :-/



-- 
Regards,
Havner

bug#16909: 24.3; scrolling *Completions* window with tab sometimes chooses a wrong window

[martin rudalics]

 >> We have to detect the moment when the window is closed automatically
 >> anyway.  At that time we can either kill the buffer or reset
 >> `other-window-scroll-buffer’.
 >
 > I’m still not sure that even using this variable is a wise idea. It might be used
 > by a user to configure his things.

By design it's a plain variable and not a user option.

 > A simple custom function that will always scroll
 > *Completions* window seems like a better choice. You can always scroll
 > by buffer name. No need to force the usage of scroll-other-window function.

IIRC the initial intention was to make the *Completions* window the
`next-window' so it would get scrolled automatically by C-M-v.  When
that failed, `other-window-scroll-buffer' was invented to handle the
case where the *Completions* window was not the next window.  I doubt
that a custom function would gain anything in this regard - it would
still have to find the window to scroll and detect when it's no more
needed.

Which obviously does not preclude the use of a function as value of
`other-window-scroll-buffer'.  Sooner or later we should move
`other-window-for-scrolling' and `scroll-other-window' to Elisp anyway,
so this should be easily doable.

 > I meant that if you only kill the buffer (after we used autocomplete) and
 > then user will scroll-other-window (for whatever reason) it won’t behave as
 > if its value is nil. We’d have to make it nil as well (as you mentioned now).

Why?  If `other-window-scroll-buffer' denotes a dead buffer, it now
should be tantamount to nil.  Or am I missing something?

 > If I had an idea where to start I would have sent a patch instead of reporting a bug :-/

I thought you had because you earlier said that

    All of this happens because a window that's chosen for displaying a window
    for completion is created with 'display-buffer', while the window that is
    scrolled with TAB during completion is choosen with a 'other-window'
    ('scroll-other-window') function. And as shown here those windows are not
    always the same.

which gave me the impression that you already did some preliminary
investigative work.

martin

bug#16909: 24.3; scrolling *Completions* window with tab sometimes chooses a wrong window

[Lukasz Pawelczyk]

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

On Wed, Mar 5, 2014 at 8:26 AM, martin rudalics <rudalics@gmx.at> wrote:

> >> We have to detect the moment when the window is closed automatically
> >> anyway.  At that time we can either kill the buffer or reset
> >> `other-window-scroll-buffer’.
> >
> > I’m still not sure that even using this variable is a wise idea. It
> might be used
> > by a user to configure his things.
>
> By design it's a plain variable and not a user option.


In that case it's ok.



> > A simple custom function that will always scroll
> > *Completions* window seems like a better choice. You can always scroll
> > by buffer name. No need to force the usage of scroll-other-window
> function.
>
> IIRC the initial intention was to make the *Completions* window the
> `next-window' so it would get scrolled automatically by C-M-v.  When
> that failed, `other-window-scroll-buffer' was invented to handle the
> case where the *Completions* window was not the next window.  I doubt
> that a custom function would gain anything in this regard - it would
> still have to find the window to scroll and detect when it's no more
> needed.
>

If that's the case, sure.


> I meant that if you only kill the buffer (after we used autocomplete) and
> > then user will scroll-other-window (for whatever reason) it won’t behave
> as
> > if its value is nil. We’d have to make it nil as well (as you mentioned
> now).
>
> Why?  If `other-window-scroll-buffer' denotes a dead buffer, it now
> should be tantamount to nil.  Or am I missing something?


If the 'other-window-scroll-buffer' is pointing to a dead buffer
'scroll-other-window'
does not fallback to its normal behaviour. You get an 'invalid buffer'
message.
So we don't have to kill the *Completions* buffer. We need to set the
variable back
to nil to resume normal user operations after using auto complete scrolling.


> If I had an idea where to start I would have sent a patch instead of
> reporting a bug :-/
>
> I thought you had because you earlier said that
>
>    All of this happens because a window that's chosen for displaying a
> window
>    for completion is created with 'display-buffer', while the window that
> is
>    scrolled with TAB during completion is choosen with a 'other-window'
>    ('scroll-other-window') function. And as shown here those windows are
> not
>    always the same.
>
> which gave me the impression that you already did some preliminary
> investigative work.


My investigation was to figure out what is going on, not precisely where to
fix it and was
based on semantic's autocomplete which behaves exactly the same. I just
thought
that reporting this in base emacs functionality might get a higher chance
to get
attention. And the mechanism and a cause seems to be exactly the same.
I didn't manage to pinpoint where it happens in case of elisp auto complete.
Only for semantic displayor one (which needs fixing as well).


-- 
Regards
Havner

[-- Attachment #2: Type: text/html, Size: 4185 bytes --]

bug#16909: 24.3; scrolling *Completions* window with tab sometimes chooses a wrong window

[martin rudalics]

 > If the 'other-window-scroll-buffer' is pointing to a dead buffer
 > 'scroll-other-window'
 > does not fallback to its normal behaviour. You get an 'invalid buffer'
 > message.

Hopefully not any more.  Did you try the latest trunk?

 > So we don't have to kill the *Completions* buffer. We need to set the
 > variable back
 > to nil to resume normal user operations after using auto complete scrolling.

We can now kill the *Completions* buffer as well.  We only have to be
sure what's easier - keeping track of `other-window-scroll-buffer' and
resetting it or killing the buffer.  Somewhere there should be a
`quit-restore-window' call around, responsible for accomplishing part of
that task already.

 > My investigation was to figure out what is going on, not precisely where to
 > fix it and was
 > based on semantic's autocomplete which behaves exactly the same. I just
 > thought
 > that reporting this in base emacs functionality might get a higher chance
 > to get
 > attention. And the mechanism and a cause seems to be exactly the same.
 > I didn't manage to pinpoint where it happens in case of elisp auto complete.
 > Only for semantic displayor one (which needs fixing as well).

Then try fixing that.  At the time of displaying the *Completions*
buffer set `other-window-scroll-buffer' to the *Completions* buffer.  At
the time of removing that window (I hope it gets removed) reset
`other-window-scroll-buffer' to nil or kill the *Completions* buffer.

Writing this should be simple once you know where to apply the changes.
The more important task is to give it some testing.

martin

bug#16909: 24.3; scrolling *Completions* window with tab sometimes chooses a wrong window

[Lukasz Pawelczyk]

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

On Wed, Mar 5, 2014 at 3:03 PM, martin rudalics <rudalics@gmx.at> wrote:

> > If the 'other-window-scroll-buffer' is pointing to a dead buffer
> > 'scroll-other-window'
> > does not fallback to its normal behaviour. You get an 'invalid buffer'
> > message.
>
> Hopefully not any more.  Did you try the latest trunk?


Yes, I just have (for the first time, my bug report was about 24.3).
And 2 things:

1. I still get invalid buffer, Meaning:
- (setq other-window-scroll-buffer "*Completions*")
- C-M-v always scrolls Completions buffer as it should
- kill the completions buffer
- C-M-v -> "Invalid buffer'

2. But most interestingly, on the latest trunk I cannot reproduce my
original issue anymore.
Contrary to 24.3 TAB for auto-complete seems to work just fine. I'll
investigate this more
to make sure it really is as it should and will report my findings.
If it's just a coincidence (my local work environment is playing tricks on
me) I'll
try to implement some fix this weekend.

Were you ever able to reproduce this issue on trunk? If so, when?


Thanks.


-- 
Regards
Havner

[-- Attachment #2: Type: text/html, Size: 1809 bytes --]

bug#16909: 24.3; scrolling *Completions* window with tab sometimes chooses a wrong window

[martin rudalics]

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

 > Yes, I just have (for the first time, my bug report was about 24.3).
 > And 2 things:
 >
 > 1. I still get invalid buffer, Meaning:
 > - (setq other-window-scroll-buffer "*Completions*")
 > - C-M-v always scrolls Completions buffer as it should
 > - kill the completions buffer
 > - C-M-v -> "Invalid buffer'

I checked in another fix.  Try now with emacs -Q

(progn
   (with-current-buffer (get-buffer-create "*foo*")
     (dotimes (i 100)
       (insert "foo......\n\n....foo....\n\n......foo\n\n....foo...."))
     (goto-char (point-min)))
   (display-buffer "*foo*")
   (with-current-buffer (get-buffer-create "*bar*")
     (dotimes (i 100)
       (insert "bar......\n\n....bar....\n\n......bar\n\n....bar...."))
     (goto-char (point-min)))
   (set-window-buffer (split-window) "*bar*")
   (setq other-window-scroll-buffer (get-buffer "*foo*")))

and do C-M-v.  It should scroll *foo*'s window.  Next do

(kill-buffer "*foo*")

It should scroll *bar*'s window now.

 > 2. But most interestingly, on the latest trunk I cannot reproduce my
 > original issue anymore.
 > Contrary to 24.3 TAB for auto-complete seems to work just fine. I'll
 > investigate this more
 > to make sure it really is as it should and will report my findings.
 > If it's just a coincidence (my local work environment is playing tricks on
 > me) I'll
 > try to implement some fix this weekend.
 >
 > Were you ever able to reproduce this issue on trunk? If so, when?

Always.  On the attached screenshot you see the selected window at the
bottom right and the *Completions* window at the bottom left.  I can't
scroll the *Completions* window via C-M-v from the selected window.  So
I'm afraid you still have to look into this.

martin

[-- Attachment #2: completions.png --]
[-- Type: image/png, Size: 17725 bytes --]

bug#16909: 24.3; scrolling *Completions* window with tab sometimes chooses a wrong window

[Lars Ingebrigtsen]

Lukasz Pawelczyk <havner@gmail.com> writes:

> Steps to reproduce (this should be reproducible even on 80x25):
> 1. Use the attached emacs config file
> 2. C-x C-f: open a long file (lets say a few screens long)
> 3. C-x 3: split window right
> 4. C-x o: move to the right window
> 5. C-x b *scratch*: display the scratch buffer in the new window
> 6. C-x 2: split the new window below
> 7. C-x o: move to the lower-right window (it should display scratch buffer as well)
> 8. Type "(g" (without quote ofc) and hit TAB, the auto-complete window should
> appear on the lower left
> 9. Hit tab few more times, the upper left window will be scrolled, not the
> *Completions* one.

(I'm going through old bug reports that unfortunately weren't resolved
at the time.)

I'm unable to reproduce this problem in Emacs 29.  Are you still seeing
this issue in recent Emacs versions?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#16909: 24.3; scrolling *Completions* window with tab sometimes chooses a wrong window

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> I'm unable to reproduce this problem in Emacs 29.  Are you still seeing
> this issue in recent Emacs versions?

More information was requested, but no response was given within a
month, so I'm closing this bug report.  If the problem still exists,
please respond to this email and we'll reopen the bug report.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no