[mini.diff] Add customization options to gen_source.git

[zelch]

Contributing guidelines

• I have read CONTRIBUTING.md
• I have read CODE_OF_CONDUCT.md

Module(s)

mini.diff

Description

My use case is fairly straight forward: I use yadm (Yet Another Dotfiles Manager), and I want mini.diff to work reliably.

yadm is essentially a wrapper around git where the repo directory and the work directory differ.

Currently, I'm doing a copy and paste of all of the git functionality from mini.diff to create a yadm source, and replacing all of the references to git with yadm, but that's a pain to maintain, and really, it should be unnecessary.

The only thing that I need to be able to customize is the git command itself, though, if I were implementing this as a general thing, I'd add the ability to add custom arguments to all command runs as well.

Ideally, I would be able to do something like:

      source = {
        MiniDiff.gen_source.git(),
        MiniDiff.gen_source.git({git_cmd = "yadm"}),
        MiniDiff.gen_source.save(),
      },

[echasnovski]

Thanks for the suggestion!

yadm is essentially a wrapper around git where the repo directory and the work directory differ.

Can you please elaborate a bit more? Currently (I think) 'mini.diff' should work regardless of where the .git directory is located. It asks git itself about where '.git' is located and then should use it.

... and I want mini.diff to work reliably.

Please describe current problem(s) with 'mini.diff' in your use case. It would be more helpful if you provide a step-by-step reproduction for people who don't know what yadm is and how to use it.

---

In theory, allowing custom path to Git executable is reasonable and doable, but I have my doubts that it will be useful in a way you describe. In particular, I don't know how using both git() and git({ exec = 'yadm' }) will work here.

[zelch]

yadm sets things up with a git directory that isn't .git

 ~❯ git rev-parse --show-gitdir
fatal: not a git repository (or any of the parent directories): .git
 ~❯ yadm rev-parse --git-dir    
/home/warp/.local/share/yadm/repo.git
 ~❯ yadm rev-parse --show-toplevel
/home/warp
 ~❯ git --git-dir=.local/share/yadm/repo.git status 
On branch main
Your branch is up to date with 'origin/main'.

nothing to commit (use -u to show untracked files)

The easiest way to interact with this is to just use the 'yadm' command, as it wraps git and adds the necessary arguments so that git can find the repository correctly.

[echasnovski]

The easiest way to interact with this is to just use the 'yadm' command, as it wraps git and adds the necessary arguments so that git can find the repository correctly.

I see. Could you please try applying the following patch to your 'mini.diff' installation and see if it makes git() source work as you expect with yadm? In a way that you described earlier: with config.source being array of { git(), git({ exec = 'yadm' }) }. I want to be sure that simply allowing different executable fixes this use case:

'mini.diff' patch

diff --git a/lua/mini/diff.lua b/lua/mini/diff.lua
index fddf111e..dd909dd7 100644
--- a/lua/mini/diff.lua
+++ b/lua/mini/diff.lua
@@ -643,7 +643,10 @@ MiniDiff.gen_source = {}
 --- - There is no capability for unstaging hunks. Use full Git client for that.
 ---
 ---@return table Source. See |MiniDiff-source-specification|.
-MiniDiff.gen_source.git = function()
+MiniDiff.gen_source.git = function(opts)
+  opts = vim.tbl_extend('force', { exec = 'git' }, opts or {})
+  local exec = opts.exec
+
   local attach = function(buf_id)
     -- Try attaching to a buffer only once
     if H.git_cache[buf_id] ~= nil then return false end
@@ -652,7 +655,7 @@ MiniDiff.gen_source.git = function()
     if path == '' then return false end
 
     H.git_cache[buf_id] = {}
-    H.git_start_watching_index(buf_id, path)
+    H.git_start_watching_index(buf_id, exec, path)
   end
 
   local detach = function(buf_id)
@@ -662,10 +665,10 @@ MiniDiff.gen_source.git = function()
   end
 
   local apply_hunks = function(buf_id, hunks)
-    local path_data = H.git_get_path_data(H.get_buf_realpath(buf_id))
+    local path_data = H.git_get_path_data(exec, H.get_buf_realpath(buf_id))
     if path_data == nil or path_data.rel_path == nil then return end
     local patch = H.git_format_patch(buf_id, hunks, path_data)
-    H.git_apply_patch(path_data, patch)
+    H.git_apply_patch(exec, path_data, patch)
   end
 
   return { name = 'git', attach = attach, detach = detach, apply_hunks = apply_hunks }
@@ -1673,7 +1676,7 @@ H.export_qf = function(opts)
 end
 
 -- Git ------------------------------------------------------------------------
-H.git_start_watching_index = function(buf_id, path)
+H.git_start_watching_index = function(buf_id, exec, path)
   -- NOTE: Watching single 'index' file is not enough as staging by Git is done
   -- via "create fresh 'index.lock' file, apply modifications, change file name
   -- to 'index'". Hence watch the whole '.git' (first level) and react only if
@@ -1702,19 +1705,19 @@ H.git_start_watching_index = function(buf_id, path)
 
     -- Set up index watching
     local git_dir_path = table.concat(stdout_feed, ''):gsub('\n+$', '')
-    H.git_setup_index_watch(buf_id, git_dir_path)
+    H.git_setup_index_watch(buf_id, exec, git_dir_path)
 
     -- Set reference text immediately
-    H.git_set_ref_text(buf_id)
+    H.git_set_ref_text(buf_id, exec)
   end
 
-  process = vim.loop.spawn('git', spawn_opts, on_exit)
+  process = vim.loop.spawn(exec, spawn_opts, on_exit)
   H.git_read_stream(stdout, stdout_feed)
 end
 
-H.git_setup_index_watch = function(buf_id, git_dir_path)
+H.git_setup_index_watch = function(buf_id, exec, git_dir_path)
   local buf_fs_event, timer = vim.loop.new_fs_event(), vim.loop.new_timer()
-  local buf_git_set_ref_text = function() H.git_set_ref_text(buf_id) end
+  local buf_git_set_ref_text = function() H.git_set_ref_text(buf_id, exec) end
 
   local watch_index = function(_, filename, _)
     if filename ~= 'index' then return end
@@ -1728,7 +1731,7 @@ H.git_setup_index_watch = function(buf_id, git_dir_path)
   H.git_cache[buf_id] = { fs_event = buf_fs_event, timer = timer }
 end
 
-H.git_set_ref_text = vim.schedule_wrap(function(buf_id)
+H.git_set_ref_text = vim.schedule_wrap(function(buf_id, exec)
   if not vim.api.nvim_buf_is_valid(buf_id) then return end
   local buf_set_ref_text = vim.schedule_wrap(function(text) pcall(MiniDiff.set_ref_text, buf_id, text) end)
 
@@ -1759,11 +1762,11 @@ H.git_set_ref_text = vim.schedule_wrap(function(buf_id)
     buf_set_ref_text(text)
   end
 
-  process = vim.loop.spawn('git', spawn_opts, on_exit)
+  process = vim.loop.spawn(exec, spawn_opts, on_exit)
   H.git_read_stream(stdout, stdout_feed)
 end)
 
-H.git_get_path_data = function(path)
+H.git_get_path_data = function(exec, path)
   -- Get path data needed for proper patch header
   local cwd, basename = vim.fn.fnamemodify(path, ':h'), vim.fn.fnamemodify(path, ':t')
   local stdout = vim.loop.new_pipe()
@@ -1781,7 +1784,7 @@ H.git_get_path_data = function(path)
     res.mode_bits, res.eol, res.rel_path = string.match(out, '^(%d+) (%S+) (.*)$')
   end
 
-  process = vim.loop.spawn('git', spawn_opts, on_exit)
+  process = vim.loop.spawn(exec, spawn_opts, on_exit)
   H.git_read_stream(stdout, stdout_feed)
   vim.wait(1000, function() return did_exit end, 1)
   return res
@@ -1818,12 +1821,12 @@ H.git_format_patch = function(buf_id, hunks, path_data)
   return res
 end
 
-H.git_apply_patch = function(path_data, patch)
+H.git_apply_patch = function(exec, path_data, patch)
   local stdin = vim.loop.new_pipe()
   local args = { 'apply', '--whitespace=nowarn', '--cached', '--unidiff-zero', '-' }
   local spawn_opts = { args = args, cwd = path_data.cwd, stdio = { stdin, nil, nil } }
   local process
-  process = vim.loop.spawn('git', spawn_opts, function() process:close() end)
+  process = vim.loop.spawn(exec, spawn_opts, function() process:close() end)
 
   -- Write patch, notify that writing is finished (shutdown), and close
   for _, l in ipairs(patch) do

[zelch]

Hm, that's only working if I put the yadm source before the non-yadm source...

But if I change all the git_cache references to H.git_cache[buf_id .. "/" .. exec] then everything seems to work!

[echasnovski]

Hm, that's only working if I put the yadm source before the non-yadm source...

That's good enough for me and probably makes more sense (usually more narrowly scoped source should be tried before more widely scoped ones). I am not really keen on complicating internal caching just for this use case alone.

I'll think a bit more if this is the most reasonable way to address this. Thank you for the testing this particular approach!

[zelch]

Sadly, putting yadm first doesn't work very well, for two reasons.

The first is that it still gets cached in git_cache as non-working, that's a pain.

The second is that scoping wise, yadm covers everything in the user's home directory, including all subdirectory trees.

So I'm pretty sure that the only ways to make this work are to either include the exec in the cache key, or to use a separate cache entirely.

[zelch]

(Sorry, migraine is kicking in, that wasn't as clear as it could be.)

Without the change for git_cache, if yadm is first mini.diff won't pick up on any non-yadm git repositories because of the caching.

[echasnovski]

Without the change for git_cache, if yadm is first mini.diff won't pick up on any non-yadm git repositories because of the caching.

Ah, I see what you mean. There still should be no reason to complicate caching that much. But the fact that it would require any change at all doesn't make this more likely to be added, unfortunately. I'll take a closer look later then.

[zelch]

Just to confirm, this is only an issue for H.git_cache, not for H.cache.

Which, on top of the diff you passed, looks like:

mini_diff.patch

[zelch]

An alternate implementation would be moving all of the H.git_* items into MiniDiff.gen_source.git, at which point the cache is unchanged simply because it's local to the source.

That's something like this:

mini.diff.patch

Though, really, at that point, I could argue that it should be in a separate module, and anything that it depends on from H should be made accessible off of MiniDiff for other source modules to use.

[zelch]

I only see one thing that's non-trivial for moving the git source to it's own submodule is the git_start_watching_index on_not_in_git, where it's doing H.cache[buf_id] = nil.

I'm not clear on what exactly that's trying to do, which makes it somewhat difficult to figure out how to expose that functionality for other sources to use.

[echasnovski]

An alternate implementation would be moving all of the H.git_* items into MiniDiff.gen_source.git, at which point the cache is unchanged simply because it's local to the source.

Yeah, that might make sense. Will have to look how to make it cleaner than putting everything inside gen_source.git function.

Though, really, at that point, I could argue that it should be in a separate module, and anything that it depends on from H should be made accessible off of MiniDiff for other source modules to use.

There won't be any separate modules for this, as it makes things too complex. There also won't be that many helpers exported, as it makes things too complex.

If that were the only way forward, then this feature would be marked as out of scope of 'mini.diff'.

[zelch]

Maybe something more like this?

mini.diff.3.patch