shell/logging: Fix E730 with verbose system({List}) #9009

ref https://github.com/neovim/neovim/issues/9001#issuecomment-421843790

Steps to reproduce:
    :set verbose=9
    :call system(['echo'])
    E730: using List as a String

src/nvim/eval.c

@@ -16471,13 +16471,12 @@ static void get_system_output_as_rettv(typval_T *argvars, typval_T *rettv,
   }
 
   if (p_verbose > 3) {
-    char buf[NUMBUFLEN];
+    char *cmdstr = shell_argv_to_str(argv);
-    const char * cmd = tv_get_string_buf(argvars, buf);
-
     verbose_enter_scroll();
-    smsg(_("Calling shell to execute: \"%s\""), cmd);
+    smsg(_("Executing command: \"%s\""), cmdstr);
     msg_puts("\n\n");
     verbose_leave_scroll();
+    xfree(cmdstr);
   }
 
   if (do_profiling == PROF_YES) {

src/nvim/eval/typval.c

@@ -2632,7 +2632,7 @@ static const char *const str_errors[] = {
 
 #undef FUNC_ERROR
 
-/// Check that given value is a string or can be converted to it
+/// Check that given value is a VimL String or can be "cast" to it.
 ///
 /// Error messages are compatible with tv_get_string_chk() previously used for
 /// the same purpose.
@@ -2805,7 +2805,7 @@ float_T tv_get_float(const typval_T *const tv)
   return 0;
 }
 
-/// Get the string value of a VimL object
+/// Get the string value of a "stringish" VimL object.
 ///
 /// @param[in]  tv  Object to get value of.
 /// @param  buf  Buffer used to hold numbers and special variables converted to
@@ -2847,7 +2847,7 @@ const char *tv_get_string_buf_chk(const typval_T *const tv, char *const buf)
   return NULL;
 }
 
-/// Get the string value of a VimL object
+/// Get the string value of a "stringish" VimL object.
 ///
 /// @warning For number and special values it uses a single, static buffer. It
 ///          may be used only once, next call to get_tv_string may reuse it. Use
@@ -2866,7 +2866,7 @@ const char *tv_get_string_chk(const typval_T *const tv)
   return tv_get_string_buf_chk(tv, mybuf);
 }
 
-/// Get the string value of a VimL object
+/// Get the string value of a "stringish" VimL object.
 ///
 /// @warning For number and special values it uses a single, static buffer. It
 ///          may be used only once, next call to get_tv_string may reuse it. Use
@@ -2888,7 +2888,7 @@ const char *tv_get_string(const typval_T *const tv)
   return tv_get_string_buf((typval_T *)tv, mybuf);
 }
 
-/// Get the string value of a VimL object
+/// Get the string value of a "stringish" VimL object.
 ///
 /// @note tv_get_string_chk() and tv_get_string_buf_chk() are similar, but
 ///       return NULL on error.

src/nvim/misc1.c

@@ -2718,7 +2718,7 @@ int call_shell(char_u *cmd, ShellOpts opts, char_u *extra_shell_arg)
 
   if (p_verbose > 3) {
     verbose_enter();
-    smsg(_("Calling shell to execute: \"%s\""), cmd == NULL ? p_sh : cmd);
+    smsg(_("Executing command: \"%s\""), cmd == NULL ? p_sh : cmd);
     msg_putchar('\n');
     verbose_leave();
   }

src/nvim/os/shell.c

@@ -80,21 +80,53 @@ char **shell_build_argv(const char *cmd, const char *extra_args)
 void shell_free_argv(char **argv)
 {
   char **p = argv;
-
   if (p == NULL) {
     // Nothing was allocated, return
     return;
   }
-
   while (*p != NULL) {
     // Free each argument
     xfree(*p);
     p++;
   }
-
   xfree(argv);
 }
 
+/// Joins shell arguments from `argv` into a new string.
+/// If the result is too long it is truncated with ellipsis ("...").
+///
+/// @returns[allocated] `argv` joined to a string.
+char *shell_argv_to_str(char **const argv)
+  FUNC_ATTR_NONNULL_ALL
+{
+  size_t n = 0;
+  char **p = argv;
+  char *rv = xcalloc(256, sizeof(*rv));
+  const size_t maxsize = (256 * sizeof(*rv));
+  if (*p == NULL) {
+    return rv;
+  }
+  while (*p != NULL) {
+    xstrlcat(rv, "'", maxsize);
+    xstrlcat(rv, *p, maxsize);
+    n = xstrlcat(rv,  "' ", maxsize);
+    if (n >= maxsize) {
+      break;
+    }
+    p++;
+  }
+  if (n < maxsize) {
+    rv[n - 1] = '\0';
+  } else {
+    // Command too long, show ellipsis: "/bin/bash 'foo' 'bar'..."
+    rv[maxsize - 4] = '.';
+    rv[maxsize - 3] = '.';
+    rv[maxsize - 2] = '.';
+    rv[maxsize - 1] = '\0';
+  }
+  return rv;
+}
+
 /// Calls the user-configured 'shell' (p_sh) for running a command or wildcard
 /// expansion.
 ///

test/functional/eval/system_spec.lua

@@ -203,23 +203,13 @@ describe('system()', function()
     end)
 
     it('prints verbose information', function()
+      screen:try_resize(72, 14)
       feed(':4verbose echo system("echo hi")<cr>')
-      screen:expect([[
+      if iswin() then
-                                                             |
+        screen:expect{any=[[Executing command: "'cmd.exe' '/s' '/c' '"echo hi"'"]]}
-        ~                                                    |
+      else
-        ~                                                    |
+        screen:expect{any=[[Executing command: "'/[^']*sh' '%-c' 'echo hi'"]]}
-        ~                                                    |
+      end
-        ~                                                    |
-        ~                                                    |
-        ~                                                    |
-        ~                                                    |
-                                                             |
-        Calling shell to execute: "echo hi"                  |
-                                                             |
-        hi                                                   |
-                                                             |
-        Press ENTER or type command to continue^              |
-      ]])
       feed('<cr>')
     end)
 

test/functional/ui/screen.lua

@@ -243,7 +243,7 @@ local ext_keys = {
 --              nothing is ignored.
 -- condition:   Function asserting some arbitrary condition. Return value is
 --              ignored, throw an error (use eq() or similar) to signal failure.
--- any:         A string that should be present on any line of the screen.
+-- any:         Lua pattern string expected to match a screen line.
 -- mode:        Expected mode as signaled by "mode_change" event
 --
 -- The following keys should be used to expect the state of various ext_

test/unit/os/shell_spec.lua

@@ -25,6 +25,7 @@ describe('shell functions', function()
     local res = cimported.shell_build_argv(
         cmd and to_cstr(cmd),
         extra_args and to_cstr(extra_args))
+    -- `res` is zero-indexed (C pointer, not Lua table)!
     local argc = 0
     local ret = {}
     -- Explicitly free everything, so if it is not in allocated memory it will
@@ -38,6 +39,26 @@ describe('shell functions', function()
     return ret
   end
 
+  local function shell_argv_to_str(argv_table)
+    -- C string array (char **).
+    local argv = (argv_table
+                  and ffi.new("char*[?]", #argv_table+1)
+                  or NULL)
+
+    local argc = 1
+    while argv_table ~= nil and argv_table[argc] ~= nil do
+      -- `argv` is zero-indexed (C pointer, not Lua table)!
+      argv[argc - 1] = to_cstr(argv_table[argc])
+      argc = argc + 1
+    end
+    if argv_table ~= nil then
+      argv[argc - 1] = NULL
+    end
+
+    local res = cimported.shell_argv_to_str(argv)
+    return ffi.string(res)
+  end
+
   local function os_system(cmd, input)
     local input_or = input and to_cstr(input) or NULL
     local input_len = (input ~= nil) and string.len(input) or 0
@@ -151,4 +172,14 @@ describe('shell functions', function()
       eq(nil, argv[3])
     end)
   end)
+
+  itp('shell_argv_to_str', function()
+    eq('', shell_argv_to_str({ nil }))
+    eq("''", shell_argv_to_str({ '' }))
+    eq("'foo' '' 'bar'", shell_argv_to_str({ 'foo', '', 'bar' }))
+    eq("'/bin/sh' '-c' 'abc  def'", shell_argv_to_str({'/bin/sh', '-c', 'abc  def'}))
+    eq("'abc  def' 'ghi  jkl'", shell_argv_to_str({'abc  def', 'ghi  jkl'}))
+    eq("'/bin/sh' '-c' 'abc  def' '"..('x'):rep(225).."...",
+       shell_argv_to_str({'/bin/sh', '-c', 'abc  def', ('x'):rep(999)}))
+  end)
 end)