Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

spacemac-layouts: In non default layouts popping buffers is behaving wrong #4956

Closed
shivamkalra opened this issue Feb 4, 2016 · 22 comments
Closed

Comments

@shivamkalra
Copy link

Description

I'm using spacemac-layouts and in all the non Default layout, behavior of popping buffers is wrong.

Reproduction guide

  • Start Emacs
  • Create new layout by pressing SPC-l 2
  • In new layout, open any file (make sure it is added to current layout SPC-l b)
  • Now open dired, navigate to some sub directories
  • From a sub-directory press q
  • You would expect to go back to parent directory but you get to file buffer instead.

Observed behaviour:
Since dired is not part of current layout, on quitting the buffer, it shows me the last buffer on spacemacs-layouts buffer stack instead of global buffer stack.

This is making dired almost impossible to use, I can only navigate parent directory by pressing ^. Same happens for various Emacs plugins that create buffers with * in front, they are not added to layouts and their history is not tracked.

Expected behaviour:
On existing the current buffer, it must go back to previous buffer I was on.

It must add all the buffer opened in current layout no matter whether it is dired, mu4e or * in front of it. However, on saving them to file it can write exclusively.

System Info

  • OS: gnu/linux
  • Emacs: 24.5.1
  • Spacemacs: 0.105.9
  • Spacemacs branch: nil (rev. a739e1f)
  • Distribution: spacemacs
  • Editing style: hybrid
  • Completion: helm
  • Layers:
((auto-completion :variables auto-completion-enable-help-tooltip t)
 better-defaults spacemacs-helm emacs-lisp gtags c-c   version-control git markdown org gnus shell syntax-checking csharp sql csharp lua elpy elfeed xkcd haskell extra-langs spell-checking ipython-notebook
 (erc :variables erc-sasl-server-regexp-list
      '("irc\.freenode\.net"))
 shell-scripts theming javascript html colors ibuffer smex github themes-megapack fasd wakatime notmuch mu4e tools
 (latex :variables latex-enable-auto-fill t))

Annoying Behavior
optimised

@robbyoconnor
Copy link
Contributor

Are you using develop or master?

@shivamkalra
Copy link
Author

develop

@Andre0991
Copy link
Contributor

I think the problem here is that spacemacs-layouts doesn't add all types of buffers to the current layout.

See #4590 (comment) and CestDiego's answer.

I think persp.el author created his package in this way intentionally, but it doesn't work very well for our use case.

@shivamkalra
Copy link
Author

@Andre0991 Correct. Default layout does not exhibit this behavior because it adds all the buffers. So, where should dired, mu4e, ERC be used? Only in Default layout?

@Andre0991
Copy link
Contributor

Well, I think we have to live with that for now.

The ERC layer has a workaround:

      (add-hook 'erc-mode #'(lambda ()
                              (persp-add-buffer (current-buffer))))

But of course this approach is ad-hoc and doesn't scale well. We have to add every
created buffer to the current layout rather than defining a rule based
on modes or names.

I think this is what CestDiego wants too. At first it seems that we
just would have to add a rule to associate new buffers with the current
layout with a hook but I'm sure it's not simple actually (otherwise it
would already be done now.)

So basically you'll have to use SPC B b a lot until this is fixed.
And you can try to fix it too, of course :)

User 104 writes:

@Andre0991 Correct. Default layout does not exhibit this behavior because it adds all the buffer. So, where should dired, mu4e, ERC be used? Only in Default buffer?


Reply to this email directly or view it on GitHub:
#4956 (comment)

Att.
André Peric Tavares

@AlejandroCatalina
Copy link
Contributor

That tweak is also in RCIRC layer. We should find a way to do this right.

@shivamkalra
Copy link
Author

Is the problem with persp-mode or spacemacs-layouts layer? I'm sure, it was working some 1 or 2 months ago.

@bmag
Copy link
Collaborator

bmag commented Feb 4, 2016

@shivamkalra The behavior originates from persp-mode, as I've managed to reproduce it with persp-mode in bare emacs (emacs -Q -l /path/to/persp-mode.el).

persp-mode hooks into find-file-hook (and some other hooks also), but the command dired doesn't use find-file, that's why opening a dired buffer via SPC a d doesn't add it to the current perspective. On the other hand, opening a dired buffer via SPC f f (by choosing a directory) does add it to the current perspective.

So yeah, we need to find some solution.

@Andre0991
Copy link
Contributor

Surprisingly, seems that there's no hook for buffer creation: http://stackoverflow.com/questions/7899949/is-there-an-emacs-hook-that-runs-after-every-buffer-is-created

One guy suggested advising get-buffer-create, but he said that "The
advice hook will only see calls from elisp, not calls from the Emacs C
core parts".

Does anyone know examples of those C calls? I know there are functions
in C in Emacs but I don't get this in the context of creating
buffers.

He also suggests after-change-major-mode-hook, which indeed would work in most cases, I think. But it would still be an ugly "solution".

Bar writes:

@shivamkalra The behavior originates from persp-mode, as I've managed to reproduce it with persp-mode in bare emacs (emacs -Q -l /path/to/persp-mode.el).

persp-mode hooks into find-file-hook (and some other hooks also), but the command dired doesn't use find-file, that's why opening a dired buffer via SPC a d doesn't add it to the current perspective. On the other hand, opening a dired buffer via SPC f f (by choosing a directory) does add it to the current perspective.

So yeah, we need to find some solution.


Reply to this email directly or view it on GitHub:
#4956 (comment)

Att.
André Peric Tavares

@shivamkalra
Copy link
Author

I partially solved the issue by following @Bad_Ptr's suggestions on:

Bad-ptr/persp-mode.el#27

(setq persp-set-frame-buffer-predicate nil)
(setq persp-when-kill-switch-to-buffer-in-perspective nil)

@Andre0991
Copy link
Contributor

@shishkin 👍

What cases are not covered by this solution?

@shivamkalra
Copy link
Author

@Andre0991 Solutions solves the original problem I described, however it does not solve following:

  1. It does not really add buffers to the layout's stack. (SPC-l b does not show dired buffers)
  2. C-x Left and C-x Right follows global buffer stack history instead per layout buffer. (I'm not sure if there is any way around that).

Otherwise, it works fine for me way better than before. We can close this issue by adding above two lines in spacemac-layouts layer.

@bmag
Copy link
Collaborator

bmag commented Feb 6, 2016

One guy suggested advising get-buffer-create, but he said that "The advice hook will only see calls from elisp, not calls from the Emacs C core parts". Does anyone know examples of those C calls? I know there are functions in C in Emacs but I don't get this in the context of creating buffers.

@Andre0991 I searched the sources of Emacs 24.5 for get-buffer-create, Fget_buffer_create, etc. and I found that there are several locations where get-buffer-create is called. Most of them seem irrelevant for user-created buffers, but among them is also start-process. I'd expect start-process to be used for several types of user-created buffers, such as terminal buffers (shell, isql, python interpreter, etc.) and some helm buffers (e.g. helm-ag).
However, when I tested, both M-x shell and M-x python-start-or-switch-repl triggered an advice that I added to get-buffer-create. It seems that advising get-buffer-create could work for our purposes, but I'm wary of advising such a low-level function (and generally advice should be avoided when possible).
For the record, this is the advice I used:

(defun hello (&rest args)
  (message "Hello: %S" args))

(advice-add #'get-buffer-create :after #'hello)

He also suggests after-change-major-mode-hook, which indeed would work in most cases, I think. But it would still be an ugly "solution".

It's possible, but also not perfect. Of the top of my head I can think of two scenarios that a solution with after-change-major-hook needs to take into account:

  • switching between major modes in the same buffer is a valid work flow for files that mix several programming languages. Granted, this isn't a problem if the buffer is already part of the layout, but can be problematic if the buffer was intentionally displayed without adding it the current layout (there's a command for this).
  • using the same buffer in two layouts. For example, say you opened a directory in one layout, and later happen to open the same directory in a different layout. Because of buffer isolation, I don't know what happens in this situation, but if Emacs reuses the buffer from the other layout then I doubt after-change-major-mode-hook is called.

As I haven't tested these two scenarios, it's possible that I'm totally wrong here and these are non-issues.

I'd like to suggest a different approach, involving buffer-list-update-hook. This hook is called by several functions, among them are get-buffer-create and select-window. On the plus side, it will catch anything that an advice for get-buffer-create will catch, and it will also catch a buffer that is used in two layouts (the 2nd scenario I mentioned above).
On the minus side, I estimate the implementation would be complicated. We'll need to keep track of the open buffers, so we can distinguish events and know when a new buffer is used. We'll need to filter out temporary buffers and other "background" buffers. We'll need not to know when not to add a buffer (e.g. when the invoked command is persp-temporarily-display-buffer).
That said, some down-sides I mentioned are also applicable to the other two approaches, so I guess it'll be complicated anyway 😆

@bmag
Copy link
Collaborator

bmag commented Feb 6, 2016

Important: I did not see the comment upstream before writing my previous comment. It appears that the latest persp-mode handles the original issue now, at least partially. However, the latest persp-mode won't be available to us Spacemacs users until #4946 is merged (you can merge it locally, of course).

@Andre0991
Copy link
Contributor

@bmag I think what you described is the way to go then, as we would have the same issues by just advising the other functions. Actually, even if Emacs had a hook for buffer creation we would still have to handle those temporary/uninteresting buffers, I guess. By the way, what are some examples of them? Would we have to filter them by name?

@Bad-ptr might be interested in this as he wants to solve the same problem, I think.

@bmag
Copy link
Collaborator

bmag commented Feb 6, 2016

Evaluate what I posted in your scratch buffer:

(defun hello (&rest args)
  (message "Hello: %S" args))

(advice-add #'get-buffer-create :after #'hello)

And after a while you'll see in the *Messages* buffer what I'm talking about.
Yes, we would have to filter them by name, unless there's some other useful parameter I don't know about.

@Andre0991
Copy link
Contributor

@bmag
Right, I see.

At least most of them already use a blank space. ("*LV*") is an exception that I got here.

But wait - we could just use same rules that list-buffers uses to filter them, no? I think that this would cover all cases (ie, any buffer that shows up on list-buffers would get in the layout).

@Andre0991
Copy link
Contributor

The list-buffers docstring is

"Display a list of existing buffers.
The list is displayed in a buffer named "Buffer List".
See `buffer-menu' for a description of the Buffer Menu.
By default, all buffers are listed except those whose names start
with a space (which are for internal use). With prefix argument
ARG, show only buffers that are visiting files."

So it only ignores buffers that start with \s (= uninteresting buffers).

I think the temporary ones don't show up on the list-buffers because disappear almost immediately. But I feel I'm missing something obvious here.

@Andre0991
Copy link
Contributor

Ping @CestDiego

Also see Bad-ptr/persp-mode.el#27 (comment) - there's an option to automatically add buffers to perspectives now.

@d12frosted
Copy link
Collaborator

Closing issue for being inactive. Also it has a proposed solution 😸 If you still wish for it to be open, let me know 😸

CC @bmag 😸

@TheKashe
Copy link

TheKashe commented Jul 5, 2018

Notmuch is affected as well

@ldionmarcil
Copy link

Was experiencing this and found this explanation:

This is because your recent files are part of another layout. They can only be part of one layout at a time. SPC b Tab only switches between buffers open in the current layout. Use SPC l a to add them to the current layout and this won't happen anymore. And yes, I agree this can be confusing.

So, after switching to a second layout, if that dired buffer was already opened in the initial layout, you must switch it to this layout (with SPC l a). Kind of annoying, but now I understand why.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants