feat: restore buffers after loading a session

[igorlfs]

Hello,

I'm on a quest to make all of my plugins (vim) "session friendly". For nvim-tree this sounds pretty straightforward, so I went ahead and implemented it myself.

Problem: nvim-tree leaves stale empty buffers after restoring a session

Solution: detect these buffers on SessionLoadPost and reopen them.

[igorlfs]

I went with this location since it was the standard, but it might a good idea have this as an eager autocmd. Otherwise, any means of "lazy loading" may end up calling setup too late (sessions could be loaded relatively early, as in, part of the initilization with nvim -S Session.vim).

[igorlfs]

There's an edge case with this approach: if nvim-tree was the only buf from a tabpage, it will not be restored. I think that's fine, as it's sort of unlikely. Could add a comment as a reference, or otherwise look into it. The vim api is not very tab friendly, but I guess it should be possible to hook an unholy :[N]tabnew...

[alex-courtis]

That's fine. We are very aware of how insufficient the tab api is and how much trouble we've had with it.

Let's stick to the happy path cases to keep it simple and reliable.

Yes please: lots of comments in here.

[igorlfs]

Am I missing something or is the CI failure unrelated?

[alex-courtis]

We've had a lot of problems with session integration in the past e.g. #1992, with nodeterministic issues surrounding plogin load and event order etc.

The outcome was that we elected not to support sessions, using a recipe instead.

First question: is an error thrown on startup, or does the session simply fail to correctly load?

Options:

• Publish your solution as a recipe
• Package your solution as an extension plugin

Are the above possible using API?

[alex-courtis]

Am I missing something or is the CI failure unrelated?

Apologies, an upstream Nvim change broke the build, fixed on master #3337

[igorlfs]

We've had a lot of problems with session integration in the past

Thanks, I should have looked this up.

The outcome was that we elected not to support sessions

That's unfortunate to hear. If this decision is final, feel free to close this PR. Else, since the discussion in the other threads is lengthy, could you propose some "up to date" requirements, with, potentially, some compromises? For instance, this comment suggests that folks using session plugins (rmagatti/auto-session in particular) should "be on their own".

The comment also mentions "persisting something about the buffer" (which is also mentioned in this comment). Regarding that, I have an idea which is already employed by some plugins (ref: kulala.nvim, nvim-dap-view): storing the state inside a global variable. Global variables are restored when 'sessionoptions' includes globals (the conditions are slightly stricter and there are other caveats, but I digress).

This approach could be leveraged to restore additional information, besides plainly reopening the buffers. At a most basic level, it could be used to properly restore the cwd. But maybe it could be used to fully restore the tree's state (although I think this would be more complex, since files may have been deleted or whatever).

If we are willing to use features only available on nightly, we can also take advantage of the (recently merged) SessionWritePre, a new event that could also be helpful with the "stateful restore".

A rough outline of what I have in mind for an MVP is:

• Leave "session plugin users on their own"
• When a :mksession is issued, use SessionWritePre to update the global variable that stores nvim-tree's state
• Setup an eager autocmd on SessionLoadPost to restore the state. What I mean by "eager" is that it should be "reachable" from plugin/nvim-tree.lua (potentially "inlined"). Given that session plugins are not interfering with their own SessionLoadPost hooks and that the user isn't using any shenanigans to lazy-load the plugin, this should be much less "race prone".

Of course, that's a really "happy path" plan. If you are uncomfortable with the 'globals' approach, I was thinking about raising a discussion upstream, regarding the "best practices for plugins handling sessions" (aka lua-plugin-sessions). If the other ideas are also "far fetched", I'm open to alternatives. And if we can't settle down on the outline, I'm happy with closing and keeping this as a module on my own config. I could publish it as a recipe as well (current implementation would is easily "copy-pasteable").

Are the above possible using API?

Current implementation is not relying on any internals.

---

First question: is an error thrown on startup, or does the session simply fail to correctly load?

Do you mean if an error is thrown on startup, or something else? Raising an error on VimEnter does not prevent the session from loading. Buffers are restored as usual. SessionLoadPre is also fine. Haven't checked other events (I'm not sure about the event's load order, if you have any specific hints, I can dig into them). I could try a more "destructive" error (e.g., manipulating windows, etc) if needed.

[alex-courtis]

We've had a lot of problems with session integration in the past

Thanks, I should have looked this up.

The outcome was that we elected not to support sessions

That's unfortunate to hear. If this decision is final, feel free to close this PR. Else, since the discussion in the other threads is lengthy, could you propose some "up to date" requirements, with, potentially, some compromises? For instance, this comment suggests that folks using session plugins (rmagatti/auto-session in particular) should "be on their own".

The problems were all around reliability and robustness - many instances of errors thrown on startup. A solution that is very defensive and absolutely does not throw exceptions is acceptable.

The comment also mentions "persisting something about the buffer" (which is also mentioned in this comment). Regarding that, I have an idea which is already employed by some plugins (ref: kulala.nvim, nvim-dap-view): storing the state inside a global variable. Global variables are restored when 'sessionoptions' includes globals (the conditions are slightly stricter and there are other caveats, but I digress).

This approach could be leveraged to restore additional information, besides plainly reopening the buffers. At a most basic level, it could be used to properly restore the cwd. But maybe it could be used to fully restore the tree's state (although I think this would be more complex, since files may have been deleted or whatever).

Setting cwd is quite achievable: nvim_tree.api.tree.open() will set nvim-tree cwd to Nvim pwd. The desired cwd can be explicitly passed via {path}

Full state could be very problematic, due to the great number of configuration options and live state changes (e.g. no-buffer filter) that we have no means of serialising or persisting.

A nvim_tree.api.tree.find_file() might be possible however there may be unforseen problems.

If we are willing to use features only available on nightly, we can also take advantage of the (recently merged) SessionWritePre, a new event that could also be helpful with the "stateful restore".

That changes everything; an official and time predictable mechanism. Yes, new Nvim features can be used via a version check to enable only when available, see Explorer

A rough outline of what I have in mind for an MVP is:

• Leave "session plugin users on their own"
• When a :mksession is issued, use SessionWritePre to update the global variable that stores nvim-tree's state
• Setup an eager autocmd on SessionLoadPost to restore the state. What I mean by "eager" is that it should be "reachable" from plugin/nvim-tree.lua (potentially "inlined"). Given that session plugins are not interfering with their own SessionLoadPost hooks and that the user isn't using any shenanigans to lazy-load the plugin, this should be much less "race prone".

Of course, that's a really "happy path" plan. If you are uncomfortable with the 'globals' approach, I was thinking about raising a discussion upstream, regarding the "best practices for plugins handling sessions" (aka lua-plugin-sessions). If the other ideas are also "far fetched", I'm open to alternatives. And if we can't settle down on the outline, I'm happy with closing and keeping this as a module on my own config. I could publish it as a recipe as well (current implementation would is easily "copy-pasteable").

A predictable SessionWritePre makes a globals approach desirable. The code would be simple on read and write, taking all the guess-work away, thus making it quite robust.

Yes to a global, however please to raise also start a discussion as there may be other unconsidered options or the opportunity for further upstream changes.

Please try this MVP:

• global write
• global read
• open with cwd
  • multiple tabs is up to you at this stage

Are the above possible using API?

Current implementation is not relying on any internals.

First question: is an error thrown on startup, or does the session simply fail to correctly load?

Do you mean if an error is thrown on startup, or something else? Raising an error on VimEnter does not prevent the session from loading. Buffers are restored as usual. SessionLoadPre is also fine. Haven't checked other events (I'm not sure about the event's load order, if you have any specific hints, I can dig into them). I could try a more "destructive" error (e.g., manipulating windows, etc) if needed.

No error right now is good, I was quite concerned. With defensive programming we can keep it that way.

[alex-courtis]

There is an alternative to globals: a json state file.

There is precedent with bookmark persistence: :help nvim_tree.config.bookmarks

[igorlfs]

Latest push now uses the "globals" approach to save and restore the cwd.

start a discussion as there may be other unconsidered options or the opportunity for further upstream changes.

Here it is!

There is an alternative to globals: a json state file.

I can look into that, given the precedent. But we can also wait for some feedback on the upstream discussion, whatever you prefer.

---

CI is failing because the event does exist on stable 🤔
Thoughts on that?

[alex-courtis]

Latest push now uses the "globals" approach to save and restore the cwd.

start a discussion as there may be other unconsidered options or the opportunity for further upstream changes.

Here it is!

There is an alternative to globals: a json state file.

I can look into that, given the precedent. But we can also wait for some feedback on the upstream discussion, whatever you prefer.

That sounds sensible. The global is a good placeholder until we learn more.

We can always refactor / change this at a later date as it is not API.

CI is failing because the event does exist on stable 🤔 Thoughts on that?

That is annoying. It looks like we'll have to use a suppression for the new path instead of the usual old path:

    vim.api.nvim_create_autocmd("SessionWritePre", { ---@diagnostic disable-line: param-type-mismatch

Those suppressions are regularly cleaned up, when we increase the minimum version of Nvim.

[igorlfs]

CI is happy now, should I squash?

[alex-courtis]

Let's run this as an experiment: we can make changes as needed and the user expectations will be correctly set.

• create an experimental config experimental.session_restore_nvim_0.13
• add doc to lua/nvim-tree/_meta/config/experimental.lua
• gate the autocommands behind the experiment flag
• create an issue similar to #2819 to describe the new functionality and give users a place to raise issues and discuss - I'll pin it

[alex-courtis]

That's fine. We are very aware of how insufficient the tab api is and how much trouble we've had with it.

Let's stick to the happy path cases to keep it simple and reliable.

Yes please: lots of comments in here.

[alex-courtis]

CI is happy now, should I squash?

Up to you. The PR will be sqash merged anyway.

[alex-courtis]

neovim/neovim#40336 (comment)

Uppercase globals seem to be a step in the right direction.

Hopefully more guidance follows.