folke / snacks.nvim

šŸæ A collection of small QoL plugins for Neovim
Apache License 2.0
1.19k stars 23 forks source link

bug: Mini.Sessions default read using `'local'` errors when user defines default `file` config #144

Closed GitMurf closed 6 days ago

GitMurf commented 6 days ago

Did you check docs and existing issues?

Neovim version (nvim -v)

NVIM v0.11.0-dev-1108+g5a86360400

Operating system/version

Windows 11

Describe the bug

The default for mini.sessions errors out if the user sets the config file value which determines the default name for the "local" project-wide session. I recommend changing the following:

https://github.com/folke/snacks.nvim/blob/9462273bf7c0e627da0f412c02daee907947078d/lua/snacks/dashboard.lua#L744-L745

... to something like this:

    { "mini.sessions", ":lua require('mini.sessions').read()" },
    { "mini.nvim", ":lua require('mini.sessions').read()" },

OR

    { "mini.sessions", ":lua require('mini.sessions').read(require('mini.sessions').config.file)" },
    { "mini.nvim", ":lua require('mini.sessions').read(require('mini.sessions').config.file)" },

Also should be able to replace require('mini.sessions') with MiniSessions if want to make less verbose.

cc'ing @echasnovski to see if he agrees and/or has an even better default?

Steps To Reproduce

N/A

Expected Behavior

N/A

Repro

No response

folke commented 6 days ago

I don't use mini.sessions, so no idea what's the correct way :) I'd for sure keep the require instead of the global, since user may be lazy-loading mini.sessions. Before a that session load is called, we already cd to the correct directory, so we want to load the session for the cwd.

I thought that was done by doing read('local')?

echasnovski commented 6 days ago

Before a that session load is called, we already cd to the correct directory, so we want to load the session for the cwd.

I thought that was done by doing read('local')?

This case indeed is configured via config.file. If you want to support only that, then using its value is the way to go indeed.

However, running setup() is necessary to have detected sessions, including local. And indeed running read() without arguments should be the best of both worlds.

Ideal code snippet is something like this: if _G.MiniSessions == nil then require('mini.sessions').setup() end; _G.MiniSessions.read().

folke commented 6 days ago

Great. Just changed it to read()

GitMurf commented 5 days ago

Thanks to both of you!

@echasnovski I hope you're proud of me that I basically got it right on the first try! šŸ˜œ šŸŽ‰

echasnovski commented 5 days ago

@echasnovski I hope you're proud of me that I basically got it right on the first try! šŸ˜œ šŸŽ‰

Yeah, nice job :+1: ! The fact that you are checking out a dashboard which is not 'mini.starter' - not so much :thinking:

GitMurf commented 5 days ago

@echasnovski you know it's impossible to control the neovim urge to always test and try every new shiny object šŸ¤£ I feel like there needs to be a Neovim Anonymous support group!