olimorris / persisted.nvim

💾 Simple session management for Neovim with git branching, autoloading and Telescope support
MIT License
418 stars 24 forks source link

Replace callbacks with user events #51

Closed olimorris closed 1 year ago

olimorris commented 1 year ago

Something I have been mulling over for a while now is replacing the mess that is user callbacks in this plugin. As time has progressed, so have the number of callbacks. Whilst they're all justified, their implementation couldn't help but feel clunky.

To remedy this, callbacks will be replaced with user events which can be hooked into via autocmds. The benefits of this are:

  1. The plugin is much cleaner
  2. Users aren't tied to keeping all of the event/callback logic in their persisted.nvim config
  3. It allows for a greater variety of options regarding hooking into the plugin such as greater statusline integration
olimorris commented 1 year ago

An example of hooking into the autocmds:

local group = vim.api.nvim_create_augroup("PersistedHooks", {})

vim.api.nvim_create_autocmd({ "User" }, {
  pattern = "PersistedSavePre",
  group = group,
  callback = function()
    -- Ensure the minimap plugin is not written into the session
    pcall(vim.cmd, "bw minimap")
  end,
})
desilinguist commented 1 year ago

Hi, I am using the following snippet from the README for telescope integration:

require("persisted").setup({
  telescope = {
    before_source = function()
      vim.api.nvim_input("<ESC>:%bd<CR>")
    end,
    after_source = function(session)
      print("Loaded session " .. session.name)
    end,
  },
})

How should I modify this to use the new events?

abulwafa commented 1 year ago

I switched the code from a callback to an auto command as shown but switching to a new session deletes all buffers in the target session

local group = vim.api.nvim_create_augroup("PersistedHooks", {})

  vim.api.nvim_create_autocmd({ "User" }, {
    pattern = "PersistedTelescopeLoadPre",
    group = group,
    callback = function()
      pcall(vim.cmd, "SessionSave")
      vim.api.nvim_input("<ESC>:%bd<CR>")
    end,
  })
desilinguist commented 1 year ago

Yeah, I just tried the same and got the same problem with all buffers being deleted. Had to revert:

local group = vim.api.nvim_create_augroup("PersistedHooks", {})

vim.api.nvim_create_autocmd({ "User" }, {
  pattern = "PersistedTelescopeLoadPre",
  group = group,
  callback = function()
    -- Close and delete all open buffers
    vim.api.nvim_input("<ESC>:%bd<CR>")
  end,
})

vim.api.nvim_create_autocmd({ "User" }, {
  pattern = "PersistedTelescopeLoadPost",
  group = group,
  callback = function(session)
    print("Loaded session " .. session.name)
  end,
})
latipun7 commented 1 year ago
vim.api.nvim_create_autocmd({ "User" }, {
  pattern = "PersistedTelescopeLoadPost",
  group = group,
  callback = function(session)
    print("Loaded session " .. session.name) -- this will not works.
  end,
})

Also, this will never work. Callback arguments would not yield field of name. It will only yield the info of this autocmd event, such as buf number etc.

Also, it seems PersistedTelescopeLoadPre is fired after the session loaded.

tmpm697 commented 1 year ago

I just update this plugin via lazy.nvim and it stopped working (same issue as @desilinguist), had to revert i guess, idk but i'm totally ok with one place setting up in persisted.lua file instead of autocmd.

EDIT: do we need to re-save session for this new one? current working commit hash for me be22f38 EDIT 2: can i use the event in the same persisted.lua config file? i don't like to separate it to another file.

olimorris commented 1 year ago

Thanks for the feedback.

@latipun7, as per e594ede82578a8bbccbe5f0c2d3ec3a4e228153e, I've moved the loading of the session from telescope into a vim.schedule block which ensures anything you run with the PersistedTelescopeLoadPreevent completes first before the session is loaded.

olimorris commented 1 year ago

@desilinguist, as per a0196d5c513e779d7dc0daf52ba03e1a1f458422, the session should now be available to event hooks. This should now work:

local group = vim.api.nvim_create_augroup("PersistedHooks", {})

vim.api.nvim_create_autocmd({ "User" }, {
  pattern = "PersistedTelescopeLoadPre",
  group = group,
  callback = function(session)
    print(session.data.branch)
  end,
})
olimorris commented 1 year ago

I just update this plugin via lazy.nvim and it stopped working (same issue as @desilinguist)

It shouldn't have stopped working. I kept the functionality in place and just warned about an upcoming deprecation.

olimorris commented 1 year ago

@desilinguist, as per a0196d5, the session should now be available to event hooks

Unfortunately this means I've had to bump the minimum version of Neovim to 0.8.0. I've created a neovim-0.7.0 tag for those who wish to pin to that for compatibility.

clue44 commented 1 year ago

It just broke for me also but I got it to work again.

My previous config (with lazy) was:

    config = function()
        require("persisted").setup()
        require("telescope").load_extension("persisted")
     end,

This now started giving this error:

Failed to run `config` for persisted.nvim                                                                                                                                                                                               
.../share/nvim/lazy/persisted.nvim/lua/persisted/config.lua:40: attempt to index local 'opts' (a nil value)                                                                                                                             
# stacktrace:                                                                                                                                                                                                                           
  - /persisted.nvim/lua/persisted/config.lua:40 _in_ **setup**                                                                                                                                                                          
  - /persisted.nvim/lua/persisted/init.lua:68 _in_ **setup**                                                                                                                                                                            
  - .config/nvim/lua/plugins/persisted.lua:7 _in_ **config** 

Changing the code to this works:

    config = function()
        require("persisted").setup({})
        require("telescope").load_extension("persisted")
     end,

The only change is adding an empty lua table to the setup function.

The funny thing is, this also works, just using config = true instead of a function:

config = true

However, then there is no way to add loading the telescope extension in the same place as the plugin.

latipun7 commented 1 year ago

@tmpm697 :

EDIT: do we need to re-save session for this new one? current working commit hash for me be22f38

Auto save should works. Unless you load the session from telescope.

EDIT 2: can i use the event in the same persisted.lua config file? i don't like to separate it to another file.

You can place the autocommand in your custom config file. For example, after calling setup. This snippet below is config if you load plugins using lazy.nvim.

config = function()
  require("persisted").setup()
  -- here is your autocommands
  require("telescope").load_extension("persisted")
end,

olimorris commented 1 year ago

Ah sorry @clue44 that's a really dumb mistake on my part. Should be fixed in 029ba65f3901f62093c9b1537f116a3729d78d98.

latipun7 commented 1 year ago

@olimorris :

as per a0196d5, the session should now be available to event hooks. This should now work:

I got an error when loading this not from Telescope events using data.

  create_aucmd("User", {
    pattern = "PersistedLoadPost",
    group = persisted_hooks,
    callback = function(session)
      vim.notify(
        "Loaded session " .. session.data.name,
        vim.log.levels.INFO,
        { title = title }
      )
    end,
  })

error:

Error detected while processing User Autocommands for "PersistedLoadPost":

Error executing lua callback: ...e/latipun/.config/lvim/lua/latipun/plugins/persisted.lua:37: attempt to concatenate field 'name' (a nil value)
stack traceback:
    ...e/latipun/.config/lvim/lua/latipun/plugins/persisted.lua:37: in function <...e/latipun/.config/lvim/lua/latipun/plugins/persisted.lua:35>
    [C]: in function 'nvim_exec_autocmds'
    ...ite/pack/lazy/opt/persisted.nvim/lua/persisted/utils.lua:90: in function 'load_session'
    ...site/pack/lazy/opt/persisted.nvim/lua/persisted/init.lua:94: in function 'load'
    [string ":lua"]:1: in main chunk
olimorris commented 1 year ago

For those who use lazy and wish to load the telescope extension, consider the following:

{
  "olimorris/persisted.nvim", -- Session management
  opts = {
    save_dir = Sessiondir .. "/",
    use_git_branch = true,
    silent = true,
    should_autosave = function()
      if vim.bo.filetype == "alpha" then return false end
      return true
    end,
  },
  config = function(_, opts)
    require("persisted").setup(opts)
    require("telescope").load_extension("persisted")
  end,
},
latipun7 commented 1 year ago

I got an error when loading this not from Telescope events using data.

Oh wait, the session data is different from telescope events. I just expect it would have the same table.

olimorris commented 1 year ago

Oh wait, the session data is different from telescope events. I just expect it would have the same table.

Yep that's always been the case but I should have documented it. The non-telescope commands just output the name (excluding the meta data such as event and buffer etc) whereas the telescope ones output branch, name, file_path etc.

Edit: This is now referenced https://github.com/olimorris/persisted.nvim#events--callbacks

tmpm697 commented 1 year ago

@latipun7

Auto save should works. Unless you load the session from telescope.

but i used telescope :(

@olimorris : telescope work now? i expect it work without any change as fallbacks still there.

olimorris commented 1 year ago

@tmpm697 everything should be working. Can you try again and if not, share your config?

tmpm697 commented 1 year ago

i use telescope = {, after_source, before_save, should_autosave. worked with open current folder like cd /path/to/saved/session/folder; nvim --> this restore previous session correctly.

but when i switch to other session, it will delete all buffers and basically give me empty buffer (switch using telescope).

i used latest origin/main.

EDIT: I think this might be your warning buffer that interfere with above events. EDIT2: can you give option to disable your warnings? EDIT3: i close and open neo-tree before and after restore session, this can be break by warning buffer I guess. EDIT4: i used vim.schedule under telescope = { before_source = <..> to delete all buffer before switching, is this cause problems with new event?

clue44 commented 1 year ago

Ah sorry @clue44 that's a really dumb mistake on my part. Should be fixed in 029ba65.

Yes indeed. Thank you!

tmpm697 commented 1 year ago

I got this error:

Error executing vim.schedule lua callback: ...re/nvim/lazy/neo-tree.nvim/lua/neo-tree/command/init.lua:165: Invalid window id: 1012
stack traceback:
    [C]: in function 'nvim_set_current_win'
    ...re/nvim/lazy/neo-tree.nvim/lua/neo-tree/command/init.lua:165: in function <...re/nvim/lazy/neo-tree.nvim/lua/neo-tree/command/init.lua:163>
Error executing vim.schedule lua callback: ...re/nvim/lazy/neo-tree.nvim/lua/neo-tree/command/init.lua:165: Invalid window id: 1023
stack traceback:
    [C]: in function 'nvim_set_current_win'
    ...re/nvim/lazy/neo-tree.nvim/lua/neo-tree/command/init.lua:165: in function <...re/nvim/lazy/neo-tree.nvim/lua/neo-tree/command/init.lua:163>

as i mentioned at edit4 above, vim.schedule(vim.cmd(<..>)) seem to have problem

desilinguist commented 1 year ago

@olimorris the autocommands now work correctly for me when switching sessions! Thank you!

latipun7 commented 1 year ago

@tmpm697

as i mentioned at edit4 above, vim.schedule(vim.cmd(<..>)) seem to have problem

Yep, you don't need vim.schedule now. Under the hood, it's already included.

Also, you could manually save the session before switching using telescope with new autocmd telescope events.

tmpm697 commented 1 year ago

what i want, i just want to close neotree before save and show it after restore or switch session via telescope, how?

tmpm697 commented 1 year ago

also got some kind of error when doing :qa, can't capture it.

tmpm697 commented 1 year ago

seem there's bug from neotree, i had to revert this plugin and neotree back to last 2022 commit, dang.

I already feel some unstable from time to time. nah i'm good now with 2022 stuff.

tmpm697 commented 1 year ago

i've testeed with latest persisted and neotree from 2022, work perfect, so issue is from neotree. thanks.

latipun7 commented 1 year ago

I have session at ~/.local/share/chezmoi, the autosave should trigger in any directory if I run nvim in that directory, and it's true and works. But the previous behavior seems lost. When I open file with nvim $HISTFILE in ~/.local/share/chezmoi, then close it, now the session contain only file $HISTFILE, which is not what I desire and not the previous behaivor. The autosave should automatically not triggered when opening file directly, i.e. nvim /path/to/file, nvim $HISTFILE, etc.

Tool Version
NVIM NVIM v0.9.0-dev-1166+g06aed7c17
Persisted 88f27dc
olimorris commented 1 year ago

@latipun7 can you raise a separate issue please?