feat(api): nvim_get_chan_info: include "argv" for jobs #15537

justinmk · web-flow · commit 0603eba6e713 · 2021-09-01T07:29:38.000-07:00
ref #15440
diff --git a/src/nvim/api/vim.c b/src/nvim/api/vim.c
@@ -2057,27 +2057,28 @@ void nvim_set_client_info(uint64_t channel_id, String name,
   rpc_set_client_info(channel_id, info);
 }
 
-/// Get information about a channel.
+/// Gets information about a channel.
 ///
 /// @returns Dictionary describing a channel, with these keys:
-///    - "stream"  the stream underlying the channel
+///    - "id"       Channel id.
+///    - "argv"     (optional) Job arguments list.
+///    - "stream"   Stream underlying the channel.
 ///         - "stdio"      stdin and stdout of this Nvim instance
 ///         - "stderr"     stderr of this Nvim instance
 ///         - "socket"     TCP/IP socket or named pipe
-///         - "job"        job with communication over its stdio
-///    -  "mode"    how data received on the channel is interpreted
-///         - "bytes"      send and receive raw bytes
-///         - "terminal"   a |terminal| instance interprets ASCII sequences
-///         - "rpc"        |RPC| communication on the channel is active
-///    -  "pty"     Name of pseudoterminal, if one is used (optional).
-///                 On a POSIX system, this will be a device path like
-///                 /dev/pts/1. Even if the name is unknown, the key will
-///                 still be present to indicate a pty is used. This is
-///                 currently the case when using winpty on windows.
-///    -  "buffer"  buffer with connected |terminal| instance (optional)
-///    -  "client"  information about the client on the other end of the
-///                 RPC channel, if it has added it using
-///                 |nvim_set_client_info()|. (optional)
+///         - "job"        Job with communication over its stdio.
+///    -  "mode"    How data received on the channel is interpreted.
+///         - "bytes"      Send and receive raw bytes.
+///         - "terminal"   |terminal| instance interprets ASCII sequences.
+///         - "rpc"        |RPC| communication on the channel is active.
+///    -  "pty"     (optional) Name of pseudoterminal. On a POSIX system this
+///                 is a device path like "/dev/pts/1". If the name is unknown,
+///                 the key will still be present if a pty is used (e.g. for
+///                 winpty on Windows).
+///    -  "buffer"  (optional) Buffer with connected |terminal| instance.
+///    -  "client"  (optional) Info about the peer (client on the other end of
+///                 the RPC channel), if provided by it via
+///                 |nvim_set_client_info()|.
 ///
 Dictionary nvim_get_chan_info(Integer chan, Error *err)
   FUNC_API_SINCE(4)
diff --git a/src/nvim/channel.c b/src/nvim/channel.c
@@ -240,6 +240,10 @@ static void free_channel_event(void **argv)
     rpc_free(chan);
   }
 
+  if (chan->streamtype == kChannelStreamProc) {
+    process_free(&chan->stream.proc);
+  }
+
   callback_reader_free(&chan->on_data);
   callback_reader_free(&chan->on_stderr);
   callback_free(&chan->on_exit);
@@ -847,13 +851,24 @@ Dictionary channel_info(uint64_t id)
 
   const char *stream_desc, *mode_desc;
   switch (chan->streamtype) {
-    case kChannelStreamProc:
+    case kChannelStreamProc: {
       stream_desc = "job";
       if (chan->stream.proc.type == kProcessTypePty) {
         const char *name = pty_process_tty_name(&chan->stream.pty);
         PUT(info, "pty", STRING_OBJ(cstr_to_string(name)));
       }
+
+      char **p = chan->stream.proc.argv;
+      Array argv = ARRAY_DICT_INIT;
+      if (p != NULL) {
+        while (*p != NULL) {
+          ADD(argv, STRING_OBJ(cstr_to_string(*p)));
+          p++;
+        }
+      }
+      PUT(info, "argv", ARRAY_OBJ(argv));
       break;
+    }
 
     case kChannelStreamStdio:
        stream_desc = "stdio";
diff --git a/src/nvim/event/process.c b/src/nvim/event/process.c
@@ -88,7 +88,7 @@ int process_spawn(Process *proc, bool in, bool out, bool err)
     } else {
       process_close(proc);
     }
-    shell_free_argv(proc->argv);
+    process_free(proc);
     proc->status = -1;
     return status;
   }
@@ -201,7 +201,7 @@ int process_wait(Process *proc, int ms, MultiQueue *events)
     // Job exited, free its resources.
     decref(proc);
     if (proc->events) {
-      // the decref call created an exit event, process it now
+      // decref() created an exit event, process it now.
       multiqueue_process_events(proc->events);
     }
   } else {
@@ -239,6 +239,15 @@ void process_stop(Process *proc) FUNC_ATTR_NONNULL_ALL
                  KILL_TIMEOUT_MS, 0);
 }
 
+// Frees process-owned resources.
+void process_free(Process *proc) FUNC_ATTR_NONNULL_ALL
+{
+  if (proc->argv != NULL) {
+    shell_free_argv(proc->argv);
+    proc->argv = NULL;
+  }
+}
+
 /// Sends SIGKILL (or SIGTERM..SIGKILL for PTY jobs) to processes that did
 /// not terminate after process_stop().
 static void children_kill_cb(uv_timer_t *handle)
@@ -269,9 +278,12 @@ static void children_kill_cb(uv_timer_t *handle)
 static void process_close_event(void **argv)
 {
   Process *proc = argv[0];
-  shell_free_argv(proc->argv);
-  if (proc->cb) {  // "on_exit" for jobstart(). See channel_job_start().
+  if (proc->cb) {
+    // User (hint: channel_job_start) is responsible for calling
+    // process_free().
     proc->cb(proc, proc->status, proc->data);
+  } else {
+    process_free(proc);
   }
 }
 
diff --git a/src/nvim/event/process.h b/src/nvim/event/process.h
@@ -26,6 +26,7 @@ struct process {
   char **argv;
   dict_T *env;
   Stream in, out, err;
+  /// Exit handler. If set, user must call process_free().
   process_exit_cb cb;
   internal_process_cb internal_exit_cb, internal_close_cb;
   bool closed, detach, overlapped;
diff --git a/test/functional/api/server_notifications_spec.lua b/test/functional/api/server_notifications_spec.lua
@@ -6,6 +6,7 @@ local meths = helpers.meths
 local exec_lua = helpers.exec_lua
 local retry = helpers.retry
 local isCI = helpers.isCI
+local assert_alive = helpers.assert_alive
 
 describe('notify', function()
   local channel
@@ -73,7 +74,7 @@ describe('notify', function()
     nvim('subscribe', 'event1')
     nvim('unsubscribe', 'doesnotexist')
     nvim('unsubscribe', 'event1')
-    eq(2, eval('1+1'))  -- Still alive?
+    assert_alive()
   end)
 
   it('cancels stale events on channel close', function()
@@ -83,12 +84,13 @@ describe('notify', function()
       end
     if helpers.pending_win32(pending) then return end
     local catchan = eval("jobstart(['cat'], {'rpc': v:true})")
-    eq({id=catchan, stream='job', mode='rpc', client = {}}, exec_lua ([[
+    local catpath = eval('exepath("cat")')
+    eq({id=catchan, argv={catpath}, stream='job', mode='rpc', client = {}}, exec_lua ([[
       vim.rpcnotify(..., "nvim_call_function", 'chanclose', {..., 'rpc'})
       vim.rpcnotify(..., "nvim_subscribe", "daily_rant")
       return vim.api.nvim_get_chan_info(...)
     ]], catchan))
-    eq(2, eval('1+1'))  -- Still alive?
+    assert_alive()
     eq({false, 'Invalid channel: '..catchan},
       exec_lua ([[ return {pcall(vim.rpcrequest, ..., 'nvim_eval', '1+1')}]], catchan))
     retry(nil, 3000, function() eq({}, meths.get_chan_info(catchan)) end) -- cat be dead :(
diff --git a/test/functional/api/vim_spec.lua b/test/functional/api/vim_spec.lua
@@ -1,6 +1,7 @@
 local helpers = require('test.functional.helpers')(after_each)
 local Screen = require('test.functional.ui.screen')
 
+local fmt = string.format
 local NIL = helpers.NIL
 local clear, nvim, eq, neq = helpers.clear, helpers.nvim, helpers.eq, helpers.neq
 local command = helpers.command
@@ -1326,10 +1327,10 @@ describe('API', function()
     end)
   end)
 
-  describe('nvim_list_chans and nvim_get_chan_info', function()
+  describe('nvim_list_chans, nvim_get_chan_info', function()
     before_each(function()
-      command('autocmd ChanOpen * let g:opened_event = copy(v:event)')
-      command('autocmd ChanInfo * let g:info_event = copy(v:event)')
+      command('autocmd ChanOpen * let g:opened_event = deepcopy(v:event)')
+      command('autocmd ChanInfo * let g:info_event = deepcopy(v:event)')
     end)
     local testinfo = {
       stream = 'stdio',
@@ -1350,7 +1351,7 @@ describe('API', function()
       eq({}, meths.get_chan_info(10))
     end)
 
-    it('works for stdio channel', function()
+    it('stream=stdio channel', function()
       eq({[1]=testinfo,[2]=stderr}, meths.list_chans())
       eq(testinfo, meths.get_chan_info(1))
       eq(stderr, meths.get_chan_info(2))
@@ -1377,11 +1378,13 @@ describe('API', function()
       eq(info, meths.get_chan_info(1))
     end)
 
-    it('works for job channel', function()
+    it('stream=job channel', function()
       eq(3, eval("jobstart(['cat'], {'rpc': v:true})"))
+      local catpath = eval('exepath("cat")')
       local info = {
         stream='job',
         id=3,
+        argv={ catpath },
         mode='rpc',
         client={},
       }
@@ -1394,6 +1397,7 @@ describe('API', function()
       info = {
         stream='job',
         id=3,
+        argv={ catpath },
         mode='rpc',
         client = {
           name='amazing-cat',
@@ -1410,14 +1414,15 @@ describe('API', function()
          pcall_err(eval, 'rpcrequest(3, "nvim_set_current_buf", -1)'))
     end)
 
-    it('works for :terminal channel', function()
-      command(":terminal")
+    it('stream=job :terminal channel', function()
+      command(':terminal')
       eq({id=1}, meths.get_current_buf())
-      eq(3, meths.buf_get_option(1, "channel"))
+      eq(3, meths.buf_get_option(1, 'channel'))
 
       local info = {
         stream='job',
         id=3,
+        argv={ eval('exepath(&shell)') },
         mode='terminal',
         buffer = 1,
         pty='?',
@@ -1431,6 +1436,38 @@ describe('API', function()
       info.buffer = {id=1}
       eq({[1]=testinfo,[2]=stderr,[3]=info}, meths.list_chans())
       eq(info, meths.get_chan_info(3))
+
+      -- :terminal with args + running process.
+      command(':exe "terminal" shellescape(v:progpath) "-u NONE -i NONE"')
+      eq(-1, eval('jobwait([&channel], 0)[0]'))  -- Running?
+      local expected2 = {
+        stream = 'job',
+        id = 4,
+        argv = (
+          iswin() and {
+            eval('&shell'),
+            '/s',
+            '/c',
+            fmt('"%s -u NONE -i NONE"', eval('shellescape(v:progpath)')),
+          } or {
+            eval('&shell'),
+            eval('&shellcmdflag'),
+            fmt('%s -u NONE -i NONE', eval('shellescape(v:progpath)')),
+          }
+        ),
+        mode = 'terminal',
+        buffer = 2,
+        pty = '?',
+      }
+      local actual2 = eval('nvim_get_chan_info(&channel)')
+      expected2.pty = actual2.pty
+      eq(expected2, actual2)
+
+      -- :terminal with args + stopped process.
+      eq(1, eval('jobstop(&channel)'))
+      eval('jobwait([&channel], 1000)')  -- Wait.
+      expected2.pty = (iswin() and '?' or '')  -- pty stream was closed.
+      eq(expected2, eval('nvim_get_chan_info(&channel)'))
     end)
   end)
 
diff --git a/test/functional/core/job_spec.lua b/test/functional/core/job_spec.lua
@@ -5,6 +5,7 @@ local clear, eq, eval, exc_exec, feed_command, feed, insert, neq, next_msg, nvim
   helpers.insert, helpers.neq, helpers.next_msg, helpers.nvim,
   helpers.nvim_dir, helpers.ok, helpers.source,
   helpers.write_file, helpers.mkdir, helpers.rmdir
+local assert_alive = helpers.assert_alive
 local command = helpers.command
 local funcs = helpers.funcs
 local os_kill = helpers.os_kill
@@ -870,7 +871,7 @@ describe('jobs', function()
     -- loop tick. This is also prevented by try-block, so feed must be used.
     feed_command("call DoIt()")
     feed('<cr>') -- press RETURN
-    eq(2,eval('1+1'))
+    assert_alive()
   end)
 
   it('jobstop() kills entire process tree #6530', function()
