From 8a4ae3d664a22cfaa3ec05635d26a8d662458c7e Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Mon, 30 Sep 2019 22:00:55 +0200 Subject: [PATCH] tui: improve handle_background_color: short-circuit (#11067) * handle_background_color: short-circuit if handled already * Unit tests for handle_background_color * set waiting_for_bg_response to false in tui_terminal_after_startup By then it should have been received. --- src/nvim/tui/input.c | 11 ++ src/nvim/tui/input.h | 5 + src/nvim/tui/tui.c | 6 ++ test/functional/terminal/tui_spec.lua | 74 +------------- test/unit/tui_spec.lua | 139 ++++++++++++++++++++++++++ 5 files changed, 165 insertions(+), 70 deletions(-) create mode 100644 test/unit/tui_spec.lua diff --git a/src/nvim/tui/input.c b/src/nvim/tui/input.c index ed9b410a19..876f00e03e 100644 --- a/src/nvim/tui/input.c +++ b/src/nvim/tui/input.c @@ -27,6 +27,7 @@ void tinput_init(TermInput *input, Loop *loop) input->loop = loop; input->paste = 0; input->in_fd = 0; + input->waiting_for_bg_response = false; input->key_buffer = rbuffer_new(KEY_BUFFER_SIZE); uv_mutex_init(&input->key_buffer_mutex); uv_cond_init(&input->key_buffer_cond); @@ -443,6 +444,9 @@ static void set_bg_deferred(void **argv) // [1] https://en.wikipedia.org/wiki/Luma_%28video%29 static bool handle_background_color(TermInput *input) { + if (!input->waiting_for_bg_response) { + return false; + } size_t count = 0; size_t component = 0; size_t header_size = 0; @@ -463,6 +467,7 @@ static bool handle_background_color(TermInput *input) } else { return false; } + input->waiting_for_bg_response = false; rbuffer_consumed(input->read_stream.buffer, header_size); RBUFFER_EACH(input->read_stream.buffer, c, i) { count = i + 1; @@ -503,6 +508,12 @@ static bool handle_background_color(TermInput *input) } return true; } +#ifdef UNIT_TESTING +bool ut_handle_background_color(TermInput *input) +{ + return handle_background_color(input); +} +#endif static void tinput_read_cb(Stream *stream, RBuffer *buf, size_t count_, void *data, bool eof) diff --git a/src/nvim/tui/input.h b/src/nvim/tui/input.h index a4071fab40..49ae32f00e 100644 --- a/src/nvim/tui/input.h +++ b/src/nvim/tui/input.h @@ -12,6 +12,7 @@ typedef struct term_input { // Phases: -1=all 0=disabled 1=first-chunk 2=continue 3=last-chunk int8_t paste; bool waiting; + bool waiting_for_bg_response; TermKey *tk; #if TERMKEY_VERSION_MAJOR > 0 || TERMKEY_VERSION_MINOR > 18 TermKey_Terminfo_Getstr_Hook *tk_ti_hook_fn; ///< libtermkey terminfo hook @@ -28,4 +29,8 @@ typedef struct term_input { # include "tui/input.h.generated.h" #endif +#ifdef UNIT_TESTING +bool ut_handle_background_color(TermInput *input); +#endif + #endif // NVIM_TUI_INPUT_H diff --git a/src/nvim/tui/tui.c b/src/nvim/tui/tui.c index 0c282a3f1f..4ca44b25f0 100644 --- a/src/nvim/tui/tui.c +++ b/src/nvim/tui/tui.c @@ -296,6 +296,7 @@ static void terminfo_start(UI *ui) unibi_out(ui, unibi_keypad_xmit); unibi_out(ui, unibi_clear_screen); // Ask the terminal to send us the background color. + data->input.waiting_for_bg_response = true; unibi_out_ext(ui, data->unibi_ext.get_bg); // Enable bracketed paste unibi_out_ext(ui, data->unibi_ext.enable_bracketed_paste); @@ -365,6 +366,11 @@ static void tui_terminal_after_startup(UI *ui) // 2.3 bug(?) which caused slow drawing during startup. #7649 unibi_out_ext(ui, data->unibi_ext.enable_focus_reporting); flush_buf(ui); + + if (data->input.waiting_for_bg_response) { + DLOG("did not get a response for terminal background query"); + data->input.waiting_for_bg_response = false; + } } static void tui_terminal_stop(UI *ui) diff --git a/test/functional/terminal/tui_spec.lua b/test/functional/terminal/tui_spec.lua index 89468359ef..ed904f27ca 100644 --- a/test/functional/terminal/tui_spec.lua +++ b/test/functional/terminal/tui_spec.lua @@ -1376,7 +1376,6 @@ describe('TUI background color', function() end) it("handles deferred background color", function() - local last_bg = 'dark' local function wait_for_bg(bg) -- Retry until the terminal response is handled. retry(100, nil, function() @@ -1394,76 +1393,11 @@ describe('TUI background color', function() ]], bg) }) end) - last_bg = bg end - local function assert_bg(colorspace, color, bg) - -- Ensure the opposite of the expected bg is active. - local other_bg = (bg == 'dark' and 'light' or 'dark') - if last_bg ~= other_bg then - feed_data(other_bg == 'light' and '\027]11;rgb:f/f/f\007' - or '\027]11;rgb:0/0/0\007') - wait_for_bg(other_bg) - end - - feed_data('\027]11;'..colorspace..':'..color..'\007') - wait_for_bg(bg) - end - - assert_bg('rgb', '0000/0000/0000', 'dark') - assert_bg('rgb', 'ffff/ffff/ffff', 'light') - assert_bg('rgb', '000/000/000', 'dark') - assert_bg('rgb', 'fff/fff/fff', 'light') - assert_bg('rgb', '00/00/00', 'dark') - assert_bg('rgb', 'ff/ff/ff', 'light') - assert_bg('rgb', '0/0/0', 'dark') - assert_bg('rgb', 'f/f/f', 'light') - - assert_bg('rgb', 'f/0/0', 'dark') - assert_bg('rgb', '0/f/0', 'light') - assert_bg('rgb', '0/0/f', 'dark') - - assert_bg('rgb', '1/1/1', 'dark') - assert_bg('rgb', '2/2/2', 'dark') - assert_bg('rgb', '3/3/3', 'dark') - assert_bg('rgb', '4/4/4', 'dark') - assert_bg('rgb', '5/5/5', 'dark') - assert_bg('rgb', '6/6/6', 'dark') - assert_bg('rgb', '7/7/7', 'dark') - assert_bg('rgb', '8/8/8', 'light') - assert_bg('rgb', '9/9/9', 'light') - assert_bg('rgb', 'a/a/a', 'light') - assert_bg('rgb', 'b/b/b', 'light') - assert_bg('rgb', 'c/c/c', 'light') - assert_bg('rgb', 'd/d/d', 'light') - assert_bg('rgb', 'e/e/e', 'light') - - assert_bg('rgb', '0/e/0', 'light') - assert_bg('rgb', '0/d/0', 'light') - assert_bg('rgb', '0/c/0', 'dark') - assert_bg('rgb', '0/b/0', 'dark') - - assert_bg('rgb', 'f/0/f', 'dark') - assert_bg('rgb', 'f/1/f', 'dark') - assert_bg('rgb', 'f/2/f', 'dark') - assert_bg('rgb', 'f/3/f', 'light') - assert_bg('rgb', 'f/4/f', 'light') - - assert_bg('rgba', '0000/0000/0000/0000', 'dark') - assert_bg('rgba', '0000/0000/0000/ffff', 'dark') - assert_bg('rgba', 'ffff/ffff/ffff/0000', 'light') - assert_bg('rgba', 'ffff/ffff/ffff/ffff', 'light') - assert_bg('rgba', '000/000/000/000', 'dark') - assert_bg('rgba', '000/000/000/fff', 'dark') - assert_bg('rgba', 'fff/fff/fff/000', 'light') - assert_bg('rgba', 'fff/fff/fff/fff', 'light') - assert_bg('rgba', '00/00/00/00', 'dark') - assert_bg('rgba', '00/00/00/ff', 'dark') - assert_bg('rgba', 'ff/ff/ff/00', 'light') - assert_bg('rgba', 'ff/ff/ff/ff', 'light') - assert_bg('rgba', '0/0/0/0', 'dark') - assert_bg('rgba', '0/0/0/f', 'dark') - assert_bg('rgba', 'f/f/f/0', 'light') - assert_bg('rgba', 'f/f/f/f', 'light') + -- Only single integration test. + -- See test/unit/tui_spec.lua for unit tests. + feed_data('\027]11;rgb:ffff/ffff/ffff\007') + wait_for_bg('light') end) end) diff --git a/test/unit/tui_spec.lua b/test/unit/tui_spec.lua new file mode 100644 index 0000000000..47dbd87b71 --- /dev/null +++ b/test/unit/tui_spec.lua @@ -0,0 +1,139 @@ +local helpers = require("test.unit.helpers")(after_each) +local cimport = helpers.cimport +local eq = helpers.eq +local ffi = helpers.ffi +local itp = helpers.gen_itp(it) +local to_cstr = helpers.to_cstr + +local cinput = cimport("./src/nvim/tui/input.h") +local rbuffer = cimport("./test/unit/fixtures/rbuffer.h") +local globals = cimport("./src/nvim/globals.h") +local multiqueue = cimport("./test/unit/fixtures/multiqueue.h") + +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 + + -- Short-circuit when not waiting for response. + term_input.waiting_for_bg_response = false + eq(false, handle_background_color(term_input)) + + local capacity = 100 + local rbuf = ffi.gc(rbuffer.rbuffer_new(capacity), rbuffer.rbuffer_free) + term_input.read_stream.buffer = rbuf + + local function assert_bg(colorspace, color, bg) + local term_response = '\027]11;'..colorspace..':'..color..'\007' + rbuffer.rbuffer_write(rbuf, to_cstr(term_response), #term_response) + + term_input.waiting_for_bg_response = true + eq(true, handle_background_color(term_input)) + eq(false, term_input.waiting_for_bg_response) + eq(1, multiqueue.multiqueue_size(events)) + + local event = multiqueue.multiqueue_get(events) + local bg_event = ffi.cast("Event*", event.argv[1]) + eq(bg, ffi.string(bg_event.argv[0])) + + -- Buffer has been consumed. + eq(0, rbuf.size) + end + + assert_bg('rgb', '0000/0000/0000', 'dark') + assert_bg('rgb', 'ffff/ffff/ffff', 'light') + assert_bg('rgb', '000/000/000', 'dark') + assert_bg('rgb', 'fff/fff/fff', 'light') + assert_bg('rgb', '00/00/00', 'dark') + assert_bg('rgb', 'ff/ff/ff', 'light') + assert_bg('rgb', '0/0/0', 'dark') + assert_bg('rgb', 'f/f/f', 'light') + + assert_bg('rgb', 'f/0/0', 'dark') + assert_bg('rgb', '0/f/0', 'light') + assert_bg('rgb', '0/0/f', 'dark') + + assert_bg('rgb', '1/1/1', 'dark') + assert_bg('rgb', '2/2/2', 'dark') + assert_bg('rgb', '3/3/3', 'dark') + assert_bg('rgb', '4/4/4', 'dark') + assert_bg('rgb', '5/5/5', 'dark') + assert_bg('rgb', '6/6/6', 'dark') + assert_bg('rgb', '7/7/7', 'dark') + assert_bg('rgb', '8/8/8', 'light') + assert_bg('rgb', '9/9/9', 'light') + assert_bg('rgb', 'a/a/a', 'light') + assert_bg('rgb', 'b/b/b', 'light') + assert_bg('rgb', 'c/c/c', 'light') + assert_bg('rgb', 'd/d/d', 'light') + assert_bg('rgb', 'e/e/e', 'light') + + assert_bg('rgb', '0/e/0', 'light') + assert_bg('rgb', '0/d/0', 'light') + assert_bg('rgb', '0/c/0', 'dark') + assert_bg('rgb', '0/b/0', 'dark') + + assert_bg('rgb', 'f/0/f', 'dark') + assert_bg('rgb', 'f/1/f', 'dark') + assert_bg('rgb', 'f/2/f', 'dark') + assert_bg('rgb', 'f/3/f', 'light') + assert_bg('rgb', 'f/4/f', 'light') + + assert_bg('rgba', '0000/0000/0000/0000', 'dark') + assert_bg('rgba', '0000/0000/0000/ffff', 'dark') + assert_bg('rgba', 'ffff/ffff/ffff/0000', 'light') + assert_bg('rgba', 'ffff/ffff/ffff/ffff', 'light') + assert_bg('rgba', '000/000/000/000', 'dark') + assert_bg('rgba', '000/000/000/fff', 'dark') + assert_bg('rgba', 'fff/fff/fff/000', 'light') + assert_bg('rgba', 'fff/fff/fff/fff', 'light') + assert_bg('rgba', '00/00/00/00', 'dark') + assert_bg('rgba', '00/00/00/ff', 'dark') + assert_bg('rgba', 'ff/ff/ff/00', 'light') + assert_bg('rgba', 'ff/ff/ff/ff', 'light') + assert_bg('rgba', '0/0/0/0', 'dark') + assert_bg('rgba', '0/0/0/f', 'dark') + assert_bg('rgba', 'f/f/f/0', 'light') + assert_bg('rgba', 'f/f/f/f', 'light') + + + -- Incomplete sequence: not necessarily correct behavior, but tests it. + 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 = true + eq(true, term_input.waiting_for_bg_response) + eq(false, handle_background_color(term_input)) + eq(false, term_input.waiting_for_bg_response) + + eq(0, multiqueue.multiqueue_size(events)) + eq(0, rbuf.size) + + + -- Does nothing when not at start of buffer. + term_response = '123\027]11;rgba:f/f/f/f\007456' + rbuffer.rbuffer_write(rbuf, to_cstr(term_response), #term_response) + + term_input.waiting_for_bg_response = true + eq(true, term_input.waiting_for_bg_response) + eq(false, handle_background_color(term_input)) + eq(true, term_input.waiting_for_bg_response) + + eq(0, multiqueue.multiqueue_size(events)) + eq(#term_response, rbuf.size) + rbuffer.rbuffer_consumed(rbuf, #term_response) + + + -- Keeps trailing buffer. + term_response = '\027]11;rgba:f/f/f/f\007456' + rbuffer.rbuffer_write(rbuf, to_cstr(term_response), #term_response) + + term_input.waiting_for_bg_response = true + eq(true, term_input.waiting_for_bg_response) + eq(true, handle_background_color(term_input)) + eq(false, term_input.waiting_for_bg_response) + + eq(1, multiqueue.multiqueue_size(events)) + eq(3, rbuf.size) + rbuffer.rbuffer_consumed(rbuf, rbuf.size) +end)