Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sessions do no longer auto-save on exit #222

Closed
LeonHeidelbach opened this issue Oct 24, 2022 · 44 comments
Closed

Sessions do no longer auto-save on exit #222

LeonHeidelbach opened this issue Oct 24, 2022 · 44 comments

Comments

@LeonHeidelbach
Copy link
Contributor

LeonHeidelbach commented Oct 24, 2022

@glepnir Sessions do no longer auto-save when closing out of neovim and a session already exists for the current directory. It would be nice to either have a setup option to re-enable this functionality natively or for you to expose the project_name function in the session module so that users could easily implement this functionality themselves. As of now, to find the correct session one would have to copy your code to get the session file name in order to check if it exists for the cwd and depending on that call "session_save" on closing out of neovim.

@glepnir
Copy link
Member

glepnir commented Oct 25, 2022

can provide a function to check the current file in session or not then do something.

@LeonHeidelbach
Copy link
Contributor Author

can provide a function to check the current file in session or not then do something.

That would be perfect. Thank you!

@glepnir
Copy link
Member

glepnir commented Oct 25, 2022

when have time do this :)

@LeonHeidelbach
Copy link
Contributor Author

when have time do this :)

Do you want me to implement this? I won't have time today, but could do it tomorrow and create a PR if you like.

@glepnir
Copy link
Member

glepnir commented Oct 25, 2022

why not :)

@Leo310
Copy link

Leo310 commented Jan 2, 2023

I have an existing session and session_auto_save_on_exit set to true, but it doesn't auto save on quit. When I run :SessionLoad it opens the file I first run :SessionSave on and not the file I last quit.

My settings:
image

@LeonHeidelbach
Copy link
Contributor Author

LeonHeidelbach commented Jan 3, 2023

@Leo310 I have implemented the feature so that it will only auto-save a session if there is more than one file open. If that's not the case, the existing session file will not be overwritten.

Edit: what's also important is, that the auto-save functionality is only triggered after first creating or loading a session for the current directory using :SessionSave or :SessionLoad. Dashboard does not automatically create or load sessions for directories you open, so you have to do that manually with a key binding or the aforementioned commands. However, you could implement such functionality yourself very easily if you need it.

@Leo310
Copy link

Leo310 commented Jan 3, 2023

Weird, because I have more then one file open and it still doesn't overwrite the existing session.

And on your edit: I did create a session with :SessionSave first, but this session is as I mentioned, just not overwritten, by the auto-save hook. Not sure what I am doing wrong

@LeonHeidelbach
Copy link
Contributor Author

@Leo310 whenever you reopen the directory, you will first have to load the session to enable auto-saving. If it still doesn't work for you, the issue has to lie somewhere else. I'm using this feature on a daily basis and it works just fine for me.

@Leo310
Copy link

Leo310 commented Jan 3, 2023

Very weird:
Small Gif I made showing that I think I am doing everythink right

  1. :SessionSave fileA
  2. :SessionLoad -> opens fileA
  3. navigating to a different fileB and quitting nvim (should auto-save now)
  4. :SessionLoad -> should open fileB but still opens fileA

Any idea why It still doesn't work?

@LeonHeidelbach
Copy link
Contributor Author

@Leo310 I found the issue. Apparently sessions are not saved when creating buffers through telescope. If you open files through NvimTree or using the standard :e ur_file.lua command, everything works as intended. This is something @glepnir will have to look into but it looks like telescope is the culprit here. Nice catch though 😄. I usually use NvimTree to open buffers, so I would have probably never noticed this. I recorded a quick video that demonstrates the issue.

showcase.mp4

@Leo310
Copy link

Leo310 commented Jan 3, 2023

@LeonHeidelbach Hmmm I think we have slightly different issues:

  • Also not working with Nvim-Tree
  • My :SessionSave does save the session but the auto_save is not overwriting it (as seen in my video)

Maybe reopen this issue?

@LeonHeidelbach
Copy link
Contributor Author

LeonHeidelbach commented Jan 3, 2023

@Leo310 I mean the auto-save feature does work for me as you can see in my video as long as I don't use telescope. The auto-save functionality is based on the existing session save method which does seem to have issues with buffers opened through telescope. You could try to verify that the custom auto-command callback does in fact get called, by simply commenting out the line pcall(vim.cmd, "NvimTreeClose") and opening NvimTree before saving the session and closing out of nvim. On reloading the session, you should see an empty NvimTree buffer when the line is commented out, which should not be present if you put it back in. If the callback function actually does get called, the auto-save method does it's thing but simply fails to save your buffer list for some other reason (potentially due to telescope). Did you try to open files not using telescope? Does the issue persist?

In any case I would suggest you open a new issue for this, since auto-saving itself works for me.

@Leo310
Copy link

Leo310 commented Jan 3, 2023

@LeonHeidelbach Ok then I will propably open a new issue.
What I am trying to say is that the :SessionLoad command always restores the state of the last :SessionSave, but not the one of the last auto-save. And the auto-save callback is also not called. I even tried without Telescope (only opened files with Nvim-Tree), but it didnt work.

@LeonHeidelbach
Copy link
Contributor Author

@Leo310 Oh, then your problem could be that you define the custom auto command within the plugin config function of your package manager. You should define the custom auto command in your main initialization lua. Give that a try and let me know if that fixes it.

@Leo310
Copy link

Leo310 commented Jan 3, 2023

@LeonHeidelbach Sadly, also doesnt work :(

@LeonHeidelbach
Copy link
Contributor Author

LeonHeidelbach commented Jan 3, 2023

@Leo310 Well, I really cannot say what's the issue without further investigation then. You could try to execute the following commands and let me know what the outputs are:

:lua print(require('dashboard').session_auto_save_on_exit) --> This should print 'true' if your config has been applied correctly
:lua print(require('dashboard.session').should_auto_save()) --> This should print 'true' if the first command prints 'true' and you have loaded or saved the current session
:lua vim.api.nvim_exec_autocmds('User', { pattern = 'DBSessionSavePre', modeline = false }) --> This should close your NvimTree buffer if you have one open

@Leo310
Copy link

Leo310 commented Jan 3, 2023

Really Appreciate your help man, but everything works as expected 🤔
I checked the plugin code and for now I also couldnt find anything.

@LeonHeidelbach
Copy link
Contributor Author

@Leo310 Sure thing. I mean I implemented this feature, so it might have been a mistake on my side, but I couldn't find anything wrong with the plugin code. Let me know if you find out what the issue was.

@Leo310
Copy link

Leo310 commented Jan 3, 2023

@LeonHeidelbach fixed it by commenting out these lines.
image
seems like maybe db.session_auto_save_on_exit wasnt set on start up of plugin?

@LeonHeidelbach
Copy link
Contributor Author

@Leo310 That is very strange. But you said that all of the commands I listed printed 'true' right? That means that the variable should've also been set correctly if this returns 'true':

:lua print(require('dashboard').session_auto_save_on_exit)

Maybe it get's reset somewhere else?

@Leo310
Copy link

Leo310 commented Jan 3, 2023

@LeonHeidelbach yeah it returns true. But I wouldnt know where this should be reset. and there is no possibility that the this variable is set after this plugin is initialized?

@LeonHeidelbach
Copy link
Contributor Author

LeonHeidelbach commented Jan 3, 2023

@Leo310 I mean it only ever gets assigned a value in the default config of the plugin here, so only changing it from outside, somewhere within your config, could potentially cause this (even though it's unlikely you intentionally reassigned the value somewhere 😄 ).

@Leo310
Copy link

Leo310 commented Jan 3, 2023

that is the only place I set it. Maybe something with the Lazy.nvim package manager and its lazy loading?

@Leo310
Copy link

Leo310 commented Jan 3, 2023

so weird

@LeonHeidelbach
Copy link
Contributor Author

Yeah, well that could be. I personally still use Packer, since it's been around for quite some time and it's mostly stable. I need my package manager to work reliably and lazy.nvim probably still has quite some issues since it's just been released.

@Leo310
Copy link

Leo310 commented Jan 3, 2023

I have no experience in building plugins or something like that so no idea what exactly could cause this in Lazy.nvim.
So then I think I will just use my hardcoded version for now and hope it is fixed one day haha

@LeonHeidelbach
Copy link
Contributor Author

I have no idea how Lazy.nvim actually works internally, but it could be a caching issue. Maybe Lazy is pre-computing some plugin tables to make things faster. I remember reading in its documentation, that it also natively implements similar functionality to impatient.nvim. But if it's a caching issue your other config values should not work either. It's a strange issue... You could maybe try to print the value of session_auto_save_on_exit where you commented out the if statement to see if it actually is false or nil.

@Leo310
Copy link

Leo310 commented Jan 3, 2023

yeah very strange. and it's actually false where I commented out the if statement

@LeonHeidelbach
Copy link
Contributor Author

LeonHeidelbach commented Jan 3, 2023

@Leo310 Okay, how about you try and change the default value here to true. If that works then we know it has to be some issue with Lazy.nvim. Maybe then try to set lazy = false in your plugin table to disable lazy loading Dashboard.

@Leo310
Copy link

Leo310 commented Jan 3, 2023

Yeah that works and is what I am doing atm.
lazy = false didnt do the trick
Would you open an issue on lazy for that or would you chill and wait?
I just have no idea where the Problem could be

@LeonHeidelbach
Copy link
Contributor Author

LeonHeidelbach commented Jan 3, 2023

I would open an issue with Lazy.nvim for that since it's a pretty big problem. It's also still unclear what is causing this exactly. If you want to be sure and take the time you could try switching your package manager to Packer and see if you are still getting this config issue with your setup. If not, there is no doubt that it really is a Lazy issue. Specifically with preserving the config state after loading the plugin.

@folke
Copy link

folke commented Jan 4, 2023

Hi! I just saw this issue.

Is it expected that a user does require('dashboard').setup() before sourcing dashboard's plugin files?

Lazy doesn't precompute anything or caches that sort of data btw.

@folke
Copy link

folke commented Jan 4, 2023

I just pushed an update that might fix this. @Leo310 would be great if you could confirm

@LeonHeidelbach
Copy link
Contributor Author

LeonHeidelbach commented Jan 5, 2023

Hey @folke! Nice to see a direct response to this random issue from you here, how did you even find this? 😄 Dashboard does not have a dedicated setup function, instead users are supposed to directly change values within the "db" settings table located here like so: require('dashboard').session_auto_save_on_exit = true. Dashboards init.lua sets default values to all settings on sourcing the plugin files, so the user should be able to change those values after the defaults have been set, which seemd to have worked for @Leo310. Checking the settings values during runtime like this :lua print(require('dashboard').session_auto_save_on_exit) showed the correct values. However, the settings seemed to have been reset to their default values or this section of code had been run before the user settings were applied. I hope this helps, and maybe your commit already fixed it 👍.

@folke
Copy link

folke commented Jan 5, 2023

The problem was that dashboard/plugin is ran before the user's config() method. I've now switched that around.

@glepnir
Copy link
Member

glepnir commented Jan 5, 2023

maybe can use event = 'VimEnter or something else to make plugin folder not load at startup.

@Leo310
Copy link

Leo310 commented Jan 5, 2023

@folke yeah it is working now 👌. Also wondering how you find this issue?

@folke
Copy link

folke commented Jan 5, 2023

I'm searching Github every now and then to find any issues that might be caused by lazy ;)

@Leo310
Copy link

Leo310 commented Jan 5, 2023

real sick man. i was a bit to lazy to open a new issue, so love your effort :)

@folke
Copy link

folke commented Jan 5, 2023

So, I had to revert that change, since that did break some plugins. I was also not really in favor of that change to begin with.

When loading a plugin, the paths are added to the rtp, plugin files are sourced and once it's completely loaded, only then the user's config() is executed.

I think it makes more sense to indeed change the dashboard code to only check if sessions need to be enabled on VimEnter.

Or you could even always create the autosave autocmd and on VimLeavePre check if you need to save it or something...

All those other autocmds are not a problem, since their callback code executes on events after sourcing the plugin file.

I hope this makes sense...

@LeonHeidelbach
Copy link
Contributor Author

LeonHeidelbach commented Jan 5, 2023

@folke maybe instead of changing when Lazy calls the config function you could introduce another setup field that can be used to set up plugins like dashboard. I am sure there are also other plugins that would require the same way of setup, so having this would make Lazy more compatible without the need of changing plugin code. Especially since Packer supports this way of setting up plugins it would make the transition for people easier without the need to tell other plugin devs to change their setup method. This could potentially take quite some time in some cases if they are even gonna do it at all.

@folke
Copy link

folke commented Jan 5, 2023

I would like to keep it simple. config runs when the plugin was loaded, meaning that everything related to the plugin is loaded and available to be used in the config function. This is also how native packages with Pakadd would work.

@glepnir 's suggestion and what i commented is an easy fix and makes more sense.

@pmatulis
Copy link

pmatulis commented Dec 4, 2023

I see that this issue is closed. Well I just hit this - no autosaving. I'm using Telescope like a billion other people.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants