emacs-ess / ESS

Emacs Speaks Statistics: ESS
https://ess.r-project.org/
GNU General Public License v3.0
622 stars 162 forks source link

Unwanted buffer behaviour in case of R error #823

Closed prosoitos closed 5 years ago

prosoitos commented 5 years ago

About 3ish months ago, my buffers and windows settings started to display a weird behaviour whenever R shows an error message:

I usually have 2 windows side by side, one with an ESS buffer for my R script and one with an iESS process and I send commands from the script to the inferior process with ess-eval-paragraph-and-step or ess-eval-line-and-step.

But now (started about 3+/- months ago), each time there is an R error, the ESS buffer gets replaced by the iESS buffer in its window (so that I end up with 2 iESS buffers side by side). My ESS buffer doesn't get killed and I can call it back into its window easily.

Also, for some types of R errors, things get even more crazy: once I recall my ESS buffer into its window, it is empty. I can restore its content simply with undo.

Finally, for some R errors, things get totally messed-up in the iESS buffer (the command which created the R error is still at the prompt, the point is further down and pressing Enter has no effect). Sometimes running undo in the iESS buffer saves it (usually several undo are necessary until I return to an empty prompt and then things are good again from there) and sometimes I have to kill the buffer altogether.

Things were working totally fine before and I did not change any setting when this started. Since it has now been a few months, nobody complained about this and the behaviour is not going back to normal, it has to be something in my settings... I would appreciate some help to figure out what it may be.

Of note, when I don't run into an R error, everything works normally in my ESS and iESS buffers.

Some info on my setup:

OS: Arch Linux (4.20.4-arch1-1-ARCH) Window Manager: EXWM (with Xorg) Emacs: GNU Emacs 26.1 ESS: latest Melpa version R: version 3.5.2 (2018-12-20)

Thanks!

jabranham commented 5 years ago

There was a change to how ESS displays buffers, but it shouldn't result in the behavior you're describing. Unfortunately, I don't experience this behavior. Is there a way to reproduce it reliably on your machine? If so, can you share that?

Do you have tracebug on? (it's on by default)

prosoitos commented 5 years ago

The behaviours I described above are very reliable and happen every single time I hit an R error.

I am not sure about tracebug, but if it is on by default, I suspect so.

prosoitos commented 5 years ago

Extremely simple situation:

I have my ESS buffer on the left window and the iESS buffer (fresh session) on the right. If I go to the iESS buffer and call a non existing object (say a for instance), the window expands to now occupy the full frame, and R sort of freezes like this:

image

I say "sort of freezes" because the cursor is still blinking and while pressing Enter doesn't work, I can save it by pressing undo until the faulty command gets deleted. From there on, things are normal again (except that I have to reset my windows of course).

prosoitos commented 5 years ago

Actually, I lied: in the above case, pressing Enter did work and produced this (often, it doesn't and undo is the only way out, or even, sometimes, killing the iESS buffer altogether):

image

Notice the space below the line with the faulty command and the double prompt on the current line. Getting back to a normal situation takes some Enter or undo presses.

prosoitos commented 5 years ago

In my normal workflow, I send commands from the ESS buffer to the iESS buffer. In this case, this is what happens:

My initial setup:

image

After I send the command a to the iESS, this is what it looks like:

image

This happens totally reliably.

prosoitos commented 5 years ago

As I mentioned, I never had this happen until an update a few months ago. Unfortunately, I do not remember precisely when as I was too busy to report the bug then and I was also hoping that it would get resolved by further updates.

Also, I don't hit R error at every command, so having to reset the windows, while quite frustrating, didn't happen enough to prevent me from working.

I did not change anything on my end when this started to happen.

jabranham commented 5 years ago

Interesting. This definitely shouldn't happen. Do you have any customization for the display-buffer variables?

prosoitos commented 5 years ago

I do. And it is very possible that my settings are a problem. I found the relevant help files quite confusing and I spent a lot of time playing with those settings until I got window and buffer behaviours which worked for me. Eventually they did though, until this thing here fell apart.

This is what I have:

(setq display-buffer-alist
   (quote
    (("magit-diff.*"
      (display-buffer-reuse-window display-buffer-pop-up-window))
     ("\\*magit-.*-popup\\*" display-buffer-below-selected)
     ("\\*helm.*\\*"
      (display-buffer-reuse-window display-buffer-pop-up-window)))))

(setq display-buffer-base-action (quote (display-buffer-same-window)))

(setq display-buffer-reuse-frames t)

(setq display-buffer-function 'popwin:display-buffer)
prosoitos commented 5 years ago

And I have no entry for ESS or iESS buffers in popwin, so that last one shouldn't have any effect.

vspinu commented 5 years ago

Do you have the most recent ESS btw? There was a fix last week that should inhibit coverage of the current window by an iESS buffer.

I think your (setq display-buffer-base-action (quote (display-buffer-same-window))) might be a culprit here. @jabranham will know better, but I think you essentially force all buffers (including iESS) to cover your current buffer. So, when we issue pop-to-buffer in our code you hijack that and essentially do switch-to-buffer instead.

jabranham commented 5 years ago

@vspinu is right here. ESS began respecting display-buffer settings in late October. By specifying display-buffer-base-action, you're saying that you want all buffers to display in the same window. If that's really what you want, you can override the behavior for the inferior R buffers by modifying your setting of buffer-display-alist. I think just changing magit-diff so it matches *R buffers might get the behavior you want: "\\(?:\\*R\\|magit-diff\\)". Can you try that and see if it works?

If it doesn't try commenting out all your display-buffer customizations and see if that does the trick.

prosoitos commented 5 years ago

Ah! Late October sounds about right :slightly_smiling_face: So I guess that is what it is indeed. And that would explain why my settings worked before.

Funny: it is the second time I have a poor setting somewhere that was initially fine because ESS was not respecting it, then created issue when ESS started to respect it. Because of this, both times, I failed to pinpoint the source of the problem because I had set the particular setting many months before it started causing issues :slightly_smiling_face:

@vspinu: I have the latest Melpa version. But I don't know whether new commits on Emacs packages which are on Melpa are pushed directly to the stable version or whether there is a development version, with only stable releases pushed to Melpa (with R packages and CRAN, all this is very clear to me. I know how to check a package version, how to install the devel version, etc. But the equivalent for Emacs packages and Melpa is confusing to me. I don't even know how to install a development version of an Emacs package, or a specific version, nor how to check the version of an installed package :confused: I guess I better do some homework and learn about this...).

Anyway, will try and let you know.

Thank you for your time!! Greatly appreciated! (and thank you for your work on ESS!).

jabranham commented 5 years ago

@prosoitos Good to hear that it's all working out now. ESS is slowly becoming a more "modern" Emacs package, which means it should start to respect global configuration options like the display-buffer stuff.

MELPA serves the latest git commit from a package (with about a 4 to 8 hour lag because it takes the server some time to build). MELPA-stable is a separate package archive that serves the latest git tag (e.g. more like how CRAN works), but some Emacs packages don't have git tags so they aren't on it.

I'm closing this issue since I think it's "fixed." You mentioned something above though that evaluating a nonexistant object in R can hang the R process? I didn't totally understand that, if you still experience it please open a new issue.

prosoitos commented 5 years ago

I removed my display-buffer-base-action and that totally solved the problem when I send a command from the ESS buffer to the iESS buffer.

So:

image

if I run ess-eval-paragraph-and-step becomes:

image

as it should.

However, if I call a directly in the iESS buffer, I still have the same weird behaviour I described above:

image

after pressing Enter in the iESS buffer becomes:

image

and if I press Enter again:

image

So it seems that there is something else going on.

prosoitos commented 5 years ago

@jabranham: thank you very much for your explanations about Melpa.

I hadn't tried things yet when I first replied, but I now have (see above).

What still doesn't work (when I trigger an R error message by a command sent from within the iESS buffer) is very much what I was trying to explain: it isn't exactly that R hangs. But as you can see in the first picture, the error message doesn't show, nor does a new prompt. The point goes down and nothing happens in the iESS buffer. If I press Enter a second time, things become messed up. I often have to undo until I remove the faulty R command or relaunch the iESS process.

With this new info, do you want me to open a new issue on this as you suggest above?

jabranham commented 5 years ago

@prosoitos thanks, I understand now. Looks like this is a small bug in ESS.

@vspinu this happens when ess--flush-accumulated-output gets called. The call to display-buffer triggers users display-buffer customizations, which we don't really need/want if the buffer is already selected. What do you think about this patch?

diff --git a/lisp/ess-tracebug.el b/lisp/ess-tracebug.el
index a0380b63..2108fa0d 100644
--- a/lisp/ess-tracebug.el
+++ b/lisp/ess-tracebug.el
@@ -1327,7 +1327,8 @@ prompts."
           (goto-char (point-min))
           (let ((case-fold-search nil))
             (when (re-search-forward "Error\\(:\\| +in\\)" nil t)
-              (display-buffer (process-buffer proc))))
+              (unless (equal (selected-window) (get-buffer-window pbuf))
+                (display-buffer (process-buffer proc)))))
           (goto-char (point-min))
           ;; First long + + in the output mirrors the sent input by the user and
           ;; is unnecessary in nowait case. A single + can be a continuation in
prosoitos commented 5 years ago

Ah! Sorry: I was wrong about popwin having no setting affecting ESS:

As I posted earlier, I have:

(setq display-buffer-function 'popwin:display-buffer)

And I actually have this in popwin:

(setq popwin:special-display-config
   (quote
    (("\\*R:.*\\*" :regexp t :width 82 :stick t))))

This is the only way I have found to have the iESS buffer open in a window on the right of the ESS buffer. This works great, but since I am probably not the only one wanting this window configuration, there may be a more straightforward way within ESS that I totally missed.

jabranham commented 5 years ago

@prosoitos No problem. You shouldn't need popwin anymore (at least, not for ess) now that we use display-buffer. Take a look at our manual (info "(ess) Controlling buffer display") and if there isn't an example there of how you want ESS to manage your buffers, let me know and I'll add one.

prosoitos commented 5 years ago

Oh great! It was easy to adapt one of the examples to reproduce my popwin configuration. Your examples actually helped me understand display-buffer-alist (which I have found not very documented and quite obscure with my current elisp knowledge) a little better.

prosoitos commented 5 years ago

Note: I still use popwin for my *R dired* buffers: I want them to pop-up at the top when I use them, which is rare. I haven't figured out how to do this with display-buffer-alist. But that's fine as popwin does the job very well.

Here is what it looks like when I call an *R dired* buffer:

image

jabranham commented 5 years ago

I think you'd have an entry that looks like this in display-buffer-alist:

("\\`\\*R dired"
 (display-buffer-reuse-window display-buffer-in-side-window)
 (side . top)
 (window-height . 0.50) ; means take up half the frame
 (reusable-frames . t))

let me know if that works for you.

prosoitos commented 5 years ago

Actually, I am running into an issue with setting the iESS buffer with display-buffer-alist:

When I am in my usual ESS setting (ESS buffer left, iESS buffer right) and with the iESS buffer active, calling anything that will create a new window is blocked (so helm-occur, helm-mini, split-window-and-jump, etc.). I can do all of these from the ESS buffer and if the window configuration changes, then I can also do it from the iESS buffer. But not before then.

Since I had no issue with popwin, I may go back to setting it that way.

But your examples have really helped me understand display-buffer-alist and that will be really invaluable for other packages, even if I don't make use of it with ESS.

Here is my new display-buffer-alist with the new R addition.

(setq display-buffer-alist
   (quote
    (("magit-diff.*"
      (display-buffer-reuse-window display-buffer-pop-up-window))
     ("\\*magit-.*-popup\\*" display-buffer-below-selected)
     ("\\*helm.*\\*"
      (display-buffer-reuse-window display-buffer-pop-up-window))
     ("*R"
      (display-buffer-reuse-window display-buffer-in-side-window)
      (side . right)
      (window-width . 0.5)
      (reusable-frames))))

Depending on what I am trying to do, I get those messages:

Cannot split side window or parent side window or Cannot make side window the only window, etc.

Note that I am mentioning this in case it is helpful on your end. But that's totally ok on my side if I don't go to the bottom of this as popwin does the job fine. So do not feel pressured to get back to me on this point unless it is useful for the writing of ESS. And it might be that this problem will not be reproducible if it is caused by some of my popwin or other settings for those new windows I am trying to create. So if you aren't getting those errors, I guess it is not worth worrying about it.

prosoitos commented 5 years ago

let me know if that works for you.

Since I decided to let popwin still handle my ESS windows, I won't try it. But you gave me the solution to my problem: I didn't realize that display-buffer-in-side-window could take a side argument with a value of top. So I knew how to put windows at the "bottom" and on the right and left "side", but not at the top.

Again, I have no doubt that this will be incredibly useful for some other window customization at some point as I have battled with this display-buffer-alist for a long time, with only limited success. So thank you!

prosoitos commented 5 years ago

Totally off topic, but I remember now why I had set (setq display-buffer-base-action (quote (display-buffer-same-window))): it was to prevent dired from opening buffers in a jumping fashion as soon as there is more than one window in a frame. With that line, I could navigate up and down folder trees in dired while staying in the same window, without all this jumping back and forth to other window which I find totally crazy (many complain about the fact that dired keeps opening new buffers, but I don't mind this. What drives me crazy is that it doesn't open them all in the same window if there are more than one window open). I'll have to find another way to achieve that.

Oh, same behaviour also in Custom mode :disappointed: (setq display-buffer-base-action (quote (display-buffer-same-window))) had solved all this jumping around between windows (when there are more than one window open) which I really don't like.

prosoitos commented 5 years ago
(setq same-window-buffer-names
   (quote
    ("(if (eq major-mode 'dired-mode) (buffer-name) nil)")))

has no effect :disappointed:

And all my attempts at adding dired mode buffers to display-buffer-alist to set display-buffer-same-window only for that mode made EXWM crash and gave error "sub-directory xxx has no alist". My Elisp skills are still very limited, so this may be the way to go and I just failed to write it properly.

If I go back to setting (setq display-buffer-base-action (quote (display-buffer-same-window))) because I can't get display-buffer to stop doing all this jumping around in several major modes, is there a way to make an exception to it for ESS? This code does not create any other issue and it may be simpler to void it in ESS than try to force the behaviour of display-buffer elsewhere??

prosoitos commented 5 years ago

edit: Sorry @jabranham: I just realized that you had already answered my last question higher up in this thread.

By specifying display-buffer-base-action, you're saying that you want all buffers to display in the same window. If that's really what you want, you can override the behavior for the inferior R buffers by modifying your setting of buffer-display-alist. I think just changing magit-diff so it matches *R buffers might get the behavior you want: "\\(?:\\*R\\|magit-diff\\)". Can you try that and see if it works?

I tried and it works like a charm. So I went back to (setq display-buffer-base-action (quote (display-buffer-same-window))), which I really prefer for many things and adding (quote (("\\*R" (display-buffer-reuse-window display-buffer-pop-up-window)))) to my display-buffer-alist works perfectly to prevent that behaviour in iESS buffer.

Thank you very much for all your time with this. It was extremely helpful.

prosoitos commented 5 years ago

@jabranham: thank you again sincerely for all your time and help in this thread: popwin is unmaintained and now conflicts with Magit, so I had to migrate all my popwin settings to display-buffer-alist. The information you gave me here were incredibly useful and I would not have managed without them.

So, thank you!!!

jabranham commented 5 years ago

Happy to help!

On April 12, 2019 6:59:12 PM CDT, Marie-Helene Burle notifications@github.com wrote:

@jabranham: thank you again sincerely for all your time and help in this thread: popwin is unmaintained and now conflicts with Magit, so I had to migrate all my popwin settings to display-buffer-alist. The information you gave me here were incredibly useful and I would not have managed without them.

So, thank you!!!

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/emacs-ess/ESS/issues/823#issuecomment-482756323