Feature: load last cwd

[josh-nz]

This is a proof of concept that, when autoload.cwd = true, instead of just loading the session named after the cwd, will load the most recent session file for that cwd.

This should work regardless of whether autosave.cwd = true is set. For example, a user could save manual sessions. Or it could be a combination of both. The idea behind this is that a user would want to resume whatever was the most recent session for that cwd. It could be extended to the more general idea of "load the last session regardless of cwd" if desired.

I've tried to keep each commit to a particular feature or slice of work.

There are a few issues with it as it stands.

1. There is still an issue with autosaving. I think this has caused bug: auto load cwd with auto save will save blank session first #60 to open again.
2. I think there needs to be consideration around the semantics of the config options and how they integrate with this proposed change.

Let's discuss 1.
With autosave.cwd = true and this implementation of autoload.cwd = true, do the following.

1. Load Neovim
2. Open some files
3. Quit. This will create the session named after the cwd.
4. Reload Neovim. This will autoload the session named after the cwd.
5. :PossessionSave foo to save a new session.
6. Quit.
7. Load Neovim.

This triggers session.lua autosave to run. The issue is autosave_skip returns true when it should return false, see #60. One of the buffers has a buffertype of '' (the default [no name] buffer I think), so hence the table is truthy and next(table) is true. This means the autosave.cwd clause in autosave_info returns true (because these expressions are negated), and since the name of this is the cwd which doesn't match foo, autosave triggers. I don't think it's appropriate to try and change the autosave.cwd clause to use the latest cwd session name, see discussion point 2 below. I haven't yet found a way to reliably have autosave_skip return false when starting Neovim. I still don't quite understand the logic in autosave_skip, in particular the or not next(unscratched_buffers) clause.

One option to solve this is to pass a skip_autosave flag down the call stack. It might be cleaner than trying to decide if autosave should be skipped by examining the state of buffers in autosave_skip.

Let's discuss 2.
I think this one needs a bit more time and thought. Initially there was a save cwd and load cwd commands, and autosave cwd and autoload cwd config options. Now there is the concept of the 'last cwd' which has been thrust upon the autoload.cwd config option. So the semantics of what a cwd session means is starting to blur. For example, if autoloading the cwd loads a session named foo, what should save cwd command do? foo is the current session, and happens to be a cwd session, but it's also not the "named" cwd session. Similarly, should :PossessionLoadCwd load the most recent session, or only the "named" cwd session?

I've left the saving the loading commands alone; I feel they should save and load the "named" cwd session. It's worth noting that otherwise to load the cwd named session via :PossessionLoad is very difficult due to the percent encoding name.

I'm not sure there is merit in having another config to "load the last cwd" session, when the implemented behaviour is, I think, what users would naturally expect. Similarly, I don't think changing the name of the autosave_info cwd clause to be the last session name is correct, it should remain as the cwd.

It's late here, and I need some sleep. Apologies for a dump of information. I think there is something useful here, especially if you want a default session auto created (based on the cwd name) but then switch to a manually named session for example for different git branches. But I do think it's important to take some time and consider what these different features and options mean. It also has impacts on how the code is designed.

[jedrzejboczar]

Thanks for this work and detailed message.

I added some comments regarding the implementation.

Regarding 1.
I haven't debugged it, but it seems like there might be a lot of edge cases. Your suggestion to pass skip_autosave down the call stack seems like a good idea to avoid some complex logic.

Regarding 2.
I would probably need to think about it some more time but my initial thoughts:

• "what should save cwd do" - I would say that everything related to cwd settings/commands should save CWD sessions (name after CWD)
• autosave.current has higher priority, so if we loaded CWD session and it happens to be the named session foo then we would save foo because autosave.current matches; if user has autosave.current=false then it would save as CWD session
• PossessionLoadCwd could probably distinguish if it uses just "the CWD session" or "last sessions that has a matching CWD" using bang (!)
• there should probably be config option to decide if autoload should only consider "the CWD session" or all sessions that match CWD, but I would have to think more about naming it
• we might also consider using vim.ui.select in the commands, so that a LoadCwd command would prompt user to select which session to load, but this needs some consideration where to put it and how to allow users to disable this behaviour (some may prefer just use the most recent one), so maybe:
  • LoadCwd - uses vim.ui.select
  • LoadCwd! - always takes the most recent one
  • LoadAutoCwd or something like that would be for "the CWD session"
• when thinking about it maybe it makes no sense to have the "only the CWD session" case, as you say, users would probably think that LoadCwd means "auto-generated CWD sessions and named sessions with matching CWD", and if we support vim.ui.select then they can choose anyway

Ok, I haven't addressed everything, but now it's late for me too :P
I dumped what I could and I'll try to get back to this review later.

[josh-nz]

I've had a few thoughts overnight. The main goal is try to keep it simple to understand. I'm ok with some things being a little more involved provided we include clear and understandable documentation so users can learn.

1. Let's start with the autoload because I think that is simple. Autoload only happens once on Neovim load, so it can only be a single thing. I think what threw me was the current config for autoload is a table. I propose it becomes autoload = "last" | "last_cwd" | "cwd". This covers all cases. Names are to be decided. In particular, this concept of the autosave cwd session (the percent encoded one) needs a concrete name, I think. It will help address user understanding of the other features, and we can explain it in the docs. I was calling it unnamed cwd since the user doesn't provide a name, but it does have a name and unnamed might have user recognition conflicts with for example the unnamed register. Because I don't have a better name right now, I'll refer to it as pe_cwd (for percent encoded) until we find a better name. Edit: we should keep the function option for autoload also to allow users custom autoload choices.

2. My thoughts this morning were that :PossessionLoadCwd and :PossessionSaveCwd only operate on the pe_cwd session. They take no arguments. The idea here is that these are simply convenience commands so a user doesn't have to call :PossessionSave ~/foo/bar/ or the load equivalent, esp when path names might get long and there is no autocomplete. We can be clear about this in the docs. Edit: it looks like there is autocomplete for :PossessionSave and :PossessionLoad. That's nice.

3. That only leaves :PossessionListCwd that I added, but I wonder if this should be removed, and the cwd is simply an optional argument to :PossessionList that filters the results. This helps further separate :PossessionSaveCwd and the :PossessionLoadCwd as convenience commands since there would no longer be this command :PossessionListCwd that operated on all sessions for the cwd. Any command that has Cwd in the name only operates on the single pe_cwd session.

PossessionLoadCwd could probably distinguish if it uses just "the CWD session" or "last sessions that has a matching CWD" using bang (!)

I'm not sure I like this idea. It is putting different meaning, and my basic Neovim understanding is that bang is used to bypass any prompts, which is how I think the other Possession commands use it. This new usage then becomes a one-off and goes against user intuition.

I also don't have any thoughts on using vim.ui.select. That could certainly be something worth considering, but I think perhaps we keep it outside the scope of this PR. There are currently ways to achieve this via the Telescope extension.

I think the above would make things pretty clear and easy to understand, and is mostly how it is currently working.

Edit: Noticing that there is autocomplete for :PossessionSave and :PossessionLoad, is there a need for :PossessionSaveCwd and :PossessionLoadCwd, and a need for the pe_cwd autoload option? Autoload options would be last or last for cwd. Something to think about.

[josh-nz]

Ok, yeah, the Github review system did not work as I was expecting. Clearly I don't know how to use this correctly ;) Hopefully you can figure it out. I was mostly just trying to add a comment to a specific commit.

[josh-nz]

From my edit comment above:

Edit: Noticing that there is autocomplete for :PossessionSave and :PossessionLoad, is there a need for :PossessionSaveCwd and :PossessionLoadCwd, and a need for the pe_cwd autoload option? Autoload options would be last or last for cwd. Something to think about.

Thinking some more, it might be that users don't have the autosave cwd option enabled, so for them, to save a session they need to think of a name or type the path (to emulate the pe_cwd auto save). So I think keeping :PossessionSaveCwd is a useful convenience command, although as I type this, could we incorporate this into :PossessionSave?

So :PossessionSave might work as:

1. Save the current session if one exists
2. Prompt for a session name if no session exists - pressing enter without a name will default to the pe_cwd session name
   If 2, then 3. Prompt to overwrite if config is set to prompt and session name exists (existing behaviour).

Or is this overloading :PossessionSave too much? I think it's still a fairly straight forwarded flow.

[jedrzejboczar]

Regarding #63 (comment):

1. This makes sense, I can't think of other behaviours. (btw, auto_cwd could also be nice alternative to pe_cwd)
2. (and the following "Edit") I would leave Possession{Load,Save}Cwd as separate commands. They would be an alternative for users that would like a "manual cwd autoload/autosave" and work just as these would.
3. As I noted in the commend, I would leave PossessionListCwd. It's more obvious what it is filtering on and we can provide directory autocompletion + vim.fn.expand.

I'm not sure I like this idea. It is putting different meaning, and my basic Neovim understanding is that bang is used to bypass any prompts, which is how I think the other Possession commands use it. This new usage then becomes a one-off and goes against user intuition.

Agreed, let's leave bang for forcing things etc.

I also don't have any thoughts on using vim.ui.select. That could certainly be something worth considering, but I think perhaps we keep it outside the scope of this PR. There are currently ways to achieve this via the Telescope extension.

Sure, let's leave it for now.

---

Regarding #63 (comment):

I'd leave PossessionSaveCwd as separate command. It would be the "manual cwd-autosave". I feel like this is to much for PossessionSave. The problem I see is that if you don't provide a name, then you get CWD as the default input, but then you have to delete it if you actually wanted a prompt for "normal" session name. I think it will be simpler to have separate commands.

• PossessionListCwd and PossessionLoadCwd would take optional argument (with directory completion)
• PossesionSaveCwd would take no arguments, it doesn't seem to make sense to specify different directory than the current one (at least I can't think of good reasons)

[jedrzejboczar]

Ok, so now, to summarize while looking at the latest version of the code, my suggestions would be (if I didn't forget about something):

• separate Possession{Save,Load,List}Cwd commands, load and list take an optional argument with directory completion (see possession.lua for how migrate uses file completion - we want dir) that defaults to cwd (vim.fn.getcwd())
• use autoload = "last" | "last_cwd" | "cwd" as you suggested - this seems like the most sensible approach

I commented on some things in different conversations but I resolved all the non-action-point conversations to leave only what is relevant now.

[josh-nz]

You're summary sounds fine and reasonable. I do have a question about :PossessionLoadCwd. If an argument (session name) is provided, I assume we want to check that the session matches the cwd, and if not, display a message to the user, and suggest they use :PossessionList instead? Eg something like
Session foo belongs to a different cwd. Use PossessionLoad foo if you want to load this session

Otherwise there is no difference between this and :PossessionLoad.

[jedrzejboczar]

No, I was thinking that the argument would be a directory to use as cwd instead od the actual cwd. So:

• with no arguments it uses vim.fn.getcwd() and selects the most recently modified session that matches this cwd
• with an argument it uses vim.fn.expand and makes the path absolute, and uses the result instead of vim.fn.getcwd; this command has the built-in directory command-completion enabled

[jedrzejboczar]

So basically this would be the same as you did with PossessionListCwd. What do you think? This way it is a shortcut for doing :cd a/b/c followed by :PossessionLoadCwd with no arguments.

[josh-nz]

Yes, that makes a lot more sense than what I was thinking.

It should only load the auto_cwd session? Or the last session for the specified dir? What if there is no auto_cwd but only a single named session?

I think keeping the *Cwd commands to operate on the just the auto_cwd makes the API easier to understand since behaviour is consistent. What are your thoughts?

Edit; I just reread your comment and you say

with no arguments it uses vim.fn.getcwd() and selects the most recently modified session that matches this cwd

So SaveCwd will always save as the auto_cwd session, but LoadCwd will always load the most recent session for the given dir, or vim.fn.getcwd() if no dir is provided.

Now I'm a bit unsure as to what the "correct" implementation should be. I doubt I'll use these commands, so I'll let you make the final choice on how loadCwd should behave.

[jedrzejboczar]

Thanks for all your work on this and other issues in this plugin. Let's wait some time, I'll also use this version and when you think it's ready then we can merge it.

[josh-nz]

There is a table that is returned from lua/possession.lua that has some commands. Do we need to add the newer commands to this? Things seem to work as is, without adding the newer commands.

I don't know what this returned table does.

[jedrzejboczar]

This was added as a convenience API shortcuts to allow e.g. for require('possession').save instead of require('possession.session').save, but now I think it was a bad idea - additional complexity with little to no benefit. As for your question whether we need to extend it - no, let's leave it as-is. Besides, I think this PR does not extend possession.session API so there would be nothing to add.

[josh-nz]

I've been thinking about PossessionLoadCwd.

The current implementation of PossessionLoadCwd loads the last session for the given cwd, or the default cwd if no argument is provided. This could be the auto-cwd session if it is the more recent one, or the only one. Or it could be a manually named session. I guess my point is, you don't know what it will load.

If I want to load a session I need to know it's name. While :PossessionLoad has autocomplete, the data for named sessions don't indicate the cwd they relate to, so this is information that I just need to know.

Which folder is foo related to, for example?

I can use PossessionList/Cwd first, and then I could see the names and cwds, and then have to then execute PossessionLoad {name}. Which is a little cumbersome, and the Cwd variant command is named differently from the load command (it has Cwd in the name). Or I could use the Telescope listing and select from there (a nicer user experience with a single command), but that requires me to have Telescope installed.

If I'm working across some Git branches, I really only care about my cwd sessions. I think this is why my first attempt at the implementation for PossessionLoadCwd listed all the sessions for the cwd. In this way, the autocomplete functions the same as for PossessionLoad, just filtered down to the cwd only. As a user, I'm also explicitly in control or what is loaded. Both commands take a session name as argument. As it currently stands, it's unknown what PossessionLoadCwd will load, and it's not user specified. I think this is what is nagging at me about the current implementation.

I'm back to wondering if PossessionLoadCwd should work as described above; autocomplete lists sessions only for the cwd. If you want to change directory, do it yourself, or use PossessionLoad {name}.

Every time I try to write something down about these commands I keep running into strange edge cases, or mismatches in the API behaviour even when trying to make them similar. Ultimately my usage of sessions is pretty limited in scope, I think it will mostly be a single auto cwd saved session and auto load most recent. So a lot of what I'm thinking about is all in my head and not necessarily representative of someone using this for real world stuff. And unfortunately I don't use Neovim enough to have strong ideas about this or real use cases. So I think I should leave the final say on how these commands behave up to others - but I can see a strong case for switching between different sessions in the same cwd, for eg with Git branch changes, or as a way to "pin" different buffers for different parts of a project, and as mentioned above, there isn't an easy way to do this without Telescope.

Apologies is this is a bit of a ramble, I've been finding it hard to effectively communicate my thoughts and ideas on these APIs.

Edit: I should state that my Neovim usage is typically a single instance in a folder. If I want to change folders I quit, cd, and reload Neovim. I might have cd hooks that change my env path and tools versions etc. So when I'm in a Neovim instance, anything I do is with that root folder and sub folders. Hence why I wouldn't use PossessionLoadCwd as it currently stands, to jump to a different cwd. I hope that by explaining my use case, it makes my thoughts I little clearer. We all have different use cases, which I think is part of the reason this is tricky to solve.

[jedrzejboczar]

Your explanation makes a lot of sense and I think it would be better to go your way and have PossessionLoadCwd work on current directory only and provide autocomplete for sessions available in that directory (and without argument load most recent session for cwd). I initially thought it would be better to have the same semantics for LoadCwd/ListCwd, but that probably would be a mistake. It's probably ok to have users first cd into a directory and then select a session.

In the future we could probably have a separate command PossessionLoadForDir <dir> [<name>] that would work linke cd + PossessionLoadCwd, but I wouldn't include it in this PR, as I don't have a strong opinion if this is needed. This could be accompanied by a two-stage Telescope picker (first picks from available cwds, then for given cwd), but as said, that's out of scope of this PR. We can always add these things later.

As for your Neovim usage with one instance per cwd - it's probably what most people use, my workflow is probably more rare - I use tab-local cwd's and exrc.nvim to load/unload specific settings when I change directories.

[josh-nz]

Ok, I'll make these changes tomorrow. I was also looking at what these commands currently do:

:PossessionSaveCwd
Save session for current working directory. Use `!` to avoid confirmation
dialog.

:PossessionLoadCwd
Load session for current working directory.

So this change will help keep this behaviour. The only remaining question is what the no argument (default) should be for PossessionLoadCwd. The current master branch will load the auto_cwd session, but you suggest it should be the latest. This would be a breaking change (potentially, if they have more than one session in a cwd) but the users will also have autocomplete to select the auto_cwd session if desired, without having to type the full name. PossessionLoad also loads the most recent session, so there is consistency there. So I think probably latest is a reasonable choice.

[josh-nz]

I don't think we need to use a vim global, a module variable seems to work for all we need here.

[jedrzejboczar]

Right, that's nice

[josh-nz]

We're not validating that the user provided name matches a cwd session. If it matches any session it will load. I think this is fine.

The autocomplete only shows cwd sessions.

[jedrzejboczar]

Seems fine

[josh-nz]

I think this takes care of all the outstanding issues. So aside from any bugs, I think work on this is complete.

To summarise the major changes:

• New autoload choices; false (off), last, last_cwd, auto_cwd, or func -> string
• Old autoload configs will continue to work, deprecated warning is shown
• PossessionListCwd has dir autocomplete
• PossessionLoadCwd has autocomplete listing only cwd sessions. Will default to last cwd session if no argument is supplied. Possible change in behaviour here for some as previous version would load auto_cwd session by default.

Closes #59 as this now supports my immediate session needs.

[jedrzejboczar]

Looks good to me, merging!

[jedrzejboczar]

Seems fine

[jedrzejboczar]

Right, that's nice

[jedrzejboczar]

@josh-nz Thanks for all your work on this! This took a lot of time, but I feel like your changes are well-thought and will be useful for many users.

[josh-nz]

Thanks for all your help and guidance with this.