shell_escape: rename; refactor

- rename to shell_xescape_xquote
- move to os/shell.c
- disallow NULL argument
- eliminate casts, nesting
- test: empty shellxquote/shellxescape

src/nvim/os/shell.c

@@ -37,51 +37,6 @@ typedef struct {
 # include "os/shell.c.generated.h"
 #endif
 
-/// Process command string with 'shellxescape' (p_sxe) and 'shellxquote'
-/// (p_sxq)
-///
-/// @param cmd Command string
-/// @return NULL if `cmd` is NULL. Otherwise, a newly allocated command string.
-///         It must be freed with `xfree` when no longer needed.
-static char *shell_escape(const char *cmd)
-  FUNC_ATTR_MALLOC FUNC_ATTR_WARN_UNUSED_RESULT
-{
-  char *ncmd;
-
-  if (cmd == NULL) {
-    ncmd = NULL;
-  } else if (*p_sxq == NUL) {
-    ncmd = xstrdup(cmd);
-  } else {
-    const char *ecmd;
-    size_t ncmd_size;
-
-    if (*p_sxe != NUL && STRCMP(p_sxq, "(") == 0) {
-      ecmd = (char *)vim_strsave_escaped_ext((char_u *)cmd, p_sxe, '^', false);
-    } else {
-      ecmd = cmd;
-    }
-    ncmd_size = strlen(ecmd) + STRLEN(p_sxq) * 2 + 1;
-    ncmd = xmalloc(ncmd_size);
-
-    // When 'shellxquote' is '(', append ')'.
-    // When 'shellxquote' is '"(', append ')"'.
-    if (STRCMP(p_sxq, "(") == 0) {
-      vim_snprintf(ncmd, ncmd_size, "(%s)", ecmd);
-    } else if (STRCMP(p_sxq, "\"(") == 0) {
-      vim_snprintf(ncmd, ncmd_size, "\"(%s)\"", ecmd);
-    } else {
-      vim_snprintf(ncmd, ncmd_size, "%s%s%s", p_sxq, ecmd, p_sxq);
-    }
-
-    if (ecmd != (const char *)cmd) {
-      xfree((void *)ecmd);
-    }
-  }
-
-  return ncmd;
-}
-
 /// Builds the argument vector for running the user-configured 'shell' (p_sh)
 /// with an optional command prefixed by 'shellcmdflag' (p_shcf).
 ///
@@ -99,13 +54,12 @@ char **shell_build_argv(const char *cmd, const char *extra_args)
   size_t i = tokenize(p_sh, rv);
 
   if (extra_args) {
-    rv[i++] = xstrdup(extra_args);   // Push a copy of `extra_args`
+    rv[i++] = xstrdup(extra_args);        // Push a copy of `extra_args`
   }
 
   if (cmd) {
-    i += tokenize(p_shcf, rv + i);   // Split 'shellcmdflag'
+    i += tokenize(p_shcf, rv + i);        // Split 'shellcmdflag'
-    rv[i++] = shell_escape(cmd);     // Process command string with
+    rv[i++] = shell_xescape_xquote(cmd);  // Copy (and escape) `cmd`.
-                                     // 'shellxescape' and 'shellxquote'
   }
 
   rv[i] = NULL;
@@ -594,3 +548,39 @@ static void shell_write_cb(Stream *stream, void *data, int status)
 {
   stream_close(stream, NULL, NULL);
 }
+
+/// Applies 'shellxescape' (p_sxe) and 'shellxquote' (p_sxq) to a command.
+///
+/// @param cmd Command string
+/// @return    Escaped/quoted command string (allocated).
+static char *shell_xescape_xquote(const char *cmd)
+  FUNC_ATTR_NONNULL_ALL FUNC_ATTR_MALLOC FUNC_ATTR_WARN_UNUSED_RESULT
+{
+  if (*p_sxq == NUL) {
+    return xstrdup(cmd);
+  }
+
+  const char *ecmd = cmd;
+  if (*p_sxe != NUL && STRCMP(p_sxq, "(") == 0) {
+    ecmd = (char *)vim_strsave_escaped_ext((char_u *)cmd, p_sxe, '^', false);
+  }
+  size_t ncmd_size = strlen(ecmd) + STRLEN(p_sxq) * 2 + 1;
+  char *ncmd = xmalloc(ncmd_size);
+
+  // When 'shellxquote' is ( append ).
+  // When 'shellxquote' is "( append )".
+  if (STRCMP(p_sxq, "(") == 0) {
+    vim_snprintf(ncmd, ncmd_size, "(%s)", ecmd);
+  } else if (STRCMP(p_sxq, "\"(") == 0) {
+    vim_snprintf(ncmd, ncmd_size, "\"(%s)\"", ecmd);
+  } else {
+    vim_snprintf(ncmd, ncmd_size, "%s%s%s", p_sxq, ecmd, p_sxq);
+  }
+
+  if (ecmd != cmd) {
+    xfree((void *)ecmd);
+  }
+
+  return ncmd;
+}
+

test/unit/os/shell_spec.lua

@@ -12,10 +12,12 @@ local to_cstr = helpers.to_cstr
 local NULL = ffi.cast('void *', 0)
 
 describe('shell functions', function()
-  setup(function()
+  before_each(function()
     -- os_system() can't work when the p_sh and p_shcf variables are unset
     cimported.p_sh = to_cstr('/bin/bash')
     cimported.p_shcf = to_cstr('-c')
+    cimported.p_sxq = to_cstr('')
+    cimported.p_sxe = to_cstr('')
   end)
 
   local function shell_build_argv(cmd, extra_args)
@@ -35,13 +37,6 @@ describe('shell functions', function()
     return ret
   end
 
-  after_each(function()
-    cimported.p_sxq = to_cstr('')
-    cimported.p_sxe = to_cstr('')
-    cimported.p_sh = to_cstr('/bin/bash')
-    cimported.p_shcf = to_cstr('-c')
-  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
@@ -121,15 +116,11 @@ describe('shell functions', function()
           '-c', 'abc  def'},
          shell_build_argv('abc  def', 'ghi  jkl'))
     end)
-  end)
 
-  describe('shell_build_argv can deal with sxe and sxq', function()
+    it('applies shellxescape (p_sxe) and shellxquote (p_sxq)', function()
-    setup(function()
       cimported.p_sxq = to_cstr('(')
       cimported.p_sxe = to_cstr('"&|<>()@^')
-    end)
 
-    it('applies shellxescape and shellxquote', function()
       local argv = ffi.cast('char**',
                         cimported.shell_build_argv(to_cstr('echo &|<>()@^'), nil))
       eq(ffi.string(argv[0]), '/bin/bash')
@@ -138,8 +129,9 @@ describe('shell functions', function()
       eq(nil, argv[3])
     end)
 
-    it('applies shellxquote when shellxquote is "\\"("', function()
+    it('applies shellxquote="(', function()
       cimported.p_sxq = to_cstr('"(')
+      cimported.p_sxe = to_cstr('"&|<>()@^')
 
       local argv = ffi.cast('char**', cimported.shell_build_argv(
                                           to_cstr('echo -n some text'), nil))
@@ -149,7 +141,7 @@ describe('shell functions', function()
       eq(nil, argv[3])
     end)
 
-    it('applies shellxquote when shellxquote is "\\""', function()
+    it('applies shellxquote="', function()
       cimported.p_sxq = to_cstr('"')
       cimported.p_sxe = to_cstr('')
 
@@ -160,5 +152,14 @@ describe('shell functions', function()
       eq(ffi.string(argv[2]), '"echo -n some text"')
       eq(nil, argv[3])
     end)
+
+    it('with empty shellxquote/shellxescape', function()
+      local argv = ffi.cast('char**', cimported.shell_build_argv(
+                                          to_cstr('echo -n some text'), nil))
+      eq(ffi.string(argv[0]), '/bin/bash')
+      eq(ffi.string(argv[1]), '-c')
+      eq(ffi.string(argv[2]), 'echo -n some text')
+      eq(nil, argv[3])
+    end)
   end)
 end)