axkirillov / hbac.nvim

Heuristic buffer auto-close
MIT License
197 stars 10 forks source link

No buffers were wiped out: bwipe 1 #16

Closed Martins3 closed 10 months ago

Martins3 commented 10 months ago

Steps to reproduce

  1. mininal init.lua
    
    local lazypath = vim.fn.stdpath("data") .. "/lazy/lazy.nvim"
    if not vim.loop.fs_stat(lazypath) then
    vim.fn.system({
    "git",
    "clone",
    "--filter=blob:none",
    "https://github.com/folke/lazy.nvim.git",
    "--branch=stable", -- latest stable release
    lazypath,
    })
    end
    vim.opt.rtp:prepend(lazypath)

require("lazy").setup({ "axkirillov/hbac.nvim", "olimorris/persisted.nvim", "nvim-telescope/telescope.nvim", "nvim-lua/plenary.nvim", }, {})

require("persisted").setup({ autoload = true }) require("hbac").setup({ })

2. open nvim and edit 11 different files to create 10 buffer
3. close nvim 
4. open nvim and here are the erros
```txt
[persisted.nvim]: Error loading the session! vim/_editor.lua:0: VimEnter Autocommands for "*"..script nvim_exec2() called at Vi
mEnter Autocommands for "*":0../home/martins3/.local/share/nvim/sessions/%home%martins3%core%linux@@main.vim, line 93: Vim(bwip
eout):E517: No buffers were wiped out: bwipe 1
Press ENTER or type command to continue

If require("hbac").setup({ }) is commented, the error message disappeared.

Additional information

  1. I changed the session manager plugin to "rmagatti/auto-session", nothing changed.
  2. nvim version:

    NVIM v0.9.4
    Build type: Release
    LuaJIT 2.1.1693350652
    
    system vimrc file: "$VIM/sysinit.vim"
    fall-back for $VIM: "
    /nix/store/4wq4i3zfc9g0g5b0d94rm36d9pfa4fqz-neovim-unwrapped-0.9.4/share/nvim
axkirillov commented 10 months ago

Hi! Thank you for submitting the issue! I wish I could help, but I don't have any idea what is happening šŸ˜… . Clearly hbac is having some conflict with the autosession plugins.

Martins3 commented 10 months ago

Thank you for the quick reply, just looking for advise. I will continue to investigate this issue. If any progress is made, I will share it here

al-ce commented 10 months ago

@Martins3 could you try this config?

local lazypath = vim.fn.stdpath("data") .. "/lazy/lazy.nvim"
if not vim.loop.fs_stat(lazypath) then
  vim.fn.system({
    "git",
    "clone",
    "--filter=blob:none",
    "https://github.com/folke/lazy.nvim.git",
    "--branch=stable", -- latest stable release
    lazypath,
  })
end
vim.opt.rtp:prepend(lazypath)

require("lazy").setup({
  {"olimorris/persisted.nvim", opts = {autoload = true}},
  {
    "axkirillov/hbac.nvim",
    event = "SessionLoadPost",
    opts = {},
  },
    "nvim-telescope/telescope.nvim",
    "nvim-lua/plenary.nvim",
}, {})

(note that we're assuming that the lazyload default for lazy.nvim is false, in case this is tried in a less-minimal config)

I don't have a full grasp of what's going on, but there seems to be some conflict with the creation of Hbac's autoclose autocommand and that autocmd running during the session load. Setting autoclose = false in the Hbac config above also seems to do the trick without need to set the lazyload event to SessionLoadPost.

Here are some lines from a session file (e.g. ~/.local/share/nvim/sessions/somefile.vim):

if expand('%') == '' && !&modified && line('$') <= 1 && getline(1) == ''
  let s:wipebuf = bufnr('%')
endif

"" ...etc

if exists('s:wipebuf') && len(win_findbuf(s:wipebuf)) == 0 && getbufvar(s:wipebuf, '&buftype') isnot# 'terminal'
  silent exe 'bwipe ' . s:wipebuf
endif

Maybe Hbac is closing the first file buffer on a session open (the file buffer that's being checked in the if expand('%') expression) during BufEnter events once the threshold is hit before the bwipe command is executed, and that causes the E517 error. That's just a guess, I've been stumbling around on this for a few days.

One thing we could try for Hbac is delaying the creation of the autoclose autocommand until SessionLoadPost if we're opening a session. For example, in hbac.autocommands

M.autoclose.autocmd = function()
    vim.api.nvim_create_autocmd({ "BufEnter" }, {
        -- autoclose logic
    })
end

M.autoclose.setup = function()
    state.autoclose_enabled = true

    if vim.fn.argc() == 0 then  -- or whatever conditions are appropriate for checking SessionLoad
        vim.api.nvim_create_autocmd({ "SessionLoadPost" }, {
            pattern = { "*" },
            callback = function()
                M.autoclose.autocmd()
            end,
            once = true,  -- I think this is appropriate here
        })
        return
    end
    M.autoclose.autocmd()
end

Do any of these approaches work for anyone?

Martins3 commented 10 months ago

I apply this patch https://github.com/Martins3/hbac.nvim/commit/f6e3e37b876951808452330a364cbcfa5fddf2c1, it now works perfectly

al-ce commented 10 months ago

I'm glad that works! (though the above code could use some cleanup) I did some session saving and loading with persisted and folke's persistence in the middle of a session, and both seem to play well with Hbac with the above changes.

We should consider whether !argc() is a sufficient condition to delay the autocmd. If I understand correctly, !argc is how all these plugins check whether to autoload a session (plus some additional checks like ignore_dir etc)

persistence and possession (different from nvim-possession) don't autoload as part of their feature set

It would be nice if there was something like a SessionLoadPre event so we weren't just inferring a session is being loaded just from !argc(). Session files set a SessionLoad var but we can't check for that until after the session file is sourced and buffers are being opened (that Hbac would close if we didn't postpone its autocmd).

imo the simplest solution for lazy.nvim users is that if you're autoloading sessions, your session plugin shouldn't be lazyloaded and that hbac should be loaded on SessionLoadPost, but adding some builtin fix into hbac would be a nice courtesy to save people trouble of figuring that out, and we would prevent adding more clutter to the readme.

@axkirillov what do you think? This load order might also be something to keep in mind for anyone tackling #15

axkirillov commented 10 months ago

I'm glad that works! (though the above code could use some cleanup) I did some session saving and loading with persisted and folke's persistence in the middle of a session, and both seem to play well with Hbac with the above changes.

We should consider whether !argc() is a sufficient condition to delay the autocmd. If I understand correctly, !argc is how all these plugins check whether to autoload a session (plus some additional checks like ignore_dir etc)

persistence and possession (different from nvim-possession) don't autoload as part of their feature set

It would be nice if there was something like a SessionLoadPre event so we weren't just inferring a session is being loaded just from !argc(). Session files set a SessionLoad var but we can't check for that until after the session file is sourced and buffers are being opened (that Hbac would close if we didn't postpone its autocmd).

imo the simplest solution for lazy.nvim users is that if you're autoloading sessions, your session plugin shouldn't be lazyloaded and that hbac should be loaded on SessionLoadPost, but adding some builtin fix into hbac would be a nice courtesy to save people trouble of figuring that out, and we would prevent adding more clutter to the readme.

@axkirillov what do you think? This load order might also be something to keep in mind for anyone tackling #15

The first part of your suggestion ('your session plugin shouldn't be lazyloaded') is what I would consider common sense / expected behaviour, since I don't see why anyone would want to lazyload a session plugin. As for event = "SessionLoadPost", it seems like a reasonable thing to suggest users to do this, since it's just one line of configuration and it seems to be solving the exact problem for the subset of people for whom this is relevant.

Introducing ad-hoc fixes in code for an unknown number of session plugins would require a lot of guesswork and maintenance burden which I am not sure is worth it in the long run šŸ˜….

al-ce commented 10 months ago

Introducing ad-hoc fixes in code for an unknown number of session plugins would require a lot of guesswork

I agree, I think this is the major pain point here. I think unless we figure out if there's a standard that we could reference in the docs, the best thing to do is for a user to figure out their best lazy-loading setup with their plugin manager.

For what it's worth, in my own setup, I autoload my session with an autocommand before hbac using lazy.nvim's init table (per lazy's docs, "init functions are always executed during startup"). So, that's before any plugin's configuration or setup call even if they're not lazy-loaded.

  {
    "folke/persistence.nvim",
    init = function()
      -- my lazyloading autocommand
    end,
    -- the rest of my persistence config
  },
  {
  -- hbac is loaded at some event later on, doesn't have to be SessionLoad 

So for lazy.nvim users, two solutions are to either set Hbac to SessionLoad post, or, if you're using autosessions, using init in your spec.

axkirillov commented 10 months ago

Introducing ad-hoc fixes in code for an unknown number of session plugins would require a lot of guesswork

I agree, I think this is the major pain point here. I think unless we figure out if there's a standard that we could reference in the docs, the best thing to do is for a user to figure out their best lazy-loading setup with their plugin manager.

For what it's worth, in my own setup, I autoload my session with an autocommand before hbac using lazy.nvim's init table (per lazy's docs, "init functions are always executed during startup"). So, that's before any plugin's configuration or setup call even if they're not lazy-loaded.

  {
    "folke/persistence.nvim",
    init = function()
      -- my lazyloading autocommand
    end,
    -- the rest of my persistence config
  },
  {
  -- hbac is loaded at some event later on, doesn't have to be SessionLoad 

So for lazy.nvim users, two solutions are to either set Hbac to SessionLoad post, or, if you're using autosessions, using init in your spec.

Both solutions seem reasonable to me. Although, I don't know if other package managers have this functionality.

@Martins3 did you try setting event = "SessionLoadPost" ?

Martins3 commented 10 months ago

I have tried, it doesn't work.

axkirillov commented 10 months ago

@Martins3 do you lazy load your session plugin?

Martins3 commented 10 months ago

terribly sorry , I think because of my weak understanding of vim, I just made a mistake. the config posted by @al-ce actually works

local lazypath = vim.fn.stdpath("data") .. "/lazy/lazy.nvim"
if not vim.loop.fs_stat(lazypath) then
  vim.fn.system({
    "git",
    "clone",
    "--filter=blob:none",
    "https://github.com/folke/lazy.nvim.git",
    "--branch=stable", -- latest stable release
    lazypath,
  })
end
vim.opt.rtp:prepend(lazypath)

require("lazy").setup({
  {"olimorris/persisted.nvim", opts = {autoload = true}},
  {
    "axkirillov/hbac.nvim",
    event = "SessionLoadPost",
    opts = {},
  },
    "nvim-telescope/telescope.nvim",
    "nvim-lua/plenary.nvim",
}, {})
axkirillov commented 10 months ago

@Martins3 ah, beautiful. Is this a viable solution for you? I would suggest using this configuration then. Then we don't have to do any code changes on our side.

al-ce commented 10 months ago

terribly sorry , I think because of my weak understanding of vim, I just made a mistake. the config posted by @al-ce actually works

No worries, there's a ton of things to try to keep track of. Glad it works! I'm sure there's other solutions too. Thanks for bringing it up, it was a nice opportunity for me to learn as well