sindrets / diffview.nvim

Single tabpage interface for easily cycling through diffs for all modified files for any git rev.
Other
3.57k stars 101 forks source link

feat(actions): added `select_{first|last}_entry` #510

Closed yungthai closed 3 weeks ago

yungthai commented 3 weeks ago

Resolves #421

There are some extraneous formatting committed (into single lines) due to the following setting within stylua.toml below. StyLua was used.

collapse_simple_statement = "Always"

After the above formatter option was introduced, I had expected short if blocks to be formatted onto single lines.

Would you please confirm whether StyLua is used as the Lua formatter? I'll tidy up those unneccessary formatting once you confirm the formatter tool & its options.

sindrets commented 3 weeks ago

This looks vastly overcomplicated for what should essentially just be view:set_file(files[1]) and view:set_file(files[#files]).

Something like this (untested):

diff --git a/lua/diffview/actions.lua b/lua/diffview/actions.lua
index 07e3c55..91c8178 100644
--- a/lua/diffview/actions.lua
+++ b/lua/diffview/actions.lua
@@ -635,6 +635,8 @@ local action_names = {
   "select_entry",
   "select_next_entry",
   "select_prev_entry",
+  "select_first_entry",
+  "select_last_entry",
   "stage_all",
   "toggle_files",
   "toggle_flatten_dirs",
diff --git a/lua/diffview/scene/views/diff/listeners.lua b/lua/diffview/scene/views/diff/listeners.lua
index e0876aa..1fb8b83 100644
--- a/lua/diffview/scene/views/diff/listeners.lua
+++ b/lua/diffview/scene/views/diff/listeners.lua
@@ -71,6 +71,14 @@ return function(view)
     select_prev_entry = function()
       view:prev_file(true)
     end,
+    select_first_entry = function()
+      local files = view.panel:ordered_file_list()
+      if files and #files > 0 then view:set_file(files[1]) end
+    end,
+    select_last_entry = function()
+      local files = view.panel:ordered_file_list()
+      if files and #files > 0 then view:set_file(files[#files]) end
+    end,
     next_entry = function()
       view.panel:highlight_next_file()
     end,
diff --git a/lua/diffview/scene/views/file_history/listeners.lua b/lua/diffview/scene/views/file_history/listeners.lua
index 897f4c1..ca8bbdf 100644
--- a/lua/diffview/scene/views/file_history/listeners.lua
+++ b/lua/diffview/scene/views/file_history/listeners.lua
@@ -59,6 +59,14 @@ return function(view)
     select_prev_entry = function()
       view:prev_item()
     end,
+    select_first_entry = function()
+      local entry = view.panel.entries[1]
+      if entry and #entry.files > 0 then view:set_file(entry.files[1]) end
+    end,
+    select_last_entry = function()
+      local entry = view.panel.entries[#view.panel.entries]
+      if entry and #entry.files > 0 then view:set_file(entry.files[#entry.files]) end
+    end,
     next_entry = function()
       view.panel:highlight_next_file()
     end,
yungthai commented 3 weeks ago

Thanks for the improvement suggestions.

I have simplified the diff file_panel listeners, as there is no need to support a motion count within that panel.

For the file history panel: There may be some benefit to navigate to the first or last file entry of +/- x number of commits, rather than just jump to the first and last file within the file history panel. I am ok with your simplified suggestion for file history too, if that's what you prefer.

sindrets commented 3 weeks ago

There may be some benefit to navigate to the first or last file entry of +/- x number of commits, rather than just jump to the first and last file within the file history panel.

No, not for an action named select_{first|last}_file. It should just do as its name implies, there's no reason for this action to use v:count. For file based jumping that respects v:count we already have [count] select_{next|prev}_file. It sounds like you want another action as well. Something like [count] select_{next|prev}_commit. That's fine, we can add that as well, but that should be a separate PR.

Also, please remove all unrelated formatting changes. It only adds noise to the diffs.

yungthai commented 3 weeks ago

Will follow up with a separate PR for the [count] select_{next|prev}_commit.