sindrets / diffview.nvim

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

Immediate trigger autocmd(BufLeave) callback when set in the view_opened hook callback #362

Closed NGPONG closed 1 year ago

NGPONG commented 1 year ago

Description

Hey, amazing plugins, thanks for all your hard work behind the scenes!

I'm having some problems writing logic for my init.lua. When I do some initialization work in the view_enter callback, which looks like this:

require("diffview").setup({
  hooks = {
    view_opened = function ()
      vim.api.nvim_create_autocmd('BufLeave', {
        group = group_id,
        pattern = { 'DiffviewFilePanel', 'DiffviewFileHistoryPanel' },
        callback = function(args)
          logger.info('leave buffer [' .. args.buf .. '].')
          tools.unhide_cursor()
        end
      })
    end,
  },
})

When I opened the Diffview panel, everything worked fine, except for immediate triggering the BufLeave event. This is confusing to me, because looking at the documentation about view_enter it says:

It's called after initializing the layout in the new tabpage (all windows are ready).

So I assume that when the view_opened event emitted, everything is ready for rendering which includes a series of Buffer-related events, such as BufferEnter, BufferLeave, etc. (I'm sorry I'm a neovim rookie, please correct me if I'm wrong, I appreciate it!)

I guess it's caused by some async initialization work that hasn't finished yet, so I used a temporary solution to solve the problem, but I believe it's not an optimal way to work

require("diffview").setup({
  hooks = {
    local timer = vim.loop.new_timer()
    timer:start(1000, 0, vim.schedule_wrap(function()
      vim.api.nvim_create_autocmd('BufLeave', {
        group = group_id,
        pattern = { 'DiffviewFilePanel', 'DiffviewFileHistoryPanel' },
        callback = function(args)
          logger.info('leave buffer [' .. args.buf .. '].')
          tools.unhide_cursor()
        end
      })
    end))
  },
})

I think this may not be a bug or may just be a problem, but I'm not quite sure why an bug tag was added, I'm not familiar with the workflow on github yet and I apologize for the confusion

Expected behavior

No response

Actual behavior

Immediate trigger BufLeave

Steps to reproduce

Already in the description

Health check

==============================================================================
diffview: require("diffview.health").check()

Checking plugin dependencies ~
- OK nvim-web-devicons installed.

Checking VCS tools ~
- The plugin requires at least one of the supported VCS tools to be valid.
- OK Git found.
- OK Git is up-to-date. (2.34.1)
- WARNING Configured `hg_cmd` is not executable: 'hg'

Neovim version

NVIM v0.10.0-dev
Build type: RelWithDebInfo
LuaJIT 2.1.0-beta3

Operating system and version

wsl2 ubuntu-22.04

sindrets commented 1 year ago

So I assume that when the view_opened event emitted, everything is ready for rendering which includes a series of Buffer-related events, such as BufferEnter, BufferLeave

You cannot make an assumption about what point in time buffer and window related events will no longer be emitted. Async code is liberally used in the plugin in order to make the interface more responsive while we're waiting for Git to provide necessary data for constructing various elements like file lists / trees, the content for the diff buffers, etc.

The description for the view_opened hook is correct: it's emitted just after the first time the initial layout has been created. This makes no promise that windows and buffers won't change beyond that point. And making such a promise makes no sense.

Your logic is too fragile if it breaks when buffer events are emitted in a way you don't expect (because this will happen a lot. i.e. just calling nvim_win_call() once will emit 2 buffer events and 2 window events). It seems like you're trying to set up some sort of logic for hiding / unhiding the cursor under certain conditions (some trickery with 'guicursor'?). In that case it seems to me that you're approaching the problem from the wrong angle: you should perform all the cursor state handling in a BufEnter event. Something like this:

vim.api.nvim_create_autocmd("BufEnter", {
  callback = function(e)
    local buf_name = vim.api.nvim_buf_get_name(e.buf)

    -- Hide the cursor if the buffer name matches any of these:
    local targets = {
      "/DiffviewFilePanel$",
      "/DiffviewFileHistoryPanel$",
    }

    for _, pattern in ipairs(targets) do
      if buf_name:match(pattern) then
        hide_cursor()
        return
      end
    end

    -- The current buffer is *not* a target: unhide the cursor
    unhide_cursor()
  end,
})

If you wish for this autocommand only to live for the lifetime of the view, you can use an autocommand group, and clear it on view_closed.

NGPONG commented 1 year ago

Hey @sindrets , thank you for such a quick and patient reply. I apologize for the time zone difference that caused me to reply to you only now.

I see your point, and you are absolutely right that the any promise to async task doesn't seem to be reliable. I appreciate the correction, it looks like I was going in the wrong direction to begin with.

The idea of the little sample you provided seems similar to what I implemented in another plugin configuration (yes, it's a weird fetish, I just wanted to make the actual experience of some plugins closer to its native responsibilities, for example there shouldn't be any editing action in the file explorer, then I would choose to hide the cursor), and the core logic is to determine if each buffer being entered needs to have the cursor disabled.

A small and silly optimization I made here is that I don't want this tedious detection logic to happen for every buffer, but to limit it to a specific buffer. So the code described in issues is created, which listens for the BufEnter and BufLeave events and passes { 'DiffviewFilePanel', 'DiffviewFileHistoryPanel' } to the pattern arg. But this logic is too fragile as you said, this doesn't cover all cases. Anyway, thanks a lot for your help, it gives me some more ideas.

I'm sorry to bother you in your busy schedule, thanks for your hard work and I think my question is solved so you can just close it out