Replace callbacks with user events

Question

Replace callbacks with user events

olimorris opened this issue 2 years ago · 30 comments

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:

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

Answer 1 · 2023-02-22T20:03:31.000Z

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,
})

Answer 2 · 2023-02-22T23:34:29.000Z

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?

Answer 3 · 2023-02-22T23:58:27.000Z

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,
  })

Answer 4 · 2023-02-23T00:04:38.000Z

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,
})

Answer 5 · 2023-02-23T03:25:12.000Z

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.

Answer 6 · 2023-02-23T04:06:26.000Z

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.

Answer 7 · 2023-02-23T10:06:36.000Z

Thanks for the feedback.

@latipun7, as per e594ede, 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.

Answer 8 · 2023-02-23T10:14:25.000Z

@desilinguist, as per a0196d5, 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,
})

Answer 9 · 2023-02-23T10:17:51.000Z

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.

Answer 10 · 2023-02-23T10:28:55.000Z

@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.

Answer 11 · 2023-02-23T10:32:30.000Z

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.

Answer 12 · 2023-02-23T10:40:34.000Z

@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,

Answer 13 · 2023-02-23T10:42:17.000Z

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

Answer 14 · 2023-02-23T10:44:41.000Z

@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

Answer 15 · 2023-02-23T10:48:28.000Z

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,
},

Answer 16 · 2023-02-23T10:53:05.000Z

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.

Answer 17 · 2023-02-23T11:09:25.000Z

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

Answer 18 · 2023-02-23T12:40:48.000Z

@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.

Answer 19 · 2023-02-23T12:56:25.000Z

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

Answer 20 · 2023-02-23T14:19:55.000Z

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?

Answer 21 · 2023-02-23T14:21:45.000Z

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

Yes indeed. Thank you!

Answer 22 · 2023-02-23T14:56:01.000Z

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

Answer 23 · 2023-02-23T15:05:31.000Z

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

Answer 24 · 2023-02-23T15:15:44.000Z

@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.

Answer 25 · 2023-02-23T15:49:40.000Z

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

Answer 26 · 2023-02-23T16:19:14.000Z

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

Answer 27 · 2023-02-23T16:44:59.000Z

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.

Answer 28 · 2023-02-23T16:56:53.000Z

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

Answer 29 · 2023-03-09T06:21:58.000Z

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`

Answer 30 · 2023-03-09T08:56:59.000Z

@latipun7 can you raise a separate issue please?