feat(actions): git-aware cut / paste / rename fs actions #2542

hinell · 2023-11-19T23:37:21Z

git-actions-demo.mov

git-aware file renames right in your nvim-tree; refactor your source tree easily
NOTE: Demo uses rebound default x,c,p,r,e,u,<c-r> keymap bindings
NOTE: Letter R signifies git renamed file (setup by confi.renderer.icons.glyphs.git

This PR allows:

Moving/renaming git-tracked files by using fs.* like file actions for cut / paste / rename
Moving/renaming non-tracked files by fs.{cut,paste} as usual
Wraps around fs.* APIs nicely, minimize redundancy
Utilizes log / info utils
Closes Git Actions #2474

Usage

No keymaps setup by default. Example:

-- $HOME/.config/nvim/lua/user/nvim-keymaps.lua
-- put this inside autocmd callback for NvimTree filetype, see `h autocmd` and `h nvim_create_autocmd()`
local opts = { buffer = autoCmdEvent.buf, nowait = true }
local api  = require "nvim-tree.api"
vim.keymap.set("n", "x"    , api.git.cut            , vim.tbl_extend("keep", { desc = "nvim-tree: git: cut file into clipboard"   }, opts)
vim.keymap.set("n", "p"    , api.git.paste          , vim.tbl_extend("keep", { desc = "nvim-tree: git: past file from clipboard"  }, opts)
vim.keymap.set("n", "r"    , api.git.rename         , vim.tbl_extend("keep", { desc = "nvim-tree: git: rename: filename"          }, opts)
vim.keymap.set("n", "e"    , api.git.rename_basename, vim.tbl_extend("keep", { desc = "nvim-tree: git: rename: filename base"     }, opts)
vim.keymap.set("n", "u"    , api.git.rename_absolute, vim.tbl_extend("keep", { desc = "nvim-tree: git: rename: filename absolute" }, opts)
vim.keymap.set("n", "<C-r>", api.git.rename_sub     , vim.tbl_extend("keep", { desc = "nvim-tree: git: rename: omit filename"	  }, opts)

API

Adds:
api.git.cut(node) - store a file under cursor into a git actions clipboard (just like api.fs.cut)
api.git.paste(node) - pop files from clipboard into a folder under cursor
api.git.rename(node) - rename file under cursor
api.git.rename_sub(node)
api.git.rename_basename(node)
api.git.rename_absolute(node)
api.git.clear_clipboard(node)
api.git.print_clipboard(node)
Utils:
git(uvSpawnOptions, onErrOrData, onExit) - a wrapper func around git cli command (uses uv)
git_is_tracked(path, cb, onExit) - checks whether given path is tracked
git_mv(path_src, path_dst, callback, onExit) - moves files around by using git
Clipboard - a class for future for clipboard; used for git actions local copy / paste stack

Design considerations

I'm aware of git runner for git but as I said earlier elsewhere - it's too limited to be used for a general purpose git commands, so this PR introduces a full-fledged proxy to git.

lua/nvim-tree/actions/git/cli.lua

alex-courtis

This is a fantastic! I love the direction you are taking with the fallback pattern, which won't disturb current users.

I think a lot of users, myself included, will end up mapping to these actions instead of the defaults.

This one will take a while, I'd like to dogfood this for some time before merge.

Big concern: repetition -> maintainability. There's a lot of duplicated code here. Suggestions:

Fold actions/git/init.lua actions/git/init.lua into the existing copy-paste.lua, rename-file.lua etc, extending the current persistence mechanisms as needed.
Use the existing git/runner.lua, exposing internal API / refactoring as necessary.
There's a lot of great improvements here that should be applied to existing code. Not in this PR. One of:
- Prefactor PR for existing copy-paste.lua etc. before this PR
- Tidy / improvement PR after merging this PR

doc/nvim-tree-lua.txt

alex-courtis · 2023-11-20T01:02:05Z

doc/nvim-tree-lua.txt

@@ -1906,9 +1906,67 @@ node.run.system()                            *nvim-tree-api.node.run.system()*
 ==============================================================================
 6.4 API GIT                                               *nvim-tree-api.git*

+This api is a set of |nvim-tree-api.fs|.* like functions that use git cli 
+for file cutting, pasting, and renaming instead of system routines.
+Many actions in the git API automatically fallback to the |nvim-tree-api.fs|


Great work.

lua/nvim-tree/actions/git/init.lua

alex-courtis · 2023-11-20T01:08:54Z

lua/nvim-tree/actions/git/cli.lua

+
+  local stdin = uv.new_pipe()
+  local stdout = uv.new_pipe()
+  local stderr = uv.new_pipe()


This functionality appears to duplicate git/runner.lua.

Let's not repeat ourselves and share that code.

Please merge:
#2551

lua/nvim-tree/actions/git/init.lua

alex-courtis · 2023-11-20T01:11:13Z

lua/nvim-tree/actions/git/init.lua

+      events._dispatch_will_rename_node(path_src, path_dst)
+      M.prompt_for_rename(node, { absolute = true })
+      -- Skip git mv
+      goto loop


goto? no thank you. Standard flow of control please.

alex-courtis · 2023-11-20T01:15:38Z

lua/nvim-tree/actions/git/clipboard.lua

+--- @class gitClipboard
+--- @field prototype gitClipboardInstance
+--- @field prototype.constructor gitClipboard
+M.Clipboard = { prototype = { constructor = M.Clipboard } }


The prototype pattern is surfacing a lot of warnings and confusing lua_language_server. Any reason we need that over an "ordinary" class?

alex-courtis · 2023-11-20T01:19:04Z

lua/nvim-tree/actions/git/init.lua

+  local prompt_default = ""
+  prompt_default = opts.absolute and node.absolute_path or prompt_default
+  prompt_default = opts.filename and fnamemodify(node.absolute_path, ":t") or prompt_default
+  prompt_default = opts.basename and fnamemodify(node.absolute_path, ":t:r") or prompt_default


This method repeats a lot of rename-file.lua.

Can we please share?

It can be shared by a separate refactor(actions): rename-file.lua PR. Are you going to accept a separate PR with rename-file.lua refactoring?

Currently rename-file.lua isn't exporting internal utils so it's not reusable. The chunk of code above si similar to (IMO very inefficient) following string comparison:

nvim-tree.lua/lua/nvim-tree/actions/fs/rename-file.lua

Lines 64 to 79 in 46e1f77

local namelen = node.name:len()

local directory = node.absolute_path:sub(0, namelen * -1 - 1)

local default_path

local prepend = ""

local append = ""

default_path = vim.fn.fnamemodify(node.absolute_path, modifier)

if modifier:sub(0, 2) == ":t" then

prepend = directory

end

if modifier == ":t:r" then

local extension = vim.fn.fnamemodify(node.name, ":e")

append = extension:len() == 0 and "" or "." .. extension

end

if modifier == ":p:h" then

default_path = default_path .. "/"

end

It can be shared by a separate refactor(actions): rename-file.lua PR. Are you going to accept a separate PR with rename-file.lua refactoring?

Absolutely! That's what I meant by "prefactor". Sorry, I shoud have been clearer.

Something like "chore(#2542): refactor rename file"

Or refactor(…): …. :)

Heh... didn't realise that was a prefix, thank you.

Please merge:
#2550

This PR is going to use common code from that PR. The refactoring is dead-simple.

alex-courtis · 2023-11-20T01:20:00Z

lua/nvim-tree/actions/git/init.lua

+---@param node table
+---@return HL_POSITION position none when clipboard empty
+---@return string|nil group only when node present in clipboard
+function M.get_highlight(node)


What is calling this?

alex-courtis · 2023-11-20T03:25:37Z

ALTERNATIVE:

This isn't particularly tightly coupled with nvim-tree. It could easily be extracted into an extension plugin.

What do you reckon @gegoune @Akmadan23 ?

Akmadan23 · 2023-11-20T09:42:15Z

This isn't particularly tightly coupled with nvim-tree.

That's true, but not completely... Nvim-tree already has some level of git integration, so it makes sense to enhance this aspect, and having these features ready out of the box would be really handy.

hinell · 2023-11-20T20:07:04Z

It could easily be extracted into an extension plugin.

@alex-courtis I agree with @Akmadan23 - support for git commands out of the box is essential.

I'm going to do some refactoring. Many modules in this project simply don't export routines that can be reused so I was forced to duplicate them. I'm under impression that the code was NOT intended to be modular/reusable in first place or authors werent even suspecting that the code might be used elsewhere.

As you can see in this PR I've exported everything to make nvim-tree git actions highly reusable. We can even use parts of it in some other plugins like lualine e.g. to check clipboard status.

I also plan to introduce simple undo history in the future, when I have enough time on my hands.

Merging with `fs` actions

Fold actions/git/init.lua actions/git/init.lua into the existing copy-paste.lua, rename-file.lua etc, extending the current persistence mechanisms as needed.

By nature, both fs and git actions are different. It's waaay better to keep them separate. In future, git actions may be extended to more git commands or even see integration with others plugins.

My suggestion is to put and exporte common / reusable code into functions or classes. This will decrease amount of code needed to be maintained.

hinell · 2023-11-20T21:11:36Z

On why this is not a plugin

tl;dr: nvim-tree doesn't provide any sane API for plugins.

It would be nice if nvim-tree borrows some nvim-neo-tree idea of Sources™, but in a sense of plugins. You can take a look how plugins for nvim-treesitter or telescope.nvim or nvim-cmp are managed. nvim-tree sadly is nowhere close.

If nvim-tree provided a generic UI Tree Widget along with standard plugins like lsp, fs, which would essentially provide various tweaks over default UI Tree widget, it would greatly simplify architecture.

Like, what if plugin spec looked like this?:

plugin.source     -- a table of funcs that export recursive table to be displayed as a tree;  user can switch sources
plugin.source_ui  -- a table of functions handling UI events emitted by nvim-tree to modify appearance
plugin.commands   -- i.e. a table actions exposed to user / others plugins (e.g. actions over focused node)
plugin.clipboard  -- integration with nvim-tree global clipboard
plugin.history    -- integration with nvim-tree actions history (i.e. undo/redo lists config)

--- etc

The one would make extending nvim-tree very simple.

Btw, nvim-neo-tree has implemented git actions long ago.

alex-courtis · 2023-11-21T00:10:56Z

I also plan to introduce simple undo history in the future, when I have enough time on my hands.

That would indeed be useful; I've reflexively tried that a few times and was disappointed.

alex-courtis · 2023-11-21T00:17:15Z

That's true, but not completely... Nvim-tree already has some level of git integration, so it makes sense to enhance this aspect, and having these features ready out of the box would be really handy.

tl;dr: nvim-tree doesn't provide any sane API for plugins.

As you can see in this PR I've exported everything to make nvim-tree git actions highly reusable. We can even use parts of it in some other plugins like lualine e.g. to check clipboard status.

Fair enough, this functionality is suited for nvim-tree itself rather than another plugin.

RE API: https://github.com/nvim-tree/nvim-tree.lua#roadmap we have been adding API as needed, however that's not practical for this case.

alex-courtis · 2023-11-21T00:19:43Z

I'm under impression that the code was NOT intended to be modular/reusable in first place or authors werent even suspecting that the code might be used elsewhere.

We are always striving to improve. Modularising / exposing is desirable.

My suggestion is to put and exporte common / reusable code into functions or classes. This will decrease amount of code needed to be maintained.

Yes please!

alex-courtis · 2023-11-26T23:15:10Z

Before I can proceed, I request the following to be merged:
* [refactor(actions): common code for rename-file #2550](https://github.com/nvim-tree/nvim-tree.lua/pull/2550)

* [refactor(git): common code to a generic git cll #2551](https://github.com/nvim-tree/nvim-tree.lua/pull/2551)
UPD: I've refactored this PR to address requested changes.

To actually quickly test this PR, you have to pull down the above PRs and apply changes as patches (no changes in your workint tree are assumed):
 gh pr checkout 2550
 gh pr checkout 2551

git co actions-git
git checkout -p git-integration-refactor lua/nvim-tree/git/runner.lua
git checkout -p actions-fs-rename lua/nvim-tree/actions/fs/rename-file.lua lua/nvim-tree/utils-ui.lua
git checkout -p actions-fs-rename lua/nvim-tree/api.lua
Use reset --hard to reset all files.

Thanks for the detailed instructions.

Unfortunately my time is limited. I'll process PRs sequentially: #2551 then #2550 then this PR.

gegoune · 2023-11-27T09:52:16Z

I have unresolved reviews from Alex that @hinell resolved. @hinell once again, review authors are to resolve their comments.

Use git cut / rename / paste actions for tracked files Use wrapper fs.* for untracked files (rename, cut, and paste) Add docs on new git actions Highlighting for clipboard cut Upgradee git cli wrapper with more functions (rm, mv, is_tracked) Cut clipboard for git actions (based on Set class)

alex-courtis · 2023-11-28T01:19:57Z

See #2551 (comment)

alex-courtis reviewed Nov 20, 2023

View reviewed changes

lua/nvim-tree/actions/git/cli.lua Outdated Show resolved Hide resolved

alex-courtis requested changes Nov 20, 2023

View reviewed changes

alex-courtis marked this pull request as draft November 26, 2023 05:44

hinell force-pushed the actions-git branch 3 times, most recently from fee8231 to 3653a09 Compare November 26, 2023 21:14

hinell force-pushed the actions-git branch 2 times, most recently from 65ffd5a to 59ffa4d Compare November 27, 2023 10:23

hinell force-pushed the actions-git branch from 59ffa4d to 70fa1b2 Compare November 27, 2023 13:52

alex-courtis closed this Nov 28, 2023

hinell deleted the actions-git branch November 28, 2023 09:33

hinell restored the actions-git branch November 28, 2023 09:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(actions): git-aware cut / paste / rename fs actions #2542

feat(actions): git-aware cut / paste / rename fs actions #2542

hinell commented Nov 19, 2023 •

edited

Loading

alex-courtis left a comment

alex-courtis Nov 20, 2023

alex-courtis Nov 20, 2023

hinell Nov 22, 2023

alex-courtis Nov 20, 2023

alex-courtis Nov 20, 2023

alex-courtis Nov 20, 2023

hinell Nov 20, 2023

alex-courtis Nov 21, 2023

gegoune Nov 21, 2023

alex-courtis Nov 21, 2023

hinell Nov 22, 2023 •

edited

Loading

alex-courtis Nov 20, 2023

alex-courtis commented Nov 20, 2023

Akmadan23 commented Nov 20, 2023

hinell commented Nov 20, 2023 •

edited

Loading

hinell commented Nov 20, 2023 •

edited

Loading

alex-courtis commented Nov 21, 2023

alex-courtis commented Nov 21, 2023

alex-courtis commented Nov 21, 2023

alex-courtis commented Nov 26, 2023

UPD: I've refactored this PR to address requested changes.

gegoune commented Nov 27, 2023

alex-courtis commented Nov 28, 2023

	local namelen = node.name:len()
	local directory = node.absolute_path:sub(0, namelen * -1 - 1)
	local default_path
	local prepend = ""
	local append = ""
	default_path = vim.fn.fnamemodify(node.absolute_path, modifier)
	if modifier:sub(0, 2) == ":t" then
	prepend = directory
	end
	if modifier == ":t:r" then
	local extension = vim.fn.fnamemodify(node.name, ":e")
	append = extension:len() == 0 and "" or "." .. extension
	end
	if modifier == ":p:h" then
	default_path = default_path .. "/"
	end

feat(actions): git-aware cut / paste / rename fs actions #2542

feat(actions): git-aware cut / paste / rename fs actions #2542

Conversation

hinell commented Nov 19, 2023 • edited Loading

Usage

API

Design considerations

alex-courtis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hinell Nov 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alex-courtis commented Nov 20, 2023

Akmadan23 commented Nov 20, 2023

hinell commented Nov 20, 2023 • edited Loading

Merging with fs actions

hinell commented Nov 20, 2023 • edited Loading

On why this is not a plugin

alex-courtis commented Nov 21, 2023

alex-courtis commented Nov 21, 2023

alex-courtis commented Nov 21, 2023

alex-courtis commented Nov 26, 2023

UPD: I've refactored this PR to address requested changes.

gegoune commented Nov 27, 2023

alex-courtis commented Nov 28, 2023

hinell commented Nov 19, 2023 •

edited

Loading

hinell Nov 22, 2023 •

edited

Loading

hinell commented Nov 20, 2023 •

edited

Loading

Merging with `fs` actions

hinell commented Nov 20, 2023 •

edited

Loading