Staging part of hunk causes signs for entire hunk to change to staged

[gegoune]

Description

Using experimental _signs_staged_enable = true is very helpful and worked pretty well for a long time until I noticed that staging part of hunk causes signs for entire hunk to change to staged. Entire hunk does not get staged though, only status column signs are incorrect.

Neovim version

NVIM v0.10.0-dev-2045+g9ee8ce2eb-Homebrew
Build type: Release
LuaJIT 2.1.1703358377

Operating system and version

MacOS 14.2.1

Expected behavior

Only signs for staged lines within larger hunk to change to staged while remaining signs stay at changed.

Actual behavior

Signs for entire hunk change to as if entire hunk was staged.

Minimal config

for name, url in pairs{
  gitsigns = 'https://github.com/lewis6991/gitsigns.nvim',
} do
  local install_path = vim.fn.fnamemodify('gitsigns_issue/'..name, ':p')
  if vim.fn.isdirectory(install_path) == 0 then
    vim.fn.system { 'git', 'clone', '--depth=1', url, install_path }
  end
  vim.opt.runtimepath:append(install_path)
end

local unstaged = 'u'
local staged = 's'
require('gitsigns').setup{
  debug_mode = true, -- You must add this to enable debug messages
  signs = {
    add = { text = unstaged },
    change = { text = unstaged },
    delete = { text = '_', show_count = true },
    topdelete = { text = '‾', show_count = true },
    changedelete = { text = unstaged, show_count = true },
    untracked = { text = 'U' },
  },
  _signs_staged = {
    add = { text = staged },
    change = { text = staged },
    delete = { text = '_', show_count = true },
    topdelete = { text = '‾', show_count = true },
    changedelete = { text = staged, show_count = true },
  },
}

Steps to reproduce

1. mkdir temp; cd temp; git init
2. echo "0\n1\n2\n3\n4\n5\n6\n7\n8\n9\n" > file
3. git add file
4. git commit -m initial
5. nvim --clean -u minimal.lua file
6. vip> # mark entire file as edited, marks should show and state u for each line
7. 3jV3j # select lines for staging
8. :'<,'>Gitsigns stage_hunk # attempt to stage selection only

Step 8. causes signs for entire hunk (buffer in our case) to change to s. But that's not correct, as per git'a diff of stage area:

$ git diff --staged
diff --git a/file b/file
index ce6c2bc..e5a25f2 100644
--- a/file
+++ b/file
@@ -1,9 +1,9 @@
 0
 1
 2
-3
-4
-5
+       3
+       4
+       5
 6
 7
 8

Gitsigns debug messages

:Gitsigns debug_messages
signs.init: Using extmark signs
signs.init: Using extmark signs
dprintf: Deriving GitSignsAdd from DiffAdd
dprintf: Deriving GitSignsChange from DiffChange
dprintf: Deriving GitSignsDelete from DiffDelete
dprintf: Deriving GitSignsChangedelete from GitSignsChange
dprintf: Deriving GitSignsTopdelete from GitSignsDelete
dprintf: Deriving GitSignsUntracked from GitSignsAdd
dprintf: Deriving GitSignsAddNr from GitSignsAdd
dprintf: Deriving GitSignsChangeNr from GitSignsChange
dprintf: Deriving GitSignsDeleteNr from GitSignsDelete
dprintf: Deriving GitSignsChangedeleteNr from GitSignsChangeNr
dprintf: Deriving GitSignsTopdeleteNr from GitSignsDeleteNr
dprintf: Deriving GitSignsUntrackedNr from GitSignsAddNr
dprintf: Deriving GitSignsAddLn from DiffAdd
dprintf: Deriving GitSignsChangeLn from DiffChange
dprintf: Deriving GitSignsChangedeleteLn from GitSignsChangeLn
dprintf: Deriving GitSignsUntrackedLn from GitSignsAddLn
dprintf: Deriving GitSignsStagedAdd from GitSignsAdd
dprintf: Deriving GitSignsStagedChange from GitSignsChange
dprintf: Deriving GitSignsStagedDelete from GitSignsDelete
dprintf: Deriving GitSignsStagedChangedelete from GitSignsChangedelete
dprintf: Deriving GitSignsStagedTopdelete from GitSignsTopdelete
dprintf: Deriving GitSignsStagedAddNr from GitSignsAddNr
dprintf: Deriving GitSignsStagedChangeNr from GitSignsChangeNr
dprintf: Deriving GitSignsStagedDeleteNr from GitSignsDeleteNr
dprintf: Deriving GitSignsStagedChangedeleteNr from GitSignsChangedeleteNr
dprintf: Deriving GitSignsStagedTopdeleteNr from GitSignsTopdeleteNr
dprintf: Deriving GitSignsStagedAddLn from GitSignsAddLn
dprintf: Deriving GitSignsStagedChangeLn from GitSignsChangeLn
dprintf: Could not derive GitSignsStagedDeleteLn
dprintf: Deriving GitSignsStagedChangedeleteLn from GitSignsChangedeleteLn
dprintf: Could not derive GitSignsStagedTopdeleteLn
dprintf: Deriving GitSignsAddPreview from DiffAdd
dprintf: Deriving GitSignsDeletePreview from DiffDelete
dprintf: Deriving GitSignsCurrentLineBlame from NonText
dprintf: Deriving GitSignsAddInline from TermCursor
dprintf: Deriving GitSignsDeleteInline from TermCursor
dprintf: Deriving GitSignsChangeInline from TermCursor
dprintf: Deriving GitSignsAddLnInline from GitSignsAddInline
dprintf: Deriving GitSignsChangeLnInline from GitSignsChangeInline
dprintf: Deriving GitSignsDeleteLnInline from GitSignsDeleteInline
dprintf: Deriving GitSignsDeleteVirtLn from DiffDelete
dprintf: Deriving GitSignsDeleteVirtLnInLine from GitSignsDeleteLnInline
dprintf: Deriving GitSignsVirtLnum from GitSignsDeleteVirtLn
attach(1): Attaching (trigger=setup)
run_job: git --version
run_job: git --version
run_job: git --no-pager --literal-pathspecs -c gc.auto=0 rev-parse --show-toplevel --git-dir --abbrev-ref HEAD
run_job: git --no-pager --literal-pathspecs -c gc.auto=0 config user.name
run_job: git --no-pager --literal-pathspecs -c gc.auto=0 rev-parse --show-toplevel --absolute-git-dir --abbrev-ref HEAD
run_job: git --no-pager --literal-pathspecs -c gc.auto=0 --git-dir /Users/gegoune/temp/.git -c core.quotepath=off ls-files --stage --others --exclude-standard --e
ol /Users/gegoune/temp/file
watch_gitdir(1): Watching git dir
run_job: git --no-pager --literal-pathspecs -c gc.auto=0 --git-dir /Users/gegoune/temp/.git show :0:file
run_job: git --no-pager --literal-pathspecs -c gc.auto=0 --git-dir /Users/gegoune/temp/.git show HEAD:file
update(1): updates: 1, jobs: 8
update(1): updates: 2, jobs: 8
update(1): updates: 3, jobs: 8
update(1): updates: 4, jobs: 8
update(1): updates: 5, jobs: 8
update(1): updates: 6, jobs: 8
update(1): updates: 7, jobs: 8
update(1): updates: 8, jobs: 8
update(1): updates: 9, jobs: 8
update(1): updates: 10, jobs: 8
update(1): updates: 11, jobs: 8
update(1): updates: 12, jobs: 8
update(1): updates: 13, jobs: 8
update(1): updates: 14, jobs: 8
update(1): updates: 15, jobs: 8
update(1): updates: 16, jobs: 8
update(1): updates: 17, jobs: 8
update(1): updates: 18, jobs: 8
update(1): updates: 19, jobs: 8
update(1): updates: 20, jobs: 8
update(1): updates: 21, jobs: 8
update(1): updates: 22, jobs: 8
update(1): updates: 23, jobs: 8
update(1): updates: 24, jobs: 8
update(1): updates: 25, jobs: 8
update(1): updates: 26, jobs: 8
cli.run: Running action 'stage_hunk' with arguments {}
run_job: git --no-pager --literal-pathspecs -c gc.auto=0 --git-dir /Users/gegoune/temp/.git apply --whitespace=nowarn --cached --unidiff-zero -
run_job: git --no-pager --literal-pathspecs -c gc.auto=0 --git-dir /Users/gegoune/temp/.git show :0:file
run_job: git --no-pager --literal-pathspecs -c gc.auto=0 --git-dir /Users/gegoune/temp/.git show HEAD:file
update(1): updates: 27, jobs: 11
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 }
run_job: git --no-pager --literal-pathspecs -c gc.auto=0 rev-parse --show-toplevel --absolute-git-dir --abbrev-ref HEAD
run_job: git --no-pager --literal-pathspecs -c gc.auto=0 --git-dir /Users/gegoune/temp/.git -c core.quotepath=off ls-files --stage --others --exclude-standard --e
ol /Users/gegoune/temp/file
run_job: git --no-pager --literal-pathspecs -c gc.auto=0 --git-dir /Users/gegoune/temp/.git show :0:file
run_job: git --no-pager --literal-pathspecs -c gc.auto=0 --git-dir /Users/gegoune/temp/.git show HEAD:file
update(1): updates: 28, jobs: 15
watcher_cb(1): Git dir update: '.gitstatus.CKvLqi' { rename = true }
run_job: git --no-pager --literal-pathspecs -c gc.auto=0 rev-parse --show-toplevel --absolute-git-dir --abbrev-ref HEAD
run_job: git --no-pager --literal-pathspecs -c gc.auto=0 --git-dir /Users/gegoune/temp/.git -c core.quotepath=off ls-files --stage --others --exclude-standard --e
ol /Users/gegoune/temp/file
run_job: git --no-pager --literal-pathspecs -c gc.auto=0 --git-dir /Users/gegoune/temp/.git show :0:file
run_job: git --no-pager --literal-pathspecs -c gc.auto=0 --git-dir /Users/gegoune/temp/.git show HEAD:file
update(1): updates: 29, jobs: 19
watcher_cb(1): Git dir update: '.gitstatus.CKvLqi' { rename = true }
run_job: git --no-pager --literal-pathspecs -c gc.auto=0 rev-parse --show-toplevel --absolute-git-dir --abbrev-ref HEAD
run_job: git --no-pager --literal-pathspecs -c gc.auto=0 --git-dir /Users/gegoune/temp/.git -c core.quotepath=off ls-files --stage --others --exclude-standard --e
ol /Users/gegoune/temp/file
run_job: git --no-pager --literal-pathspecs -c gc.auto=0 --git-dir /Users/gegoune/temp/.git show :0:file
run_job: git --no-pager --literal-pathspecs -c gc.auto=0 --git-dir /Users/gegoune/temp/.git show HEAD:file
update(1): updates: 30, jobs: 23
cli.run: Running action 'debug_messages' with arguments {}

[andrewbraxton]

Priority of this should probably be bumped up since #1039 is now merged.

[ambroisie]

I can confirm that using signs_staged_enable = false does fix the issue of incorrect reporting.

[lewis6991]

I don't think this issue is fully correct. See #1102 (comment)

[YodaEmbedding]

I think the following patch (PR #1152) may fix the issue:

diff --git a/lua/gitsigns/manager.lua b/lua/gitsigns/manager.lua
index 4a5c07a..1f5edfc 100644
--- a/lua/gitsigns/manager.lua
+++ b/lua/gitsigns/manager.lua
@@ -26,30 +26,49 @@ local M = {}
 --- @param bot integer
 --- @param clear? boolean
 --- @param untracked boolean
-local function apply_win_signs0(bufnr, signs, hunks, top, bot, clear, untracked)
+--- @return Gitsigns.Sign[]
+local function calc_win_signs(bufnr, signs, hunks, top, bot, clear, untracked)
   if clear then
+    -- WARN: Stateful/impure action.
     signs:remove(bufnr) -- Remove all signs
   end
 
-  for i, hunk in ipairs(hunks or {}) do
+  --- @type Gitsigns.Sign[]
+  local win_signs = {}
+
+  if clear and not hunks then
+    --- @type Gitsigns.Hunk.Hunk
+    local hunk = hunks[1]
+
     --- @type Gitsigns.Hunk.Hunk?
-    local next = hunks[i + 1]
+    local next = hunks[2]
 
     -- To stop the sign column width changing too much, if there are signs to be
     -- added but none of them are visible in the window, then make sure to add at
     -- least one sign. Only do this on the first call after an update when we all
     -- the signs have been cleared.
-    if clear and i == 1 then
-      signs:add(bufnr, Hunks.calc_signs(hunk, next, hunk.added.start, hunk.added.start, untracked))
+    for _, s in ipairs(Hunks.calc_signs(hunk, next, hunk.added.start, hunk.added.start, untracked)) do
+      win_signs[#win_signs + 1] = s
     end
+  end
+
+  -- Calculate signs for the visible hunks.
+  for i, hunk in ipairs(hunks or {}) do
+    --- @type Gitsigns.Hunk.Hunk?
+    local next = hunks[i + 1]
 
     if top <= hunk.vend and bot >= hunk.added.start then
-      signs:add(bufnr, Hunks.calc_signs(hunk, next, top, bot, untracked))
+      for _, s in ipairs(Hunks.calc_signs(hunk, next, top, bot, untracked)) do
+        win_signs[#win_signs + 1] = s
+      end
     end
+
     if hunk.added.start > bot then
       break
     end
   end
+
+  return win_signs
 end
 
 --- @param bufnr integer
@@ -59,9 +78,45 @@ end
 local function apply_win_signs(bufnr, top, bot, clear)
   local bcache = assert(cache[bufnr])
   local untracked = bcache.git_obj.object_name == nil
-  apply_win_signs0(bufnr, signs_normal, bcache.hunks, top, bot, clear, untracked)
+
+  -- Collect all normal signs (by line number).
+  --- @type table<integer, Gitsigns.Sign[]>
+  local normal_signs_by_lnum = {}
+  local normal_signs = calc_win_signs(bufnr, signs_normal, bcache.hunks, top, bot, clear, untracked)
+  for _, s in ipairs(normal_signs) do
+    normal_signs_by_lnum[s.lnum] = normal_signs_by_lnum[s.lnum] or {}
+    table.insert(normal_signs_by_lnum[s.lnum], s)
+  end
+
+  -- Collect all staged signs (by line number).
+  --- @type table<integer, Gitsigns.Sign[]>
+  local staged_signs_by_lnum = {}
+  if signs_staged then
+    local staged_signs =
+      calc_win_signs(bufnr, signs_staged, bcache.hunks_staged, top, bot, clear, false)
+    for _, s in ipairs(staged_signs) do
+      if not normal_signs_by_lnum[s.lnum] then
+        staged_signs_by_lnum[s.lnum] = staged_signs_by_lnum[s.lnum] or {}
+        table.insert(staged_signs_by_lnum[s.lnum], s)
+      end
+    end
+  end
+
+  -- Flatten and sort staged signs by line number.
+  --- @type Gitsigns.Sign[]
+  local staged_signs = {}
+  --- @type integer[]
+  local staged_signs_lnums = vim.tbl_keys(staged_signs_by_lnum)
+  table.sort(staged_signs_lnums)
+  for _, lnum in ipairs(staged_signs_lnums) do
+    for _, s in ipairs(staged_signs_by_lnum[lnum]) do
+      table.insert(staged_signs, s)
+    end
+  end
+
+  signs_normal:add(bufnr, normal_signs)
   if signs_staged then
-    apply_win_signs0(bufnr, signs_staged, bcache.hunks_staged, top, bot, clear, false)
+    signs_staged:add(bufnr, staged_signs)
   end
 end
 

Not a lua programmer, so it's not particularly pretty.