Merge pull request #4364 from ZyX-I/proper-e-term

Replace hack used to run TermOpen with nested modifier

src/nvim/main.c

@@ -317,15 +317,16 @@ int main(int argc, char **argv)
   }
 
   // open terminals when opening files that start with term://
-  do_cmdline_cmd("autocmd BufReadCmd term://* "
+#define PROTO "term://"
+  do_cmdline_cmd("autocmd BufReadCmd " PROTO "* nested "
                  ":call termopen( "
                  // Capture the command string
                  "matchstr(expand(\"<amatch>\"), "
-                 "'\\c\\mterm://\\%(.\\{-}//\\%(\\d\\+:\\)\\?\\)\\?\\zs.*'), "
+                 "'\\c\\m" PROTO "\\%(.\\{-}//\\%(\\d\\+:\\)\\?\\)\\?\\zs.*'), "
                  // capture the working directory
                  "{'cwd': get(matchlist(expand(\"<amatch>\"), "
-                 "'\\c\\mterm://\\(.\\{-}\\)//'), 1, '')})"
-                 "|doautocmd TermOpen");
+                 "'\\c\\m" PROTO "\\(.\\{-}\\)//'), 1, '')})");
+#undef PROTO
 
   /* Execute --cmd arguments. */
   exe_pre_commands(&params);

test/functional/fixtures/shell-test.c

@@ -1,25 +1,59 @@
-// A simple implementation of a shell for testing
-// `termopen([&sh, &shcf, '{cmd'}])` and `termopen([&sh])`.
-//
-// If launched with no arguments, prints "ready $ ", otherwise prints
-// "ready $ {cmd}\n".
-
 #include <stdio.h>
 #include <string.h>
+#include <stdint.h>
+
+static void help(void)
+{
+  puts("A simple implementation of a shell for testing termopen().");
+  puts("");
+  puts("Usage:");
+  puts("  shell-test --help");
+  puts("    Prints this help to stdout.");
+  puts("  shell-test");
+  puts("  shell-test EXE");
+  puts("    Prints \"ready $ \" to stderr.");
+  puts("  shell-test EXE \"prog args...\"");
+  puts("    Prints \"ready $ prog args...\\n\" 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\"'");
+  puts("      0: test");
+  puts("      ...");
+  puts("      96: test");
+  puts("    will be printed because byte `a' is equal to 97.");
+}
 
 int main(int argc, char **argv)
 {
-  fprintf(stderr, "ready $ ");
-
-  if (argc == 3) {
-    // argv should be {"terminal-test", "EXE", "prog args..."}
-    if (strcmp(argv[1], "EXE") != 0) {
-      fprintf(stderr, "first argument must be 'EXE'\n");
-      return 2;
-    }
-
-    fprintf(stderr, "%s\n", argv[2]);
+  if (argc == 2 && strcmp(argv[1], "--help") == 0) {
+    help();
   }
 
-  return 0;
+  if (argc >= 2) {
+    if (strcmp(argv[1], "EXE") == 0) {
+      fprintf(stderr, "ready $ ");
+      if (argc >= 3) {
+        fprintf(stderr, "%s\n", argv[2]);
+      }
+    } else if (strcmp(argv[1], "REP") == 0) {
+      if (argc < 4) {
+        fprintf(stderr, "Not enough REP arguments\n");
+        return 4;
+      }
+      uint8_t number = (uint8_t) *argv[2];
+      for (uint8_t i = 0; i < number; i++) {
+        printf("%d: %s\n", (int) i, argv[3]);
+      }
+    } else {
+      fprintf(stderr, "Unknown first argument\n");
+      return 3;
+    }
+    return 0;
+  } else if (argc == 1) {
+    fprintf(stderr, "ready $ ");
+    return 0;
+  } else {
+    fprintf(stderr, "Missing first argument\n");
+    return 2;
+  }
 }

test/functional/terminal/edit_spec.lua

@@ -0,0 +1,76 @@
+local helpers = require('test.functional.helpers')
+local screen = require('test.functional.ui.screen')
+
+local curbufmeths = helpers.curbufmeths
+local curwinmeths = helpers.curwinmeths
+local nvim_dir = helpers.nvim_dir
+local command = helpers.command
+local meths = helpers.meths
+local clear = helpers.clear
+local eq = helpers.eq
+
+describe(':edit term://*', function()
+  local get_screen = function(columns, lines)
+    local scr = screen.new(columns, lines)
+    scr:attach(false)
+    return scr
+  end
+
+  before_each(function()
+    clear()
+    meths.set_option('shell', nvim_dir .. '/shell-test')
+    meths.set_option('shellcmdflag', 'EXE')
+  end)
+
+  it('runs TermOpen event', function()
+    meths.set_var('termopen_runs', {})
+    command('autocmd TermOpen * :call add(g:termopen_runs, expand("<amatch>"))')
+    command('edit term://')
+    termopen_runs = meths.get_var('termopen_runs')
+    eq(1, #termopen_runs)
+    eq(termopen_runs[1], termopen_runs[1]:match('^term://.//%d+:$'))
+  end)
+
+  it('runs TermOpen early enough to respect terminal_scrollback_buffer_size', function()
+    local columns, lines = 20, 4
+    local scr = get_screen(columns, lines)
+    local rep = 'a'
+    meths.set_option('shellcmdflag', 'REP ' .. rep)
+    local rep_size = rep:byte()
+    local sb = 10
+    local gsb = 20
+    meths.set_var('terminal_scrollback_buffer_size', gsb)
+    command('autocmd TermOpen * :let b:terminal_scrollback_buffer_size = '
+            .. tostring(sb))
+    command('edit term://foobar')
+    local bufcontents = {}
+    local winheight = curwinmeths.get_height()
+    -- I have no idea why there is + 4 needed. But otherwise it works fine with 
+    -- different scrollbacks.
+    local shift = -4
+    local buf_cont_start = rep_size - 1 - sb - winheight - shift
+    local bufline = function(i) return ('%d: foobar'):format(i) end
+    for i = buf_cont_start,(rep_size - 1) do
+      bufcontents[#bufcontents + 1] = bufline(i)
+    end
+    bufcontents[#bufcontents + 1] = ''
+    bufcontents[#bufcontents + 1] = '[Process exited 0]'
+    -- Do not ask me why displayed screen is one line *before* buffer
+    -- contents: buffer starts with 87:, screen with 86:.
+    local exp_screen = '\n'
+    local did_cursor = false
+    local shift = 10
+    for i = 0,(winheight - 1) do
+      local line = bufline(buf_cont_start + i - 1)
+      exp_screen = (exp_screen
+                    .. (did_cursor and '' or '^')
+                    .. line
+                    .. (' '):rep(columns - #line)
+                    .. '|\n')
+      did_cursor = true
+    end
+    exp_screen = exp_screen .. (' '):rep(columns) .. '|\n'
+    scr:expect(exp_screen)
+    eq(bufcontents, curbufmeths.get_line_slice(1, -1, true, true))
+  end)
+end)

test/functional/ui/screen.lua

@@ -219,12 +219,22 @@ function Screen:expect(expected, attr_ids, attr_ignore)
   local ids = attr_ids or self._default_attr_ids
   local ignore = attr_ignore or self._default_attr_ignore
   self:wait(function()
+    local actual_rows = {}
     for i = 1, self._height do
-      local expected_row = expected_rows[i]
-      local actual_row = self:_row_repr(self._rows[i], ids, ignore)
-      if expected_row ~= actual_row then
-        return 'Row '..tostring(i)..' didn\'t match.\nExpected: "'..
-               expected_row..'"\nActual:   "'..actual_row..'"'
+      actual_rows[i] = self:_row_repr(self._rows[i], ids, ignore)
+    end
+    for i = 1, self._height do
+      if expected_rows[i] ~= actual_rows[i] then
+        local msg_expected_rows = {}
+        for i = 1, #expected_rows do msg_expected_rows[i] = expected_rows[i] end
+        msg_expected_rows[i] = '*' .. msg_expected_rows[i]
+        actual_rows[i] = '*' .. actual_rows[i]
+        msg = (
+          'Row ' .. tostring(i) .. ' didn\'t match.\n'
+          .. 'Expected:\n|' .. table.concat(msg_expected_rows, '|\n|') .. '|\n'
+          .. 'Actual:\n|' .. table.concat(actual_rows, '|\n|') .. '|'
+        )
+        return msg
       end
     end
   end)