Merge pull request #7844 from bfredl/shellout

shell: use msg functions for :!cmd so UTF-8 and binary is supported.

10 files changed, 80 insertions, 81 deletions

diff --git a/src/nvim/eval.c b/src/nvim/eval.c
index 1d483eee18..923b4527c2 100644
--- a/src/nvim/eval.c
+++ b/src/nvim/eval.c
@@ -19372,14 +19372,10 @@ void ex_echo(exarg_T *eap)
             }
             msg_putchar_attr((uint8_t)(*p), echo_attr);
           } else {
-            if (has_mbyte) {
-              int i = (*mb_ptr2len)((const char_u *)p);
+            int i = (*mb_ptr2len)((const char_u *)p);
 
-              (void)msg_outtrans_len_attr((char_u *)p, i, echo_attr);
-              p += i - 1;
-            } else {
-              (void)msg_outtrans_len_attr((char_u *)p, 1, echo_attr);
-            }
+            (void)msg_outtrans_len_attr((char_u *)p, i, echo_attr);
+            p += i - 1;
           }
         }
       }
diff --git a/src/nvim/ex_cmds.c b/src/nvim/ex_cmds.c
index 6ffd255dc7..9cfcecf8ad 100644
--- a/src/nvim/ex_cmds.c
+++ b/src/nvim/ex_cmds.c
@@ -1175,7 +1175,7 @@ static void do_filter(
   // to read the error messages. Otherwise errors are ignored, so you can see
   // the error messages from the command that appear on stdout; use 'u' to fix
   // the text.
-  // Pass on the kShellDoOut flag when the output is being redirected.
+  // Pass on the kShellOptDoOut flag when the output is being redirected.
   if (call_shell(
         cmd_buf,
         kShellOptFilter | shell_flags,
diff --git a/src/nvim/os/shell.c b/src/nvim/os/shell.c
index c205e4b3af..11e6a76939 100644
--- a/src/nvim/os/shell.c
+++ b/src/nvim/os/shell.c
@@ -123,6 +123,9 @@ int os_call_shell(char_u *cmd, ShellOpts opts, char_u *extra_args)
     if (opts & kShellOptRead) {
       output_ptr = &output;
       forward_output = false;
+    } else if (opts & kShellOptDoOut) {
+      // Caller has already redirected output
+      forward_output = false;
     }
   }
 
@@ -257,11 +260,25 @@ static int do_os_system(char **argv,
   // busy state.
   ui_busy_start();
   ui_flush();
+  if (forward_output) {
+    msg_sb_eol();
+    msg_start();
+    msg_no_more = true;
+    lines_left = -1;
+  }
   int exitcode = process_wait(proc, -1, NULL);
   if (!got_int && out_data_decide_throttle(0)) {
     // Last chunk of output was skipped; display it now.
     out_data_ring(NULL, SIZE_MAX);
   }
+  if (forward_output) {
+    // caller should decide if wait_return is invoked
+    no_wait_return++;
+    msg_end();
+    no_wait_return--;
+    msg_no_more = false;
+  }
+
   ui_busy_stop();
 
   // prepare the out parameters if requested
@@ -436,47 +453,17 @@ static void out_data_ring(char *output, size_t size)
 static void out_data_append_to_screen(char *output, size_t remaining,
                                       bool new_line)
 {
-  static colnr_T last_col = 0;  // Column of last row to append to.
-
-  size_t off = 0;
-  int last_row = (int)Rows - 1;
-
-  while (output != NULL && off < remaining) {
-    // Found end of line?
-    if (output[off] == NL) {
-      // Can we start a new line or do we need to continue the last one?
-      if (last_col == 0) {
-        screen_del_lines(0, 0, 1, (int)Rows, NULL);
-      }
-      screen_puts_len((char_u *)output, (int)off, last_row, last_col, 0);
-      last_col = 0;
-
-      size_t skip = off + 1;
-      output += skip;
-      remaining -= skip;
-      off = 0;
-      continue;
-    }
-
-    // TODO(bfredl): using msg_puts would be better until
-    // terminal emulation is implemented.
-    if (output[off] < 0x20) {
-      output[off] = ' ';
-    }
-
-    off++;
-  }
+  char *p = output, *end = output + remaining;
+  while (p < end) {
+    if (*p == '\n' || *p == '\r' || *p == TAB) {
+      msg_putchar_attr((uint8_t)(*p), 0);
+      p++;
+    } else {
+      int i = *p ? mb_ptr2len_len((char_u *)p, (int)(end-p)) : 1;
 
-  if (output != NULL && remaining) {
-    if (last_col == 0) {
-      screen_del_lines(0, 0, 1, (int)Rows, NULL);
+      (void)msg_outtrans_len_attr((char_u *)p, i, 0);
+      p += i;
     }
-    screen_puts_len((char_u *)output, (int)remaining, last_row, last_col, 0);
-    last_col += (colnr_T)remaining;
-  }
-
-  if (new_line) {
-    last_col = 0;
   }
 
   ui_flush();
diff --git a/src/nvim/quickfix.c b/src/nvim/quickfix.c
index 63252df3dc..ea2d13761c 100644
--- a/src/nvim/quickfix.c
+++ b/src/nvim/quickfix.c
@@ -3148,10 +3148,9 @@ void ex_make(exarg_T *eap)
   }
   msg_start();
   MSG_PUTS(":!");
-  msg_outtrans((char_u *) cmd);  // show what we are doing
+  msg_outtrans((char_u *)cmd);  // show what we are doing
 
-  // let the shell know if we are redirecting output or not
-  do_shell((char_u *) cmd, *p_sp != NUL ? kShellOptDoOut : 0);
+  do_shell((char_u *)cmd, 0);
 
 
   res = qf_init(wp, fname, (eap->cmdidx != CMD_make
diff --git a/test/functional/api/vim_spec.lua b/test/functional/api/vim_spec.lua
index a92acd36b1..0a0cb2e91c 100644
--- a/test/functional/api/vim_spec.lua
+++ b/test/functional/api/vim_spec.lua
@@ -15,6 +15,7 @@ local command = helpers.command
 local intchar2lua = global_helpers.intchar2lua
 local format_string = global_helpers.format_string
 local mergedicts_copy = global_helpers.mergedicts_copy
+local uname = global_helpers.uname
 
 describe('api', function()
   before_each(clear)
@@ -99,8 +100,9 @@ describe('api', function()
           [[echo nvim_command_output('echo "nested1\nnested2"') | ls]]))
     end)
 
-    it('does not return shell |:!| output', function()
-      eq(':!echo "foo"\r\n', nvim('command_output', [[!echo "foo"]]))
+    it('returns shell |:!| output', function()
+      local win_lf = (uname() == 'Windows' and '\r') or ''
+      eq(':!echo foo\r\n\nfoo'..win_lf..'\n', nvim('command_output', [[!echo foo]]))
     end)
 
     it("parse error: fails (specific error), does NOT update v:errmsg", function()
diff --git a/test/functional/eval/execute_spec.lua b/test/functional/eval/execute_spec.lua
index 91966ed3dd..c866359520 100644
--- a/test/functional/eval/execute_spec.lua
+++ b/test/functional/eval/execute_spec.lua
@@ -1,4 +1,5 @@
 local helpers = require('test.functional.helpers')(after_each)
+local global_helpers = require('test.helpers')
 local eq = helpers.eq
 local eval = helpers.eval
 local clear = helpers.clear
@@ -9,6 +10,7 @@ local funcs = helpers.funcs
 local Screen = require('test.functional.ui.screen')
 local command = helpers.command
 local feed = helpers.feed
+local uname = global_helpers.uname
 
 describe('execute()', function()
   before_each(clear)
@@ -118,9 +120,11 @@ describe('execute()', function()
     feed('<CR>')
   end)
 
-  -- This matches Vim behavior.
-  it('does not capture shell-command output', function()
-    eq('\n:!echo "foo"\13\n', funcs.execute('!echo "foo"'))
+  -- This deviates from vim behavior, but is consistent
+  -- with how nvim currently displays the output.
+  it('does capture shell-command output', function()
+    local win_lf = (uname() == 'Windows' and '\13') or ''
+    eq('\n:!echo foo\r\n\nfoo'..win_lf..'\n', funcs.execute('!echo foo'))
   end)
 
   describe('{silent} argument', function()
diff --git a/test/functional/ex_cmds/bang_filter_spec.lua b/test/functional/ex_cmds/bang_filter_spec.lua
index aaec983b73..636d732161 100644
--- a/test/functional/ex_cmds/bang_filter_spec.lua
+++ b/test/functional/ex_cmds/bang_filter_spec.lua
@@ -3,13 +3,14 @@
 local helpers = require('test.functional.helpers')(after_each)
 local feed, command, clear = helpers.feed, helpers.command, helpers.clear
 local mkdir, write_file, rmdir = helpers.mkdir, helpers.write_file, helpers.rmdir
+local feed_command = helpers.feed_command
 
 if helpers.pending_win32(pending) then return end
 
 local Screen = require('test.functional.ui.screen')
 
 
-describe('issues', function()
+describe(':! command', function()
   local screen
 
   before_each(function()
@@ -19,7 +20,12 @@ describe('issues', function()
     write_file('bang_filter_spec/f1', 'f1')
     write_file('bang_filter_spec/f2', 'f2')
     write_file('bang_filter_spec/f3', 'f3')
-    screen = Screen.new()
+    screen = Screen.new(53,10)
+    screen:set_default_attr_ids({
+      [1] = {bold = true, foreground = Screen.colors.Blue1},
+      [2] = {foreground = Screen.colors.Blue1},
+      [3] = {bold = true, foreground = Screen.colors.SeaGreen4},
+    })
     screen:attach()
   end)
 
@@ -27,25 +33,37 @@ describe('issues', function()
     rmdir('bang_filter_spec')
   end)
 
-  it('#3269 Last line of shell output is not truncated', function()
+  it("doesn't truncate Last line of shell output #3269", function()
     command([[nnoremap <silent>\l :!ls bang_filter_spec<cr>]])
     feed([[\l]])
     screen:expect([[
-      ~                                                    |
-      ~                                                    |
-      ~                                                    |
-      ~                                                    |
-      ~                                                    |
-      ~                                                    |
-      ~                                                    |
-      ~                                                    |
+      {1:~                                                    }|
+      {1:~                                                    }|
+      {1:~                                                    }|
+      {1:~                                                    }|
       :!ls bang_filter_spec                                |
-                                                           |
       f1                                                   |
       f2                                                   |
       f3                                                   |
-      Press ENTER or type command to continue^              |
+                                                           |
+      {3:Press ENTER or type command to continue}^              |
     ]])
   end)
 
+  it('handles binary and multibyte data', function()
+    feed_command('!cat test/functional/fixtures/shell_data.txt')
+    screen:expect([[
+      {1:~                                                    }|
+      {1:~                                                    }|
+      {1:~                                                    }|
+      :!cat test/functional/fixtures/shell_data.txt        |
+      {2:^@^A^B^C^D^E^F^G^H}                                   |
+      {2:^N^O^P^Q^R^S^T^U^V^W^X^Y^Z^[^\^]^^^_}                 |
+      ö 한글 {2:<a5><c3>}                                      |
+      t       {2:<ff>}                                         |
+                                                           |
+      {3:Press ENTER or type command to continue}^              |
+  ]])
+  end)
+
 end)
diff --git a/test/functional/fixtures/shell_data.txt b/test/functional/fixtures/shell_data.txt
new file mode 100644
index 0000000000..ef3506c5b1
--- /dev/null
+++ b/test/functional/fixtures/shell_data.txt
diff --git a/test/functional/ui/bufhl_spec.lua b/test/functional/ui/bufhl_spec.lua
index 091c45596d..5b38921e50 100644
--- a/test/functional/ui/bufhl_spec.lua
+++ b/test/functional/ui/bufhl_spec.lua
@@ -2,11 +2,11 @@ local helpers = require('test.functional.helpers')(after_each)
 local Screen = require('test.functional.ui.screen')
 
 local clear, feed, insert = helpers.clear, helpers.feed, helpers.insert
-local command, request, neq = helpers.command, helpers.request, helpers.neq
+local command, neq = helpers.command, helpers.neq
+local curbufmeths = helpers.curbufmeths
 
 describe('Buffer highlighting', function()
   local screen
-  local curbuf
 
   before_each(function()
     clear()
@@ -25,21 +25,14 @@ describe('Buffer highlighting', function()
       [9] = {foreground = Screen.colors.SlateBlue, underline = true},
       [10] = {foreground = Screen.colors.Red}
     })
-    curbuf = request('nvim_get_current_buf')
   end)
 
   after_each(function()
     screen:detach()
   end)
 
-  local function add_hl(...)
-    return request('nvim_buf_add_highlight', curbuf, ...)
-  end
-
-  local function clear_hl(...)
-    return request('nvim_buf_clear_highlight', curbuf, ...)
-  end
-
+  local add_hl = curbufmeths.add_highlight
+  local clear_hl = curbufmeths.clear_highlight
 
   it('works', function()
     insert([[
diff --git a/test/functional/ui/output_spec.lua b/test/functional/ui/output_spec.lua
index c6d564e8dc..da3f474e08 100644
--- a/test/functional/ui/output_spec.lua
+++ b/test/functional/ui/output_spec.lua
@@ -33,8 +33,8 @@ describe("shell command :!", function()
       {4:~                                                 }|
       {4:~                                                 }|
       {4:~                                                 }|
+      {4:~                                                 }|
       :!printf foo; sleep 200                           |
-                                                        |
       foo                                               |
       {3:-- TERMINAL --}                                    |
     ]])
@@ -56,11 +56,11 @@ describe("shell command :!", function()
     -- Final chunk of output should always be displayed, never skipped.
     -- (Throttling is non-deterministic, this test is merely a sanity check.)
     screen:expect([[
-      XXXXXXXXXX 2996                                   |
       XXXXXXXXXX 2997                                   |
       XXXXXXXXXX 2998                                   |
       XXXXXXXXXX 2999                                   |
       XXXXXXXXXX 3000                                   |
+                                                        |
       {10:Press ENTER or type command to continue}{1: }          |
       {3:-- TERMINAL --}                                    |
     ]])