natecraddock / sessions.nvim

a simple session manager plugin
MIT License
140 stars 9 forks source link

Add named session support for absolute mode #8

Open leaty opened 11 months ago

leaty commented 11 months ago

Thanks for the great plugin!

I added this feature to support my workflow, it allows naming sessions when using absolute mode rather than only storing them as paths split by dots. My fuzzy finder now lists named sessions, similar to how the good old dominickng/fzf-session.vim works.

I'm not sure if this is something you want for this project but I'll leave a PR here just in case.

tranzystorekk commented 11 months ago

Hi, thank you for making a feature proposal!

I'm not opposed to haiving a "named session mode", but I'd like to be cautious about globally switching the behavior of get_session_path with a config flag so drastically.

Maybe a separate command set would make the distinction clearer, like SessionsNamedSave, SessionsNamedLoad?

Does your workflow include using the API or just the user commands?

leaty commented 11 months ago

Does your workflow include using the API or just the user commands?

I'm actually using both right now (:SessionsSave manually and sessions.load for finder).

Maybe a separate command set would make the distinction clearer, like SessionsNamedSave, SessionsNamedLoad?

If you find that better it could work as well. Options:

  1. Functions save/load takes an optional arg to call e.g. get_session_path_by_name(name) instead
  2. Separate save/load functions for named (seems a bit superfluous to me)

Any other ideas?

natecraddock commented 11 months ago

I actually think this is okay. I'd like to test it before merging though. I'll try to test it later tonight

natecraddock commented 11 months ago

Alright, I just looked into this some more. Seeing this PR actually caused me to see some confusing behavior in the way that sessions.nvim currently works.

Currently, if config.absolute is true, but a path is still provided to the commands or API functions, the session file will be created in the working directory.

It's been about a year since the absolute feature was added, and it seems I didn't think things through enough or test it well. Or maybe this was my original intent? It is hard to say.

I think a more logical behavior is this:

To ensure this works well, a simple check at startup can be done

What do you both think about this? If I understand correctly, this should solve the problem and not require a new config option.

My only worry is that maybe someone is currently relying on the undocumented behavior where in absolute mode, relative session saving still works.

tranzystorekk commented 11 months ago

I like the proposal above, just a note to self and others to document the path argument's new "name/filepath" behavior, both for the API and the commands.

leaty commented 11 months ago

@natecraddock Yes this pretty much sums up my original thought of how absolute mode worked, until realizing it didn't. I am all for it as this is perfect for my workflow.

However I just want to mention that it's probably safe to assume there will be some people (not me) wanting to use the functionality of both at the same time, where different user commands (and save/load taking an optional arg) is more beneficial than a hard config switch.

natecraddock commented 11 months ago

However I just want to mention that it's probably safe to assume there will be some people (not me) wanting to use the functionality of both at the same time, where different user commands (and save/load taking an optional arg) is more beneficial than a hard config switch.

In the case that there are some people relying on the mixed behavior, here is one more idea.

Add a new option in the config mode. It would default to "relative". The other options are "mixed" or "absolute". If the option is "relative" or "mixed", then the current behavior would work. The config.absolute option would set the mode to "mixed".

If the mode is "absolute", then the more logical absolute behavior I proposed above would be used.

I have no strong opinion either way (adding a new config or just fixing the current behavior and hoping no one notices). Do either of you want to work on this? Or should I do it?

leaty commented 10 months ago

I think your last idea is the best for changing behavior via config, backwards compatible and all.

I just want to clarify what I mean with using both at the same time. While using (real) absolute for one's main projects- pretty much like "workspaces", there may still be incentive for storing project-local sessions, or pretty much anywhere.

In such cases I figure something like the following could be better, supporting relative and absolute path (has nothing to do with absolute mode) as well as simply relative to session_filepath (SessionsSaveAs is placeholder for something better):

# Relative and absolute
:SessionsSave path/to/session => ./path/to/session
:SessionsSave /path/to/session => /path/to/session

# Relative to session_filepath
:SessionsSaveAs path/to/session => config.session_filepath .. '/path/to/session'
:SessionsSaveAs named_session => config.session_filepath .. '/named_session'

Having said that, if you prefer config-only I can give it a go at implementing mode. Let me know!

natecraddock commented 10 months ago

I just want to clarify what I mean with using both at the same time. While using (real) absolute for one's main projects- pretty much like "workspaces", there may still be incentive for storing project-local sessions, or pretty much anywhere.

In such cases I figure something like the following could be better, supporting relative and absolute path (has nothing to do with absolute mode) as well as simply relative to session_filepath (SessionsSaveAs is placeholder for something better):

Thank you for clarifying! This actually does help a lot.

Now I have yet another idea on how to approach this 🙃 I am now wondering if part of the issue is that there is only config.session_filepath and it is conflated with both a filename, and a location.

I originally added it to save me from specifying my ./.nvim/session path, and the after absolute mode was added I also used it for the absolute directory prefix. So maybe this needs to be split into two configs? A path and a name? I haven't given it too much thought, just figured I'd put it out there.

Having said that, if you prefer config-only I can give it a go at implementing mode. Let me know!

At this point, you (and others) use this plugin much more than I do, so I trust your judgement. I do think that something beyond the current state of this PR needs to be done, but I'm not strongly attached to any one solution. So if you want to do a command or update the config, either way I trust y'all!