Skip to content

Event.TreeAttachedPost #1869

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

Closed
alex-courtis opened this issue Dec 31, 2022 · 20 comments · Fixed by #1877
Closed

Event.TreeAttachedPost #1869

alex-courtis opened this issue Dec 31, 2022 · 20 comments · Fixed by #1877
Labels
API API or Event

Comments

@alex-courtis
Copy link
Member

Event fired following on_attach call.

Context:
#1858
mrjones2014/legendary.nvim#289 (comment)

Legendary has a requirement to access / mutate user's mappings set via their on_attach.

@alex-courtis
Copy link
Member Author

@mrjones2014 I would be grateful if you tested the new event:

cd /path/to/nvim-tree.lua
git pull
git checkout 1869-add-on-attach-post-event

:help nvim-tree-events

@mrjones2014
Copy link
Collaborator

Working on testing this today.

@mrjones2014
Copy link
Collaborator

mrjones2014 commented Jan 2, 2023

So, I can probably make this work by using a FileType autocmd as a fallback, but the current API is not sufficient if the user is lazy-loading. The current events API requires that nvim-tree is loaded in order to subscribe to events. This doesn't really work if the user is lazy-loading nvim-tree, because it won't be loaded when legendary.nvim config runs, and so we won't be able to do require('nvim-tree.api').

Would it be possible to also use Neovim's built-in Autocmd mechanisms? See :h User to fire custom autocmd events. This way, I can subscribe to the event without nvim-tree necessarily being loaded.

I think the dispatch system you have could be updated to support this somewhat easily. I think you could do something like this:

-- This gives a way for `autocmd` handlers to access your custom payload
-- since the payload for our `doautocmd` call will be the nvim-tree event name
M.payload = nil

local function dispatch(event_name, payload)
  -- temporarily store the payload
  M.payload = payload
  vim.cmd(string.format('doautocmd User NvimTree_%s', event_name))
  vim.schedule_wrap(function() M.payload = nil end)
  for _, handler in pairs(get_handlers(event_name)) do
    local success, error = pcall(handler, payload)
    if not success then
      notify.error("Handler for event " .. event_name .. " errored. " .. vim.inspect(error))
    end
  end
end

This would allow hooking into nvim-tree events without forcing nvim-tree to be loaded like so:

require('legendary').autocmd({
  'User',
  function()
    local payload = vim.deepcopy(require('nvim-tree.events').payload)
    -- use the payload if needed, or do whatever is needed
  end,
  opts = { pattern = 'NvimTree_TreeAttachedPost' },
})

@mrjones2014
Copy link
Collaborator

This would be backwards compatible, as the existing event API still exists, and subscribing using the API doesn't subscribe any autocmds, so users can choose to use one or the other.

@mrjones2014
Copy link
Collaborator

Also, we would need a way to tell if Event.TreeAttachedPost has already fired or not.

@mrjones2014
Copy link
Collaborator

I'm also noticing that we only get the event if the user has supplied a custom on_attach function -- we would need the event to fire regardless so that we know when it's safe to grab up-to-date keymaps.

@mrjones2014
Copy link
Collaborator

Hm, one other thing I'm noticing, nvim-tree doesn't necessarily have a stable buffer ID. It closes and recreates the buffer. This will potentially be solvable on the legendary.nvim side with mrjones2014/legendary.nvim#256

@alex-courtis
Copy link
Member Author

I'm also noticing that we only get the event if the user has supplied a custom on_attach function -- we would need the event to fire regardless so that we know when it's safe to grab up-to-date keymaps.

Ah yes, I was thinking of this as only for on_attach however it really should apply to all.

You'll then be able to examine buffer mappings directly or call config.mappings.current

Thinking about this some more: would it be useful to have TreeAttachPre and TreeAttachPost? You could use that to calculate the keymap delta i.e. the keymaps that the user has added.

@alex-courtis alex-courtis changed the title Event.OnAttachPost Event.TreeAttachedPost Jan 2, 2023
@mrjones2014
Copy link
Collaborator

I don't personally have a use-case for TreeAttachPre, but I would need a way to know if TreeAttachPost has already happened or not -- that is, have the user-configured keymaps been applied yet or do we need to set up the event listener to wait for them?

@alex-courtis
Copy link
Member Author

has already happened or not

I'm sorry I don't think I'm understanding...

User's on_attach or mappings.list will have been mapped at TreeAttachedPost. It's fired only when the tree buffer is created, and this mapping only occurs when creating the buffer.

@alex-courtis
Copy link
Member Author

Hm, one other thing I'm noticing, nvim-tree doesn't necessarily have a stable buffer ID. It closes and recreates the buffer. This will potentially be solvable on the legendary.nvim side with mrjones2014/legendary.nvim#256

Yes. The buffer will be deleted when the tree's window is closed. We use a simple vim.api.nvim_win_close(tree_win, true). Each tab has a separate tree buffer in its window. This could be changed.

I'm not sure that's something you should need to work around: how difficult is it?

@mrjones2014
Copy link
Collaborator

has already happened or not

Basically, in a lazy-loaded environment, I won't be able to tell if nvim-tree config ran before or after legendary.nvim config. If nvim-tree runs first, then the tree may have already been attached, in which case we wouldn't get the event, and we wouldn't load the keymaps into legendary.nvim. Does that make sense?

I'm not sure that's something you should need to work around: how difficult is it?

I have a PR up in legendary.nvim that allows filtering by filetype but I don't love the implementation (it adds a fair bit of complexity and requires doing multiple filtering passes). Ideally nvim-tree would have a stable buffer ID, so that setting the opts.buffer option on keymaps in legendary.nvim would work properly.

@alex-courtis
Copy link
Member Author

Basically, in a lazy-loaded environment, I won't be able to tell if nvim-tree config ran before or after legendary.nvim config. If nvim-tree runs first, then the tree may have already been attached, in which case we wouldn't get the event, and we wouldn't load the keymaps into legendary.nvim. Does that make sense?

I see. I did not consider the ordering case.

Options:

  • add a NvimTreeSetupPost event; you can subscribe to events before calling setup
  • add API like api.is_setup

@alex-courtis
Copy link
Member Author

Would it be possible to also use Neovim's built-in Autocmd mechanisms? See :h User to fire custom autocmd events. This way, I can subscribe to the event without nvim-tree necessarily being loaded.

I'll need to consider this one carefully; we went with a custom eventing system for simplicity and ease of end user use.

Additional events are always possible and cheap to implement.

@mrjones2014
Copy link
Collaborator

The main reason I think using the built-in autocmd system would be a lot better is for lazy-loading environments. Normally I would do something like local ok, nvim_tree = pcall(require, 'nvim-tree') to see if it is loaded, however in the newer plugin manager lazy.nvim, doing so will cause nvim-tree to be loaded.

So basically if the end-user is using lazy.nvim as their plugin manager, there isn't really a way for legendary.nvim to lazily hook into nvim-tree without forcing it to load, potentially breaking the user's lazy-loading setups. This becomes a non-issue if I could just subscribe to a built-in autocmd autocmd User NvimTree_TreeAttachPost because I can subscribe to that without loading nvim-tree at all.

add API like api.is_setup

I think something like this is needed either way so that I can detect if nvim-tree has loaded before or after legendary.nvim is loaded.

@alex-courtis
Copy link
Member Author

Normally I would do something like local ok, nvim_tree = pcall(require, 'nvim-tree') to see if it is loaded, however in the newer plugin manager lazy.nvim, doing so will cause nvim-tree to be loaded.

I see. I was thinking in terms of Packer.

Does lazy call nvim-tree's setup? nvim-tree won't do anything until setup is called, however the user can still subscribe to events at that time.

@mrjones2014
Copy link
Collaborator

In lazy.nvim, whenever the plugin is loaded, it will call the config function of the plugin spec which typically does call the .setup() function for the plugin.

This is the root of what I'm trying to do -- lazily hook into nvim-trees API without forcefully loading the plugin itself, which may be undesirable in lazy-loaded environments.

@alex-courtis
Copy link
Member Author

I understand. I am learning a lot today.

It sounds like we need three things:

  1. au for nvim-tree required
  2. au for nvim-tree setup
  3. require("nvim-tree").is_setup()

It seems that you would need to create your autocommands before the lazy is started.

@mrjones2014
Copy link
Collaborator

Yeah, I concur. I think that should be sufficient.

@alex-courtis
Copy link
Member Author

Opened #1893

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API API or Event
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants