From c3b0fbb89be46320e53fed3adbe41b89b9399ebc Mon Sep 17 00:00:00 2001
From: Lewis Russell <lewis6991@gmail.com>
Date: Mon, 19 Jun 2023 10:13:23 +0100
Subject: [PATCH] feat(watcher): move from libuv poll to file event watcher

---
 .github/workflows/ci.yml     |  1 -
 lua/gitsigns.lua             | 18 +++++---
 lua/gitsigns/attach.lua      |  4 --
 lua/gitsigns/cache.lua       |  4 +-
 lua/gitsigns/config.lua      |  6 +--
 lua/gitsigns/manager.lua     | 85 ++++++++++++++++++++++--------------
 test/gitdir_watcher_spec.lua | 34 ++++++++++++---
 test/gitsigns_spec.lua       |  5 +--
 test/gs_helpers.lua          |  4 +-
 test/highlights_spec.lua     |  5 +--
 vim.yml                      | 22 ++++++++++
 11 files changed, 124 insertions(+), 64 deletions(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 3527c4882..551c94e46 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -18,7 +18,6 @@ jobs:
     runs-on: ubuntu-latest
 
     strategy:
-      fail-fast: true
       matrix:
         neovim_branch:
           - 'v0.8.3'
diff --git a/lua/gitsigns.lua b/lua/gitsigns.lua
index 62d4e23d8..334e29c43 100644
--- a/lua/gitsigns.lua
+++ b/lua/gitsigns.lua
@@ -13,7 +13,7 @@ local uv = vim.loop
 
 local M = {}
 
-local cwd_watcher ---@type uv_fs_poll_t
+local cwd_watcher ---@type uv_fs_event_t?
 
 local update_cwd_head = void(function()
   local paths = vim.fs.find('.git', {
@@ -29,7 +29,7 @@ local update_cwd_head = void(function()
   if cwd_watcher then
     cwd_watcher:stop()
   else
-    cwd_watcher = assert(uv.new_fs_poll())
+    cwd_watcher = assert(uv.new_fs_event())
   end
 
   local cwd = assert(vim.loop.cwd())
@@ -70,10 +70,18 @@ local update_cwd_head = void(function()
     return
   end
 
+  local debounce_trailing = require('gitsigns.debounce').debounce_trailing
+
+  local update_head = debounce_trailing(100, void(function()
+    local new_head = git.get_repo_info(cwd).abbrev_head
+    scheduler()
+    vim.g.gitsigns_head = new_head
+  end))
+
   -- Watch .git/HEAD to detect branch changes
   cwd_watcher:start(
     towatch,
-    config.watch_gitdir.interval,
+    {},
     void(function(err)
       local __FUNC__ = 'cwd_watcher_cb'
       if err then
@@ -82,9 +90,7 @@ local update_cwd_head = void(function()
       end
       dprint('Git cwd dir update')
 
-      local new_head = git.get_repo_info(cwd).abbrev_head
-      scheduler()
-      vim.g.gitsigns_head = new_head
+      update_head()
     end)
   )
 end)
diff --git a/lua/gitsigns/attach.lua b/lua/gitsigns/attach.lua
index b6e304e82..e44cd379c 100644
--- a/lua/gitsigns/attach.lua
+++ b/lua/gitsigns/attach.lua
@@ -352,10 +352,6 @@ local attach_throttled = throttle_by_id(function(cbuf, ctx, aucmd)
 
   -- Initial update
   manager.update(cbuf, cache[cbuf])
-
-  if config.keymaps and not vim.tbl_isempty(config.keymaps) then
-    require('gitsigns.mappings')(config.keymaps, cbuf)
-  end
 end)
 
 --- Detach Gitsigns from all buffers it is attached to.
diff --git a/lua/gitsigns/cache.lua b/lua/gitsigns/cache.lua
index 54cf3e90d..93841bda4 100644
--- a/lua/gitsigns/cache.lua
+++ b/lua/gitsigns/cache.lua
@@ -17,7 +17,7 @@ local M = {
 --- @field hunks_staged?      Gitsigns.Hunk.Hunk[]
 ---
 --- @field staged_diffs       Gitsigns.Hunk.Hunk[]
---- @field gitdir_watcher?    uv_poll_t
+--- @field gitdir_watcher?    uv_fs_event_t
 --- @field git_obj            Gitsigns.GitObj
 --- @field commit?            string
 local CacheEntry = M.CacheEntry
@@ -57,6 +57,8 @@ function CacheEntry:invalidate()
   self.hunks_staged = nil
 end
 
+--- @param o Gitsigns.CacheEntry
+--- @return Gitsigns.CacheEntry
 function CacheEntry.new(o)
   o.staged_diffs = o.staged_diffs or {}
   return setmetatable(o, { __index = CacheEntry })
diff --git a/lua/gitsigns/config.lua b/lua/gitsigns/config.lua
index 0f44127d4..00d9413cd 100644
--- a/lua/gitsigns/config.lua
+++ b/lua/gitsigns/config.lua
@@ -62,7 +62,7 @@ end
 --- @field sign_priority integer
 --- @field _on_attach_pre fun(bufnr: integer, callback: fun(_: table))
 --- @field on_attach fun(bufnr: integer)
---- @field watch_gitdir { enable: boolean, interval: integer, follow_files: boolean }
+--- @field watch_gitdir { enable: boolean, follow_files: boolean }
 --- @field max_file_length integer
 --- @field update_debounce integer
 --- @field status_formatter fun(_: table<string,any>): string
@@ -295,7 +295,6 @@ M.schema = {
     deep_extend = true,
     default = {
       enable = true,
-      interval = 1000,
       follow_files = true,
     },
     description = [[
@@ -307,9 +306,6 @@ M.schema = {
         • `enable`:
             Whether the watcher is enabled.
 
-        • `interval`:
-            Interval the watcher waits between polls of the gitdir in milliseconds.
-
         • `follow_files`:
             If a file is moved with `git mv`, switch the buffer to the new location.
     ]],
diff --git a/lua/gitsigns/manager.lua b/lua/gitsigns/manager.lua
index 3bf342955..54d7872c3 100644
--- a/lua/gitsigns/manager.lua
+++ b/lua/gitsigns/manager.lua
@@ -497,7 +497,7 @@ local function handle_moved(bufnr, bcache, old_relpath)
       git_obj.orig_relpath = nil
       do_update = true
     end
-  else
+  -- else
     -- File removed from index, do nothing
   end
 
@@ -519,59 +519,78 @@ local function handle_moved(bufnr, bcache, old_relpath)
   end
 end
 
+-- vim.inspect but on one line
+--- @param x any
+--- @return string
+local function inspect(x)
+  return vim.inspect(x, {indent = '', newline = ' '})
+end
+
+local watch_gitdir_handler = debounce_trailing(100, void(function(bufnr)
+  local bcache = cache[bufnr]
+
+  if not bcache then
+    -- Very occasionally an external git operation may cause the buffer to
+    -- detach and update the git dir simultaneously. When this happens this
+    -- handler will trigger but there will be no cache.
+    dprint('Has detached, aborting')
+    return
+  end
+
+  local git_obj = bcache.git_obj
+
+  git_obj.repo:update_abbrev_head()
+
+  scheduler()
+  Status:update(bufnr, { head = git_obj.repo.abbrev_head })
+
+  local was_tracked = git_obj.object_name ~= nil
+  local old_relpath = git_obj.relpath
+
+  git_obj:update_file_info()
+
+  if config.watch_gitdir.follow_files and was_tracked and not git_obj.object_name then
+    -- File was tracked but is no longer tracked. Must of been removed or
+    -- moved. Check if it was moved and switch to it.
+    handle_moved(bufnr, bcache, old_relpath)
+  end
+
+  bcache:invalidate()
+
+  M.update(bufnr, bcache)
+end))
+
 --- @param bufnr integer
 --- @param gitdir string
---- @return uv_fs_poll_t?
+--- @return uv_fs_event_t?
 function M.watch_gitdir(bufnr, gitdir)
   if not config.watch_gitdir.enable then
     return
   end
 
   dprintf('Watching git dir')
-  local w = assert(uv.new_fs_poll())
+  local w = assert(uv.new_fs_event())
   w:start(
     gitdir,
-    config.watch_gitdir.interval,
-    void(function(err)
+    {},
+    function(err, filename, events)
       local __FUNC__ = 'watcher_cb'
       if err then
         dprintf('Git dir update error: %s', err)
         return
       end
-      dprint('Git dir update')
 
-      local bcache = cache[bufnr]
+      local info = string.format("Git dir update: '%s' %s", filename, inspect(events))
 
-      if not bcache then
-        -- Very occasionally an external git operation may cause the buffer to
-        -- detach and update the git dir simultaneously. When this happens this
-        -- handler will trigger but there will be no cache.
-        dprint('Has detached, aborting')
+      if vim.endswith(filename, '.lock') then
+        dprintf("%s (ignoring)", info)
         return
       end
 
-      local git_obj = bcache.git_obj
-
-      git_obj.repo:update_abbrev_head()
-
-      scheduler()
-      Status:update(bufnr, { head = git_obj.repo.abbrev_head })
+      dprint(info)
 
-      local was_tracked = git_obj.object_name ~= nil
-      local old_relpath = git_obj.relpath
-
-      git_obj:update_file_info()
-
-      if config.watch_gitdir.follow_files and was_tracked and not git_obj.object_name then
-        -- File was tracked but is no longer tracked. Must of been removed or
-        -- moved. Check if it was moved and switch to it.
-        handle_moved(bufnr, bcache, old_relpath)
-      end
-
-      bcache:invalidate()
-
-      M.update(bufnr, bcache)
-    end)
+      watch_gitdir_handler(bufnr)
+    end
   )
   return w
 end
diff --git a/test/gitdir_watcher_spec.lua b/test/gitdir_watcher_spec.lua
index b8574a509..2c1575bc7 100644
--- a/test/gitdir_watcher_spec.lua
+++ b/test/gitdir_watcher_spec.lua
@@ -1,7 +1,5 @@
 local helpers = require('test.gs_helpers')
 
-local Screen = require('test.screen')
-
 local clear           = helpers.clear
 local exec_lua        = helpers.exec_lua
 local edit            = helpers.edit
@@ -11,11 +9,11 @@ local cleanup         = helpers.cleanup
 local command         = helpers.command
 local test_config     = helpers.test_config
 local match_debug_messages = helpers.match_debug_messages
+local match_dag = helpers.match_dag
 local p               = helpers.p
 local setup_gitsigns  = helpers.setup_gitsigns
 local test_file       = helpers.test_file
 local git             = helpers.git
-local get_buf_name    = helpers.curbufmeths.get_name
 
 local it = helpers.it(it)
 
@@ -63,8 +61,16 @@ describe('gitdir_watcher', function()
     local test_file2 = test_file..'2'
     git{'mv', test_file, test_file2}
 
+    match_dag {
+      "watcher_cb(1): Git dir update: 'index.lock' { rename = true } (ignoring)",
+      "watcher_cb(1): Git dir update: 'index' { rename = true }",
+      "watcher_cb(1): Git dir update: 'index' { rename = true }",
+    }
+
     match_debug_messages {
-      'watcher_cb(1): Git dir update',
+      p"watcher_cb%(1%): .*",
+      p"watcher_cb%(1%): .*",
+      p"watcher_cb%(1%): .*",
       p'run_job: git .* rev%-parse %-%-show%-toplevel %-%-absolute%-git%-dir %-%-abbrev%-ref HEAD',
       p('run_job: git .* ls%-files .* '..vim.pesc(test_file)),
       p'run_job: git .* diff %-%-name%-status %-C %-%-cached',
@@ -83,8 +89,16 @@ describe('gitdir_watcher', function()
 
     git{'mv', test_file2, test_file3}
 
+    match_dag {
+      "watcher_cb(1): Git dir update: 'index.lock' { rename = true } (ignoring)",
+      "watcher_cb(1): Git dir update: 'index' { rename = true }",
+      "watcher_cb(1): Git dir update: 'index' { rename = true }",
+    }
+
     match_debug_messages {
-      'watcher_cb(1): Git dir update',
+      p"watcher_cb%(1%): .*",
+      p"watcher_cb%(1%): .*",
+      p"watcher_cb%(1%): .*",
       p'run_job: git .* rev%-parse %-%-show%-toplevel %-%-absolute%-git%-dir %-%-abbrev%-ref HEAD',
       p('run_job: git .* ls%-files .* '..vim.pesc(test_file2)),
       p'run_job: git .* diff %-%-name%-status %-C %-%-cached',
@@ -101,8 +115,16 @@ describe('gitdir_watcher', function()
 
     git{'mv', test_file3, test_file}
 
+    match_dag {
+      "watcher_cb(1): Git dir update: 'index.lock' { rename = true } (ignoring)",
+      "watcher_cb(1): Git dir update: 'index' { rename = true }",
+      "watcher_cb(1): Git dir update: 'index' { rename = true }",
+    }
+
     match_debug_messages {
-      'watcher_cb(1): Git dir update',
+      p"watcher_cb%(1%): .*",
+      p"watcher_cb%(1%): .*",
+      p"watcher_cb%(1%): .*",
       p'run_job: git .* rev%-parse %-%-show%-toplevel %-%-absolute%-git%-dir %-%-abbrev%-ref HEAD',
       p('run_job: git .* ls%-files .* '..vim.pesc(test_file3)),
       p'run_job: git .* diff %-%-name%-status %-C %-%-cached',
diff --git a/test/gitsigns_spec.lua b/test/gitsigns_spec.lua
index b1eeb81a4..2a342729b 100644
--- a/test/gitsigns_spec.lua
+++ b/test/gitsigns_spec.lua
@@ -21,7 +21,6 @@ local test_file       = helpers.test_file
 local git             = helpers.git
 local scratch         = helpers.scratch
 local newfile         = helpers.newfile
-local debug_messages  = helpers.debug_messages
 local match_dag       = helpers.match_dag
 local match_lines     = helpers.match_lines
 local p               = helpers.p
@@ -87,7 +86,7 @@ describe('gitsigns', function()
     edit(test_file)
 
     expectf(function()
-      match_dag(debug_messages(), {
+      match_dag {
         'attach(1): Attaching (trigger=BufReadPost)',
         p'run_job: git .* config user.name',
         p'run_job: git .* rev%-parse %-%-show%-toplevel %-%-absolute%-git%-dir %-%-abbrev%-ref HEAD',
@@ -95,7 +94,7 @@ describe('gitsigns', function()
         'watch_gitdir(1): Watching git dir',
         p'run_job: git .* show :0:dummy.txt',
         'update(1): updates: 1, jobs: 6'
-      })
+      }
     end)
 
     check {
diff --git a/test/gs_helpers.lua b/test/gs_helpers.lua
index 6d4f69262..acc8dbc79 100644
--- a/test/gs_helpers.lua
+++ b/test/gs_helpers.lua
@@ -211,9 +211,9 @@ function M.debug_messages()
   return exec_lua("return require'gitsigns.debug.log'.messages")
 end
 
-function M.match_dag(lines, spec)
+function M.match_dag(spec)
   for _, s in ipairs(spec) do
-    match_lines2(lines, {s})
+    match_lines2(M.debug_messages(), {s})
   end
 end
 
diff --git a/test/highlights_spec.lua b/test/highlights_spec.lua
index 8400b2f86..c8da55f7c 100644
--- a/test/highlights_spec.lua
+++ b/test/highlights_spec.lua
@@ -9,7 +9,6 @@ local cleanup       = helpers.cleanup
 local test_config   = helpers.test_config
 local expectf       = helpers.expectf
 local match_dag     = helpers.match_dag
-local debug_messages = helpers.debug_messages
 local p             = helpers.p
 local setup_gitsigns = helpers.setup_gitsigns
 
@@ -61,7 +60,7 @@ describe('highlights', function()
     setup_gitsigns(config)
 
     expectf(function()
-      match_dag(debug_messages(), {
+      match_dag {
         p'Deriving GitSignsAdd from DiffAdd',
         p'Deriving GitSignsAddLn from DiffAdd',
         p'Deriving GitSignsAddNr from GitSignsAdd',
@@ -69,7 +68,7 @@ describe('highlights', function()
         p'Deriving GitSignsChangeNr from GitSignsChange',
         p'Deriving GitSignsDelete from DiffDelete',
         p'Deriving GitSignsDeleteNr from GitSignsDelete',
-      })
+      }
     end)
 
     -- eq('GitSignsChange xxx links to DiffChange',
diff --git a/vim.yml b/vim.yml
index c257a9f31..848a1988e 100644
--- a/vim.yml
+++ b/vim.yml
@@ -13,3 +13,25 @@ globals:
   package.config:
     any: true
     property: read-only
+
+  # busted
+  describe:
+    args:
+      - type: string
+        required: true
+      - type: function
+        required: true
+  it:
+    args:
+      - type: string
+        required: true
+      - type: function
+        required: true
+  before_each:
+    args:
+      - type: function
+        required: true
+  after_each:
+    args:
+      - type: function
+        required: true