unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Zoom: a window management minor mode -- best practices and questions
@ 2018-05-02 16:31 Andrea Cardaci
  2018-05-02 17:32 ` Eli Zaretskii
  0 siblings, 1 reply; 23+ messages in thread
From: Andrea Cardaci @ 2018-05-02 16:31 UTC (permalink / raw)
  To: emacs-devel; +Cc: Eli Zaretskii

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

Hi,

This is a follow-up discussion for a bug I reported (
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=31312), since the
conversation was moving away from the bug itself I'm starting a new thread
as suggested by Eli.

> In that case, could you explain in more detail what zoom.el attempts
> to do

Zoom (https://github.com/cyrus-and/zoom) aims to enforce a fixed window
layout so that the selected window is always *big enough*. I think the
screencast at the project page is quite clear about the usage.

The implementation is very simple, every time a window change is *detected*
the following happens:
1. `balance-windows` is called;
2. the selected window is resized.

This, in practice, has the effect of disabling the manual resizing of
windows.

The problem is that to implement the "every time a window change is
detected" part I currently hook several functions, specifically:

    (add-hook 'window-size-change-functions #'zoom--handler)
    (advice-add #'select-window :after #'zoom--handler)

This is clearly an hack-ish solution but Zoom works fairly well most of the
times. On the other thread Martin suggested to replace `advice-add` with an
hook to `buffer-list-update-hook`, but the main issue remains that
`zoom--handler` actually resizes windows in the
`window-size-change-functions` hook.

Moreover, this solution causes the handler to be called several times and
most of them are useless. (Additional guards can be added but still...)

> and what does it need from Emacs core to be able to do that?

My knowledge of the Emacs internals is quite limited but at the highest
level of abstraction what AFAIK is missing is a way to enforce custom
window layouts and manage the windows size. This part seems quite fragile /
extremely hard to work with (just look at the implementation of
`balance-windows`). In addition to that there are some features that
further complicate the situation, e.g., side windows and not resizable
windows.

Said that, I'm definitely not in a position to make any feature requests
but I would appreciate any feedback or suggestions.


Thanks,

Andrea

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Zoom: a window management minor mode -- best practices and questions
  2018-05-02 16:31 Zoom: a window management minor mode -- best practices and questions Andrea Cardaci
@ 2018-05-02 17:32 ` Eli Zaretskii
  2018-05-02 18:41   ` Andrea Cardaci
  0 siblings, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2018-05-02 17:32 UTC (permalink / raw)
  To: Andrea Cardaci; +Cc: emacs-devel

> From: Andrea Cardaci <cyrus.and@gmail.com>
> Date: Wed, 2 May 2018 18:31:11 +0200
> Cc: Eli Zaretskii <eliz@gnu.org>
> 
> Zoom (https://github.com/cyrus-and/zoom) aims to enforce a fixed window layout so that the selected window
> is always *big enough*. I think the screencast at the project page is quite clear about the usage.

I've seen the screencast before asking the question, but couldn't
grasp the intent well enough, probably because the display in
screencast switches windows too quickly, and it's hard to understand
what Zoom attempts to do.

> The implementation is very simple, every time a window change is *detected* the following happens:
> 1. `balance-windows` is called;
> 2. the selected window is resized.
> 
> This, in practice, has the effect of disabling the manual resizing of windows.
> 
> The problem is that to implement the "every time a window change is detected" part I currently hook several
> functions, specifically:
> 
>     (add-hook 'window-size-change-functions #'zoom--handler)
>     (advice-add #'select-window :after #'zoom--handler)

I understand why you need to hook select-window, but not why you need
the other hook.  Is it because there are too many functions that could
modify the window size, and you didn't want to hook all of them?

Did you try to use pre-redisplay-functions instead?  That doesn't
necessarily tell you that the selected window is going to change or
its dimensions are about to change, but determining that for a single
window shouldn't be too expensive, right?

Alternatively, we could provide some control to disable resizing of
the selected window -- would that be enough, in addition to hooking
select-widnow using the method suggested by martin?

> My knowledge of the Emacs internals is quite limited but at the highest level of abstraction what AFAIK is
> missing is a way to enforce custom window layouts and manage the windows size. This part seems quite
> fragile / extremely hard to work with (just look at the implementation of `balance-windows`). In addition to that
> there are some features that further complicate the situation, e.g., side windows and not resizable windows.

Maybe Martin will suggest a good way of doing that using the existing
facilities, if they are enough.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Zoom: a window management minor mode -- best practices and questions
  2018-05-02 17:32 ` Eli Zaretskii
@ 2018-05-02 18:41   ` Andrea Cardaci
  2018-05-02 18:58     ` Eli Zaretskii
  2018-05-03  7:11     ` martin rudalics
  0 siblings, 2 replies; 23+ messages in thread
From: Andrea Cardaci @ 2018-05-02 18:41 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

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

Thanks for the answer.

> I understand why you need to hook select-window, but not why you need
> the other hook.  Is it because there are too many functions that could
> modify the window size, and you didn't want to hook all of them?

The handler should be triggered in these cases:
- a window is resized;
- a window is selected;
- a window is created (this is actually included in the "a window is resized"
case);
- a window changes its buffer (because it's possible to exclude certain
windows from zooming so the layout should be updated accordingly).

So yes, instead of trying to figure out what the functions that cause such
events are (probably way too many) I simply tried to hook on the effects of
such actions.

> Did you try to use pre-redisplay-functions instead?

Not yet, but a quick test shows that it gets called *very* often, even when
Emacs is idle. I should really implement some additional guard if I decide
to go that way to avoid incurring in performance issues.

Also it seems to not work well with the `track-mouse` variable that I use
to delay the re-layout if there is some mouse selection in progress.

Besides, doesn't this in principle suffer of the same
do-something-that-triggers-the-hook-inside-such-hook problem like
`window-size-change-functions`?

> Alternatively, we could provide some control to disable resizing of
> the selected window

Please elaborate on this point. How can it help this situation?

On 2 May 2018 at 19:32, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Andrea Cardaci <cyrus.and@gmail.com>
> > Date: Wed, 2 May 2018 18:31:11 +0200
> > Cc: Eli Zaretskii <eliz@gnu.org>
> >
> > Zoom (https://github.com/cyrus-and/zoom) aims to enforce a fixed window
> layout so that the selected window
> > is always *big enough*. I think the screencast at the project page is
> quite clear about the usage.
>
> I've seen the screencast before asking the question, but couldn't
> grasp the intent well enough, probably because the display in
> screencast switches windows too quickly, and it's hard to understand
> what Zoom attempts to do.
>
> > The implementation is very simple, every time a window change is
> *detected* the following happens:
> > 1. `balance-windows` is called;
> > 2. the selected window is resized.
> >
> > This, in practice, has the effect of disabling the manual resizing of
> windows.
> >
> > The problem is that to implement the "every time a window change is
> detected" part I currently hook several
> > functions, specifically:
> >
> >     (add-hook 'window-size-change-functions #'zoom--handler)
> >     (advice-add #'select-window :after #'zoom--handler)
>
> I understand why you need to hook select-window, but not why you need
> the other hook.  Is it because there are too many functions that could
> modify the window size, and you didn't want to hook all of them?
>
> Did you try to use pre-redisplay-functions instead?  That doesn't
> necessarily tell you that the selected window is going to change or
> its dimensions are about to change, but determining that for a single
> window shouldn't be too expensive, right?
>
> Alternatively, we could provide some control to disable resizing of
> the selected window -- would that be enough, in addition to hooking
> select-widnow using the method suggested by martin?
>
> > My knowledge of the Emacs internals is quite limited but at the highest
> level of abstraction what AFAIK is
> > missing is a way to enforce custom window layouts and manage the windows
> size. This part seems quite
> > fragile / extremely hard to work with (just look at the implementation
> of `balance-windows`). In addition to that
> > there are some features that further complicate the situation, e.g.,
> side windows and not resizable windows.
>
> Maybe Martin will suggest a good way of doing that using the existing
> facilities, if they are enough.
>

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Zoom: a window management minor mode -- best practices and questions
  2018-05-02 18:41   ` Andrea Cardaci
@ 2018-05-02 18:58     ` Eli Zaretskii
  2018-05-03  7:11       ` martin rudalics
  2018-05-03  9:46       ` Andrea Cardaci
  2018-05-03  7:11     ` martin rudalics
  1 sibling, 2 replies; 23+ messages in thread
From: Eli Zaretskii @ 2018-05-02 18:58 UTC (permalink / raw)
  To: Andrea Cardaci; +Cc: emacs-devel

> From: Andrea Cardaci <cyrus.and@gmail.com>
> Date: Wed, 2 May 2018 20:41:38 +0200
> Cc: emacs-devel@gnu.org
> 
> The handler should be triggered in these cases:
> - a window is resized;

This is the one I mentioned as missing.

> - a window is selected;

Hooking select-window will take care of this one.

> - a window is created (this is actually included in the "a window is resized" case);

If you mean that the selected window is resized as result of creating
a new window, then yes, "window is resized" takes care of this.  (What
about deleting a window, btw?)

> - a window changes its buffer (because it's possible to exclude certain windows from zooming so the layout
> should be updated accordingly).

The hook suggested by Martin should take care of this.

> Besides, doesn't this in principle suffer of the same do-something-that-triggers-the-hook-inside-such-hook
> problem like `window-size-change-functions`?

No, because pre-redisplay-functions are run _before_ anything
significant in redisplay, so your chances to confuse redisplay are
nil.

> > Alternatively, we could provide some control to disable resizing of
> > the selected window
> 
> Please elaborate on this point. How can it help this situation?

Well, as you said, the main purpose of Zoom is to prevent resizing of
the selected window due to Emacs's own considerations.  My
interpretation of this was that you want to control the size of the
selected window, and disallow anything else changing it.  So if you
change the size at select-window time, and the size is not allowed to
change after that, you have reached your purpose, right?



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Zoom: a window management minor mode -- best practices and questions
  2018-05-02 18:41   ` Andrea Cardaci
  2018-05-02 18:58     ` Eli Zaretskii
@ 2018-05-03  7:11     ` martin rudalics
  2018-05-03  9:47       ` Andrea Cardaci
  1 sibling, 1 reply; 23+ messages in thread
From: martin rudalics @ 2018-05-03  7:11 UTC (permalink / raw)
  To: Andrea Cardaci, Eli Zaretskii; +Cc: emacs-devel

 > The handler should be triggered in these cases:
 > - a window is resized;
 > - a window is selected;
 > - a window is created (this is actually included in the "a window is resized"
 > case);
 > - a window changes its buffer (because it's possible to exclude certain
 > windows from zooming so the layout should be updated accordingly).

There are probably other ones like when a window gets deleted or the
configuration changes but all these should get caught by your code.
I'm not sure why hooking 'minibuffer-setup-hook' is needed, I suppose
it is not, at least for Emacs 26.  Other than that your code seems
valid to me (even disabling 'window-configuration-change-hook' in
'balance-windows' is OK, that hook should not get called there any
more).

Obviously, 'window-size-change-functions' is meant for applications to
react to size changes and possibly readjust buffer text shown in a
window whose size changed.  You should warn other applications to make
sure they _append_ their functions to this hook in order to be aware
of your adjustments.  Maybe you should re-prepend your function when
you find out that other ones have been prepended in between.

Otherwise, I would use 'buffer-list-update-hook' instead of advising
'select-window' to make sure that all occurences of selecting a window
get caught.  And I would experimentally try to not zoom immediately in
'buffer-list-update-hook' and 'window-configuration-change-hook' but
simply feed these occurrencs to 'pre-redisplay-function' to reduce the
number of times you zoom.  'window-pixel-height-before-size-change'
and 'window-pixel-width-before-size-change' should allow to easily do
the 'window-size-change-functions' part in 'pre-redisplay-function' as
well.  Though my personal experiences with 'pre-redisplay-function'
are not bright enough to recommend it for sure.

martin



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Zoom: a window management minor mode -- best practices and questions
  2018-05-02 18:58     ` Eli Zaretskii
@ 2018-05-03  7:11       ` martin rudalics
  2018-05-03  9:50         ` Andrea Cardaci
  2018-05-03  9:46       ` Andrea Cardaci
  1 sibling, 1 reply; 23+ messages in thread
From: martin rudalics @ 2018-05-03  7:11 UTC (permalink / raw)
  To: Eli Zaretskii, Andrea Cardaci; +Cc: emacs-devel

 > Well, as you said, the main purpose of Zoom is to prevent resizing of
 > the selected window due to Emacs's own considerations.  My
 > interpretation of this was that you want to control the size of the
 > selected window, and disallow anything else changing it.  So if you
 > change the size at select-window time, and the size is not allowed to
 > change after that, you have reached your purpose, right?

IUUC zoom.el does not necessarily prevent resizing of the selected
window (we allow fixing or preserving window sizes to do that).  It
tries to have the selected window stand out by making it "very" large.

martin



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Zoom: a window management minor mode -- best practices and questions
  2018-05-02 18:58     ` Eli Zaretskii
  2018-05-03  7:11       ` martin rudalics
@ 2018-05-03  9:46       ` Andrea Cardaci
  1 sibling, 0 replies; 23+ messages in thread
From: Andrea Cardaci @ 2018-05-03  9:46 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

> If you mean that the selected window is resized as result of creating
> a new window, then yes, "window is resized" takes care of this.  (What
> about deleting a window, btw?)

Yes I mean that and the same applies when a window is deleted.

> No, because pre-redisplay-functions are run _before_ anything
> significant in redisplay, so your chances to confuse redisplay are
> nil.

Got it, thanks.

> Well, as you said, the main purpose of Zoom is to prevent resizing of
> the selected window due to Emacs's own considerations.  My
> interpretation of this was that you want to control the size of the
> selected window, and disallow anything else changing it.  So if you
> change the size at select-window time, and the size is not allowed to
> change after that, you have reached your purpose, right?

Hm maybe, but not only that, the reason why I use `balance-windows`
before resizing the selected window is ensure some consistency in the
sizes of other windows too.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Zoom: a window management minor mode -- best practices and questions
  2018-05-03  7:11     ` martin rudalics
@ 2018-05-03  9:47       ` Andrea Cardaci
  2018-05-07 12:32         ` Andrea Cardaci
  0 siblings, 1 reply; 23+ messages in thread
From: Andrea Cardaci @ 2018-05-03  9:47 UTC (permalink / raw)
  To: martin rudalics; +Cc: Eli Zaretskii, emacs-devel

> There are probably other ones like when a window gets deleted or the
> configuration changes but all these should get caught by your code.

Yep, I forgot to mention when a window gets deleted.

> I'm not sure why hooking 'minibuffer-setup-hook' is needed, I suppose
> it is not, at least for Emacs 26.  Other than that your code seems
> valid to me (even disabling 'window-configuration-change-hook' in
> 'balance-windows' is OK, that hook should not get called there any
> more).

`minibuffer-setup-hook` is needed to detect when the minibuffer is
selected because there's an option to preserve the currently zoomed
window in that case to avoid quick and useless layout changes.

> Obviously, 'window-size-change-functions' is meant for applications to
> react to size changes and possibly readjust buffer text shown in a
> window whose size changed.  You should warn other applications to make
> sure they _append_ their functions to this hook in order to be aware
> of your adjustments.  Maybe you should re-prepend your function when
> you find out that other ones have been prepended in between.

That's a interesting advice.

> Otherwise, I would use 'buffer-list-update-hook' instead of advising
> 'select-window' to make sure that all occurences of selecting a window
> get caught.  And I would experimentally try to not zoom immediately in
> 'buffer-list-update-hook' and 'window-configuration-change-hook' but
> simply feed these occurrencs to 'pre-redisplay-function' to reduce the
> number of times you zoom.  'window-pixel-height-before-size-change'
> and 'window-pixel-width-before-size-change' should allow to easily do
> the 'window-size-change-functions' part in 'pre-redisplay-function' as
> well.  Though my personal experiences with 'pre-redisplay-function'
> are not bright enough to recommend it for sure.

I will try this approach, thanks.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Zoom: a window management minor mode -- best practices and questions
  2018-05-03  7:11       ` martin rudalics
@ 2018-05-03  9:50         ` Andrea Cardaci
  0 siblings, 0 replies; 23+ messages in thread
From: Andrea Cardaci @ 2018-05-03  9:50 UTC (permalink / raw)
  To: martin rudalics; +Cc: Eli Zaretskii, emacs-devel

> IUUC zoom.el does not necessarily prevent resizing of the selected
> window (we allow fixing or preserving window sizes to do that).  It
> tries to have the selected window stand out by making it "very" large.

No it doesn't but that's a nice side effect and not only for the
selected window, I don't want the user messes up the layout if
`zoom-mode` is enabled, for the same reason I disable the mouse drag
events on windows borders (I noticed that under certain circumstances
some glitches appear).



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Zoom: a window management minor mode -- best practices and questions
  2018-05-03  9:47       ` Andrea Cardaci
@ 2018-05-07 12:32         ` Andrea Cardaci
  2018-05-07 18:19           ` Eli Zaretskii
  2018-05-09  7:00           ` martin rudalics
  0 siblings, 2 replies; 23+ messages in thread
From: Andrea Cardaci @ 2018-05-07 12:32 UTC (permalink / raw)
  To: martin rudalics; +Cc: Eli Zaretskii, emacs-devel

> > Otherwise, I would use 'buffer-list-update-hook' instead of advising
> > 'select-window' to make sure that all occurences of selecting a window
> > get caught.  And I would experimentally try to not zoom immediately in
> > 'buffer-list-update-hook' and 'window-configuration-change-hook' but
> > simply feed these occurrencs to 'pre-redisplay-function' to reduce the
> > number of times you zoom.  'window-pixel-height-before-size-change'
> > and 'window-pixel-width-before-size-change' should allow to easily do
> > the 'window-size-change-functions' part in 'pre-redisplay-function' as
> > well.  Though my personal experiences with 'pre-redisplay-function'
> > are not bright enough to recommend it for sure.
>
> I will try this approach, thanks.

I've been trying this, I like the separation between checking whether
a relayout is needed or not and the actual relayout.

There's one problem with `buffer-list-update-hook` though, it gets
called (multiple times) even wen the buffer list is not changed, e.g.,
simply by clicking in the buffer. Is this the expected behaviour?

Besides this, if there's no way to get rid of false positives in event
handling (i.e., a relayout is triggered but no actual change happened)
I guess I need a way to decide if a relayout is *really* needed. A
naive way would be saving the current window and buffer but I'm not
sure if we can do better.

Also, oddly enough, `pre-redisplay-function` is never called on macOS
(Emacs 26.1)...



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Zoom: a window management minor mode -- best practices and questions
  2018-05-07 12:32         ` Andrea Cardaci
@ 2018-05-07 18:19           ` Eli Zaretskii
  2018-05-08 10:40             ` Andrea Cardaci
  2018-05-09  7:00           ` martin rudalics
  1 sibling, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2018-05-07 18:19 UTC (permalink / raw)
  To: Andrea Cardaci; +Cc: rudalics, emacs-devel

> From: Andrea Cardaci <cyrus.and@gmail.com>
> Date: Mon, 7 May 2018 14:32:28 +0200
> Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org
> 
> There's one problem with `buffer-list-update-hook` though, it gets
> called (multiple times) even wen the buffer list is not changed, e.g.,
> simply by clicking in the buffer. Is this the expected behaviour?

I don't remember (perhaps Martin does).  But if you show a C-level
backtrace from such a call to buffer-list-update-hook, it will be easy
to say whether this is expected or not.

> Besides this, if there's no way to get rid of false positives in event
> handling (i.e., a relayout is triggered but no actual change happened)

Do you mean that pre-redisplay-function is called?  If not, what
exactly do you mean by "relayout is triggered"?

> Also, oddly enough, `pre-redisplay-function` is never called on macOS
> (Emacs 26.1)...

That's strange.  The most frequent call to pre-redisplay-function is
in prepare_menu_bars; are you saying that function is never called on
macOS?  If you put a breakpoint inside that function, does it never
break?



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Zoom: a window management minor mode -- best practices and questions
  2018-05-07 18:19           ` Eli Zaretskii
@ 2018-05-08 10:40             ` Andrea Cardaci
  2018-05-08 14:53               ` Noam Postavsky
  2018-05-08 17:52               ` Eli Zaretskii
  0 siblings, 2 replies; 23+ messages in thread
From: Andrea Cardaci @ 2018-05-08 10:40 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: martin rudalics, emacs-devel

> I don't remember (perhaps Martin does).  But if you show a C-level
> backtrace from such a call to buffer-list-update-hook, it will be easy
> to say whether this is expected or not.

Is there a simple way to dump a backtrace from Emacs?

It's easy to reproduce tough:

(defun foo ()
  (message "buffer-list-update-hook"))

(add-hook 'buffer-list-update-hook 'foo)

then simply click in the selected window.

Anyway according to the documentation this hook is called by
`select-window` which is actually called if you click on a window,
even if it is the selected one.

> Do you mean that pre-redisplay-function is called?  If not, what
> exactly do you mean by "relayout is triggered"?

Yeah, I mean I collect all the events which tell me that a relayout is
needed (window created/destroyed, window/buffer selected, etc.) then
finally I perform the actual relayout. If a subsequent event tells me
that a relayout is needed but no window has been actually
created/destroyed etc. then it's a false positive, in this case for
example it happens when `pre-redisplay-function` is called, e.g., when
the user clicks in the selected window.

(By relayout I mean what Zoom does: `balance-windows` and resize the
selected window.)

> That's strange.  The most frequent call to pre-redisplay-function is
> in prepare_menu_bars; are you saying that function is never called on
> macOS?

Well, there's more than that, it doesn't seem to be a macOS thing. If I do:

(defun foo (x)
  (message "pre-redisplay-function"))

(add-hook 'pre-redisplay-function 'foo)

Almost always, the hook is not called when I interact with windows and
the text selection mark becomes invisible for future selections (!),
even in the terminal. Otherwise (but I cannot reproduce it anymore...)
the hook is called very frequently, even when Emacs is idle.

Emacs 26.0.90 on Linux, using the -Q option.

> If you put a breakpoint inside that function, does it never break?

`prepare_menu_bars` does break but `FUNCTIONP
(Vpre_redisplay_function)` is false so `safe__call1 (true,
Vpre_redisplay_function, windows);` is never executed.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Zoom: a window management minor mode -- best practices and questions
  2018-05-08 10:40             ` Andrea Cardaci
@ 2018-05-08 14:53               ` Noam Postavsky
  2018-05-08 15:03                 ` Andrea Cardaci
  2018-05-09 12:33                 ` Stefan Monnier
  2018-05-08 17:52               ` Eli Zaretskii
  1 sibling, 2 replies; 23+ messages in thread
From: Noam Postavsky @ 2018-05-08 14:53 UTC (permalink / raw)
  To: Andrea Cardaci; +Cc: martin rudalics, Eli Zaretskii, Emacs developers

On 8 May 2018 at 06:40, Andrea Cardaci <cyrus.and@gmail.com> wrote:

> (defun foo (x)
>   (message "pre-redisplay-function"))
>
> (add-hook 'pre-redisplay-function 'foo)

`add-hook' is for variables which are lists of functions, for
pre-redisplay-function which holds a single function, you should use
add-function:

(add-function :after pre-redisplay-function #'foo)



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Zoom: a window management minor mode -- best practices and questions
  2018-05-08 14:53               ` Noam Postavsky
@ 2018-05-08 15:03                 ` Andrea Cardaci
  2018-05-09 12:33                 ` Stefan Monnier
  1 sibling, 0 replies; 23+ messages in thread
From: Andrea Cardaci @ 2018-05-08 15:03 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: martin rudalics, Eli Zaretskii, Emacs developers

> you should use add-function

Thanks Noam, I didn't know that.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Zoom: a window management minor mode -- best practices and questions
  2018-05-08 10:40             ` Andrea Cardaci
  2018-05-08 14:53               ` Noam Postavsky
@ 2018-05-08 17:52               ` Eli Zaretskii
  1 sibling, 0 replies; 23+ messages in thread
From: Eli Zaretskii @ 2018-05-08 17:52 UTC (permalink / raw)
  To: Andrea Cardaci; +Cc: rudalics, emacs-devel

> From: Andrea Cardaci <cyrus.and@gmail.com>
> Date: Tue, 8 May 2018 12:40:01 +0200
> Cc: martin rudalics <rudalics@gmx.at>, emacs-devel@gnu.org
> 
> > I don't remember (perhaps Martin does).  But if you show a C-level
> > backtrace from such a call to buffer-list-update-hook, it will be easy
> > to say whether this is expected or not.
> 
> Is there a simple way to dump a backtrace from Emacs?
> 
> It's easy to reproduce tough:
> 
> (defun foo ()
>   (message "buffer-list-update-hook"))
> 
> (add-hook 'buffer-list-update-hook 'foo)
> 
> then simply click in the selected window.
> 
> Anyway according to the documentation this hook is called by
> `select-window` which is actually called if you click on a window,
> even if it is the selected one.

Yes, and the comments in select-window say that it's important it
moves the selected window's buffer to the front of various alists,
which is done by record_buffer.  So I guess this is a feature.  Maybe
we could be smarter, and avoid calling the hook if the buffer is
already at the front of those alists.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Zoom: a window management minor mode -- best practices and questions
  2018-05-07 12:32         ` Andrea Cardaci
  2018-05-07 18:19           ` Eli Zaretskii
@ 2018-05-09  7:00           ` martin rudalics
  2018-05-09 16:06             ` Andrea Cardaci
  1 sibling, 1 reply; 23+ messages in thread
From: martin rudalics @ 2018-05-09  7:00 UTC (permalink / raw)
  To: Andrea Cardaci; +Cc: Eli Zaretskii, emacs-devel

 > There's one problem with `buffer-list-update-hook` though, it gets
 > called (multiple times) even wen the buffer list is not changed, e.g.,
 > simply by clicking in the buffer. Is this the expected behaviour?

Yes and no.  Some users accumulate several hundred entries in their
buffer lists.  For them, checking the buffer list every time a buffer
is recorded could create an unnecessary performance burden.  So it's
expected that the function on the hook remembers the old buffer list
and compares it against the new one if this is of any importance (I
suppose that in many cases checking the head of the list is all that's
needed).

 > Besides this, if there's no way to get rid of false positives in event
 > handling (i.e., a relayout is triggered but no actual change happened)
 > I guess I need a way to decide if a relayout is *really* needed. A
 > naive way would be saving the current window and buffer but I'm not
 > sure if we can do better.

For example 'switch-to-buffer' does

(select-window (selected-window)))

in order to record the buffer it displays to make sure that a "false
positive" is made when the selected window already displays the
buffer.  So saving the selected window and the buffer it displays is
the best thing one can do.

The Elisp manual mentions this as follows:

    However, when its NORECORD argument is `nil', `select-window'
updates the buffer list and thus indirectly runs the normal hook
`buffer-list-update-hook' (*note Buffer List::).  Consequently, that
hook provides a reasonable way to run a function whenever a window gets
selected more "permanently".

    Since `buffer-list-update-hook' is also run by functions that are
not related to window management, it will usually make sense to save the
value of the selected window somewhere and compare it with the value of
`selected-window' while running that hook.  Also, to avoid false
positives when using `buffer-list-update-hook', it is good practice
that every `select-window' call supposed to select a window only
temporarily passes a non-`nil' NORECORD argument.  If possible, the
macro `with-selected-window' (see below) should be used in such cases.

martin



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Zoom: a window management minor mode -- best practices and questions
  2018-05-08 14:53               ` Noam Postavsky
  2018-05-08 15:03                 ` Andrea Cardaci
@ 2018-05-09 12:33                 ` Stefan Monnier
  1 sibling, 0 replies; 23+ messages in thread
From: Stefan Monnier @ 2018-05-09 12:33 UTC (permalink / raw)
  To: emacs-devel

>> (add-hook 'pre-redisplay-function 'foo)
>
> `add-hook' is for variables which are lists of functions, for
> pre-redisplay-function which holds a single function, you should use
> add-function:
>
> (add-function :after pre-redisplay-function #'foo)

Indeed.
And there's also pre-redisplay-functions which you can use with add-hook
and is slightly simpler to use.


        Stefan




^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Zoom: a window management minor mode -- best practices and questions
  2018-05-09  7:00           ` martin rudalics
@ 2018-05-09 16:06             ` Andrea Cardaci
  2018-05-10  6:27               ` martin rudalics
  0 siblings, 1 reply; 23+ messages in thread
From: Andrea Cardaci @ 2018-05-09 16:06 UTC (permalink / raw)
  To: martin rudalics; +Cc: Eli Zaretskii, Emacs developers

> So saving the selected window and the buffer it displays is
> the best thing one can do.

I'm not sure this is enough, it would not catch the case where the selected
window/buffer doesn't change but others do.

I'm experimenting with the following expression to be used as a guard (I
need `track-mouse` to properly delay the update during a mouse selection):

       (format "%s%s" (window-list) track-mouse)

In this solution I simply hook `pre-redisplay-function` and use the above
to avoid unnecessary relayouts. It seems to work so far...



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Zoom: a window management minor mode -- best practices and questions
  2018-05-09 16:06             ` Andrea Cardaci
@ 2018-05-10  6:27               ` martin rudalics
  2018-05-10 10:11                 ` Andrea Cardaci
  0 siblings, 1 reply; 23+ messages in thread
From: martin rudalics @ 2018-05-10  6:27 UTC (permalink / raw)
  To: Andrea Cardaci; +Cc: Eli Zaretskii, Emacs developers

 > I'm not sure this is enough, it would not catch the case where the selected
 > window/buffer doesn't change but others do.

But 'set-window-buffer' calls both 'buffer-list-update-hook' and
'window-configuration-change-hook' so what are you missing?

martin



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Zoom: a window management minor mode -- best practices and questions
  2018-05-10  6:27               ` martin rudalics
@ 2018-05-10 10:11                 ` Andrea Cardaci
  2018-05-10 10:27                   ` martin rudalics
  0 siblings, 1 reply; 23+ messages in thread
From: Andrea Cardaci @ 2018-05-10 10:11 UTC (permalink / raw)
  To: martin rudalics; +Cc: Eli Zaretskii, Emacs developers

> But 'set-window-buffer' calls both 'buffer-list-update-hook' and
> 'window-configuration-change-hook' so what are you missing?

I hook `pre-redisplay-function` only and perform the update only when the
following expression changes:

      (format "%s%s" (window-list) track-mouse)

So any event that causes `pre-redisplay-function` to be called but doesn't
change the expression won't trigger the update.

If I simply save the selected window/buffer then, for example, a simple
window split (`C-x 2`) is not recognized because it does cause a redisplay
but it doesn't change the selected window/buffer.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Zoom: a window management minor mode -- best practices and questions
  2018-05-10 10:11                 ` Andrea Cardaci
@ 2018-05-10 10:27                   ` martin rudalics
  2018-05-10 10:34                     ` Andrea Cardaci
  0 siblings, 1 reply; 23+ messages in thread
From: martin rudalics @ 2018-05-10 10:27 UTC (permalink / raw)
  To: Andrea Cardaci; +Cc: Eli Zaretskii, Emacs developers

 > I hook `pre-redisplay-function` only and perform the update only when the
 > following expression changes:
 >
 >        (format "%s%s" (window-list) track-mouse)
 >
 > So any event that causes `pre-redisplay-function` to be called but doesn't
 > change the expression won't trigger the update.

This should work although I suppose (cons track-mouse (window-list))
would do as well.

martin



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Zoom: a window management minor mode -- best practices and questions
  2018-05-10 10:27                   ` martin rudalics
@ 2018-05-10 10:34                     ` Andrea Cardaci
  2018-05-10 10:37                       ` martin rudalics
  0 siblings, 1 reply; 23+ messages in thread
From: Andrea Cardaci @ 2018-05-10 10:34 UTC (permalink / raw)
  To: martin rudalics; +Cc: Eli Zaretskii, Emacs developers

> This should work although I suppose (cons track-mouse (window-list))
> would do as well.

It doesn't because keeping just the window object doesn't retain the
visited buffer at the time:

     (setq x (selected-window))
     (switch-to-buffer "foo")
     (setq y (selected-window))
     (equal x y) ; t

So formatting is IMHO a nice workaround to storing also the buffer name.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Zoom: a window management minor mode -- best practices and questions
  2018-05-10 10:34                     ` Andrea Cardaci
@ 2018-05-10 10:37                       ` martin rudalics
  0 siblings, 0 replies; 23+ messages in thread
From: martin rudalics @ 2018-05-10 10:37 UTC (permalink / raw)
  To: Andrea Cardaci; +Cc: Eli Zaretskii, Emacs developers

 > It doesn't because keeping just the window object doesn't retain the
 > visited buffer at the time:
 >
 >       (setq x (selected-window))
 >       (switch-to-buffer "foo")
 >       (setq y (selected-window))
 >       (equal x y) ; t
 >
 > So formatting is IMHO a nice workaround to storing also the buffer name.

You're right.

martin



^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2018-05-10 10:37 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-05-02 16:31 Zoom: a window management minor mode -- best practices and questions Andrea Cardaci
2018-05-02 17:32 ` Eli Zaretskii
2018-05-02 18:41   ` Andrea Cardaci
2018-05-02 18:58     ` Eli Zaretskii
2018-05-03  7:11       ` martin rudalics
2018-05-03  9:50         ` Andrea Cardaci
2018-05-03  9:46       ` Andrea Cardaci
2018-05-03  7:11     ` martin rudalics
2018-05-03  9:47       ` Andrea Cardaci
2018-05-07 12:32         ` Andrea Cardaci
2018-05-07 18:19           ` Eli Zaretskii
2018-05-08 10:40             ` Andrea Cardaci
2018-05-08 14:53               ` Noam Postavsky
2018-05-08 15:03                 ` Andrea Cardaci
2018-05-09 12:33                 ` Stefan Monnier
2018-05-08 17:52               ` Eli Zaretskii
2018-05-09  7:00           ` martin rudalics
2018-05-09 16:06             ` Andrea Cardaci
2018-05-10  6:27               ` martin rudalics
2018-05-10 10:11                 ` Andrea Cardaci
2018-05-10 10:27                   ` martin rudalics
2018-05-10 10:34                     ` Andrea Cardaci
2018-05-10 10:37                       ` martin rudalics

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).