Inconvenient Lua API

[Iskustvo]

Description

Thank you for this plugin and sorry for the long post (I wrote it locally, before seeing the template :disappointed: )! I'm well aware that this contains a mix of bugs, feature requests and questions for help, but I honestly believe that as a whole, it should help you better understand struggles and frustration of everyday users with current API. Please bear with me on this one, and if you prefer, I'll later split it into smaller issues (whichever you consider "in scope").

Goal

I want to create <Enter> mapping which will:

• In case when line under cursor isn't commited, show popup with such message.
• Otherwise, open new Vim tab and show commit's changes (diff) which introduced line under cursor.
• Work "recursively", i.e., be able to use this keymap on a line in parent's snapshot to go further down the blame history.

Check existing support (technical breakdown)

• In case when line under cursor isn't commited, show window with such message -> gitsigns.blame_line
• Blame the line under cursor and get commit SHA -> gitsigns.blame_line ?
• Open snapshot of a file in blamed commit -> gitsigns.show
• Turn that snapshot into before/after diff -> gitsigns.diffthis

It seems quite reassuring that this is "in scope" of the plugin and should be easy to implement, right?

Implementation

Blame the line under cursor and get commit SHA

Unfortunately, gitsigns.blame_line seems to be the only API available for the job. Issues:

• It doesn't just return blamed line as string, but it opens a popup window which has it.
• It's asynchronous (but at least accepts "then" callback for chaining operations).

Because of these, the only way to extract blamed commit SHA via Gitsigns is the following:

-- Open popup window with commit of blamed line.
gitsigns.blame_line(
    { full = false },
    function()
        -- In order to focus opened popup window, blame_line needs to be called again.
        gitsigns.blame_line(
            {},
            function()
                -- Now that popup is focused, extract commit SHA from the start of it.
                local blamed_commit = vim.fn.getline(1):match("^(%x+)")

                -- Close the focused popup.
                vim.cmd(":quit")

                -- If commit is unavailable (i.e. local changes), reopen the popup window, but without focus.
                if blamed_commit == nil then
                    gitsigns.blame_line()
                    return
                end

                -- Rest of the code that has to be executed synchronously after blamed commit is extracted.
            end)
    end)

Preferred solution: Having something like gitsigns.get_blame_line which will synchronously return blamed line for opened file snapshot.

Open snapshot of a file in blamed commit

Here, gitsigns.show does just what we want, right? Issues:

• It's also asynchronous, but unlike other functions, it doesn't accept callback which will be executed upon completion.
• Even if it did accept callback, it's hard to define when it should be invoked:
  • After buffer with snapshot was opened and focused?
  • After Gitsigns attaches to it?
  • After Gitsigns correctly sets the base to snapshot's parent?

Because of this, inconvenient code is forced on us again:

-- Open popup window with commit SHA of blamed line.
gitsigns.blame_line(
    { full = false },
    function()
        -- In order to focus opened popup window, blame_line needs to be called again.
        gitsigns.blame_line(
            {},
            function()
                -- Now that popup is focused, extract commit SHA from the start of it.
                local blamed_commit = vim.fn.getline(1):match("^(%x+)")

                -- Close the focused popup.
                vim.cmd(":quit")

                -- If commit is unavailable (i.e. local changes), reopen the popup window, but without focus.
                if blamed_commit == nil then
                    gitsigns.blame_line()
                    return
                end

                -- Create new tab which is viewing current version of the file.
                vim.cmd(":tabnew %")

                -- React to opening of the gitsigns.show (below).
                vim.api.nvim_create_autocmd("User", {
                    pattern = "GitSignsUpdate",
                    callback = function()
                        -- Rest of the code that has to be executed synchronously after gitsigns.show.
                    end,
                    once = true
                })

                -- Open and focus blamed version of the file in new tab.
                gitsigns.show(blamed_commit)
            end)
    end)

Turn that snapshot into before/after diff

Again, this also looks like a thing that can't go wrong. We just have to diffthis, right?

Issues:

• In order to successfully perform diffthis, "Gitsigns" had to already finish all of its configuration magic, and there's no specific event for that.
• You may have noticed GitSignsUpdate event in last code snippet, but that one is fired multiple times (at least twice) for every new opened file, either by a bug or design.
• Reacting only on N-th occurrence of this event is just asking for trouble down the road, so that's why we're waiting only for first one, to know that at least Gitsigns is available in that buffer.
• Because we are reacting only to first occurrence (which might not have its base correctly setup), we have to specify revision manually - gitsigns.diffthis(blamed_commit .. "^")
• Unfortunately, for reasons unknown to me, this doesn't work recursively, i.e., it misbehaves on second attempt to go deeper in blame history.

Only thing that fixed "recursive" behavior was doing diffthis synchronously after manually changing the base:

-- Open popup window with commit SHA of blamed line.
gitsigns.blame_line(
    { full = false },
    function()
        -- In order to focus opened popup window, blame_line needs to be called again.
        gitsigns.blame_line(
            {},
            function()
                -- Now that popup is focused, extract commit SHA from the start of it.
                local blamed_commit = vim.fn.getline(1):match("^(%x+)")

                -- Close the focused popup.
                vim.cmd(":quit")

                -- If commit is unavailable (i.e. local changes), reopen the popup window, but without focus.
                if blamed_commit == nil then
                    gitsigns.blame_line()
                    return
                end

                -- Create new tab which is viewing current version of the file.
                vim.cmd(":tabnew %")

                -- React to opening of the gitsigns.show (below).
                vim.api.nvim_create_autocmd("User", {
                    pattern = "GitSignsUpdate",
                    callback = function()
                        -- Diff blamed version of the file with its parent's version of the same file.
                        gitsigns.change_base(blamed_commit .. "^", false, gitsigns.diffthis)
                    end,
                    once = true
                })

                -- Open and focus blamed version of the file in new tab.
                gitsigns.show(blamed_commit)
            end)
    end)

Test environment

• Execute the following bash script to create dummy git repo for testing in /tmp:

  mkdir /tmp/gitsigns_test
  cd /tmp/gitsigns_test
  git init
  echo -e "aaa\nbbb\nccc" > file
  git add file
  git commit -m "add 3 lines"
  echo -e "aaaa\nbbbb\nccc" > file
  git commit -am "modify first two lines"
  echo -e "aaaa\nbbbbb\ncccc" > file
  git commit -am "modify last two lines"

• Use minimal.lua when testing.

Outstanding bugs

Wrong caching (I assume)

Steps to reproduce:

• Open dummy file from testing directory in Neovim.

• Navigate to last line (cccc).

• Press key that will execute our mapping (<Enter> for me).

• Observe that newly created Vim tab has correct diff and that each buffer has correct "signs".

• Close the tab with diff: :tabclose

• Observe that "signs" in the original file are "copied" from our showed snapshot and don't represent the correct state.

• Execute :Gitsigns blame_line on marked lines.

• Observe that Gitsigns shows "Not Commited Yet" message, even though there are no local (nor staged) changes.

Initial commits are missing signs and cannot be blamed.

Steps to reproduce:

• Open dummy file from testing directory in Neovim.
• Navigate to first line (aaaa).
• Press key that should execute our mapping (<Enter> for me).
• Observe that newly created Vim tab has correct diff, but buffer with initial commit (left window) is missing "signs".
• Navigate to snapshot of the initial commit.
• Execute :Gitsigns blame_line on any line.
• Observe that command was just ignored, it didn't spawn popup window, even though all lines were added in that commit.

NOTE: After retesting with minimal.lua, I see the following error, which doesn't popup otherwise.

Error executing luv callback:
...fig/nvim/gitsigns_issue/gitsigns//lua/gitsigns/async.lua:85: The coroutine failed with this message: ...onfig/nvim/gitsigns_issue/gitsigns//lua/gitsigns/git.lua:516: (ERROR) ...onfig/nvim/gitsigns_issue/gitsigns//lua/gitsigns/git.lua(516
): fatal: Not a valid object name fff6a8ba^^

This makes perfect sense given that fff6a8ba^ is initial commit and fff6a8ba^^ is therefore invalid. But it again makes me stuck because the only way to avoid this is to manually invoke git and check this edge case. Gitsigns doesn't provide me any way to handle it gracefully.

Neovim version

NVIM v0.10.0

Operating system and version

Arch Linux (x86_64) Kernel: 6.9.2-arch1-1

Expected behavior

No response

Actual behavior

Everything is in description.

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

require('gitsigns').setup {
    debug_mode = true, -- You must add this to enable debug messages
    on_attach = function(buffer_number)
        local gitsigns = require('gitsigns')
        vim.keymap.set(
            'n',
            '<Enter>',
            function()
                -- Open popup window with commit SHA of blamed line.
                gitsigns.blame_line(
                    { full = false },
                    function()
                        -- In order to focus opened popup window, blame_line needs to be called again.
                        gitsigns.blame_line({},
                            function()
                                -- Now that popup is focused, extract commit SHA from the start of it.
                                local blamed_commit = vim.fn.getline(1):match("^(%x+)")

                                -- Close the focused popup.
                                vim.cmd(":quit")

                                -- If commit is unavailable (i.e. local changes), reopen the popup window, but without focus.
                                if blamed_commit == nil then
                                    gitsigns.blame_line()
                                    return
                                end

                                -- Create new tab which is viewing current version of the file.
                                vim.cmd(":tabnew %")

                                -- React to opening of the gitsigns.show (below) and diff it.
                                vim.api.nvim_create_autocmd("User", {
                                    pattern = "GitSignsUpdate",
                                    callback = function()
                                        -- Diff blamed version of the file with its parent's version of the same file.
                                        gitsigns.change_base(blamed_commit .. "^", false, gitsigns.diffthis)
                                        -- gitsigns.diffthis(blamed_commit .. "^")
                                    end,
                                    once = true
                                })

                                -- Open and focus blamed version of the file in new tab.
                                gitsigns.show(blamed_commit)
                            end)
                    end)
            end,
            { buffer = buffer_number, desc = "[Enter] the diff mode between current's line commit and its parent" })
    end
}

Steps to reproduce

Everything is in description.

Gitsigns debug messages

No response

[lewis6991]

Looks like you are trying to use the functions outside their intended use.

Consider using vim.system({'git', ...}):wait() directly.

[Iskustvo]

Well, yeah, since Lua API is designed for convenient mapping, not programming :(

While I can use git directly, given that I still like and want to have this plugin, it feels like a bad idea:

• I would have to repeat same git commands which Gitsigns will also do independently.
• It's quite inconvenient to implement caching which already exists in Gitsings (and it would be bad to duplicate it).
• Maintaining and debugging edge cases (e.g. like initial commit) in user config also doesn't seem too appealing.

I would like to split this into following more concrete issues:

• [Enhancement] Expose Lua API with gitsigns.get_blame_line() which will synchronously return blamed line for opened file snapshot
• [Enhancement] Refine Gitsigns events so that user can accurately react on Gitsigns commands/functions (e.g. after gitsigns.show() has opened a file snapshot, attached itself to it and set appropriate base)
• [Enhancement] Extend {base}/{revision} parameters to also accept special value "parent" which will correctly handle plugin behavior for initial commits, i.e. in diffthis() it should:
  • In case of normal file -> Open diff between current version of the file and version of parent's commit (if it exists).
  • In case of Gitsigns's file snapshot from different commit -> Open diff between that snapshot and the one from its parent's commit (if it exists).
  • In case of initial commit (detected like this and hopefuly cached) -> Open popup with message like "Initial commit - Cannot diff parent".
• [Bug] gitsigns.blame_line() incorrectly shows "Not Committed Yet" for lines in showed file snapshots that were modified by parent commit
• [Bug] When Gitsigns attaches to a snapshot of a file in initial commit, "signs" don't indicate that all lines were added in that commit, as they do for all other commits (compared to their parent's snapshot)

Of course, this whole "feature" (minus the new Vim tab) can be internally implemented inside of plugin and exposed as a function, but I'm not sure how many people would use it, so it's questionable. On the other hand, I really feel like Gitsings should have building blocks to allow anyone to implement this in efficient/convenient way, given the scope that it already has.

What do you think, should I proceed and create proposed issues?

[lewis6991]

You can create issues, but I'll consider each separately, in conjunction with all other issues and feature requests.

There would also be no timeline for these either since I use this plugin mostly for myself and have limited time.

[Iskustvo]

No problem, I fully understand the situation you're in. Regardless, thank you for taking the time for this!

I guess I'll just spawn smaller tickets aiming to achieve this mapping as I go, given that behavior is constantly changed by your work. At the very least, your last commit solved this one already:

[Bug] When Gitsigns attaches to a snapshot of a file in initial commit, "signs" don't indicate that all lines were added in that commit, as they do for all other commits (compared to their parent's snapshot)

[Iskustvo]

Hey @lewis6991, with latest changes I'm able to get (mostly) desired behavior :tada: Unfortunately, it's now even more error-prone and uglier than before, but at least it works :smile:

    vim.keymap.set(
        'n',
        '<Enter>',
        function()
            -- Open popup window with commit SHA of blamed line.
            gitsigns.blame_line(
                { full = false },
                function()
                    -- In order to focus opened popup window, blame_line needs to be called again.
                    gitsigns.blame_line({},
                        function()
                            -- Now that popup is focused, extract commit SHA from the start of it.
                            local blamed_commit = vim.fn.getline(1):match("^(%x+)")

                            -- Close the focused popup.
                            vim.cmd(":quit")

                            -- If commit is unavailable (i.e. local changes), reopen the popup window, but without focus.
                            if blamed_commit == nil then
                                gitsigns.blame_line()
                                return
                            end

                            -- Create new tab which is viewing current version of the file.
                            vim.cmd(":tabnew %")

                            -- React to opening of the gitsigns.show (below) and diff it.
                            -- FIXME: Note that diff doesn't work until all 3 updates happened, so ignore first two.
                            local callback_count = 0
                            vim.api.nvim_create_autocmd("User", {
                                pattern = "GitSignsUpdate",
                                callback = function()
                                    callback_count = callback_count + 1
                                    if (callback_count < 3) then
                                        return false
                                    end

                                    -- Diff blamed version of the file with its parent's version of the same file.
                                    gitsigns.diffthis(blamed_commit .. "^")

                                    return true
                                end,
                            })

                            -- Open and focus blamed version of the file in new tab.
                            gitsigns.show(blamed_commit)
                        end)
                end)
        end,
        { buffer = buffer_number, desc = "[Enter] the diff mode between current's line commit and its parent" })

Further improvements:

• After gitsigns.show() is executed, I have to wait for 3 events (have no idea what they're doing, assuming they are changing the base and whatnot) on the newly opened buffer before I can successfully execute gitsigns.diffthis(). That code is just hideous and it will easily break with unrelated plugin updates, so I'm really not looking forward to maintain it like this :smile: I have noticed that you recently introduced then callback for gitsigns.show() and this one seemed like the ideal solution for the problem. Unfortunately, that callback seems to consistently be executed after first update event, so it did not help :disappointed: Can we delay its invocation after last expected update event, so that it happens after newly showed buffer is fully functional for further operations (like gitsignds.diffthis() in my case)? - #1091
• Synchronous gitsigns.blame_line() to remove half of the bloat in the snippet above - #1045
• Edge case handling for initial commits. Obviously, in the snippet above, I'm just slapping parent reference(^) to previous commit when invoking gitsigns.show() and gitsigns.diffthis(), but in case when I reach initial commit, these will fail and I don't have a way to react to it and stop the execution when error is encountered. It would be nice to be able to react on errors like these. On the positive side, this can at least be tested on user side before making such requests from the plugin.

Obviously, last two can be handled on the user side (for last one, it may even be the best solution), but I really think first one should be somehow handled in the plugin itself. Therefore, I'll open another issue for that one. (Done)

I know there are no timelines and appreciate you working on this at all, but if you have some immediate feedback/suggestion, I'd like to hear them!

[Iskustvo]

Hey @lewis6991, after recent fixes, I think this one is in quite good shape now. Only #1045 remains from my wish list and since we already have a ticket for it, I'll close this epic. Thank you for all the work and patience!

For the reference, I ended up with following implementation for now:

    vim.keymap.set(
        'n',
        '<Enter>',
        function()
            -- Open popup window with commit SHA of blamed line.
            gitsigns.blame_line(
                { full = false },
                function()
                    -- In order to focus opened popup window, blame_line needs to be called again.
                    gitsigns.blame_line({},
                        function()
                            -- Now that popup is focused, extract commit SHA from the start of it.
                            local blamed_commit = vim.fn.getline(1):match("^(%x+)")

                            -- Close the focused popup.
                            vim.cmd(":quit")

                            -- If commit is unavailable (i.e. local changes), reopen the popup window, but without focus.
                            if blamed_commit == nil then
                                gitsigns.blame_line()
                                return
                            end

                            -- Compute parent commit of the blamed one.
                            local blamed_commit_parent = blamed_commit .. "^"
                            local result = vim.system({ "git", "rev-parse", "--short", blamed_commit_parent }):wait()
                            if result.code ~= 0 then
                                print("Ignoring the command since commit " .. blamed_commit_parent .. " doesn't exist!")
                                return
                            end

                            -- Use concrete commit SHA rather than SHA and ^ notation.
                            blamed_commit_parent = result.stdout:match("%S+")

                            -- Create new tab which is viewing current version of the file.
                            vim.cmd(":tabnew %")

                            -- Use new tab to show changes that blamed commit introduced.
                            gitsigns.show(blamed_commit, function() gitsigns.diffthis(blamed_commit_parent) end)
                        end)
                end)
        end,
        { buffer = buffer_number, desc = "[Enter] new tab and show previous changes that modified current line" })