alexmurray / flycheck-posframe

Show flycheck errors via posframe.el
60 stars 9 forks source link

Posframe lingers even when current buffer is changed #21

Closed anonimitoraf closed 3 years ago

anonimitoraf commented 3 years ago

I've noticed that a posframe lingers when I switch buffers

Steps to reproduce:

anonimitoraf commented 3 years ago

Is this just a matter of hiding any shown posframes in window-configuration-change-hook?

alexmurray commented 3 years ago

We already hide it in window-configuration-change-hook - see https://github.com/alexmurray/flycheck-posframe/blob/master/flycheck-posframe.el#L143 and https://github.com/alexmurray/flycheck-posframe/blob/master/flycheck-posframe.el#L228 and https://github.com/alexmurray/flycheck-posframe/blob/master/flycheck-posframe.el#L161

Sometimes I can reproduce this too though but I am not sure how it is occurring at the moment... perhaps we need to just unconditionally add flycheck-posframe-hide-posframe to window-configuration-change-hook when flycheck-posframe is enabled rather than doing it only when showing the posframe...

anonimitoraf commented 3 years ago

Ohhh, I see. Just to put it out there, I've only started to use Emacs, but this feels to me like it might be a race condition? I'll see if I can debug later

anonimitoraf commented 3 years ago

I found out that changing: (add-hook hook #'flycheck-posframe-hide-posframe nil t) to (add-hook hook #'flycheck-posframe-hide-posframe nil nil) seems to fix the issue.

I read about that arg here: https://www.gnu.org/software/emacs/manual/html_node/elisp/Setting-Hooks.html

If local is non-nil, that says to add function to the buffer-local hook list instead of to the global hook list. This makes the hook buffer-local and adds t to the buffer-local value. The latter acts as a flag to run the hook functions in the default value as well as in the local value.

Are there downsides to adding this hook to the global hook list?

flatwhatson commented 3 years ago

It seems that window-configuration-change-hook runs buffer-local hooks for the buffer you've switched to, but not the buffer you've switched from. This means that flycheck-posframe-hide-posframe doesn't get called until you switch back to the buffer which was showing the posframe!

I think that @anonimitoraf's solution of using the global window-configuration-change-hook is reasonable.

anonimitoraf commented 3 years ago

Actually, in that case, the counterpart remove-hooks should target global too, i.e.:

    (remove-hook hook #'flycheck-posframe-hide-posframe t)
    (remove-hook hook #'flycheck-posframe-maybe-hide-posframe t)

from t -> nil

Alternatively, now that you bring up this point:

It seems that window-configuration-change-hook runs buffer-local hooks for the buffer you've switched to, but not the buffer you've switched from

Are there window-*-hooks that get applied on the buffer you switch from?

flatwhatson commented 3 years ago

Actually, in that case, the counterpart remove-hooks should target global too

Yep!

Are there window-*-hooks that get applied on the buffer you switch from?

You can read about the hooks for window events in the manual. Whether the window configuration hooks are run before or after a buffer has been switched isn't exactly clear, but it does mention that they run during redisplay, and it makes sense that redisplay would occur after switching buffer and not before.

I really think that the global window-configuration-change-hook is the correct choice here to hide the posframe on the next window-modifying event.

alexmurray commented 3 years ago

This should be fixed now (thanks @anonimitoraf) - I will close this for now but feel free to reopen if it you see this again.