From db5d2c86b32818e78e413bea6e218bb30b1bf209 Mon Sep 17 00:00:00 2001 From: erw7 Date: Wed, 11 Mar 2020 20:27:22 +0900 Subject: TUI: fix bracket paste getting stuck fixes #11699, #11991. --- src/nvim/tui/input.c | 92 +++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 70 insertions(+), 22 deletions(-) diff --git a/src/nvim/tui/input.c b/src/nvim/tui/input.c index 94c326a5eb..62e0858301 100644 --- a/src/nvim/tui/input.c +++ b/src/nvim/tui/input.c @@ -22,6 +22,12 @@ #define KEY_BUFFER_SIZE 0xfff +typedef enum { + kIncomplete = -1, + kNotApplicable = 0, + kComplete = 1, +} HandleState; + #ifdef INCLUDE_GENERATED_DECLARATIONS # include "tui/input.c.generated.h" #endif @@ -339,8 +345,14 @@ static void tk_getkeys(TermInput *input, bool force) static void tinput_timer_cb(TimeWatcher *watcher, void *data) { - tk_getkeys(data, true); - tinput_flush(data, true); + TermInput *input = (TermInput *)data; + // If the raw buffer is not empty, process the raw buffer first because it is + // processing an incomplete bracketed paster sequence. + if (rbuffer_size(input->read_stream.buffer)) { + handle_raw_buffer(input, true); + } + tk_getkeys(input, true); + tinput_flush(input, true); } /// Handle focus events. @@ -365,19 +377,22 @@ static bool handle_focus_event(TermInput *input) return false; } -static bool handle_bracketed_paste(TermInput *input) +#define START_PASTE "\x1b[200~" +#define END_PASTE "\x1b[201~" +static HandleState handle_bracketed_paste(TermInput *input) { - if (rbuffer_size(input->read_stream.buffer) > 5 - && (!rbuffer_cmp(input->read_stream.buffer, "\x1b[200~", 6) - || !rbuffer_cmp(input->read_stream.buffer, "\x1b[201~", 6))) { + size_t buf_size = rbuffer_size(input->read_stream.buffer); + if (buf_size > 5 + && (!rbuffer_cmp(input->read_stream.buffer, START_PASTE, 6) + || !rbuffer_cmp(input->read_stream.buffer, END_PASTE, 6))) { bool enable = *rbuffer_get(input->read_stream.buffer, 4) == '0'; if (input->paste && enable) { - return false; // Pasting "start paste" code literally. + return kNotApplicable; // Pasting "start paste" code literally. } // Advance past the sequence rbuffer_consumed(input->read_stream.buffer, 6); if (!!input->paste == enable) { - return true; // Spurious "disable paste" code. + return kComplete; // Spurious "disable paste" code. } if (enable) { @@ -392,9 +407,15 @@ static bool handle_bracketed_paste(TermInput *input) // Paste phase: "disabled". input->paste = 0; } - return true; + return kComplete; + } else if (buf_size < 6 + && (!rbuffer_cmp(input->read_stream.buffer, START_PASTE, buf_size) + || !rbuffer_cmp(input->read_stream.buffer, + END_PASTE, buf_size))) { + // Wait for further input, as the sequence may be split. + return kIncomplete; } - return false; + return kNotApplicable; } // ESC NUL => @@ -518,21 +539,21 @@ bool ut_handle_background_color(TermInput *input) } #endif -static void tinput_read_cb(Stream *stream, RBuffer *buf, size_t count_, - void *data, bool eof) +static void handle_raw_buffer(TermInput *input, bool force) { - TermInput *input = data; - - if (eof) { - loop_schedule_fast(&main_loop, event_create(tinput_done_event, 0)); - return; - } + HandleState is_paste; do { - if (handle_focus_event(input) - || handle_bracketed_paste(input) - || handle_forced_escape(input) - || handle_background_color(input)) { + if (!force + && (handle_focus_event(input) + || (is_paste = handle_bracketed_paste(input)) != kNotApplicable + || handle_forced_escape(input) + || handle_background_color(input))) { + if (is_paste == kIncomplete) { + // Wait for the next input, leaving it in the raw buffer due to an + // incomplete sequence. + return; + } continue; } @@ -578,7 +599,34 @@ static void tinput_read_cb(Stream *stream, RBuffer *buf, size_t count_, } } } while (rbuffer_size(input->read_stream.buffer)); +} + +static void tinput_read_cb(Stream *stream, RBuffer *buf, size_t count_, + void *data, bool eof) +{ + TermInput *input = data; + + if (eof) { + loop_schedule_fast(&main_loop, event_create(tinput_done_event, 0)); + return; + } + + handle_raw_buffer(input, false); tinput_flush(input, true); + + // An incomplete sequence was found. Leave it in the raw buffer and wait for + // the next input. + if (rbuffer_size(input->read_stream.buffer)) { + // If 'ttimeout' is not set, start the timer with a timeout of 0 to process + // the next input. + long ms = input->ttimeout ? + (input->ttimeoutlen >= 0 ? input->ttimeoutlen : 0) : 0; + // Stop the current timer if already running + time_watcher_stop(&input->timer_handle); + time_watcher_start(&input->timer_handle, tinput_timer_cb, (uint32_t)ms, 0); + return; + } + // Make sure the next input escape sequence fits into the ring buffer without // wraparound, else it could be misinterpreted (because rbuffer_read_ptr() // exposes the underlying buffer to callers unaware of the wraparound). -- cgit From 8923a3df83aebb1e779240c461a5bcc46c62d74e Mon Sep 17 00:00:00 2001 From: erw7 Date: Wed, 11 Mar 2020 20:56:19 +0900 Subject: TUI: fix processing of bg color response Terminal responses may be fragmented. In that case, the problem that was not processed normally and was processed in the same way as user input is corrected. fixes #11393. --- src/nvim/tui/input.c | 39 +++++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/src/nvim/tui/input.c b/src/nvim/tui/input.c index 62e0858301..2c9c8b78f3 100644 --- a/src/nvim/tui/input.c +++ b/src/nvim/tui/input.c @@ -462,39 +462,47 @@ static void set_bg_deferred(void **argv) // ignored in the calculations. // // [1] https://en.wikipedia.org/wiki/Luma_%28video%29 -static bool handle_background_color(TermInput *input) +static HandleState handle_background_color(TermInput *input) { if (input->waiting_for_bg_response <= 0) { - return false; + return kNotApplicable; } size_t count = 0; size_t component = 0; size_t header_size = 0; size_t num_components = 0; + size_t buf_size = rbuffer_size(input->read_stream.buffer); uint16_t rgb[] = { 0, 0, 0 }; uint16_t rgb_max[] = { 0, 0, 0 }; bool eat_backslash = false; bool done = false; bool bad = false; - if (rbuffer_size(input->read_stream.buffer) >= 9 + if (buf_size >= 9 && !rbuffer_cmp(input->read_stream.buffer, "\x1b]11;rgb:", 9)) { header_size = 9; num_components = 3; - } else if (rbuffer_size(input->read_stream.buffer) >= 10 + } else if (buf_size >= 10 && !rbuffer_cmp(input->read_stream.buffer, "\x1b]11;rgba:", 10)) { header_size = 10; num_components = 4; + } else if (buf_size < 10 + && !rbuffer_cmp(input->read_stream.buffer, + "\x1b]11;rgba", buf_size)) { + // An incomplete sequence was found, waiting for the next input. + return kIncomplete; } else { input->waiting_for_bg_response--; if (input->waiting_for_bg_response == 0) { DLOG("did not get a response for terminal background query"); } - return false; + return kNotApplicable; } - input->waiting_for_bg_response = 0; - rbuffer_consumed(input->read_stream.buffer, header_size); RBUFFER_EACH(input->read_stream.buffer, c, i) { count = i + 1; + // Skip the header. + if (i < header_size) { + continue; + } if (eat_backslash) { done = true; break; @@ -516,8 +524,8 @@ static bool handle_background_color(TermInput *input) bad = true; } } - rbuffer_consumed(input->read_stream.buffer, count); if (done && !bad && rgb_max[0] && rgb_max[1] && rgb_max[2]) { + rbuffer_consumed(input->read_stream.buffer, count); double r = (double)rgb[0] / (double)rgb_max[0]; double g = (double)rgb[1] / (double)rgb_max[1]; double b = (double)rgb[2] / (double)rgb_max[2]; @@ -526,11 +534,17 @@ static bool handle_background_color(TermInput *input) DLOG("bg response: %s", bgvalue); loop_schedule_deferred(&main_loop, event_create(set_bg_deferred, 1, bgvalue)); + input->waiting_for_bg_response = 0; + } else if (!done && !bad) { + // An incomplete sequence was found, waiting for the next input. + return kIncomplete; } else { + input->waiting_for_bg_response = 0; + rbuffer_consumed(input->read_stream.buffer, count); DLOG("failed to parse bg response"); - return false; + return kNotApplicable; } - return true; + return kComplete; } #ifdef UNIT_TESTING bool ut_handle_background_color(TermInput *input) @@ -542,14 +556,15 @@ bool ut_handle_background_color(TermInput *input) static void handle_raw_buffer(TermInput *input, bool force) { HandleState is_paste; + HandleState is_bc; do { if (!force && (handle_focus_event(input) || (is_paste = handle_bracketed_paste(input)) != kNotApplicable || handle_forced_escape(input) - || handle_background_color(input))) { - if (is_paste == kIncomplete) { + || (is_bc = handle_background_color(input)) != kNotApplicable)) { + if (is_paste == kIncomplete || is_bc == kIncomplete) { // Wait for the next input, leaving it in the raw buffer due to an // incomplete sequence. return; -- cgit From 6c1e1fe772f99ab655402e92f14b782867b35eec Mon Sep 17 00:00:00 2001 From: erw7 Date: Thu, 12 Mar 2020 12:16:10 +0900 Subject: test,unit: Change test according to change of bg color response processing Adjust the test for handle_background_color() according to bd0275182b1c1b14c43dc4fc7e9f9da05071e56c. --- src/nvim/tui/input.c | 4 +++- src/nvim/tui/input.h | 8 +++++++- test/unit/tui_spec.lua | 43 +++++++++++++++++++++++++++++++++++++------ 3 files changed, 47 insertions(+), 8 deletions(-) diff --git a/src/nvim/tui/input.c b/src/nvim/tui/input.c index 2c9c8b78f3..124f96e039 100644 --- a/src/nvim/tui/input.c +++ b/src/nvim/tui/input.c @@ -22,11 +22,13 @@ #define KEY_BUFFER_SIZE 0xfff +#ifndef UNIT_TESTING typedef enum { kIncomplete = -1, kNotApplicable = 0, kComplete = 1, } HandleState; +#endif #ifdef INCLUDE_GENERATED_DECLARATIONS # include "tui/input.c.generated.h" @@ -547,7 +549,7 @@ static HandleState handle_background_color(TermInput *input) return kComplete; } #ifdef UNIT_TESTING -bool ut_handle_background_color(TermInput *input) +HandleState ut_handle_background_color(TermInput *input) { return handle_background_color(input); } diff --git a/src/nvim/tui/input.h b/src/nvim/tui/input.h index b30546c815..ed76455189 100644 --- a/src/nvim/tui/input.h +++ b/src/nvim/tui/input.h @@ -32,7 +32,13 @@ typedef struct term_input { #endif #ifdef UNIT_TESTING -bool ut_handle_background_color(TermInput *input); +typedef enum { + kIncomplete = -1, + kNotApplicable = 0, + kComplete = 1, +} HandleState; + +HandleState ut_handle_background_color(TermInput *input); #endif #endif // NVIM_TUI_INPUT_H diff --git a/test/unit/tui_spec.lua b/test/unit/tui_spec.lua index e6b5c889d7..36ce4a1493 100644 --- a/test/unit/tui_spec.lua +++ b/test/unit/tui_spec.lua @@ -14,10 +14,13 @@ itp('handle_background_color', function() local handle_background_color = cinput.ut_handle_background_color local term_input = ffi.new('TermInput', {}) local events = globals.main_loop.thread_events + local kIncomplete = cinput.kIncomplete + local kNotApplicable = cinput.kNotApplicable + local kComplete = cinput.kComplete -- Short-circuit when not waiting for response. term_input.waiting_for_bg_response = 0 - eq(false, handle_background_color(term_input)) + eq(kNotApplicable, handle_background_color(term_input)) local capacity = 100 local rbuf = ffi.gc(rbuffer.rbuffer_new(capacity), rbuffer.rbuffer_free) @@ -28,7 +31,7 @@ itp('handle_background_color', function() rbuffer.rbuffer_write(rbuf, to_cstr(term_response), #term_response) term_input.waiting_for_bg_response = 1 - eq(true, handle_background_color(term_input)) + eq(kComplete, handle_background_color(term_input)) eq(0, term_input.waiting_for_bg_response) eq(1, multiqueue.multiqueue_size(events)) @@ -97,14 +100,42 @@ itp('handle_background_color', function() assert_bg('rgba', 'f/f/f/f', 'light') - -- Incomplete sequence: not necessarily correct behavior, but tests it. + -- Incomplete sequence: necessarily correct behavior. local term_response = '\027]11;rgba:f/f/f/f' -- missing '\007 rbuffer.rbuffer_write(rbuf, to_cstr(term_response), #term_response) term_input.waiting_for_bg_response = 1 - eq(false, handle_background_color(term_input)) + eq(kIncomplete, handle_background_color(term_input)) + eq(1, term_input.waiting_for_bg_response) + eq(#term_response, rbuf.size) + + term_response = '\007' + rbuffer.rbuffer_write(rbuf, to_cstr(term_response), #term_response) + eq(kComplete, handle_background_color(term_input)) + eq(0, term_input.waiting_for_bg_response) + + local event = multiqueue.multiqueue_get(events) + local bg_event = ffi.cast("Event*", event.argv[1]) + eq('light', ffi.string(bg_event.argv[0])) + eq(0, multiqueue.multiqueue_size(events)) + eq(0, rbuf.size) + + term_response = '\027]11;rg' + rbuffer.rbuffer_write(rbuf, to_cstr(term_response), #term_response) + + term_input.waiting_for_bg_response = 1 + eq(kIncomplete, handle_background_color(term_input)) + eq(1, term_input.waiting_for_bg_response) + eq(#term_response, rbuf.size) + + term_response = 'ba:f/f/f/f\007' + rbuffer.rbuffer_write(rbuf, to_cstr(term_response), #term_response) + eq(kComplete, handle_background_color(term_input)) eq(0, term_input.waiting_for_bg_response) + event = multiqueue.multiqueue_get(events) + bg_event = ffi.cast("Event*", event.argv[1]) + eq('light', ffi.string(bg_event.argv[0])) eq(0, multiqueue.multiqueue_size(events)) eq(0, rbuf.size) @@ -114,7 +145,7 @@ itp('handle_background_color', function() rbuffer.rbuffer_write(rbuf, to_cstr(term_response), #term_response) term_input.waiting_for_bg_response = 3 - eq(false, handle_background_color(term_input)) + eq(kNotApplicable, handle_background_color(term_input)) eq(2, term_input.waiting_for_bg_response) eq(0, multiqueue.multiqueue_size(events)) @@ -127,7 +158,7 @@ itp('handle_background_color', function() rbuffer.rbuffer_write(rbuf, to_cstr(term_response), #term_response) term_input.waiting_for_bg_response = 1 - eq(true, handle_background_color(term_input)) + eq(kComplete, handle_background_color(term_input)) eq(0, term_input.waiting_for_bg_response) eq(1, multiqueue.multiqueue_size(events)) -- cgit From a539a12d723dd9c9ac63df815d35cd0204199b96 Mon Sep 17 00:00:00 2001 From: erw7 Date: Thu, 29 Oct 2020 14:01:34 +0900 Subject: test: add functional test for segmented response from terminal --- test/functional/terminal/tui_spec.lua | 152 +++++++++++++++++++++++++++++++--- 1 file changed, 139 insertions(+), 13 deletions(-) diff --git a/test/functional/terminal/tui_spec.lua b/test/functional/terminal/tui_spec.lua index 5948ab3a64..3ef41fde1d 100644 --- a/test/functional/terminal/tui_spec.lua +++ b/test/functional/terminal/tui_spec.lua @@ -727,6 +727,38 @@ describe('TUI', function() ]]) end) + it('paste: split "start paste" code', function() + feed_data('i') + -- Send split "start paste" sequence. + feed_data('\027[2') + feed_data('00~pasted from terminal\027[201~') + screen:expect([[ + pasted from terminal{1: } | + {4:~ }| + {4:~ }| + {4:~ }| + {5:[No Name] [+] }| + {3:-- INSERT --} | + {3:-- TERMINAL --} | + ]]) + end) + + it('paste: split "stop paste" code', function() + feed_data('i') + -- Send split "stop paste" sequence. + feed_data('\027[200~pasted from terminal\027[20') + feed_data('1~') + screen:expect([[ + pasted from terminal{1: } | + {4:~ }| + {4:~ }| + {4:~ }| + {5:[No Name] [+] }| + {3:-- INSERT --} | + {3:-- TERMINAL --} | + ]]) + end) + it('allows termguicolors to be set at runtime', function() screen:set_option('rgb', true) screen:set_default_attr_ids({ @@ -1452,15 +1484,108 @@ describe("TUI", function() end) -it('TUI bg color triggers OptionSet event on terminal-response', function() - -- Only single integration test. - -- See test/unit/tui_spec.lua for unit tests. - clear() - local screen = thelpers.screen_setup(0, '["'..nvim_prog - ..'", "-u", "NONE", "-i", "NONE", "--cmd", "set noswapfile", ' - ..'"-c", "autocmd OptionSet background echo \\"did OptionSet, yay!\\""]') +describe('TUI bg color', function() + local screen + + local function setup() + -- Only single integration test. + -- See test/unit/tui_spec.lua for unit tests. + clear() + screen = thelpers.screen_setup(0, '["'..nvim_prog + ..'", "-u", "NONE", "-i", "NONE", "--cmd", "set noswapfile", ' + ..'"-c", "autocmd OptionSet background echo \\"did OptionSet, yay!\\""]') + end + + before_each(setup) + + it('triggers OptionSet event on unsplit terminal-response', function() + screen:expect([[ + {1: } | + {4:~ }| + {4:~ }| + {4:~ }| + {5:[No Name] 0,0-1 All}| + | + {3:-- TERMINAL --} | + ]]) + feed_data('\027]11;rgb:ffff/ffff/ffff\007') + screen:expect{any='did OptionSet, yay!'} + + feed_data(':echo "new_bg=".&background\n') + screen:expect{any='new_bg=light'} + + setup() + screen:expect([[ + {1: } | + {4:~ }| + {4:~ }| + {4:~ }| + {5:[No Name] 0,0-1 All}| + | + {3:-- TERMINAL --} | + ]]) + feed_data('\027]11;rgba:ffff/ffff/ffff/8000\027\\') + screen:expect{any='did OptionSet, yay!'} + + feed_data(':echo "new_bg=".&background\n') + screen:expect{any='new_bg=light'} + end) + + it('triggers OptionSet event with split terminal-response', function() + screen:expect([[ + {1: } | + {4:~ }| + {4:~ }| + {4:~ }| + {5:[No Name] 0,0-1 All}| + | + {3:-- TERMINAL --} | + ]]) + -- Send a background response with the OSC command part split. + feed_data('\027]11;rgb') + feed_data(':ffff/ffff/ffff\027\\') + screen:expect{any='did OptionSet, yay!'} + + feed_data(':echo "new_bg=".&background\n') + screen:expect{any='new_bg=light'} + + setup() + screen:expect([[ + {1: } | + {4:~ }| + {4:~ }| + {4:~ }| + {5:[No Name] 0,0-1 All}| + | + {3:-- TERMINAL --} | + ]]) + -- Send a background response with the Pt portion split. + feed_data('\027]11;rgba:ffff/fff') + feed_data('f/ffff/8000\007') + screen:expect{any='did OptionSet, yay!'} + + feed_data(':echo "new_bg=".&background\n') + screen:expect{any='new_bg=light'} + end) + + it('not triggers OptionSet event with invalid terminal-response', function() + screen:expect([[ + {1: } | + {4:~ }| + {4:~ }| + {4:~ }| + {5:[No Name] 0,0-1 All}| + | + {3:-- TERMINAL --} | + ]]) + feed_data('\027]11;rgb:ffff/ffff/ffff/8000\027\\') + screen:expect_unchanged() + + feed_data(':echo "new_bg=".&background\n') + screen:expect{any='new_bg=dark'} - screen:expect([[ + setup() + screen:expect([[ {1: } | {4:~ }| {4:~ }| @@ -1468,10 +1593,11 @@ it('TUI bg color triggers OptionSet event on terminal-response', function() {5:[No Name] 0,0-1 All}| | {3:-- TERMINAL --} | - ]]) - feed_data('\027]11;rgb:ffff/ffff/ffff\007') - screen:expect{any='did OptionSet, yay!'} + ]]) + feed_data('\027]11;rgba:ffff/foo/ffff/8000\007') + screen:expect_unchanged() - feed_data(':echo "new_bg=".&background\n') - screen:expect{any='new_bg=light'} + feed_data(':echo "new_bg=".&background\n') + screen:expect{any='new_bg=dark'} + end) end) -- cgit