terminal: Support extra arguments in 'shell'. #4504

Tokenize p_sh if used as default in ex_terminal(). Previously p_sh was
used as the first arg in a list when calling termopen(), this would try
to call an untokenized version of shell, meaning if you had an argument
in 'shell':
    set shell=/bin/bash\ --login
the command would fail.

Helped-by: oni-link <knil.ino@gmail.com>

Closes #3999

src/nvim/ex_docmd.c

@@ -9627,26 +9627,38 @@ static void ex_folddo(exarg_T *eap)
 
 static void ex_terminal(exarg_T *eap)
 {
-  char *name = (char *)p_sh;  // Default to 'shell' if {cmd} is not given.
+  char ex_cmd[1024];
-  bool mustfree = false;
-  char *lquote = "['";
-  char *rquote = "']";
 
-  if (*eap->arg != NUL) {
+  if (*eap->arg != NUL) {  // Run {cmd} in 'shell'.
-    name = (char *)vim_strsave_escaped(eap->arg, (char_u *)"\"\\");
+    char *name = (char *)vim_strsave_escaped(eap->arg, (char_u *)"\"\\");
-    mustfree = true;
+    snprintf(ex_cmd, sizeof(ex_cmd),
-    lquote = rquote = "\"";
+             ":enew%s | call termopen(\"%s\") | startinsert",
-  }
+             eap->forceit ? "!" : "", name);
-
-  char ex_cmd[512];
-  snprintf(ex_cmd, sizeof(ex_cmd),
-           ":enew%s | call termopen(%s%s%s) | startinsert",
-           eap->forceit==TRUE ? "!" : "", lquote, name, rquote);
-  do_cmdline_cmd(ex_cmd);
-
-  if (mustfree) {
     xfree(name);
+  } else {  // No {cmd}: run the job with tokenized 'shell'.
+    if (*p_sh == NUL) {
+      EMSG(_(e_shellempty));
+      return;
+    }
+
+    char **argv = shell_build_argv(NULL, NULL);
+    char **p = argv;
+    char tempstring[512];
+    char shell_argv[512] = { 0 };
+
+    while (*p != NULL) {
+      snprintf(tempstring, sizeof(tempstring), ",\"%s\"", *p);
+      xstrlcat(shell_argv, tempstring, sizeof(shell_argv));
+      p++;
+    }
+    shell_free_argv(argv);
+
+    snprintf(ex_cmd, sizeof(ex_cmd),
+             ":enew%s | call termopen([%s]) | startinsert",
+             eap->forceit ? "!" : "", shell_argv + 1);
   }
+
+  do_cmdline_cmd(ex_cmd);
 }
 
 /// Checks if `cmd` is "previewable" (i.e. supported by 'inccommand').

src/nvim/os/shell.c

@@ -39,12 +39,13 @@ typedef struct {
 #endif
 
 /// Builds the argument vector for running the user-configured 'shell' (p_sh)
-/// with an optional command prefixed by 'shellcmdflag' (p_shcf).
+/// with an optional command prefixed by 'shellcmdflag' (p_shcf). E.g.:
+///
+///   ["shell", "-extra_args", "-shellcmdflag", "command with spaces"]
 ///
 /// @param cmd Command string, or NULL to run an interactive shell.
 /// @param extra_args Extra arguments to the shell, or NULL.
-/// @return A newly allocated argument vector. It must be freed with
+/// @return Newly allocated argument vector. Must be freed with shell_free_argv.
-///         `shell_free_argv` when no longer needed.
 char **shell_build_argv(const char *cmd, const char *extra_args)
   FUNC_ATTR_NONNULL_RET FUNC_ATTR_MALLOC
 {

test/functional/fixtures/shell-test.c

@@ -12,8 +12,12 @@ static void help(void)
   puts("  shell-test");
   puts("  shell-test EXE");
   puts("    Prints \"ready $ \" to stderr.");
+  puts("  shell-test -t {prompt text}");
+  puts("    Prints \"{prompt text} $ \" to stderr.");
   puts("  shell-test EXE \"prog args...\"");
   puts("    Prints \"ready $ prog args...\\n\" to stderr.");
+  puts("  shell-test -t {prompt text} EXE \"prog args...\"");
+  puts("    Prints \"{prompt text} $ progs args...\" to stderr.");
   puts("  shell-test REP {byte} \"line line line\"");
   puts("    Prints \"{lnr}: line line line\\n\" to stdout {byte} times.");
   puts("    I.e. for `shell-test REP ab \"test\"'");
@@ -30,7 +34,17 @@ int main(int argc, char **argv)
   }
 
   if (argc >= 2) {
-    if (strcmp(argv[1], "EXE") == 0) {
+    if (strcmp(argv[1], "-t") == 0) {
+      if (argc < 3) {
+        fprintf(stderr,"Missing prompt text for -t option\n");
+        return 5;
+      } else {
+        fprintf(stderr, "%s $ ", argv[2]);
+        if (argc >= 5 && (strcmp(argv[3], "EXE") == 0)) {
+          fprintf(stderr, "%s\n", argv[4]);
+        }
+      }
+    } else if (strcmp(argv[1], "EXE") == 0) {
       fprintf(stderr, "ready $ ");
       if (argc >= 3) {
         fprintf(stderr, "%s\n", argv[2]);

test/functional/terminal/ex_terminal_spec.lua

@@ -79,6 +79,30 @@ describe(':terminal (with fake shell)', function()
     ]])
   end)
 
+  it("with no argument, and 'shell' is set to empty string", function()
+    nvim('set_option', 'shell', '')
+    terminal_with_fake_shell()
+    wait()
+    screen:expect([[
+      ^                                                  |
+      ~                                                 |
+      ~                                                 |
+      E91: 'shell' option is empty                      |
+    ]])
+  end)
+
+  it("with no argument, but 'shell' has arguments, acts like termopen()", function()
+    nvim('set_option', 'shell', nvim_dir..'/shell-test -t jeff')
+    terminal_with_fake_shell()
+    wait()
+    screen:expect([[
+      jeff $                                            |
+      [Process exited 0]                                |
+                                                        |
+      -- TERMINAL --                                    |
+    ]])
+  end)
+
   it('executes a given command through the shell', function()
     terminal_with_fake_shell('echo hi')
     wait()
@@ -90,6 +114,18 @@ describe(':terminal (with fake shell)', function()
     ]])
   end)
 
+  it("executes a given command through the shell, when 'shell' has arguments", function()
+    nvim('set_option', 'shell', nvim_dir..'/shell-test -t jeff')
+    terminal_with_fake_shell('echo hi')
+    wait()
+    screen:expect([[
+      jeff $ echo hi                                    |
+                                                        |
+      [Process exited 0]                                |
+      -- TERMINAL --                                    |
+    ]])
+  end)
+
   it('allows quotes and slashes', function()
     terminal_with_fake_shell([[echo 'hello' \ "world"]])
     wait()