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

@@ -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)

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

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)

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)
+    -- Only single integration test.
-      -- Ensure the opposite of the expected bg is active.
+    -- See test/unit/tui_spec.lua for unit tests.
-      local other_bg = (bg == 'dark' and 'light' or 'dark')
+    feed_data('\027]11;rgb:ffff/ffff/ffff\007')
-      if last_bg ~= other_bg then
+    wait_for_bg('light')
-        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')
   end)
 end)

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)