spacemacs/default-pop-shell conflict with window-purpose

[deb0ch]

Description

When having a window layout with one edit purposed window and a general purposed window side by side, SPC ' (spacemacs/default-pop-shell) has a strange behaviour when used from the edit window.

@bmag, I feel like this one is for you 😸

Reproduction guide 🪲

• Start Emacs
• open a code buffer and its corresponding magit-status buffer:
  
• select the code window
• SPC ' (spacemacs/default-pop-shell)

Observed behaviour: 👀 💔

Expected behaviour: ❤️ 😄

Note that this does not happen when the magit window is selected.

System Info 💻

• OS: gnu/linux
• Emacs: 24.5.1
• Spacemacs: 0.200.7
• Spacemacs branch: develop (rev. dc36181)
• Graphic display: t
• Distribution: spacemacs
• Editing style: hybrid
• Completion: helm
• Layers:

((auto-completion :variables :disabled-for markdown org erc)
 (better-defaults :variables better-defaults-move-to-beginning-of-code-first nil better-defaults-move-to-end-of-code-first t)
 (c-c++ :variables c-c++-default-mode-for-headers 'c++-mode c-c++-enable-clang-support nil)
 (colors :variables colors-enable-nyan-cat-progress-bar nil)
 common-lisp csv emacs-lisp
 (erc :packages
      (not erc-social-graph)
      :variables erc-server-list
      '(("jinan.parrot.biz" :ssl t :port "7000" :nick "ThomdB" :full-name "Thomas de Beauchene")))
 git github gtags helm markdown multiple-cursors org pdf-tools
 (python :variables python-enable-yapf-format-on-save t python-sort-imports-on-save t)
 ranger
 (shell :variables shell-default-height 50 shell-default-position 'bottom)
 shell-scripts spacemacs-layouts
 (syntax-checking :variables syntax-checking-use-original-bitmaps nil)
 themes-megapack version-control vim-empty-lines xkcd ycmd)

[bmag]

Thanks for the report, this is caused by a bug in shell-pop and Emacs 25.1. It was already reported upstream kyagi/shell-pop-el#51 and also here #7691. The reports are from two months ago, but no response from upstream yet.

[deb0ch]

Damn, two month 🙀

Note that I'm running 24.5.1, so the bug is not only in Emacs 25.1 👀

[bmag]

Since you're running 24.5 it might be a different bug so I'm reopening the issue. However, I'm unable to reproduce it on Emacs 24.5...

[deb0ch]

By the way, I just tested on master and it doesn't happen. It doesn't happen either on develop when I deactivate purpose-mode from M-x.

It also happens if you replace the magit buffer with a *Messages* buffer, also on general purpose.

This is why I think it's related to purpose.

[deb0ch]

Hey hey, just pinging in. Have you had time to investigate what's going on here ? 😸

[bmag]

nope

[deb0ch]

I just got a better understanding of this issue. I just realised that I saw it fixed one day automagically and that it was because we started assigning the purpose terminal to terminals, instead of general.

What happened here is that pop-shell was trying to pop a new terminal but its buffer had a purpose of general so it was sent to the magit window. That most likely confused shell-pop in some way that it created a new window with no buffer, and opened this window from the wrong place hence the weird window configuration.

I just reproduced this bug today as I was working on integrating vterm terminal to Spacemacs (#11552).

[bmag]

I figured the technical details causing the bug. It breaks into a few parts.

General flow of shell-pop command:

• (1) if terminal buffer doesn't exist:
  • (1.1) create a window for the buffer and select the window (buffer doesn't exist yet)
  • (1.2) call a function to create and display the buffer. The function is assumed to display the buffer in the selected window. The function is configurable via shell-pop-shell-type and defaults to just calling the function shell.
  • (1.3) rename the shell buffer. By default renames to "*shell-1*" or "*shell*-1" or something like that (the number can be greater than 1).
• (2) if terminal buffer exists but isn't shown:
  • (2.1) create a window for the buffer and select the window (buffer isn't shown yet)
  • (2.2) display the buffer with switch-to-buffer

Starting with Emacs 25.1, the shell function changed its behavior to display the shell buffer in a new/other window, instead of in the selected window. This breaks the assumption in (1.2), and shell-pop upstream hasn't fixed the issue yet. This is the source of the upsrteam issue linked above. This bug used to affect Spacemacs until some time ago, but doesn't anymore.

In Spacemacs (develop branch) we use different shell-pop-shell-type for each of our custom shell-pop commands (SPC a s e/i/m/T/t). For shell buffers we now use inferior-shell instead of the shell-pop's default shell function. What inferior-shell does is first (switch-to-buffer "*shell*") which creates and displays the buffer, and then it calls (shell "*shell*") which sets up the buffer as a shell buffer (and doesn't affect the buffer's display because its already displayed).

Now how window-purpose factors into this? It affects where the buffer is displayed in steps (1.2) and (2.2). Usually the selected window after (1.1) or (2.1) has an edit purpose, and usually switch-to-buffer is the function called to display the shell buffer, and usually the shell buffer has a terminal purpose. By default, window-purpose will look for an existing window with a terminal purpose, and if it finds one that is where it will display the shell buffer, which is not what we want. However, if it doesn't find such a window, it will switch-to-buffer display the shell buffer in the selected window, which is indeed what we want. So, to summarize, if you have some normal shell buffer opened in some window (not as a shell-pop buffer), and call SPC a s i (spacemacs/shell-pop-inferior-shell), it will create a new window in the side, but display the shell-pop buffer in the previously existing shell window instead of the new window.

However, in Spacemacs we have the dotspacemacs-switch-to-buffer-prefers-purpose variable, which changes how window-purpose alters switch-to-buffer behavior. When the value is t we get default window-purpose behavior and the above-mentioned bug does occur. However, when the variable is set to nil, switch-to-buffer will try to display the buffer in the selected window before looking for another window with a matching purpose. The default value is nil, which means this bug shouldn't appear anymore in a default Spacemacs installation (develop branch).

To further complicate things, some functions such as shell will display a buffer before setting its major-mode, which could change its purpose (as seen by window-purpose). This means in the display phase the buffer could have a different purpose than intended. By default, window-purpose give the terminal purpose to buffers whose major mode derives from comint-mode, eshell-mode or term-mode, and to buffers whose name equals "*shell*" (this is literal, not a regexp). This might have been a problem with SPC a s i before we implemented spacemacs/shell-pop-inferior-shell, but now it isn't anymore.

I haven't looked at the code for SPC a s e/m/T/t (the eshell, multi-term, term and ansi-term shell-pop commands), but I suspect they behave the same.

@deb0ch the above should explain the automagic fix and how exactly shell-pop was confused. The bottom line is that we still have a bug to fix when dotspacemacs-switch-to-buffer-prefers-purpose is t (maybe wrap shell-pop-up in a without-purpose via advice).

I didn't review vterm yet, but am planning to do so.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Please let us know if this issue is still valid!