diff options
-rw-r--r-- | runtime/doc/various.txt | 12 | ||||
-rw-r--r-- | runtime/doc/vim_diff.txt | 5 | ||||
-rw-r--r-- | src/nvim/main.c | 2 | ||||
-rw-r--r-- | src/nvim/os/shell.c | 91 | ||||
-rw-r--r-- | test/functional/eval/execute_spec.lua | 17 | ||||
-rw-r--r-- | test/functional/terminal/helpers.lua | 1 | ||||
-rw-r--r-- | test/functional/ui/output_spec.lua | 8 | ||||
-rw-r--r-- | test/functional/ui/screen.lua | 47 |
8 files changed, 158 insertions, 25 deletions
diff --git a/runtime/doc/various.txt b/runtime/doc/various.txt index a1bf379d86..625416146e 100644 --- a/runtime/doc/various.txt +++ b/runtime/doc/various.txt @@ -255,14 +255,20 @@ g8 Print the hex values of the bytes used in the backslashes are before the newline, only one is removed. - On Unix the command normally runs in a non-interactive - shell. If you want an interactive shell to be used - (to use aliases) set 'shellcmdflag' to "-ic". + The command runs in a non-interactive shell connected + to a pipe (not a terminal). Use |:terminal| to run an + interactive shell connected to a terminal. + For Win32 also see |:!start|. After the command has been executed, the timestamp and size of the current file is checked |timestamp|. + If the command produces a lot of output the displayed + output will be "throttled" so the command can execute + quickly without waiting for the display. This only + affects the display, no data is lost. + Vim redraws the screen after the command is finished, because it may have printed any text. This requires a hit-enter prompt, so that you can read any messages. diff --git a/runtime/doc/vim_diff.txt b/runtime/doc/vim_diff.txt index c4795bec57..1940bc55fa 100644 --- a/runtime/doc/vim_diff.txt +++ b/runtime/doc/vim_diff.txt @@ -155,6 +155,11 @@ are always available and may be used simultaneously in separate plugins. The |system()| does not support writing/reading "backgrounded" commands. |E5677| +Nvim truncates ("throttles") shell-command messages echoed by |:!|, |:grep|, +and |:make|. No data is lost, this only affects display. This makes things +faster, but may seem weird for commands like ":!cat foo". Use ":te cat foo" +instead, |:terminal| output is never throttled. + |mkdir()| behaviour changed: 1. Assuming /tmp/foo does not exist and /tmp can be written to mkdir('/tmp/foo/bar', 'p', 0700) will create both /tmp/foo and /tmp/foo/bar diff --git a/src/nvim/main.c b/src/nvim/main.c index 9b9976ac0a..12227565f3 100644 --- a/src/nvim/main.c +++ b/src/nvim/main.c @@ -534,7 +534,7 @@ int main(int argc, char **argv) } TIME_MSG("before starting main loop"); - ILOG("Starting Neovim main loop."); + ILOG("starting main loop"); /* * Call the main command loop. This never returns. diff --git a/src/nvim/os/shell.c b/src/nvim/os/shell.c index 9e6effd21b..a300984f63 100644 --- a/src/nvim/os/shell.c +++ b/src/nvim/os/shell.c @@ -187,6 +187,8 @@ static int do_os_system(char **argv, bool silent, bool forward_output) { + out_data_throttle(NULL, 0); // Initialize throttle for this shell command. + // the output buffer DynamicBuffer buf = DYNAMIC_BUFFER_INIT; stream_read_cb data_cb = system_data_cb; @@ -253,8 +255,8 @@ static int do_os_system(char **argv, wstream_set_write_cb(&in, shell_write_cb, NULL); } - // invoke busy_start here so event_poll_until wont change the busy state for - // the UI + // Invoke busy_start here so LOOP_PROCESS_EVENTS_UNTIL will not change the + // busy state. ui_busy_start(); ui_flush(); int status = process_wait(proc, -1, NULL); @@ -309,6 +311,86 @@ static void system_data_cb(Stream *stream, RBuffer *buf, size_t count, dbuf->len += nread; } +/// Tracks output received for the current executing shell command. To avoid +/// flooding the UI, output is periodically skipped and a pulsing "..." is +/// shown instead. Tracking depends on the synchronous/blocking nature of ":!". +// +/// Purpose: +/// 1. CTRL-C is more responsive. #1234 #5396 +/// 2. Improves performance of :! (UI, esp. TUI, is the bottleneck here). +/// 3. Avoids OOM during long-running, spammy :!. +/// +/// Note: +/// - Throttling "solves" the issue for *all* UIs, on all platforms. +/// - Unlikely that users will miss useful output: humans do not read >100KB. +/// - Caveat: Affects execute(':!foo'), but that is not a "very important" +/// case; system('foo') should be used for large outputs. +/// +/// Vim does not need this hack because: +/// 1. :! in terminal-Vim runs in cooked mode, so CTRL-C is caught by the +/// terminal and raises SIGINT out-of-band. +/// 2. :! in terminal-Vim uses a tty (Nvim uses pipes), so commands +/// (e.g. `git grep`) may page themselves. +/// +/// @returns true if output was skipped and pulse was displayed +static bool out_data_throttle(char *output, size_t size) +{ +#define NS_1_SECOND 1000000000 // 1s, in ns +#define THRESHOLD 1024 * 10 // 10KB, "a few screenfuls" of data. + static uint64_t started = 0; // Start time of the current throttle. + static size_t received = 0; // Bytes observed since last throttle. + static size_t visit = 0; // "Pulse" count of the current throttle. + static size_t max_visits = 0; + static char pulse_msg[] = { ' ', ' ', ' ', '\0' }; + + if (output == NULL) { + started = received = visit = 0; + max_visits = 10; + return false; + } + + received += size; + if (received < THRESHOLD + // Display at least the first chunk of output even if it is >=THRESHOLD. + || (!started && received < size + 1000)) { + return false; + } + + if (!visit) { + started = os_hrtime(); + } + + if (visit >= max_visits) { + uint64_t since = os_hrtime() - started; + if (since < NS_1_SECOND) { + // Adjust max_visits based on the current relative performance. + // Each "pulse" period should last >=1 second so that it is perceptible. + max_visits = (2 * max_visits); + } else { + received = visit = 0; + } + } + + if (received && ++visit <= max_visits) { + // Pulse "..." at the bottom of the screen. + size_t tick = (visit == max_visits) + ? 3 // Force all dots "..." on last visit. + : (visit + 1) % 4; + pulse_msg[0] = (tick == 0) ? ' ' : '.'; + pulse_msg[1] = (tick == 0 || 1 == tick) ? ' ' : '.'; + pulse_msg[2] = (tick == 0 || 1 == tick || 2 == tick) ? ' ' : '.'; + if (visit == 1) { + screen_del_lines(0, 0, 1, (int)Rows, NULL); + } + int lastrow = (int)Rows - 1; + screen_puts_len((char_u *)pulse_msg, ARRAY_SIZE(pulse_msg), lastrow, 0, 0); + ui_flush(); + return true; + } + + return false; +} + /// Continue to append data to last screen line. /// /// @param output Data to append to screen lines. @@ -319,6 +401,11 @@ static void append_to_screen_end(char *output, size_t remaining, bool new_line) // Column of last row to start appending data to. static colnr_T last_col = 0; + if (out_data_throttle(output, remaining)) { + last_col = 0; + return; + } + size_t off = 0; int last_row = (int)Rows - 1; diff --git a/test/functional/eval/execute_spec.lua b/test/functional/eval/execute_spec.lua index b5b481435a..fc13c0a72b 100644 --- a/test/functional/eval/execute_spec.lua +++ b/test/functional/eval/execute_spec.lua @@ -12,16 +12,16 @@ local feed = helpers.feed describe('execute()', function() before_each(clear) - it('returns the same result with :redir', function() + it('captures the same result as :redir', function() eq(redir_exec('messages'), funcs.execute('messages')) end) - it('returns the output of the commands if the argument is List', function() + it('captures the concatenated outputs of a List of commands', function() eq("foobar", funcs.execute({'echon "foo"', 'echon "bar"'})) eq("\nfoo\nbar", funcs.execute({'echo "foo"', 'echo "bar"'})) end) - it('supports the nested redirection', function() + it('supports nested redirection', function() source([[ function! g:Foo() let a = '' @@ -43,17 +43,17 @@ describe('execute()', function() eq('42', funcs.execute([[echon execute("echon execute('echon 42')")]])) end) - it('returns the transformed string', function() + it('captures a transformed string', function() eq('^A', funcs.execute('echon "\\<C-a>"')) end) - it('returns the empty string if the argument list is empty', function() + it('returns empty string if the argument list is empty', function() eq('', funcs.execute({})) eq(0, exc_exec('let g:ret = execute(v:_null_list)')) eq('', eval('g:ret')) end) - it('returns the errors', function() + it('captures errors', function() local ret ret = exc_exec('call execute(0.0)') eq('Vim(call):E806: using Float as a String', ret) @@ -69,6 +69,11 @@ describe('execute()', function() eq('Vim(call):E729: using Funcref as a String', ret) end) + -- This matches Vim behavior. + it('does not capture shell-command output', function() + eq('\n:!echo "foo"\13\n', funcs.execute('!echo "foo"')) + end) + it('silences command run inside', function() local screen = Screen.new(40, 5) screen:attach() diff --git a/test/functional/terminal/helpers.lua b/test/functional/terminal/helpers.lua index aacf109f2f..ae5e6d4b1f 100644 --- a/test/functional/terminal/helpers.lua +++ b/test/functional/terminal/helpers.lua @@ -51,6 +51,7 @@ local function screen_setup(extra_height, command) [7] = {foreground = 130}, [8] = {foreground = 15, background = 1}, -- error message [9] = {foreground = 4}, + [10] = {foreground = 2}, -- "Press ENTER" in embedded :terminal session. }) screen:attach({rgb=false}) diff --git a/test/functional/ui/output_spec.lua b/test/functional/ui/output_spec.lua index c58bbe9147..4aae2edfaa 100644 --- a/test/functional/ui/output_spec.lua +++ b/test/functional/ui/output_spec.lua @@ -1,5 +1,6 @@ local session = require('test.functional.helpers')(after_each) local child_session = require('test.functional.terminal.helpers') +local Screen = require('test.functional.ui.screen') if session.pending_win32(pending) then return end @@ -39,4 +40,11 @@ describe("shell command :!", function() {3:-- TERMINAL --} | ]]) end) + + it("throttles shell-command output greater than ~20KB", function() + child_session.feed_data( + ":!for i in $(seq 2 3000); do echo XXXXXXXXXX; done\n") + -- If a line with only a dot "." appears, then throttling was triggered. + screen:expect("\n.", nil, nil, nil, true) + end) end) diff --git a/test/functional/ui/screen.lua b/test/functional/ui/screen.lua index ebe8af35eb..2581b36711 100644 --- a/test/functional/ui/screen.lua +++ b/test/functional/ui/screen.lua @@ -126,7 +126,7 @@ end do local spawn, nvim_prog = helpers.spawn, helpers.nvim_prog local session = spawn({nvim_prog, '-u', 'NONE', '-i', 'NONE', '-N', '--embed'}) - local status, rv = session:request('vim_get_color_map') + local status, rv = session:request('nvim_get_color_map') if not status then print('failed to get color map') os.exit(1) @@ -207,7 +207,15 @@ function Screen:try_resize(columns, rows) uimeths.try_resize(columns, rows) end -function Screen:expect(expected, attr_ids, attr_ignore, condition) +-- Asserts that `expected` eventually matches the screen state. +-- +-- expected: Expected screen state (string). +-- attr_ids: Text attribute definitions. +-- attr_ignore: Ignored text attributes. +-- condition: Function asserting some arbitrary condition. +-- any: true: Succeed if `expected` matches ANY screen line(s). +-- false (default): `expected` must match screen exactly. +function Screen:expect(expected, attr_ids, attr_ignore, condition, any) -- remove the last line and dedent expected = dedent(expected:gsub('\n[ ]+$', '')) local expected_rows = {} @@ -229,21 +237,34 @@ function Screen:expect(expected, attr_ids, attr_ignore, condition) for i = 1, self._height do 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 j = 1, #expected_rows do - msg_expected_rows[j] = expected_rows[j] - end - msg_expected_rows[i] = '*' .. msg_expected_rows[i] - actual_rows[i] = '*' .. actual_rows[i] + + if any then + -- Search for `expected` anywhere in the screen lines. + local actual_screen_str = table.concat(actual_rows, '\n') + if nil == string.find(actual_screen_str, expected) then return ( - 'Row ' .. tostring(i) .. ' didn\'t match.\n' - .. 'Expected:\n|' .. table.concat(msg_expected_rows, '|\n|') .. '|\n' - .. 'Actual:\n|' .. table.concat(actual_rows, '|\n|') .. '|\n\n' .. [[ + 'Failed to match any screen lines.\n' + .. 'Expected (anywhere): "' .. expected .. '"\n' + .. 'Actual:\n |' .. table.concat(actual_rows, '|\n |') .. '|\n\n') + end + else + -- `expected` must match the screen lines exactly. + for i = 1, self._height do + if expected_rows[i] ~= actual_rows[i] then + local msg_expected_rows = {} + for j = 1, #expected_rows do + msg_expected_rows[j] = expected_rows[j] + end + msg_expected_rows[i] = '*' .. msg_expected_rows[i] + actual_rows[i] = '*' .. actual_rows[i] + return ( + 'Row ' .. tostring(i) .. ' did not match.\n' + ..'Expected:\n |'..table.concat(msg_expected_rows, '|\n |')..'|\n' + ..'Actual:\n |'..table.concat(actual_rows, '|\n |')..'|\n\n'..[[ To print the expect() call that would assert the current screen state, use screen:snaphot_util(). In case of non-deterministic failures, use screen:redraw_debug() to show all intermediate screen states. ]]) + end end end end) |