gcv / julia-snail

An Emacs development environment for Julia
GNU General Public License v3.0
230 stars 21 forks source link

window arrangement and buffer display problem when calling julia-snail #8

Closed orialb closed 4 years ago

orialb commented 4 years ago

I noticed some glitches with the buffers being displayed after calling julia-snail , after the first call I see that following: image

i.e. the julia-process buffer is displayed instead of the edited julia file buffer.

When I call julia-snail afterwards from the julia file (in a state when the REPL buffer is not currently displayed), only the REPL buffer is displayed instead of the julia file (i.e. the window is not split)

Here is my display-buffer-alist:

(((lambda
    (bufname _action)
    (and
     (string-match-p "\\*julia" bufname)
     (with-current-buffer bufname
       (bound-and-true-p julia-snail-repl-mode))))
  (display-buffer-reuse-window display-buffer-pop-up-window))
 ("\\*julia"
  (display-buffer-reuse-window display-buffer-same-window))
 (popwin:display-buffer-condition popwin:display-buffer-action))
gcv commented 4 years ago

I wonder if this behavior has something to do with popwin. The code does not at any point call pop-to-buffer or display-buffer on the process buffer, so I'm surprised to see it make an appearance.

Try 1: What happens if you change display-buffer-alist to explicitly not display the process buffer?

(add-to-list 'display-buffer-alist
             ;; suppress popping up *julia* process
             '("\\*julia.*process$"
               ;; actions:
               (display-buffer-no-window)))

Try 2: What happens if you put (bury-buffer process-buf) in julia-snail--repl-enable, as the very first thing in the let form which binds process-buf?

orialb commented 4 years ago

Both suggestions don't solve the issue unfortunately. I also tried disabling popwin but it didn't change anything.

orialb commented 4 years ago

I just want to emphasize that this is a minor thing at the moment and I can also try to further debug it by myself in case this happens only for me.

gcv commented 4 years ago

It'd be super-helpful if you could step through the initialization code in the Elisp debugger and try to narrow down what causes the process buffer window to pop up. Please instrument julia-snail--repl-enable for debugging using C-u C-M-x and run julia-snail and step through until the undesired buffer appears.

orialb commented 4 years ago

It seems that it is due to perspective-mode- the code that makes the process buffer appear is in line 229

(when (fboundp 'persp-add-buffer) ; perspective-el support
      (persp-add-buffer process-buf))

at least from what I can gather, by default when persp-add-buffer is called without further arguments it will switch to the buffer that was added, unless a global variable persp-switch-to-added-buffer is set to nil.

The following change in the call to persp-add-buffer seems to prevent from the buffer to appear:

    (when (fboundp 'persp-add-buffer) ; perspective-el support
      (persp-add-buffer process-buf nil '(switchorno nil)))
gcv commented 4 years ago

Oh! I should have known, since you mentioned you use Spacemacs.

Background. I currently maintain (but did not originally author) the Perspective package. That package was forked years ago into persp-mode, with a different package name but the same minor mode name and conflicting namespaces. So persp-add-buffer has a different signature in the fork than in the original. Spacemacs ships with the fork, which I (obviously) don't use.

I fixed the problem on the Snail side by adding support for both. The problem should be fixed in https://github.com/gcv/julia-snail/commit/13ddf77079d07326291a1f9ea87d33521d49537d.

Thanks for helping track this down.

orialb commented 4 years ago

with a different package name but the same minor mode name and conflicting namespaces.

That is indeed a very confusing situation :)

I checked the fix again, it seems that the call to persp-add-buffer I provided above is not the correct one. It still worked when I tested it in a non-default perspective, but for the wrong reason. It should be:

    (when (and (featurep 'persp-mode) (bound-and-true-p persp-mode)) ; persp-mode support
      (declare-function persp-add-buffer "persp-mode.el")
      (persp-add-buffer process-buf (get-current-persp) nil))

(basically I got confused about the correct call signature because the function was defined with cl-defun and I was not aware of its syntax for optional arguments. I'm quite a beginner in (e)lisp).

Sorry for the confusion (I can also do a PR myself if you wish).

gcv commented 4 years ago

Fixed in https://github.com/gcv/julia-snail/commit/a77a99ab70384efc8411865eb6f313211454393b

Great catch, thank you!

orialb commented 4 years ago

Great!

Just one more question regarding buffer display (again I might need to open a new issue, but want to verify if this is an issue). I do the following:

  1. Open a Julia file and run julia-snail for the first time.
  2. Window is split with julia file and the REPL buffer displayed (as expected after the fix): image
  3. I close the REPL window to continue working on the julia file in full screen: image
  4. Later I run julia-snail again because I want to check something in the REPL. What I see is that the source file buffer is replaced by the REPL buffer instead of the window being split: image

Is this the expected behavior?

gcv commented 4 years ago

No, not expected behavior. I think it happens because I called pop-to-buffer-same-window instead of vanilla pop-to-buffer for established REPLs, but my personal configuration has additional tweaks which prevent Emacs from doing what you saw. Not sure why I thought that was a good idea. I pushed https://github.com/gcv/julia-snail/commit/6d8c457ca7a1786221cf9e4863ec1433afacb864 which should fix it. Let me know.

In general, I want window behavior to be easy for users to tweak to match the desired workflow using display-buffer-alist (which, while brutally confusing, is the official Emacs method for dealing with windows), and anything which interferes with a user's desired display-buffer-alist configuration should be considered a bug.

After thinking about this, I made a couple of other changes and updated the documentation. As a result, I removed the recommended display-buffer-alist changes from the use-package invocation. They are much too intrusive. I also updated my explanations of how to configure window behavior in the README.

So: as soon as the latest change shows up in MELPA (probably timestamped 20200317), please make that change to your use-package and remove the Snail-specific display-buffer-alist tweaks. Update julia-snail. Restart Emacs to be absolutely sure the use-package change takes. Then let me know if the window behavior you observe makes sense, including with error and documentation windows.

gcv commented 4 years ago

Looks like all relevant changes are in MELPA now.

orialb commented 4 years ago

thanks for the fix! the julia REPL window display now works nicely as expected (i.e. opens a new window if it does not currently display).

I'll keep playing with the package and let you know if I encounter any other problems with window display.