tiagovla / scope.nvim

Revolutionize Your Neovim Tab Workflow: Introducing Enhanced Tab Scoping!
406 stars 21 forks source link

Problems in combination with Session Manager #1

Closed nikbrunner closed 1 year ago

nikbrunner commented 2 years ago

Hi there, thank you for this great little plugin. It is exactly what I need, but there seems to be a problem in combination with the Session Manager.

When I run save_current_session() all tabs get discarded for the session. This issue does not occur without scope. Sometimes I even run into a nvim segmentation fault.

I am using Neovim 0.7, but this also occurred with 0.8.

I would really appreciate the help here :) Thanks again!

tiagovla commented 2 years ago

I think one way to deal with this would be to save scope's state within the session and restore it. Also, check if all hidden buffers are correctly saved https://github.com/Shatur/neovim-session-manager/blob/4f9129a6fd80dc0a5111349bfb447ff243c7d504/lua/session_manager/utils.lua#L75-L79. I will look into this when I get more time.

nikbrunner commented 2 years ago

Alright. Thank you very much. I even paused using the Session Manager in favor of Scope, because how much it improves my workflow. :)

It certainly seems to me saving hidden buffers correctly, since when I load up a session, all other “non-active” buffers are already loaded up. But it is possible that I don't understand this correctly.

If I can help out with logs or something, just let me know

musjj commented 1 year ago

I wonder what's the correct way of caching the buffers? From my experience, buffer ids aren't consistent throughout sessions, so saving buffer ids wouldn't be enough.

tiagovla commented 1 year ago

I wonder what's the correct way of caching the buffers? From my experience, buffer ids aren't consistent throughout sessions, so saving buffer ids wouldn't be enough.

Absolute filepath + hidden state should be enough to reconstruct it, I suppose.

EpsilonKu commented 1 year ago

Any progress?

tiagovla commented 1 year ago

I tried to store the state in the session file in this branch. There are two commands ScopeLoadSession and ScopeSaveSession to be triggered after loading a session and before :mksession, respectively. I still need to test it. It should require globals, buffers, tabpages on vim.opt.sessionoptions.

EpsilonKu commented 1 year ago

Ok, I will test with 'rmagatti/auto-session'

musjj commented 1 year ago

Thanks for implementing this! But may I suggest abstracting the functions a bit more?

The current method forces the user to save all globals across sessions, which might not be ideal for some users. For example, possession.nvim allows the user to store arbitrary data within the session file, so there may not be any need to store globals at all.

We can split {load,save}_session() into {serialize,deserialize}_state and {load,save}_session. And then, we can do something like:

function M.load_session()
    if vim.g.ScopeState ~= nil then
        M.deserialize_state(vim.g.ScopeState)
    end
end

function M.save_session()
    vim.g.ScopeState = M.serialize_state()
end

Plus an option to enable/disable the autocmd that reads the global:

require("scope").setup { restore_state = false }

This way, the user can now manually handle the state management all by themselves (by calling require("session").{serialize,deserialize}_state), in case they're unsatisfied with the default method.

tiagovla commented 1 year ago

Thanks for implementing this! But may I suggest abstracting the functions a bit more?

The current method forces the user to save all globals across sessions, which might not be ideal for some users. For example, possession.nvim allows the user to store arbitrary data within the session file, so there may not be any need to store globals at all.

We can split {load,save}_session() into {serialize,deserialize}_state and {load,save}_session. And then, we can do something like:

function M.load_session()
    if vim.g.ScopeState ~= nil then
        M.deserialize_state(vim.g.ScopeState)
    end
end

function M.save_session()
    vim.g.ScopeState = M.serialize_state()
end

Plus an option to enable/disable the autocmd that reads the global:

require("scope").setup { restore_state = false }

This way, the user can now manually handle the state management all by themselves (by calling require("session").{serialize,deserialize}_state), in case they're unsatisfied with the default method.

That was just a draft of a solution that would not require any intervention of the session managers, I will make this behavior optional. I was playing with the SessionLoadPost, I wish there was one before saving the session file as well. I've added your contributions, thank you!

EmreDogann commented 1 year ago

I don't know if I'm missing something, but what is the purpose of restore_state? I ask this because in init.lua the SessionLoadPost autocommand it's supposed to register is commented out, meaning restore_state does nothing:

https://github.com/tiagovla/scope.nvim/blob/107214792bb77a35588e5112dde629aa316c80cc/lua/scope/init.lua#L28-L32

tiagovla commented 1 year ago

I don't know if I'm missing something, but what is the purpose of restore_state? I ask this because in init.lua the SessionLoadPost autocommand it's supposed to register is commented out, meaning restore_state does nothing:

https://github.com/tiagovla/scope.nvim/blob/107214792bb77a35588e5112dde629aa316c80cc/lua/scope/init.lua#L28-L32

I was hoping for it to seamlessly integrate with any session plugin, but unfortunately, it's not functioning perfectly yet. I have submitted a pull request on neovim to include an additional session event, and I'm currently awaiting their response. In the meantime, you can use the commands ScopeSaveState and ScopeLoadState based on the hooks provided by your session plugin.

EmreDogann commented 1 year ago

Ah, I see. With the plugin manager I'm using (nvim-possession) I was running into issues with ScopeLoadState from the post session load hook where sometimes it seemed that it wasn't loading correctly. I wasn't able to fix that so I uncommented that line above to have the plugin do the autoloading and it seems to work so far.

tiagovla commented 1 year ago

Ah, I see. With the plugin manager I'm using (nvim-possession) I was running into issues with ScopeLoadState from the post session load hook where sometimes it seemed that it wasn't loading correctly. I wasn't able to fix that so I uncommented that line above to have the plugin do the autoloading and it seems to work so far.

That's helpful information. Please feel free to reach out if you encounter any issues. It appears that this autocommand might be executing multiple times, once for each buffer. Since I'm not using a session plugin, I'm uncertain if this could potentially lead to any problems.

EmreDogann commented 1 year ago

I think in this case the README should be changed to mention this fact or at least mention that the option currently doesn't work. Because the example in the readme has set restore_state = true, implying that the option does something by toggling it.

tiagovla commented 1 year ago

@EmreDogann I removed it from the README and also uncommented it in case you want to use it unofficially. I will wait until my neovim's PR is approved or denied to work on that.

tiagovla commented 1 year ago

If there are any problems with a specific session manager, please open new issues.