axkirillov / hbac.nvim

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

feat(state): keep global state of pinned buffers for session persistence #22

Closed Nimjii closed 3 months ago

Nimjii commented 10 months ago

Hey again!

This should fix #15.

I also made set_all a local function, as it seemed kind of useless to export it (I can remove that change tho, if you don't like it). I won't lie, I don't use any session plugins, so I am not exactly sure on how they operate. If this has any issues, feel free to point them out.

al-ce commented 10 months ago

Thanks for making this PR! This would be a nice feature and I think your changes are elegant and easy to follow.

I should note that I'm testing this by using mksession as my session tool (as opposed to a plugin that uses JSON or something else). I am executing mksession before I quit and I am using this command to restore the session: nvim -u minimal.lua -S Session.vim This is my minimal config.

minimal config ```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) vim.go.sessionoptions = "buffers,curdir,tabpages,winsize,help,globals" require("lazy").setup({ {"Nimjii/hbac.nvim", opts = {}, }, }, {}) ```

The first thing that caught my eye is that vim.g.hbac_buffers is all lowercase, and if you look at point 2 in :h mksession it notes

  1. Restores global variables that start with an uppercase letter and contain at least one lowercase letter, if 'sessionoptions' contains "globals".

The pins weren't restoring for me when I reloaded my session with the current commit. After changing the global variable to Hbac_buffers throughout the project, the pins were being restored (with some caveats, see below). Does it work for you as it currently is? Do you have a session method you're using?

Second, the unpin_all function wasn't working for me anymore, I think it just needs an early return when pin_value is false. Maybe we just want to separate the logic instead of calling set_all for both pin_all and unpin_all at this point?

With these two changes, it mostly works! But if I have more buffers open than my threshold setting, even if they're all pinned, and if I save the session, quit, and try to reload it, the pinned bufs aren't as i'd expect. There's kind of a mix of conditions that I'm trying out (threshold value, amount of buffers opened/pinned/unpinned on session save and quit, whether my config sets autoclose to false, whether I re-pin the buffers on the session restore and then quit and reload the session, etc.), but I'm afraid of adding more confusion to the mix with my interpretations or possible fixes since I don't fully understand it, so I'm going to sit on it for a few days. Also maybe it's just me. Can you please experiment with those when you have a chance? (I suggest setting a low threshold for testing). Let me know if my comments are helpful or if I'm missing something and it should be working.

Thanks for working on this, I'm excited about this feature! We should also add a note in the README that sessionoptions should include globals for this to work

Nimjii commented 10 months ago

Thanks for all the feedback. I will get to the more thorough testing, when I find the time.

axkirillov commented 6 months ago

@Nimjii hi, what's the status here? are you still planning to merge?

axkirillov commented 3 months ago

@Nimjii seems like you abandoned the idea, so I'll close it for now