bug#8358: 24.0.50; `minibuffer-scroll-window' with active minibuffer: always *Completions*?

bug#8358: 24.0.50; `minibuffer-scroll-window' with active minibuffer: always *Completions*?

[Drew Adams]

`minibuffer-scroll-window' is the window to be scrolled by
`scroll-other-window' from the minibuffer.
 
The doc suggests that setting `minibuffer-scroll-window' to some window
when the minibuffer is active would make other-window scrolling use that
window.  E.g. the doc for `scroll-other-window' includes this:
 
"If in the minibuffer, `minibuffer-scroll-window' if non-nil specifies
the window to scroll.  This takes precedence over
`other-window-scroll-buffer'."
 
It doesn't say that the *Completions* window is always the other window
scrolled.  But `minibuffer-scroll-window' always seems to be reset to
the *Completions* window, AFAICT.
 
Help me understand how to make some other window than the *Completions*
window the target to be scrolled by `scroll-other-window' from the
minibuffer.
 
emacs -Q
 
C-x d RET
C-x 4 b *scratch* RET
 
(defun foo ()
 (setq minibuffer-scroll-window 
       (get-buffer-window (get-buffer "*scratch*") 0))
 (message "fffff, MSW: %S" minibuffer-scroll-window) (sleep-for 2))
 
(add-hook 'completion-setup-hook 'foo 'append)
 
(defun bar (arg)
  (interactive "P")
  (message "BBBBB, MSW: %S" minibuffer-scroll-window) (sleep-for 2)
  (scroll-other-window arg))
 
(define-key minibuffer-local-completion-map "\C-\M-v" 'bar)
 
Return focus to the Dired window.
 
M-x for TAB C-M-v
 
That shows that `minibuffer-scroll-window' is the *Completions* window
when `C-M-v' is pressed, even though `minibuffer-scroll-window' was the
*scratch* window just after *Completions* was shown.
 
I looked at the code in minibuffer.el and window.c to try to understand
where `minibuffer-scroll-window' is getting reset (to *Completions*),
but I haven't understood, so far.
 
(`minibuffer-complete' does set `minibuffer-scroll-window' (e.g. to
nil), but debugging that function shows that it doesn't seem to make a
difference here.  And my own code, where I also see the problem, doesn't
even call `minibuffer-complete'.)
 
Help appreciated.  Is this a bug?  Shouldn't you be able to set the
window to be scrolled (using `scroll-other-window') during minibuffer
input to be some window other than *Completions*?  How can I do that?
Just where is `minibuffer-scroll-window' getting set and reset?  Thx.
 
In GNU Emacs 24.0.50.1 (i386-mingw-nt5.1.2600)
 of 2011-03-21 on 3249CTO
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (4.5) --no-opt --cflags
-Ic:/imagesupport/include'
 

bug#8358: 24.0.50; `minibuffer-scroll-window' with active minibuffer: always *Completions*?

[martin rudalics]

 > I looked at the code in minibuffer.el and window.c to try to understand
 > where `minibuffer-scroll-window' is getting reset (to *Completions*),
 > but I haven't understood, so far.

`with-output-to-temp-buffer' which displays the *Completions* buffer
sets `minibuffer-scroll-window' to the window showing *Completions*.
This happens _after_ running `completion-setup-hook' so your "foo"
doesn't really do what you expected.

 > Help appreciated.  Is this a bug?  Shouldn't you be able to set the
 > window to be scrolled (using `scroll-other-window') during minibuffer
 > input to be some window other than *Completions*?  How can I do that?
 > Just where is `minibuffer-scroll-window' getting set and reset?  Thx.

Try doing

(add-hook 'temp-buffer-show-hook 'foo 'append)

instead.  Or write your own `temp-buffer-show-function'.

martin

bug#8358: 24.0.50; `minibuffer-scroll-window' with active minibuffer: always *Completions*?

[Drew Adams]

> `with-output-to-temp-buffer' which displays the *Completions* buffer
> sets `minibuffer-scroll-window' to the window showing *Completions*.

Thanks for the info.  But why should that happen?  `with-output-to-temp-buffer'
is supposed to be general, for any temporary buffer.  It is not supposed to be
specific to *Completions* or *Help* or any other given temporary buffer.

And the doc of `scroll-other-window' says that "`minibuffer-scroll-window' if
non-nil specifies the window to scroll."  It doesn't say that it always
specifies the *Completions* window.

And the doc of `minibuffer-scroll-window' says "Non-nil means it is the window
that C-M-v in minibuffer should scroll."  It doesn't say that C-M-v in the
minibuffer always scrolls *Completions*.

There is nothing to indicate that things are in fact hard-coded so that the
window to scroll when you are in the minibuffer is always *Completions*.  And
there is nothing to indicate that that is the intention (design).

On the contrary.  This is a variable, created presumably to let you change the
window to be scrolled from the minibuffer.  And the doc supports this as the
intention.

> This happens _after_ running `completion-setup-hook' so your "foo"
> doesn't really do what you expected.
> 
> > Shouldn't you be able to set the window to be scrolled
> > (using `scroll-other-window') during minibuffer
> > input to be some window other than *Completions*?  How can 
> > I do that?
> 
> Try doing
> (add-hook 'temp-buffer-show-hook 'foo 'append)
> instead.  Or write your own `temp-buffer-show-function'.

Thanks, but such a workaround is a sledge hammer here.  `temp-buffer-show-hook'
is general, and it should not be necessary to add and remove stuff just to get
`minibuffer-scroll-window' to act as a variable.

I appreciate the implementation info, but this seems like a bug to me.
`minibuffer-scroll-window' is used only when the minibuffer is active, and it is
apparently always set, in that case, to the *Completions* window.  This was
created as a variable presumably so that programs could change the window to be
scrolled from the minibuffer.

It's not clear whether you are just explaining what currently happens (thank
you) or you are also saying that this is not a bug.  What's the point of
`minibuffer-scroll-window' if it is always effectively *Completions*?

Other than this hard-coded case, user code has easy control over
`scroll-other-window(-down)'.  Can we please fix this so that
`minibuffer-scroll-window' acts as advertised?

bug#8358: 24.0.50; `minibuffer-scroll-window' with active minibuffer: always *Completions*?

[martin rudalics]

 >> `with-output-to-temp-buffer' which displays the *Completions* buffer
 >> sets `minibuffer-scroll-window' to the window showing *Completions*.
 >
 > Thanks for the info.  But why should that happen?  `with-output-to-temp-buffer'
 > is supposed to be general, for any temporary buffer.  It is not supposed to be
 > specific to *Completions* or *Help* or any other given temporary buffer.
 >
 > And the doc of `scroll-other-window' says that "`minibuffer-scroll-window' if
 > non-nil specifies the window to scroll."  It doesn't say that it always
 > specifies the *Completions* window.
 >
 > And the doc of `minibuffer-scroll-window' says "Non-nil means it is the window
 > that C-M-v in minibuffer should scroll."  It doesn't say that C-M-v in the
 > minibuffer always scrolls *Completions*.
 >
 > There is nothing to indicate that things are in fact hard-coded so that the
 > window to scroll when you are in the minibuffer is always *Completions*.  And
 > there is nothing to indicate that that is the intention (design).
 >
 > On the contrary.  This is a variable, created presumably to let you change the
 > window to be scrolled from the minibuffer.  And the doc supports this as the
 > intention.

`minibuffer-scroll-window' is not a user option.  So by design it's a
variable any code is allowed to change.  I suppose that it's set in
`with-output-to-temp-buffer' because that macro is quite opaque so the
caller isn't even informed about which window displays the buffer.

 >> Try doing
 >> (add-hook 'temp-buffer-show-hook 'foo 'append)
 >> instead.  Or write your own `temp-buffer-show-function'.
 >
 > Thanks, but such a workaround is a sledge hammer here.  `temp-buffer-show-hook'
 > is general, and it should not be necessary to add and remove stuff just to get
 > `minibuffer-scroll-window' to act as a variable.
 >
 > I appreciate the implementation info, but this seems like a bug to me.

`temp-buffer-show-hook' is provided to override the built-in behavior of
`with-output-to-temp-buffer'.  If I were annoyed by the behavior I would
use it to fix such problems.

 > `minibuffer-scroll-window' is used only when the minibuffer is active, and it is
 > apparently always set, in that case, to the *Completions* window.  This was
 > created as a variable presumably so that programs could change the window to be
 > scrolled from the minibuffer.

Yes.

 > It's not clear whether you are just explaining what currently happens (thank
 > you) or you are also saying that this is not a bug.  What's the point of
 > `minibuffer-scroll-window' if it is always effectively *Completions*?

I'm only trying to explain what happens, I don't use C-M-v myself.  But
I suppose that if you removed the corresponding assignment from
`with-output-to-temp-buffer', the completions coder would add a similar
assignment to `temp-buffer-show-hook'.

 > Other than this hard-coded case, user code has easy control over
 > `scroll-other-window(-down)'.  Can we please fix this so that
 > `minibuffer-scroll-window' acts as advertised?

The doc-string of `minibuffer-scroll-window' says

   Non-nil means it is the window that C-M-v in minibuffer should scroll.

so IIUC it acts as advertised.  If the window has been deleted in the
meantime, it's slightly out of synch but this problem is handled by
`other-window-for-scrolling'.  But you probably should try to find out
why it has been designed this way and/or propose an appropriate fix for
the completions code.

martin

bug#8358: 24.0.50; `minibuffer-scroll-window' with active minibuffer: always *Completions*?

[Drew Adams]

> `minibuffer-scroll-window' is not a user option.  So by design it's a
> variable any code is allowed to change.

Yes, any code, including user code.  And it is explicitly called out in the doc
of `scroll-other-window', to let programmers know about it.  We don't generally
do that for internal variables that are used only to always produce the same
visible behavior.

Yes, I realize that the window object used for *Completions* changes.  But if
that were the only reason for this variable then I doubt that it would figure in
the user documentation.

> I suppose that it's set in `with-output-to-temp-buffer'
> because that macro is quite opaque so the caller isn't
> even informed about which window displays the buffer.

Your "because" has nothing to do with the clause it is applied to (AFAICT).
That the caller of `with-output-to-temp-buffer' might not know which window
displays the temp buffer is not a reason to make the window that is to be
scrolled always be the *Completions* window.  That just doesn't follow.

> `temp-buffer-show-hook' is provided to override the built-in 
> behavior of `with-output-to-temp-buffer'.  If I were annoyed by the 
> behavior I would use it to fix such problems.

It won't do the job needed anyway.  In a single minibuffer input reading I
dynamically update the set of matching completions according to what the user
types (similar to the incremental completion of ido or icomplete, but using
*Completions*).

Depending on what the user does (e.g., particular minibuffer keys s?he hits),
the "other" window to be scrolled needs to change.  Think, for example, of a
minibuffer key that displays some information in another window, which the user
might then want to scroll.

I would like to set `minibuffer-scroll-window' to that window (whatever it might
be, depending on the context).  But when I do that `minibuffer-scroll-window'
keeps getting reset to the *Completions* window (presumably because updating the
set of matching completions redisplays *Completions*).

> I'm only trying to explain what happens

I appreciate that; thank you.
 
> The doc-string of `minibuffer-scroll-window' says
> 
>    Non-nil means it is the window that C-M-v in minibuffer 
>    should scroll.
> 
> so IIUC it acts as advertised.

You mean because *Completions* is in fact the window that gets scrolled? That's
putting the cart a bit before the horse.

As I said, if the intention were only that *Completions* should always be
scrolled, then the doc would say so: in the minibuffer, the *Completions* window
is the other window scrolled.  It could add that the *Completions* window is the
value of `minibuffer-scroll-window' (but it need not even bother).

If that were the intention then the doc for `minibuffer-scroll-window' would
simply say that it is the *Completions* window, which you can scroll from the
minibuffer using C-M-v.  And the variable might as well be named something like
`completions-window' in that case.

But that is not at all what the doc indicates.  It suggests strongly that
`minibuffer-scroll-window' can be set to control which window gets scrolled.

> If the window has been deleted in the
> meantime, it's slightly out of synch but this problem is handled by
> `other-window-for-scrolling'.  But you probably should try to find
> out why it has been designed this way and/or propose an 
> appropriate fix for the completions code.

I guess I'll just forget about `minibuffer-scroll-window' and
`scroll-other-window', and simply roll my own command that scrolls the window
that I tell it too.  If Emacs Dev is not interested in fixing this, so be it.

bug#8358: 24.0.50; `minibuffer-scroll-window' with active minibuffer: always *Completions*?

[martin rudalics]

 >> `minibuffer-scroll-window' is not a user option.  So by design it's a
 >> variable any code is allowed to change.
 >
 > Yes, any code, including user code.  And it is explicitly called out in the doc
 > of `scroll-other-window', to let programmers know about it.  We don't generally
 > do that for internal variables that are used only to always produce the same
 > visible behavior.

I still don't understand why you can't or don't want to change it in
your code.  Or did I miss something?

 > Yes, I realize that the window object used for *Completions* changes.  But if
 > that were the only reason for this variable then I doubt that it would figure in
 > the user documentation.
 >
 >> I suppose that it's set in `with-output-to-temp-buffer'
 >> because that macro is quite opaque so the caller isn't
 >> even informed about which window displays the buffer.
 >
 > Your "because" has nothing to do with the clause it is applied to (AFAICT).
 > That the caller of `with-output-to-temp-buffer' might not know which window
 > displays the temp buffer is not a reason to make the window that is to be
 > scrolled always be the *Completions* window.  That just doesn't follow.

Why do you use the term "always" here?  `minibuffer-scroll-window' is
set to the window used for displaying the temporary buffer which can be
*Completions* or *Help* or whatever you have.

 >> `temp-buffer-show-hook' is provided to override the built-in
 >> behavior of `with-output-to-temp-buffer'.  If I were annoyed by the
 >> behavior I would use it to fix such problems.
 >
 > It won't do the job needed anyway.

It will do the job for this particular case.

 > In a single minibuffer input reading I
 > dynamically update the set of matching completions according to what the user
 > types (similar to the incremental completion of ido or icomplete, but using
 > *Completions*).
 >
 > Depending on what the user does (e.g., particular minibuffer keys s?he hits),
 > the "other" window to be scrolled needs to change.  Think, for example, of a
 > minibuffer key that displays some information in another window, which the user
 > might then want to scroll.
 >
 > I would like to set `minibuffer-scroll-window' to that window (whatever it might
 > be, depending on the context).  But when I do that `minibuffer-scroll-window'
 > keeps getting reset to the *Completions* window (presumably because updating the
 > set of matching completions redisplays *Completions*).

So define a variable `my-minibuffer-scroll-window' and whenever
`with-output-to-temp-buffer' steals the window from you reset
`minibuffer-scroll-window' to `my-minibuffer-scroll-window' in
`temp-buffer-show-hook'.

 >> The doc-string of `minibuffer-scroll-window' says
 >>
 >>    Non-nil means it is the window that C-M-v in minibuffer
 >>    should scroll.
 >>
 >> so IIUC it acts as advertised.
 >
 > You mean because *Completions* is in fact the window that gets scrolled? That's
 > putting the cart a bit before the horse.

I don't follow you here.  What would you tell people using your code
when they complain that you set `minibuffer-scroll-window' to a window
they don't want to scroll?

 > As I said, if the intention were only that *Completions* should always be
 > scrolled, then the doc would say so: in the minibuffer, the *Completions* window
 > is the other window scrolled.  It could add that the *Completions* window is the
 > value of `minibuffer-scroll-window' (but it need not even bother).

The only doc that could be fixed in this respect is that of
`with-output-to-temp-buffer'.

 > If that were the intention then the doc for `minibuffer-scroll-window' would
 > simply say that it is the *Completions* window, which you can scroll from the
 > minibuffer using C-M-v.  And the variable might as well be named something like
 > `completions-window' in that case.

But every window used by `with-output-to-temp-buffer' is affected, not
only the *Completions* window.

 > But that is not at all what the doc indicates.  It suggests strongly that
 > `minibuffer-scroll-window' can be set to control which window gets scrolled.

It can.  `with-output-to-temp-buffer' makes use of that property.  And
you can override it in `temp-buffer-show-hook'.

martin

bug#8358: 24.0.50; `minibuffer-scroll-window' with active minibuffer: always *Completions*?

[Drew Adams]

> I still don't understand why you can't or don't want to change it in
> your code.  Or did I miss something?

I don't see an easy fix.  See what I said about incremental completion etc., if
you're interested.

>  > Your "because" has nothing to do with the clause it is 
>  > applied to (AFAICT). That the caller of
>  > `with-output-to-temp-buffer' might not know which window
>  > displays the temp buffer is not a reason to make the 
>  > window that is to be scrolled always be the *Completions*
>  > window.  That just doesn't follow.
> 
> Why do you use the term "always" here?  `minibuffer-scroll-window' is
> set to the window used for displaying the temporary buffer 
> which can be *Completions* or *Help* or whatever you have.

Whenever the minibuffer is used for completion, and *Completions* is displayed,
it is automatically set to the *Completions* window.  So not quite always, since
you can use the minibuffer without completion.

>  > I would like to set `minibuffer-scroll-window' to that 
>  > window (whatever it might be, depending on the context).
>  > But when I do that `minibuffer-scroll-window'
>  > keeps getting reset to the *Completions* window 
>  > (presumably because updating the
>  > set of matching completions redisplays *Completions*).
> 
> So define a variable `my-minibuffer-scroll-window' and whenever
> `with-output-to-temp-buffer' steals the window from you reset
> `minibuffer-scroll-window' to `my-minibuffer-scroll-window' in
> `temp-buffer-show-hook'.

If `temp-buffer-show-hook' is the only culprit, then that might be a workaround.
One way or another I will find a solution for myself (the easiest thing is to
just define a new scroll command that respects a new variable).

That doesn't change the fact (IMO) that this behavior represents a bug.

> I don't follow you here.  What would you tell people using your code
> when they complain that you set `minibuffer-scroll-window' to a window
> they don't want to scroll?

Users of my code already scroll *Completions* using different keys (which
invokes different code), anyway.  And they would appreciate being able to scroll
the other windows they interact with during completion.

> The only doc that could be fixed in this respect is that of
> `with-output-to-temp-buffer'.

>  > But that is not at all what the doc indicates.  It 
>  > suggests strongly that `minibuffer-scroll-window' can be
>  > set to control which window gets scrolled.
> 
> It can.  `with-output-to-temp-buffer' makes use of that property.  And
> you can override it in `temp-buffer-show-hook'.

Should be able to set it at any time and have it take effect (not be wiped out
by `with-output-to-temp-buffer'.  Anyway, it's clear we don't agree.  The simple
solution (trivial in this case) is for me to just roll my own and forget about
using `scroll-other-window(-down)' and `minibuffer-scroll-window', since they
are essentially hard-coded during completion.

bug#8358: 24.0.50; `minibuffer-scroll-window' with active minibuffer: always *Completions*?

[Stefan Monnier]

> Should be able to set it at any time and have it take effect (not be
> wiped out by `with-output-to-temp-buffer'.  Anyway, it's clear we
> don't agree.  The simple solution (trivial in this case) is for me to
> just roll my own and forget about using `scroll-other-window(-down)'
> and `minibuffer-scroll-window', since they are essentially hard-coded
> during completion.

Here's my question:
given that we want (by default) scroll-other-window to scroll the
*Completions* buffer when called from the minibuffer and *Completions*
has been displayed, how else would you implement this in a way that
also satisfies your needs?

IOW send us some kind of patch to the Emacs code that does not change
the current default behavior, but makes it flexible in the way
you expect.


        Stefan

bug#8358: 24.0.50; `minibuffer-scroll-window' with active minibuffer: always *Completions*?

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

> `minibuffer-scroll-window' is not a user option.  So by design it's a
> variable any code is allowed to change.  I suppose that it's set in
> `with-output-to-temp-buffer' because that macro is quite opaque so the
> caller isn't even informed about which window displays the buffer.

Skimming this bug report, it seems like everything is working as
designed here, so I'm closing this bug report.

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

bug#8358: 24.0.50; `minibuffer-scroll-window' with active minibuffer: always *Completions*?

[Stefan Monnier]

> Thanks for the info.  But why should that happen?

Because minibuffer.el wants minibuffer-scroll-window to point to
*Completions* after popping up a list of completions.

> On the contrary.  This is a variable, created presumably to let you
> change the window to be scrolled from the minibuffer.  And the doc
> supports this as the intention.

No, not to "let you ..." but to "let code ...".


        Stefan

bug#8358: 24.0.50; `minibuffer-scroll-window' with active minibuffer: always *Completions*?

[Drew Adams]

> > Thanks for the info.  But why should that happen?
> 
> Because minibuffer.el wants minibuffer-scroll-window to point to
> *Completions* after popping up a list of completions.

And even if it was already given another window as value, apparently.

> > On the contrary.  This is a variable, created presumably to let you
> > change the window to be scrolled from the minibuffer.  And the doc
> > supports this as the intention.
> 
> No, not to "let you ..." but to "let code ...".

So you are excluding user code.  OK, your intention is clear.

In that case, why document it?  The doc gives a completely different impression
- it suggests that user code can set this variable to determine which window
gets scrolled from the minibuffer by `scroll-other-window'.  (And it can, like
it or not - except during completion.)

And this variable is quite old, certainly much older than the minibuffer.el code
that makes one use of it.  And it was explicitly exposed to users and documented
from the outset (with no exception made for completion).

But hey, who said history doesn't allow revision?  I do however suggest you
change the doc in that case so it fits your intention better.  And what do I
know anyway?  Maybe it never should have been documented; maybe it was always
intended as internal-only.

Anyway, I got the message; I'll just roll my own here, since
`minibuffer-scroll-window' is useless in this context and anyway not intended
for user code.

That's trivial, and I would have done it from the beginning, but it sounded from
the doc like `scroll-other-window' and `minibuffer-scroll-window' were
ready-made to scroll a particular window from the minibuffer.  Even during
completion.  Silly me.

It seems, in any case, that it is only the use of completion that locks code out
from binding/setting the variable usefully.  Other uses of the minibuffer pose
no such limitation.  `widget-choose', for instance, binds the variable on the
fly during minibuffer input, but it does not try to do that during completion.

So if you change your mind and decide it's OK for user code to use this
variable, then you might want to change the doc just a bit to mention that
completion is an exception: *Completions* is always the other window to scroll.

bug#8358: 24.0.50; `minibuffer-scroll-window' with active minibuffer: always *Completions*?

[martin rudalics]

 >> No, not to "let you ..." but to "let code ...".
 >
 > So you are excluding user code.

On the contrary.  All code is created equal.

martin

bug#8358: 24.0.50; `minibuffer-scroll-window' with active minibuffer: always *Completions*?

[Drew Adams]

>  >> No, not to "let you ..." but to "let code ...".
>  >
>  > So you are excluding user code.
> 
> On the contrary.  All code is created equal.

Then Stefan's remark was meaningless.  I was obviously talking about user code
when I said "let you".  Everything I wrote was about doing this from user code.