syl20bnr / spacemacs

A community-driven Emacs distribution - The best editor is neither Emacs nor Vim, it's Emacs *and* Vim!
http://spacemacs.org
GNU General Public License v3.0
23.56k stars 4.9k forks source link

[defaults] Add leader key binding for `other-window-prefix` #16453

Closed fnussbaum closed 1 day ago

fnussbaum commented 1 week ago

This is handy enough to deserve a more prominent binding than the default C-x 4 4.

I'm not sure whether adding the top level leader key binding SPC O like that is controversial, but this command is a lot more convenient on a short binding, because it is always composed with another command. I use it a lot, for example as SPC O g d to jump to definition in another window, or SPC O RET to open a link in another window.

I have checked that SPC O is not used yet by any layer, and only the lowercase SPC o is reserved for user bindings.

fnussbaum commented 1 week ago

It turns out that SPC O g d is not the best example, as there is g D for that already. I still like the consistency of other-window-prefix for all kinds of commands that display some buffer.

kassick commented 1 week ago

Hmmm this made me find a bug upstream ...

purpose-mode sets display-buffer-overriding-action to (purpose--action-function . nil) here -- something valid, as the documentation for clearly states that the car can be either a function or a list of functions.

On the other hand, display-buffer-override-next-command wants to push a new element to display-buffer-overriding-action's car (window.el). Now display-buffer-overriding-action's car is now a cons cell and not a list.

Later, when it wants to delq an element from it it (window.el)... well, that cons cell is not a list.

This causes following window commands to fail with a wrong-type-argument listp ... message until you evaluate (setq display-buffer-overriding-action (cons (list purpose--action-function) nil)) to stop the callback from failing.

Maybe we should have this snippet somewhere until that is fixed...


(advice-add 'display-buffer-override-next-command :before (lambda (&rest _)
                                                            (unless (listp (car display-buffer-overriding-action))
                                                              (setcar display-buffer-overriding-action (list (car display-buffer-overriding-action))))))
fnussbaum commented 1 week ago

Oh nice thanks for catching that and fixing it promptly! Seems like your fix is going to be included in Emacs 30, so I agree that we should include that advice snippet somewhere in Spacemacs for Emacs < 30.1. I think that could be a separate PR @kassick

smile13241324 commented 1 day ago

@fnussbaum your pr looks pretty good, care to remove the draft flag?

fnussbaum commented 1 day ago

Thank you, I will come back to this in a few weeks. I have implemented a more general kind of prefix command in my configuration that optionally allows to display buffers in other workspaces and layouts and would like to propose to upstream that instead, but this is going to take some more time.