feat: make :restart work for remote UI (#34354)

runtime/doc/gui.txt

@@ -78,13 +78,13 @@ Restart Nvim
                 This fails when changes have been made and Vim refuses to
                 |abandon| the current buffer.
 
-                Note: This only works if the UI and server are on the same
+                Note: If the current UI hasn't implemented the "restart" UI
-                system.
+                event, this command is equivalent to `:qall`.
+                Note: Only works if the UI and server are on the same system.
                 Note: Not supported on Windows yet.
 
 :restart!
-                Force restarts the embedded server irrespective of unsaved
+                Force restarts the Nvim server, abandoning unsaved buffers.
-                buffers.
 
 ------------------------------------------------------------------------------
 GUI commands

runtime/doc/ui.txt

@@ -248,10 +248,13 @@ the editor.
 	Indicates to the UI that it must stop rendering the cursor. This event
 	is misnamed and does not actually have anything to do with busyness.
 
-["restart"] ~
+["restart", progpath, argv] ~
-	|:restart| command was used. The UI should stop-and-restart the Nvim
+	|:restart| command has been used and the Nvim server is about to exit.
-	server using the same startup arguments |v:argv|, and reattach to the
+	The UI should wait for the server to exit, and then start a new server
-	new server.
+	using `progpath` as the full path to the Nvim executable |v:progpath| and
+	`argv` as its arguments |v:argv|, and reattach to the new server.
+	Note: |--embed| and |--headless| are excluded from `argv`, and the client
+	should decide itself whether to add either flag.
 
 ["suspend"] ~
 	|:suspend| command or |CTRL-Z| mapping is used. A terminal client (or

src/nvim/api/ui.c

@@ -7,6 +7,7 @@
 #include <string.h>
 
 #include "klib/kvec.h"
+#include "nvim/api/private/converter.h"
 #include "nvim/api/private/defs.h"
 #include "nvim/api/private/helpers.h"
 #include "nvim/api/private/validate.h"
@@ -17,6 +18,7 @@
 #include "nvim/channel.h"
 #include "nvim/channel_defs.h"
 #include "nvim/eval.h"
+#include "nvim/eval/typval.h"
 #include "nvim/event/defs.h"
 #include "nvim/event/loop.h"
 #include "nvim/event/multiqueue.h"
@@ -239,6 +241,47 @@ void nvim_ui_detach(uint64_t channel_id, Error *err)
   remote_ui_disconnect(channel_id);
 }
 
+/// Sends a "restart" UI event to the UI on the given channel.
+///
+/// @return  false if there is no UI on the channel, otherwise true
+bool remote_ui_restart(uint64_t channel_id, Error *err)
+{
+  RemoteUI *ui = pmap_get(uint64_t)(&connected_uis, channel_id);
+  if (!ui) {
+    api_set_error(err, kErrorTypeException,
+                  "UI not attached to channel: %" PRId64, channel_id);
+    return false;
+  }
+
+  MAXSIZE_TEMP_ARRAY(args, 2);
+
+  ADD_C(args, CSTR_AS_OBJ(get_vim_var_str(VV_PROGPATH)));
+
+  Arena arena = ARENA_EMPTY;
+  const list_T *l = get_vim_var_list(VV_ARGV);
+  int argc = tv_list_len(l);
+  assert(argc > 0);
+  Array argv = arena_array(&arena, (size_t)argc + 1);
+  bool had_minmin = false;
+  TV_LIST_ITER_CONST(l, li, {
+    const char *arg = tv_get_string(TV_LIST_ITEM_TV(li));
+    if (argv.size > 0 && !had_minmin && strequal(arg, "--")) {
+      had_minmin = true;
+    }
+    // Exclude --embed/--headless from `argv`, as the client may start the server in a
+    // different way than how the server was originally started.
+    if (argv.size == 0 || had_minmin
+        || (!strequal(arg, "--embed") && !strequal(arg, "--headless"))) {
+      ADD_C(argv, CSTR_AS_OBJ(arg));
+    }
+  });
+  ADD_C(args, ARRAY_OBJ(argv));
+
+  push_call(ui, "restart", args);
+  arena_mem_free(arena_finish(&arena));
+  return true;
+}
+
 // TODO(bfredl): use me to detach a specific ui from the server
 void remote_ui_stop(RemoteUI *ui)
 {

src/nvim/api/ui_events.in.h

@@ -29,8 +29,8 @@ void visual_bell(void)
   FUNC_API_SINCE(3);
 void flush(void)
   FUNC_API_SINCE(3) FUNC_API_REMOTE_IMPL;
-void restart(void)
+void restart(String progpath, Array argv)
-  FUNC_API_SINCE(14) FUNC_API_REMOTE_ONLY FUNC_API_CLIENT_IMPL;
+  FUNC_API_SINCE(14) FUNC_API_REMOTE_ONLY FUNC_API_REMOTE_IMPL FUNC_API_CLIENT_IMPL;
 void suspend(void)
   FUNC_API_SINCE(3);
 void set_title(String title)

src/nvim/ex_cmds.lua

@@ -2184,13 +2184,13 @@ M.cmds = {
     command = 'quitall',
     flags = bit.bor(BANG, TRLBAR),
     addr_type = 'ADDR_NONE',
-    func = 'ex_quit_all',
+    func = 'ex_quitall_or_restart',
   },
   {
     command = 'qall',
     flags = bit.bor(BANG, TRLBAR, CMDWIN, LOCK_OK),
     addr_type = 'ADDR_NONE',
-    func = 'ex_quit_all',
+    func = 'ex_quitall_or_restart',
   },
   {
     command = 'read',
@@ -2250,7 +2250,7 @@ M.cmds = {
     command = 'restart',
     flags = bit.bor(BANG, TRLBAR),
     addr_type = 'ADDR_NONE',
-    func = 'ex_restart',
+    func = 'ex_quitall_or_restart',
   },
   {
     command = 'retab',

src/nvim/ex_docmd.c

@@ -4828,16 +4828,23 @@ int before_quit_all(exarg_T *eap)
 }
 
 /// ":qall": try to quit all windows
-static void ex_quit_all(exarg_T *eap)
+/// ":restart": restart the Nvim server
+static void ex_quitall_or_restart(exarg_T *eap)
 {
   if (before_quit_all(eap) == FAIL) {
     return;
   }
   exiting = true;
-  if (eap->forceit || !check_changed_any(false, false)) {
+  Error err = ERROR_INIT;
+  if ((eap->forceit || !check_changed_any(false, false))
+      && (eap->cmdidx != CMD_restart || remote_ui_restart(current_ui, &err))) {
     getout(0);
   }
   not_exiting();
+  if (ERROR_SET(&err)) {
+    emsg(err.msg);  // UI disappeared already?
+    api_clear_error(&err);
+  }
 }
 
 /// ":close": close current window, unless it is the last one
@@ -5592,31 +5599,6 @@ static void ex_detach(exarg_T *eap)
   }
 }
 
-/// ":restart" command
-/// Restarts the server by delegating the work to the UI.
-static void ex_restart(exarg_T *eap)
-{
-  bool forceit = eap && eap->forceit;
-
-  win_T *wp = curwin;
-
-  // If any buffer is changed and not saved, we cannot restart.
-  // But if called using bang (!), we will force restart.
-  if ((!buf_hide(wp->w_buffer)
-       && check_changed(wp->w_buffer, (p_awa ? CCGD_AW : 0)
-                        | (forceit ? CCGD_FORCEIT : 0)
-                        | CCGD_EXCMD))
-      || check_more(true, forceit) == FAIL
-      || check_changed_any(forceit, true)) {
-    if (!forceit) {
-      return;
-    }
-  }
-
-  // Send an ui restart event.
-  ui_call_restart();
-}
-
 /// ":mode":
 /// If no argument given, get the screen size and redraw.
 static void ex_mode(exarg_T *eap)

src/nvim/main.c

@@ -334,7 +334,8 @@ int main(int argc, char **argv)
 
   if (use_builtin_ui && !remote_ui) {
     ui_client_forward_stdin = !stdin_isatty;
-    uint64_t rv = ui_client_start_server(params.argc, params.argv);
+    uint64_t rv = ui_client_start_server(get_vim_var_str(VV_PROGPATH),
+                                         (size_t)params.argc, params.argv);
     if (!rv) {
       fprintf(stderr, "Failed to start Nvim server!\n");
       os_exit(1);

src/nvim/msgpack_rpc/channel.c

@@ -499,8 +499,13 @@ static void rpc_close_event(void **argv)
     if (!is_ui_client) {
       // Avoid hanging when there are no other UIs and a prompt is triggered on exit.
       remote_ui_disconnect(channel->id);
+    } else {
+      ui_client_may_restart_server();
+      if (ui_client_channel_id != channel->id) {
+        // A new server has been started. Don't exit.
+        return;
+      }
     }
-
     if (!channel->detach) {
       exit_on_closed_chan(channel->exit_status == -1 ? 0 : channel->exit_status);
     }

src/nvim/ui_client.c

@@ -7,12 +7,9 @@
 
 #include "nvim/api/keysets_defs.h"
 #include "nvim/api/private/defs.h"
-#include "nvim/api/private/dispatch.h"
 #include "nvim/api/private/helpers.h"
 #include "nvim/channel.h"
 #include "nvim/channel_defs.h"
-#include "nvim/eval.h"
-#include "nvim/eval/typval.h"
 #include "nvim/eval/typval_defs.h"
 #include "nvim/event/multiqueue.h"
 #include "nvim/globals.h"
@@ -25,7 +22,6 @@
 #include "nvim/msgpack_rpc/channel.h"
 #include "nvim/msgpack_rpc/channel_defs.h"
 #include "nvim/os/os.h"
-#include "nvim/os/os_defs.h"
 #include "nvim/profile.h"
 #include "nvim/tui/tui.h"
 #include "nvim/tui/tui_defs.h"
@@ -51,14 +47,13 @@ static bool ui_client_is_remote = false;
 #endif
 // uncrustify:on
 
-uint64_t ui_client_start_server(int argc, char **argv)
+uint64_t ui_client_start_server(const char *exepath, size_t argc, char **argv)
 {
-  varnumber_T exit_status;
+  char **args = xmalloc((2 + argc) * sizeof(char *));
-  char **args = xmalloc(((size_t)(2 + argc)) * sizeof(char *));
   int args_idx = 0;
   args[args_idx++] = xstrdup(argv[0]);
   args[args_idx++] = xstrdup("--embed");
-  for (int i = 1; i < argc; i++) {
+  for (size_t i = 1; i < argc; i++) {
     args[args_idx++] = xstrdup(argv[i]);
   }
   args[args_idx++] = NULL;
@@ -72,7 +67,8 @@ uint64_t ui_client_start_server(int argc, char **argv)
 #else
   bool detach = true;
 #endif
-  Channel *channel = channel_job_start(args, get_vim_var_str(VV_PROGPATH),
+  varnumber_T exit_status;
+  Channel *channel = channel_job_start(args, exepath,
                                        CALLBACK_READER_INIT, on_err, CALLBACK_NONE,
                                        false, true, true, detach, kChannelStdinPipe,
                                        NULL, 0, 0, NULL, &exit_status);
@@ -287,57 +283,69 @@ void ui_client_event_raw_line(GridLineEvent *g)
                (const schar_T *)grid_line_buf_char, grid_line_buf_attr);
 }
 
-/// Restarts the embedded server without killing the UI.
+/// When a "restart" UI event is received, its arguments are saved here when
+/// waiting for the server to exit.
+static Array restart_args = ARRAY_DICT_INIT;
+static bool restart_pending = false;
+
 void ui_client_event_restart(Array args)
 {
-  // 1. Client-side server detach.
+  // NB: don't send nvim_ui_detach to server, as it may have already exited.
-  ui_client_detach();
+  // ui_client_detach();
 
-  // 2. Close ui client channel (auto kills the `nvim --embed` server due to self-exit).
+  // Save the arguments for ui_client_may_restart_server() later.
-  const char *error;
+  api_free_array(restart_args);
-  bool success = channel_close(ui_client_channel_id, kChannelPartAll, &error);
+  restart_args = copy_array(args, NULL);
-  if (!success) {
+  restart_pending = true;
-    ELOG("%s", error);
-    return;
 }
 
-  // 3. Get v:argv.
+/// Called when the current server has exited.
-  typval_T *tv = get_vim_var_tv(VV_ARGV);
+void ui_client_may_restart_server(void)
-  if (tv->v_type != VAR_LIST || tv->vval.v_list == NULL) {
+{
-    ELOG("failed to get vim var typval");
+  if (!restart_pending) {
     return;
   }
-  list_T *l = tv->vval.v_list;
+  restart_pending = false;
-  int argc = tv_list_len(l);
 
-  // Assert to be positive for safe conversion to size_t.
+  size_t argc;
-  assert(argc > 0);
+  char **argv = NULL;
+  if (restart_args.size < 2
+      || restart_args.items[0].type != kObjectTypeString
+      || restart_args.items[1].type != kObjectTypeArray
+      || (argc = restart_args.items[1].data.array.size) < 1) {
+    ELOG("Error handling ui event 'restart'");
+    goto cleanup;
+  }
 
-  char **argv = xmalloc(sizeof(char *) * ((size_t)argc + 1));
+  // 1. Get executable path and command-line arguments.
-  listitem_T *li = tv_list_first(l);
+  const char *exepath = restart_args.items[0].data.string.data;
-  for (int i = 0; i < argc && li != NULL; i++, li = TV_LIST_ITEM_NEXT(l, li)) {
+  argv = xcalloc(argc + 1, sizeof(char *));
-    if (TV_LIST_ITEM_TV(li)->v_type == VAR_STRING && TV_LIST_ITEM_TV(li)->vval.v_string != NULL) {
+  for (size_t i = 0; i < argc; i++) {
-      argv[i] = TV_LIST_ITEM_TV(li)->vval.v_string;
+    if (restart_args.items[1].data.array.items[i].type == kObjectTypeString) {
-    } else {
+      argv[i] = restart_args.items[1].data.array.items[i].data.string.data;
+    }
+    if (argv[i] == NULL) {
       argv[i] = "";
     }
   }
-  argv[argc] = NULL;
 
-  // 4. Start a new `nvim --embed` server.
+  // 2. Start a new `nvim --embed` server.
-  uint64_t rv = ui_client_start_server(argc, argv);
+  uint64_t rv = ui_client_start_server(exepath, argc, argv);
   if (!rv) {
     ELOG("failed to start nvim server");
     goto cleanup;
   }
 
-  // 5. Client-side server re-attach.
+  // 3. Client-side server re-attach.
   ui_client_channel_id = rv;
+  ui_client_is_remote = false;
   ui_client_attach(tui_width, tui_height, tui_term, tui_rgb);
 
   ILOG("restarted server id=%" PRId64, rv);
 cleanup:
   xfree(argv);
+  api_free_array(restart_args);
+  restart_args = (Array)ARRAY_DICT_INIT;
 }
 
 #ifdef EXITFREE

src/nvim/ui_client.h

@@ -20,9 +20,6 @@ EXTERN uint64_t ui_client_channel_id INIT( = 0);
 // exit status from embedded nvim process
 EXTERN int ui_client_exit_status INIT( = 0);
 
-// TODO(bfredl): the current structure for how tui and ui_client.c communicate is a bit awkward.
-// This will be restructured as part of The UI Devirtualization Project.
-
 /// Whether ui client has sent nvim_ui_attach yet
 EXTERN bool ui_client_attached INIT( = false);
 

test/functional/terminal/tui_spec.lua

@@ -168,11 +168,14 @@ describe('TUI :restart', function()
       n.check_close()
     end)
 
+    local server_pipe = new_pipename()
     local screen = tt.setup_child_nvim({
       '-u',
       'NONE',
       '-i',
       'NONE',
+      '--listen',
+      server_pipe,
       '--cmd',
       'colorscheme vim',
       '--cmd',
@@ -189,6 +192,16 @@ describe('TUI :restart', function()
       {3:-- TERMINAL --}                                    |
     ]]
     screen:expect(s0)
+    local server_session = n.connect(server_pipe)
+    local _, server_pid = server_session:request('nvim_call_function', 'getpid', {})
+
+    local function restart_pid_check()
+      server_session:close()
+      server_session = n.connect(server_pipe)
+      local _, new_pid = server_session:request('nvim_call_function', 'getpid', {})
+      t.neq(server_pid, new_pid)
+      server_pid = new_pid
+    end
 
     tt.feed_data(':1restart\013')
     screen:expect({ any = vim.pesc('{8:E481: No range allowed}') })
@@ -199,6 +212,7 @@ describe('TUI :restart', function()
     -- Check ":restart" on an unmodified buffer.
     tt.feed_data(':restart\013')
     screen:expect(s0)
+    restart_pid_check()
 
     tt.feed_data('ithis will be removed\027')
     screen:expect([[
@@ -224,6 +238,21 @@ describe('TUI :restart', function()
     -- Check ":restart!".
     tt.feed_data(':restart!\013')
     screen:expect(s0)
+    restart_pid_check()
+
+    tt.feed_data(':echo\n')
+    screen:expect([[
+      ^                                                  |
+      {4:~                                                 }|*3
+      {5:[No Name]                                         }|
+                                                        |
+      {3:-- TERMINAL --}                                    |
+    ]])
+
+    -- No --listen conflict when server exit is delayed.
+    feed_data(':lua vim.schedule(function() vim.wait(100) end); vim.cmd.restart()\n')
+    screen:expect(s0)
+    restart_pid_check()
 
     screen:try_resize(60, 6)
     screen:expect([[
@@ -234,7 +263,7 @@ describe('TUI :restart', function()
       {3:-- TERMINAL --}                                              |
     ]])
 
-    --- Check that ":restart" uses the updated size after terminal resize
+    --- Check that ":restart" uses the updated size after terminal resize.
     tt.feed_data(':restart\013')
     screen:expect([[
       ^                                                            |
@@ -243,6 +272,7 @@ describe('TUI :restart', function()
       {MATCH:%d+ +}|
       {3:-- TERMINAL --}                                              |
     ]])
+    restart_pid_check()
   end)
 end)
 
@@ -3641,25 +3671,22 @@ describe('TUI client', function()
     })
 
     feed_data('iHello, World')
-    screen_server:expect {
+    screen_server:expect([[
-      grid = [[
       Hello, World^                                      |
       {4:~                                                 }|*3
       {5:[No Name] [+]                                     }|
       {3:-- INSERT --}                                      |
       {3:-- TERMINAL --}                                    |
-    ]],
+    ]])
-    }
     feed_data('\027')
-    screen_server:expect {
+    local s0 = [[
-      grid = [[
       Hello, Worl^d                                      |
       {4:~                                                 }|*3
       {5:[No Name] [+]                                     }|
                                                         |
       {3:-- TERMINAL --}                                    |
-    ]],
+    ]]
-    }
+    screen_server:expect(s0)
 
     set_session(client_super)
     local screen_client = tt.setup_child_nvim({
@@ -3667,28 +3694,31 @@ describe('TUI client', function()
       '--server',
       server_pipe,
     })
-
+    screen_client:expect(s0)
-    screen_client:expect {
-      grid = [[
-      Hello, Worl^d                                      |
-      {4:~                                                 }|*3
-      {5:[No Name] [+]                                     }|
-                                                        |
-      {3:-- TERMINAL --}                                    |
-    ]],
-    }
 
     -- grid smaller than containing terminal window is cleared properly
     feed_data(":call setline(1,['a'->repeat(&columns)]->repeat(&lines))\n")
     feed_data('0:set lines=3\n')
-    screen_server:expect {
+    local s1 = [[
-      grid = [[
       ^aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|
       {5:[No Name] [+]                                     }|
                                                         |*4
       {3:-- TERMINAL --}                                    |
-    ]],
+    ]]
-    }
+    screen_client:expect(s1)
+    screen_server:expect(s1)
+
+    -- Run :restart! on the remote client.
+    -- The remote client should start a new server while the original one should exit.
+    feed_data(':restart!\n')
+    screen_client:expect([[
+      ^                                                  |
+      {4:~                                                 }|*3
+      {5:[No Name]                                         }|
+                                                        |
+      {3:-- TERMINAL --}                                    |
+    ]])
+    screen_server:expect({ any = vim.pesc('[Process exited 0]') })
 
     feed_data(':q!\n')
 
@@ -3712,33 +3742,53 @@ describe('TUI client', function()
       server_pipe,
     })
 
-    screen_client:expect {
+    screen_client:expect([[
-      grid = [[
       Halloj^!                                           |
       {4:~                                                 }|*4
                                                         |
       {3:-- TERMINAL --}                                    |
-    ]],
+    ]])
-    }
 
     -- No heap-use-after-free when receiving UI events after deadly signal #22184
     server:request('nvim_input', ('a'):rep(1000))
     exec_lua([[vim.uv.kill(vim.fn.jobpid(vim.bo.channel), 'sigterm')]])
-    screen_client:expect {
+    screen_client:expect([[
-      grid = [[
       Nvim: Caught deadly signal 'SIGTERM'              |
                                                         |
       [Process exited 1]^                                |
                                                         |*3
       {3:-- TERMINAL --}                                    |
-    ]],
+    ]])
-    }
 
     eq(0, api.nvim_get_vvar('shell_error'))
     -- exits on input eof #22244
     fn.system({ nvim_prog, '--remote-ui', '--server', server_pipe })
     eq(1, api.nvim_get_vvar('shell_error'))
 
+    command('bwipe!')
+    fn.jobstart({ nvim_prog, '--remote-ui', '--server', server_pipe }, { term = true })
+    command('startinsert')
+    screen_client:expect([[
+      {4:<<<}aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|
+      aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|*3
+      aaaaaa^                                            |
+      {3:-- INSERT --}                                      |
+      {3:-- TERMINAL --}                                    |
+    ]])
+
+    -- Run :restart! on the client.
+    -- The client should start a new server while the original server should exit.
+    feed_data('\027:restart!\n')
+    screen_client:expect([[
+      ^                                                  |
+      {4:~                                                 }|*4
+                                                        |
+      {3:-- TERMINAL --}                                    |
+    ]])
+    retry(nil, nil, function()
+      eq(nil, vim.uv.fs_stat(server_pipe))
+    end)
+
     client_super:close()
     server:close()
     if is_os('mac') then