New R processes are not shown anymore

[frederic-santos]

Hi everyone,

Since the last update, I get Wrong type argument: window-live-p, nil when trying to eval some code with C-c C-c on a source file with no active session.

Pretty easy to reproduce:

1. Open a fresh Emacs session
2. Create a source.R file with the content x <- 2
3. Try to execute it with C-c C-c or whatever.

This works again if the R session is created manually with M-x R.

[frederic-santos]

Well, for me the problem comes from commit 0ee7582 (everything's fine with previous commits).
I think I understand what this commit does, but:

1. With my full ESS setup, I have the issue described above
2. I also tried with a very minimal init.el file, using a setup that comes directly from the ESS manual:

   (require 'ess-site)
   (setq display-buffer-alist
         `(("*R Dired"
            (display-buffer-reuse-window display-buffer-in-side-window)
            (side . right)
            (slot . -1)
            (window-width . 0.33)
            (reusable-frames . nil))
           ("*R"
            (display-buffer-reuse-window display-buffer-at-bottom)
            (window-width . 0.5)
            (reusable-frames . nil))
           ("*Help"
            (display-buffer-reuse-window display-buffer-in-side-window)
            (side . right)
            (slot . 1)
            (window-width . 0.33)
            (reusable-frames . nil))))
   

   This code is supposed to display the *R* buffer at the bottom left. Result in my case: I've no error, but the *R* buffer is not displayed at all (it is only available in the background).

I understand the problem raised (and solved) by commit 0ee7582, but is this really the intended behavior? And in this case, what are the appropriate (new) instructions to customize display-buffer-alist?

[jabranham]

Thanks. I do wonder whether we should just apply the following. It means you couldn't do the normal eg C-c C-c commands without the R process always being shown, but I get the sense pretty much everyone wants it to be shown anyway.

diff --git a/lisp/ess-inf.el b/lisp/ess-inf.el
index f425cc02..4dbcfc7b 100644
--- a/lisp/ess-inf.el
+++ b/lisp/ess-inf.el
@@ -812,7 +812,8 @@ to `ess-completing-read'."
                   (ess--with-no-pop-to-buffer
                     (ess-start-process-specific ess-language ess-dialect))
                   (caar ess-process-name-list))))))
-    (unless noswitch
+    (if noswitch
+        (display-buffer (ess-get-process-buffer proc))
       (pop-to-buffer (ess-get-process-buffer proc)))
     proc))
 

[jabranham]

Alternatively, we could be a little less opinionated and just ensure we show the R buffer if we've started a new process with something like:

diff --git a/lisp/ess-inf.el b/lisp/ess-inf.el
index f425cc02..51553eaf 100644
--- a/lisp/ess-inf.el
+++ b/lisp/ess-inf.el
@@ -812,6 +812,7 @@ to `ess-completing-read'."
                   (ess--with-no-pop-to-buffer
                     (ess-start-process-specific ess-language ess-dialect))
                   (caar ess-process-name-list))))))
+    (when auto-started? (display-buffer (ess-get-process-buffer proc)))
     (unless noswitch
       (pop-to-buffer (ess-get-process-buffer proc)))
     proc))

[frederic-santos]

Alternatively, we could be a little less opinionated and just ensure we show the R buffer if we've started a new process with something like [...]

Thanks, this is okay for me. :-) This fix would do the trick... at least for a first part.

Actually, I found the reason for the bug Wrong type argument: window-live-p, nil, that I still have with my full config. This is actually related to helm, but this is really a different bug (maybe I should open another issue?), although it started with the same commit.

The reproduce the bug, create a file minimal init file init_ess.el with the following content:

(package-initialize)
(eval-when-compile
  (require 'use-package))

(use-package helm
  :ensure t
  :config
  (helm-mode 1))

(use-package ess
  :ensure t
  :init
  (require 'ess-site))

Then, open emacs with emacs -q -l init_ess.el, create an R file with any dumb instruction such as x <- 2, and just try a C-c C-c on that instruction. You'll have the error helm-internal: Wrong type argument: window-live-p, nil. It does not seem to be a bug from helm, since using any ESS version prior to commit 0ee7582 is enough to get rid of the bug.

[dankessler]

I've encountered the helm variant of this issue (Wrong type argument: window-live-p, nil) when trying to e.g., switch to process (C-c C-z from an R-mode buffer) when using both helm and ess. I created a fresh emacs config, added MELPA, installed both helm and ESS, and was able to replicate the error.

My emacs debugging skills are quite crude, but I think I've isolated the issue.

Ultimately, when launching a new R process, we call run-ess-R, which in turn calls inferior-ess, and that then calls inferior-ess--maybe-prompt-startup-directory. If the variable ess-ask-for-ess-directory is non-nil (by default it seems to be t), it will prompts the user for the starting directory. A bunch of further calls lead to read-file-name, and if helm-mode is active, it will do this in helm which requires popping up a brand new buffer to do the helm selection.

However, since the commit discussed above, all of this is running inside of the ess--with-no-pop-to-buffer macro which uses display-buffer-overriding-action so that all internal calls to pop-buffer or display-buffer will not display the buffer and return nil, and the wrong-type error is happening because when helm tries to use display-buffer, nothing happens and it returns nil (which is not what helm-internal was expecting).

Here's a fairly minimal example that illustrates the problem (it doesn't immediately error out, but for some reasons throws me into the debugger and you can't see the prompt pop up).

(require 'ess-site)
(let ((display-buffer-overriding-action '(display-buffer-no-window (allow-no-window . t))))
  (save-current-buffer
    (ess-prompt-for-directory "~/" "Prompt: ")))

If I set the variable ess-ask-for-ess-directory to be nil, I no longer have the problem and can still leave helm mode enabled, which is a decent workaround for now since I can just change the working directory from within R. However, wrapping everything in the macro that prevents display-buffer from functioning will, of course, break anything that needs it to work.

[dankessler]

I have an idea for a way to fix this (without breaking what 0ee7582 was trying to accomplish), but it seems like my specific issue (helm + ESS breaks launching new inferior R shells) is maybe different than what this issue is originally about (i.e., "New R processes are not shown anymore"). Should I open a new issue specifically for helm + ESS and make a PR against that, or work against this existing issue?

[frederic-santos]

Thanks @dankessler !
Also, should I make a PR myself for the changes suggested by @jabranham? (Hoping that both our PRs will be compatible...)

[matogoro]

Hi all, I'm experiencing the same problem and the workaround from @dankessler definitely works for now. If there's a more elegant solution, I would definitely love to hear it! Even if it involves opening a new issue, or implementing a fix for the next version of ESS.

Big thanks to all of you!

[vspinu]

Given that this issue severely interferes with the user flow I am fixing it with the second @jabranham's proposal.

Relevant commits/issues: 21cb413 fixed #987 but introduced two-window issue #1067 which was fixed by 0ee7582 but introduced this issue.

To be frank I lost track of all the nuances that the new display-buffer aware mechanism had to deal with. @jabranham @lionel- would you mind putting together a list of requirements which the current implementation has to conform to? With the purpose of adding a comprehensive set of tests.

[lionel-]

@jabranham @lionel- would you mind putting together a list of requirements which the current implementation has to conform to?

I was planning to if no one else was going to do it before release. That is low in my priority list though. I don't use display-buffer for processes myself.

[lionel-]

The fix in 983c54b might call display-buffer twice in some cases so I've followed-up with 9f81477 which IIUC best implements the behaviour we want.

[bryce-carson]

I am encountering this error while using 4fefd0f.

I encounter the error regardless of the value of the ess-ask-for-directory variable, default (t) or nil.

  (setq ess-ask-for-ess-directory nil) ;if you don't want to be prompted each time you start an interactive R session
  (setq ess-startup-directory "~/")

If I set the variable ess-ask-for-ess-directory to be nil, I no longer have the problem and can still leave helm mode enabled, which is a decent workaround for now since I can just change the working directory from within R. However, wrapping everything in the macro that prevents display-buffer from functioning will, of course, break anything that needs it to work.

As I'm using Spacemacs, here is my .spacemacs, in case any part of my configuration is influencing how Helm or ESS are being loaded and used apart from the ess-ask-for-directory variable.

[dankessler]

@bryce-carson I recently migrated to spacemacs myself, too. I assume what you mean by encountering the error is illustrated by

1. Open an R file (e.g., R.tmp)
2. Attempt to evaluate the buffer with , s b
3. Receive error Wrong type argument: window-live-p, nil in the minibuffer

This happens to me, also. My current workflow is to instead first start the inferior shell with , ' (which does prompt me for the starting directory, presumably because the internals of spacemacs/ess-start-repl bypass the issue we're discussing here) and then subsequently sending things to the REPL works nicely (all of the stuff under the , s prefix).

For whatever reason, I didn't migrate the "set ess-ask-for-ess-directory to nil" hack when I migrated to spacemacs since the above works for me, but I'm surprised the hack isn't working for you.
I tried manually setting the value of ess-ask-for-ess-directory to nil with M-: (setq ess-ask-for-ess-directory nil) RET, and then repeating the steps above, and I did get an R prompt, so I'm not sure why things aren't working for you. I wonder if perhaps it's not getting re-set to t somewhere downstream of your user config.
Can you try uncommenting that fix in your .spacemacs, relaunching, trying to replicate the error, and then interrogating the value of ess-ask-for-ess-directory with SPC h d v ess-ask-for-ess-directory RET to confirm that it's truly set to nil?

Out of curiosity, I tried adding (setq ess-ask-for-ess-directory nil) to the end of my dotspacemacs/user-config defun, tried steps 1-2 above, and things "worked", so I'm not sure why that fix isn't working for you.

[bryce-carson]

Can you try uncommenting that fix in your .spacemacs, relaunching, trying to replicate the error, and then interrogating the value of ess-ask-for-ess-directory with SPC h d v ess-ask-for-ess-directory RET to confirm that it's truly set to nil?

I'll do this in a day or so. For now, I've switched back to Ivy, rather than Helm, as I need to make progress on my Shiny app for a meeting this Friday.

I'll investigate my configuration further when I have the time.

[bryce-carson]

@dankessler,

Out of curiosity, I tried adding (setq ess-ask-for-ess-directory nil) to the end of my dotspacemacs/user-config defun, tried steps 1-2 above, and things "worked", so I'm not sure why that fix isn't working for you.

The uncommented setqs had no effect after restarting Emacs (1. SPC q q; 2. emacs); the value of the variable is indeed still t, so there indeed may be something in my configuration that is resetting it to t after dotspacemacs/user-config is called.

Lastly, manually evaluating (setq ess-ask-for-ess-directory nil) does return nil, the changes are reflected by describe-variable, and the normal behaviour is restored.

I'll investigate my .spacemacs. As for this issue, do you think that it is possible 983c54b & 9f81477 haven't corrected it fully? I'd love to contribute more, but I'm only at (info "(eintr)buffer walk through") of my Emacs journey, and I only got as far as identifying ess-request-a-process as an unhappy function when "debugging" things myself. I'll be watching #1112.

[nettoyoussef]

I am using:

• Arch: 5.16.2
• Emacs: 29.05
• ESS: 04ebabb
• Helm: b9acaecbc05970b9c85eff8f2898b2410066bac6

And can confirm that this problem still persists: ess-eval-region > Wrong type argument: window-live-p, nil in a new R file without a ESS session active.

Maybe this merits a second look, @vspinu?

If I start beforehand an ESS session with run-ess-r, the problem doesn't occur.
Also setting (setq ess-ask-for-ess-directory nil) in the config solves the problem (but creates another, because sometimes we want a different root - like the project, and not the specific code folder - to be the working directory).

[vspinu]

@nettoyoussef could you maybe start anew issue for this with a back trace and steps to reproduce? It's a lot of stuff here, probably no longer relevant.