test(remote): add tests for --remote

This also fixes a fair number of issues found in running the tests

4 files changed, 179 insertions, 43 deletions

diff --git a/runtime/lua/vim/_editor.lua b/runtime/lua/vim/_editor.lua
index 5f3d66e108..869a2706ac 100644
--- a/runtime/lua/vim/_editor.lua
+++ b/runtime/lua/vim/_editor.lua
@@ -636,17 +636,10 @@ function vim.pretty_print(...)
   return ...
 end
 
-local function __rpcrequest(...)
-  return vim.api.nvim_call_function("rpcrequest", {...})
-end
-
 function vim._cs_remote(rcid, args)
-
   local f_silent = false
   local f_wait = false
   local f_tab = false
-  local should_exit = true
-  local command = 'edit '
 
   local subcmd = string.sub(args[1],10)
 
@@ -672,40 +665,42 @@ function vim._cs_remote(rcid, args)
     f_wait = true
     f_silent = true
   elseif subcmd == 'send' then
-    __rpcrequest(rcid, 'nvim_input', args[2])
-    return { should_exit = should_exit, tabbed = f_tab, files = 0 }
-    -- should we show warning if --server doesn't exist in --send and --expr?
+    if rcid == 0 then
+      vim.cmd('echoerr "E247: Remote server does not exist. Send failed."')
+      return
+    end
+    vim.fn.rpcrequest(rcid, 'nvim_input', args[2])
+    return { should_exit = true, tabbed = false }
   elseif subcmd == 'expr' then
-    local res = __rpcrequest(rcid, 'vim_eval', args[2])
-    print(res)
-    return { should_exit = should_exit, tabbed = f_tab, files = 0 }
+    if rcid == 0 then
+      vim.cmd('echoerr "E247: Remote server does not exist. Send expression failed."')
+      return
+    end
+    vim.fn.rpcrequest(rcid, 'nvim_eval', args[2])
+    return { should_exit = true, tabbed = false }
   else
-    print('--remote subcommand not found')
+    vim.cmd('echoerr "Unknown option argument: ' .. args[1] .. '"')
+    return
   end
 
-  table.remove(args,1)
-
-  if not f_silent and rcid == 0 then
-    print('Remote server does not exist.')
-  end
-
-  if f_silent and rcid == 0 then
-    print('Remote server does not exist. starting new server')
+  if rcid == 0 then
+    if not f_silent then
+      vim.cmd('echohl WarningMsg | echomsg "E247: Remote server does not exist. Editing locally" | echohl None')
+    end
     should_exit = false
-  end
-
-  if f_tab then command = 'tabedit ' end
-
-  if rcid ~= 0 then
-    for _, key in ipairs(args) do
-      __rpcrequest(rcid, 'nvim_command', command .. key)
+  else
+    local command = {}
+    if f_tab then table.insert(command, 'tab') end
+    table.insert(command, 'drop')
+    for i = 2, #args do
+      table.insert(command, vim.fn.fnameescape(args[i]))
     end
+    vim.fn.rpcrequest(rcid, 'nvim_command', table.concat(command, ' '))
   end
 
   return {
-    should_exit = should_exit,
+    should_exit = rcid ~= 0,
     tabbed = f_tab,
-    files = table.getn(args)
   }
 end
 
diff --git a/src/nvim/api/vim.c b/src/nvim/api/vim.c
index b82a7553cb..b691dee2ef 100644
--- a/src/nvim/api/vim.c
+++ b/src/nvim/api/vim.c
@@ -1997,7 +1997,7 @@ Array nvim_get_proc_children(Integer pid, Error *err)
     DLOG("fallback to vim._os_proc_children()");
     Array a = ARRAY_DICT_INIT;
     ADD(a, INTEGER_OBJ(pid));
-    String s = cstr_to_string("return vim._os_proc_children(select(...))");
+    String s = cstr_to_string("return vim._os_proc_children(...)");
     Object o = nlua_exec(s, a, err);
     api_free_string(s);
     api_free_array(a);
diff --git a/src/nvim/main.c b/src/nvim/main.c
index 843ca5f855..a3588ac5df 100644
--- a/src/nvim/main.c
+++ b/src/nvim/main.c
@@ -821,7 +821,6 @@ static void handle_remote_client(mparm_T *params, int remote_args,
 
     Boolean should_exit = true;
     Boolean tabbed;
-    int files;
 
     int t_argc = remote_args;
     Array args = ARRAY_DICT_INIT;
@@ -839,12 +838,17 @@ static void handle_remote_client(mparm_T *params, int remote_args,
     Object o = nlua_exec(s, a, &err);
     api_free_string(s);
     api_free_array(a);
+    if (ERROR_SET(&err)) {
+      mch_errmsg(err.msg);
+      mch_errmsg("\n");
+      os_exit(2);
+    }
 
     if (o.type == kObjectTypeDictionary) {
       rvobj.data.dictionary = o.data.dictionary;
-    } else if (!ERROR_SET(&err)) {
-      api_set_error(&err, kErrorTypeException,
-                    "Function returned unexpected value");
+    } else {
+      mch_errmsg("vim._cs_remote returned unexpected value\n");
+      os_exit(3);
     }
 
     for (size_t i = 0; i < rvobj.data.dictionary.size ; i++) {
@@ -853,18 +857,15 @@ static void handle_remote_client(mparm_T *params, int remote_args,
         tabbed = rvobj.data.dictionary.items[i].value.data.boolean;
       } else if (strcmp(rvobj.data.dictionary.items[i].key.data, "should_exit") == 0) {
         should_exit = rvobj.data.dictionary.items[i].value.data.boolean;
-      } else if (strcmp(rvobj.data.dictionary.items[i].key.data, "files") == 0) {
-        files = (int)rvobj.data.dictionary.items[i].value.data.integer;
       }
     }
 
     if (should_exit) {
       os_exit(0);
-    } else {
-      if (tabbed) {
-        params->window_count = files;
-        params->window_layout = WIN_TABS;
-      }
+    }
+    if (tabbed) {
+      params->window_count = argc - remote_args - 1;
+      params->window_layout = WIN_TABS;
     }
 }
 
diff --git a/test/functional/core/remote_spec.lua b/test/functional/core/remote_spec.lua
new file mode 100644
index 0000000000..e86d15ec69
--- /dev/null
+++ b/test/functional/core/remote_spec.lua
@@ -0,0 +1,140 @@
+local helpers = require('test.functional.helpers')(after_each)
+
+local clear = helpers.clear
+local command = helpers.command
+local eq = helpers.eq
+local expect = helpers.expect
+local funcs = helpers.funcs
+local insert = helpers.insert
+local meths = helpers.meths
+local new_argv = helpers.new_argv
+local neq = helpers.neq
+local run = helpers.run
+local set_session = helpers.set_session
+local spawn = helpers.spawn
+local tmpname = helpers.tmpname
+local write_file = helpers.write_file
+
+describe('Remote', function()
+  local fname, other_fname
+  local contents = 'The call is coming from outside the process'
+  local other_contents = "A second file's contents"
+
+  before_each(function()
+    fname = tmpname() .. ' with spaces in the filename'
+    other_fname = tmpname()
+    write_file(fname, contents)
+    write_file(other_fname, other_contents)
+  end)
+
+  describe('connect to server and', function()
+    local server
+    before_each(function()
+      server = spawn(new_argv(), true)
+      set_session(server)
+    end)
+
+    after_each(function()
+      server:close()
+    end)
+
+    function run_remote(...)
+      set_session(server)
+      local addr = funcs.serverlist()[1]
+      local client_argv = new_argv({args={'--server', addr, ...}})
+
+      -- Create an nvim instance just to run the remote-invoking nvim. We want
+      -- to wait for the remote instance to exit and calling jobwait blocks
+      -- the event loop. If the server event loop is blocked, it can't process
+      -- our incoming --remote calls.
+      local client_starter = spawn(new_argv(), false, nil, true)
+      set_session(client_starter)
+      local client_job_id = funcs.jobstart(client_argv)
+      eq({ 0 }, funcs.jobwait({client_job_id}))
+      client_starter:close()
+      set_session(server)
+    end
+
+    it('edit a single file', function()
+      run_remote('--remote', fname)
+      expect(contents)
+      eq(2, #funcs.getbufinfo())
+    end)
+
+    it('tab edit a single file with a non-changed buffer', function()
+      run_remote('--remote-tab', fname)
+      expect(contents)
+      eq(1, #funcs.gettabinfo())
+    end)
+
+    it('tab edit a single file with a changed buffer', function()
+      insert('hello')
+      run_remote('--remote-tab', fname)
+      expect(contents)
+      eq(2, #funcs.gettabinfo())
+    end)
+
+    it('edit multiple files', function()
+      run_remote('--remote', fname, other_fname)
+      expect(contents)
+      command('next')
+      expect(other_contents)
+      eq(3, #funcs.getbufinfo())
+    end)
+
+    it('send keys', function()
+      run_remote('--remote-send', ':edit '..fname..'<CR><C-W>v')
+      expect(contents)
+      eq(2, #funcs.getwininfo())
+      -- Only a single buffer as we're using edit and not drop like --remote does
+      eq(1, #funcs.getbufinfo())
+    end)
+
+    it('evaluate expressions', function()
+      run_remote('--remote-expr', 'setline(1, "Yo")')
+      expect('Yo')
+    end)
+  end)
+
+  it('creates server if not found', function()
+    clear('--remote', fname)
+    expect(contents)
+    eq(1, #funcs.getbufinfo())
+    -- Since we didn't pass silent, we should get a complaint
+    neq(nil, string.find(meths.exec('messages', true), 'E247'))
+  end)
+
+  it('creates server if not found with tabs', function()
+    clear('--remote-tab-silent', fname, other_fname)
+    expect(contents)
+    eq(2, #funcs.gettabinfo())
+    eq(2, #funcs.getbufinfo())
+    -- We passed silent, so no message should be issued about the server not being found
+    eq(nil, string.find(meths.exec('messages', true), 'E247'))
+  end)
+
+  describe('exits with error on', function()
+    local function run_and_check_exit_code(...)
+      local bogus_argv = new_argv(...)
+
+      -- Create an nvim instance just to run the remote-invoking nvim. We want
+      -- to wait for the remote instance to exit and calling jobwait blocks
+      -- the event loop. If the server event loop is blocked, it can't process
+      -- our incoming --remote calls.
+      local client_starter = clear()
+      local bogus_job_id = funcs.jobstart(bogus_argv)
+      eq({2}, funcs.jobwait({bogus_job_id}))
+    end
+    it('bogus subcommand', function()
+      run_and_check_exit_code('--remote-bogus')
+    end)
+
+    it('send without server', function()
+      run_and_check_exit_code('--remote-send', 'i')
+    end)
+
+    it('expr without server', function()
+      run_and_check_exit_code('--remote-expr', 'setline(1, "Yo")')
+    end)
+  end)
+end)