term: use an argument vector for termopen().

Old behaviour: termopen('cmd') would run `&shell &shcf "cmd"`, which
caused the functional tests to fail on some systems due to the process
not "owning" the terminal. Also, it is inconsistent with jobstart().

Modify termopen() so that &shell is not invoked, but maintain the old
behaviour with :terminal. Factor the common code for building the
argument vector from jobstart() and modify the functional tests to call
termopen() instead of :terminal (fixes #2354).

Also:
 * Add a 'name' option for termopen() so that `:terminal {cmd}` produces
   a buffer named "term//{cwd}/{cmd}" and termopen() users can customize
   the name.
 * Update the documentation.
 * Add functional tests for `:terminal` sinse its behaviour now differs
   from termopen(). Add "test/functional/fixtures/shell-test.c" and move
   "test/functional/job/tty-test.c" there, too.

Helped-by: Justin M. Keyes <@justinmk>

1 files changed, 50 insertions, 32 deletions

diff --git a/src/nvim/eval.c b/src/nvim/eval.c
index 1065f1caf9..ea5dbc8a3f 100644
--- a/src/nvim/eval.c
+++ b/src/nvim/eval.c
@@ -10779,47 +10779,62 @@ static void f_jobresize(typval_T *argvars, typval_T *rettv)
   rettv->vval.v_number = 1;
 }
 
-// "jobstart()" function
-static void f_jobstart(typval_T *argvars, typval_T *rettv)
+static char **list_to_argv(list_T *args)
 {
-  rettv->v_type = VAR_NUMBER;
-  rettv->vval.v_number = 0;
-
-  if (check_restricted() || check_secure()) {
-    return;
-  }
-
-  if (argvars[0].v_type != VAR_LIST
-      || (argvars[1].v_type != VAR_DICT && argvars[1].v_type != VAR_UNKNOWN)) {
-    // Wrong argument types
-    EMSG(_(e_invarg));
-    return;
-  }
-
-  list_T *args = argvars[0].vval.v_list;
-  // Assert that all list items are strings
   for (listitem_T *arg = args->lv_first; arg != NULL; arg = arg->li_next) {
     if (arg->li_tv.v_type != VAR_STRING) {
       EMSG(_(e_invarg));
-      return;
+      return NULL;
     }
   }
 
   int argc = args->lv_len;
   if (!argc) {
     EMSG(_("Argument vector must have at least one item"));
-    return;
+    return NULL;
   }
 
   assert(args->lv_first);
- 
+
   const char_u *exe = get_tv_string(&args->lv_first->li_tv);
   if (!os_can_exe(exe, NULL)) {
     // String is not executable
     EMSG2(e_jobexe, exe);
+    return NULL;
+  }
+  
+  // Build the argument vector
+  int i = 0;
+  char **argv = xcalloc(argc + 1, sizeof(char *));
+  for (listitem_T *arg = args->lv_first; arg != NULL; arg = arg->li_next) {
+    argv[i++] = xstrdup((char *) get_tv_string(&arg->li_tv));
+  }
+
+  return argv;
+}
+
+// "jobstart()" function
+static void f_jobstart(typval_T *argvars, typval_T *rettv)
+{
+  rettv->v_type = VAR_NUMBER;
+  rettv->vval.v_number = 0;
+
+  if (check_restricted() || check_secure()) {
+    return;
+  }
+
+  if (argvars[0].v_type != VAR_LIST
+      || (argvars[1].v_type != VAR_DICT && argvars[1].v_type != VAR_UNKNOWN)) {
+    // Wrong argument types
+    EMSG(_(e_invarg));
     return;
   }
 
+  char **argv = list_to_argv(argvars[0].vval.v_list);
+  if (!argv) {
+    return;  // Did error message in list_to_argv.
+  }
+
   dict_T *job_opts = NULL;
   ufunc_T *on_stdout = NULL, *on_stderr = NULL, *on_exit = NULL;
   if (argvars[1].v_type == VAR_DICT) {
@@ -10829,13 +10844,6 @@ static void f_jobstart(typval_T *argvars, typval_T *rettv)
     }
   }
 
-  // Build the argument vector
-  int i = 0;
-  char **argv = xcalloc(argc + 1, sizeof(char *));
-  for (listitem_T *arg = args->lv_first; arg != NULL; arg = arg->li_next) {
-    argv[i++] = xstrdup((char *) get_tv_string(&arg->li_tv));
-  }
-
   JobOptions opts = common_job_options(argv, on_stdout, on_stderr, on_exit,
       job_opts);
 
@@ -15155,13 +15163,18 @@ static void f_termopen(typval_T *argvars, typval_T *rettv)
     return;
   }
 
-  if (argvars[0].v_type != VAR_STRING
+  if (argvars[0].v_type != VAR_LIST
       || (argvars[1].v_type != VAR_DICT && argvars[1].v_type != VAR_UNKNOWN)) {
     // Wrong argument types
     EMSG(_(e_invarg));
     return;
   }
 
+  char **argv = list_to_argv(argvars[0].vval.v_list);
+  if (!argv) {
+    return;  // Did error message in list_to_argv.
+  }
+
   ufunc_T *on_stdout = NULL, *on_stderr = NULL, *on_exit = NULL;
   dict_T *job_opts = NULL;
   if (argvars[1].v_type == VAR_DICT) {
@@ -15171,7 +15184,6 @@ static void f_termopen(typval_T *argvars, typval_T *rettv)
     }
   }
 
-  char **argv = shell_build_argv((char *)argvars[0].vval.v_string, NULL);
   JobOptions opts = common_job_options(argv, on_stdout, on_stderr, on_exit,
       job_opts);
   opts.pty = true;
@@ -15197,10 +15209,16 @@ static void f_termopen(typval_T *argvars, typval_T *rettv)
     cwd = (char *)argvars[1].vval.v_string;
   }
   int pid = job_pid(job);
+
+  // Get the desired name of the buffer.
+  char *name = job_opts ?
+    (char *)get_dict_string(job_opts, (char_u *)"name", false) : NULL;
+  if (!name) {
+    name = argv[0];
+  }
   char buf[1024];
   // format the title with the pid to conform with the term:// URI 
-  snprintf(buf, sizeof(buf), "term://%s//%d:%s", cwd, pid,
-      (char *)argvars[0].vval.v_string);
+  snprintf(buf, sizeof(buf), "term://%s//%d:%s", cwd, pid, name);
   // at this point the buffer has no terminal instance associated yet, so unset
   // the 'swapfile' option to ensure no swap file will be created
   curbuf->b_p_swf = false;