-
-
Notifications
You must be signed in to change notification settings - Fork 617
feat: add TreePreOpen event #3105
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Could you please give us some examples why is it needed? |
So in my personal setup, I wanted a way to auto adjust split sizes in a tabpage, but the opening/closing of the NvimTree window would usually reset the layout. Also, none of the builtin Win events in NeoVim get triggered before a window is created or removed; so I wasn't able to use those either. When I saw the TreeOpen event, I thought to add this TreePreOpen event, which is what I needed. Frankly I don't currently have a use of TreePreClose, but just added it for symmetry :) |
Test Case: -- subscribe to events
api.events.subscribe(api.events.Event.TreePreOpen, function(data)
log.line("dev", "Event.TreePreOpen %s", vim.inspect(data))
end)
api.events.subscribe(api.events.Event.TreeOpen, function(data)
log.line("dev", "Event.TreeOpen %s", vim.inspect(data))
end)
api.events.subscribe(api.events.Event.TreePreClose, function(data)
log.line("dev", "Event.TreePreClose %s", vim.inspect(data))
end)
api.events.subscribe(api.events.Event.TreeClose, function(data)
log.line("dev", "Event.TreeClose %s", vim.inspect(data))
end)
-- map
vim.keymap.set("n", "<leader>c", function() api.tree.open({ current_window = true }) end, { noremap = true })
vim.keymap.set("n", "<leader>n", function() api.tree.open({ current_window = false }) end, { noremap = true }) Open in current then
Open in new then
Open two windows, open in current, then
|
At a minimum we need to fix 2. This is a bug and will be more of a problem for PreOpen events than the Open event, as users are more likely to be executing non-idempotent actions during PreOpen. How? I reckon we could move it down into view: |
I'd also like to fix 1. Events not firing is not acceptable and we're compounding the problem with a new event. It should be achievable via a vim event listener. What are your thoughts on all of this @gegoune ? |
I've invited you to write access to this project @devansh08 ; you can push directly instead of having to fork every time. |
Sure @alex-courtis, will take a look at and try to implement separately what you have suggested for these cases. Open to thoughts from others as well. |
So in lib, the open event is fired from
For this, I see that NvimTreeClose triggers WinClosed event, via vim.api.nvim_create_autocmd("WinClosed", { pattern = "*", callback = function(e) print("WinClosed: " .. vim.inspect(e)) end })
vim.api.nvim_create_autocmd("QuitPre", { pattern = "*", callback = function(e) print("QuitPre: " .. vim.inspect(e)) end })
-- Running `:q` in the NvimTree buffer (with current_window true or false)
--[[ Output:
QuitPre: {
buf = 9,
event = "QuitPre",
file = "/home/devansh/NvimTree_1",
id = 146,
match = "/home/devansh/NvimTree_1"
}
WinClosed: {
buf = 9,
event = "WinClosed",
file = "1011",
id = 145,
match = "1011"
}
]] Thoughts ? |
Unfortunately |
Sounds great... it will be a pleasure to have complete and reliable events! |
Ah, I see... Alright, will remove the event dispatch from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed invalid events._dispatch_on_tree_pre_open()
in lib.lua
Close, other windows present:
-
:q TreePreClose not called -
:NvimTreeClose
Open:
-
vim.keymap.set("n", "<leader>c", function() api.tree.open({ current_window = true }) end, { noremap = true })
TreePreOpen not called -
vim.keymap.set("n", "<leader>n", function() api.tree.open({ current_window = false }) end, { noremap = true })
-
:NvimTreeOpen -
:NvimTreeToggle
The open case seems to be resolved if a matching events._dispatch_on_tree_pre_open()
is added to view.open_in_win
The close case is problematic; needs some thought.
See #3105 (comment)
Apologies for not asking; in the interest of speed I merged #3107 and rebased this branch. |
Looking at add debug -- Handles event dispatch when tree is closed by `:q`
create_nvim_tree_autocmd("WinClosed", {
pattern = "*",
---@param ev vim.api.keyset.create_autocmd.callback_args
callback = function(ev)
if vim.api.nvim_get_option_value("filetype", { buf = ev.buf }) == "NvimTree" then
log.line("dev", "au WinClosed\nev=%s\nwins=%s", vim.inspect(ev), vim.inspect(vim.api.nvim_list_wins()))
require("nvim-tree.events")._dispatch_on_tree_close()
end
end,
}) shows
That resolves the pre close event problem - this event is pre close rather than close. Open question: how to we fire the closed event? Edit: suggestion: don't have a PreClose event:
|
Apologies for the long train of thought for what is now a small change. Action needed:
|
Yeah that makes sense. The best that could be done is a pre and post on buffer close rather than window close, using Will remove |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All test cases from both PRs successful.
Many thanks for this most difficult contribution.
IMO events are the most difficult bit of vim to get right.
Thanks for the help along the way! 😃 |
This adds 2 new events
TreePreOpen
andTreePreClose
, which get triggered beforeTreeOpen
andTreeClose
events. They work similar toTreeOpen
andTreeClose
, in terms of the handler function.This should help running logic before the NvimTree window is created or removed.
Let me know your thoughts.
Thanks!