fix(edit.lua): ensure proper termination of external terminal sessions

[PMassicotte]

I often have an issue when using :wq. I am getting this message in my terminal which do not close automatically 50% of the time:

R 4.5.2 ❯ [I] Connection with R.nvim was lost

I have used this fix for a while and I am not getting this terminal closing issue anymore:

• When closing Neovim with auto_quit = true and an external terminal (kitty, tmux, etc.), R often fails to quit because the quit() command sent as terminal text races with rnvimserver shutdown.
• Send q('no') via the nvimcom TCP channel first, which is processed directly by R's idle handler and bypasses the terminal frontend's input pipeline

[PMassicotte]

I just had the same issue again, so my solution reduce the occurrence, but not completely fix it.

[jalvesaq]

Maybe a loop with a 100 ms sleep waiting for a variable to become true could solve it...

[jalvesaq]

No... the sleep is already there, and it's not enough.

[PMassicotte]

Am I the only one experiencing that?

[jalvesaq]

I confirm that auto_quit=true isn't working when external_term is "wezterm_split".

[jalvesaq]

With this change:

diff --git a/lua/r/edit.lua b/lua/r/edit.lua
index 63fbe0e..0be963d 100644
--- a/lua/r/edit.lua
+++ b/lua/r/edit.lua
@@ -126,6 +126,16 @@ M.add_for_deletion = function(fname)
 end
 
 M.vim_leave = function()
+    vim.cmd.echo(
+        "'Terminal running: "
+            .. tostring(require("r.job").is_running("Terminal emulator"))
+            .. "'"
+    )
+    vim.cmd.redraw()
+    vim.wait(2000)
+    vim.cmd.echo("'vim_leave before loop: " .. tostring(vim.g.R_Nvim_status) .. "'")
+    vim.cmd.redraw()
+    vim.wait(2000)
     if vim.g.R_Nvim_status == 7 and config.auto_quit then
         if config.external_term ~= "" then
             require("r.run").send_to_nvimcom("E", "q('no')")

We can see that the "Terminal emulator" job is no longer running (no commands can be sent to R), and R_Nvim_status value is 1 after the loop (rnvimserver was already killed by Neovim, that is, it's too late to try to send a commad to nvimcom).

[jalvesaq]

Changing the event from VimLeave to VimLeavePre doesn't solve the issue. I think it would be better if Neovim closed the connections with running jobs after the tasks triggered by VimLeavePre were finished.

[jalvesaq]

I was wrong. This always returns false:

:lua print(require("r.job").is_running("Terminal emulator"))

Then, this seems to fix the bug (but create another one):

diff --git a/lua/r/server.lua b/lua/r/server.lua
index e75feb6..3c34566 100644
--- a/lua/r/server.lua
+++ b/lua/r/server.lua
@@ -114,6 +114,7 @@ local start_rnvimserver = function()
     if config.objbr_opendf then rns_env.RNVIM_OPENDF = "TRUE" end
     if config.objbr_openlist then rns_env.RNVIM_OPENLS = "TRUE" end
     if config.objbr_allnames then rns_env.RNVIM_OBJBR_ALLNAMES = "TRUE" end
+    if config.auto_quit then rns_env.RNVIM_AUTO_QUIT = "TRUE" end
     rns_env.RNVIM_RPATH = config.R_cmd
     rns_env.RNVIM_MAX_DEPTH = tostring(config.compl_data.max_depth)
     local disable_parts = {}
diff --git a/rnvimserver/rnvimserver.c b/rnvimserver/rnvimserver.c
index 9422475..a4a0c2d 100644
--- a/rnvimserver/rnvimserver.c
+++ b/rnvimserver/rnvimserver.c
@@ -417,6 +417,8 @@ static void handle_exe_cmd(const char *params) {
  */
 static void handle_exit(const char *method) {
     Log("Received \"%s\" notification. Shutting down.\n", method);
+    if (getenv("RNVIM_AUTO_QUIT"))
+        nvimcom_eval("q('no')");
     exit(0);
 }
 

The new bug is that R will quit if our language server is intentionally shut down by the user, even if there is no intent to quit R.

[PMassicotte]

I think this is working now, need to test a bit more.

[jalvesaq]

Great solution! No new bug introduced.

Some notes:

• Maybe we don't need to create the variables r_ready and auto_quit_pending because they would be used only once. We may use the code that creates them directly.

• In edit.lua, if vim.g.R_nvim_auto_quit_pending is true, config.external_term will not be "". So, you don't need to check if config.external_term ~= "".

• The old code (with vim.wait) will never run.

So, this perhaps is better:

    if config.auto_quit then
        if vim.g.R_Nvim_status == 7 or vim.g.R_nvim_auto_quit_pending then
            vim.g.R_nvim_auto_quit_pending = nil
            require("r.send").cmd('quit(save = "no")')
        else
            require("r.run").quit_R("nosave")
            local i = 30
            while i > 0 and vim.g.R_Nvim_status == 7 do
                vim.wait(100)
                i = i - 1
            end
        end
    end

[PMassicotte]

Thank you very much for taking time to test and your comments!

[jalvesaq]

I will look again later today. I think we don't need the old code with vim.wait().

[PMassicotte]

True I forget to check for that, this is now dead code I think.

[jalvesaq]

Your solution is always valid, whether R is running in an external terminal or not. Hence, I think this minor change concludes the fix:

diff --git a/lua/r/lsp/init.lua b/lua/r/lsp/init.lua
index 31e961e..9b2a539 100644
--- a/lua/r/lsp/init.lua
+++ b/lua/r/lsp/init.lua
@@ -759,7 +759,7 @@ end
 ---@param client integer Client handle
 local function on_exit(code, signal, client)
     local cfg = require("r.config").get_config()
-    if vim.g.R_Nvim_status == 7 and cfg.auto_quit and cfg.external_term ~= "" then
+    if vim.g.R_Nvim_status == 7 and cfg.auto_quit then
         vim.g.R_nvim_auto_quit_pending = true
     end
 

[PMassicotte]

Your solution is always valid, whether R is running in an external terminal or not.

True!

[jalvesaq]

I will merge it now. Thank you!