rmagatti / auto-session

A small automated session manager for Neovim
MIT License
1.13k stars 46 forks source link

[FEATURE] Auto SaveSession on VimExit if there are more than one visible window/tab with supported file types #316

Closed powerman closed 6 days ago

powerman commented 1 week ago

Is your feature request related to a problem? Please describe. Sometimes you're not planning to continue previous session and just wanna quickly edit single file… but in process it turns out there are more files involved in current editing task and you end up temporary exiting vim with a lot of open files with intention to continue this editing session later.

Right now in this use case you'll have to manually run SaveSession before exiting. But since you are already get used to sessions working automatically, you always forget to do this!

Describe the solution you'd like I'd like to add extra logic on VimExit: if auto-session wasn't enabled on start (e.g. nvim was started without args) and there are more than one visible window/tab with supported (by auto-sessions) file types then call SaveSession (overwriting previous session, if there was one).

Describe alternatives you've considered It doesn't looks like current API provide enough information to make it possible to implement such a logic in user's autocmd. Probably it's possible to add something trivial like "call SaveSession if there are more than one open window/tab" but there are significant shortcomings: it'll hurt performance by doing duplicate save in case auto-session was already enabled, it will overwrite previous session when it's not needed (e.g. if we've just one user file opened plus several help windows).

Additional context This feature was implemented in https://github.com/powerman/vim-plugin-autosess and I miss it! :smile:

cameronr commented 1 week ago

303 is a potentially related feature request. If there was an option to load a session (if it exists) when launching with an argument, would that work for your use case?

powerman commented 1 week ago

No, I don't think so. My use case is:

  1. Start vim to quickly edit one file instead of restoring previous session for cwd. This is often happens to "oh, let me glance at that file…", often by pressing F4 in Midnight Commander, without any intention to continue work on previous large session saved for cwd.
  2. If everything goes as planned then this quick editing session won't became complicated, even if more files will be temporary opened they will be quickly closed, thus and at the end (VimExit) we won't have more than one file opened. In this case old session for cwd shouldn't be modified (if it exists).
  3. If current editing session turns out to be more complicated and I'll have more than one opened (visible in tab/window) files at VimExit then I want to save current session (overwriting old one for cwd if it exists).

The main idea is: session with more than one opened (visible) file is valuable and must be automatically saved in any case. (And session with just one visible file is not worth saving because Vim/Neovim is able to restore single file view/state without plugins.) If I don't have current session (because I started Neovim with args) and don't want to overwrite existing previous session for cwd then I'll bother to close all files but one before exiting Neovim.

Even in case I wanna quickly check a couple of files and run nvim file1 file2 but I don't want to overwrite previous session I'll bother to close one of these files before exiting. And if I won't close one of them then session should be saved/overwritten.

cameronr commented 1 week ago

Ok, that's a clear and helpful explanation of what you're looking for. I want to think about the other use cases a bit to figure out how they could all fit together.

cameronr commented 1 week ago

@powerman I have some WIP code for handling sessions with args. Are you up for testing it to see if it works for your use case?

powerman commented 1 week ago

Cool! Sure, I'm ready. Quick hint how to setup Lazy to switch a plugin to some branch would be useful if you remember it of top of your head, otherwise I'll check the docs.

cameronr commented 1 week ago

ok, you'll need to change your config as follows:

  -- 'rmagatti/auto-session',
  'cameronr/auto-session',
  branch = 'args-testing',

and add this parameter to your call to auto-session setup:

      args_handling = 'replace_session_only_if_multiple_buffers',

restart nvim, then bring up Lazy and find auto-session and update it (it should show up at the top saying there's an origin mismatch). then quit nvim.

now try going to a directory that has a session and if you run nvim somefile (or files) it will check and see if there is more than one loaded/visible buffer on exit and use that as the new session if so.

let me know how that works for you. thanks!

powerman commented 1 week ago

There are some corner cases where I'm not sure about expected behaviour:

@cameronr Almost done! :smile:

cameronr commented 1 week ago

Thanks for the testing! I currently have the code counting loaded, file backed buffers (as opposed to visible ones):

function Lib.count_supported_buffers()
  local supported = 0

  local buffers = vim.api.nvim_list_bufs()

  for _, buf in ipairs(buffers) do
    -- Check if the buffer is valid and loaded
    if vim.api.nvim_buf_is_valid(buf) and vim.api.nvim_buf_is_loaded(buf) then
      local file_name = vim.api.nvim_buf_get_name(buf)
      if Lib.is_readable(file_name) then
        supported = supported + 1
        Lib.logger.debug("is supported: " .. file_name .. " count: " .. vim.inspect(supported))
      end
    end
  end

  return supported
end

to count visible windows, it would be something like this:

  local supported = 0

  local tabpages = vim.api.nvim_list_tabpages()
  for _, tabpage in ipairs(tabpages) do
    local windows = vim.api.nvim_tabpage_list_wins(tabpage)
    for _, window in ipairs(windows) do
      local buffer = vim.api.nvim_win_get_buf(window)
      local file_name = vim.api.nvim_buf_get_name(buffer)
      if Lib.is_readable(file_name) then
        supported = supported + 1
      end
    end
  end

  return supported

For me personally, I tend to use few windows and more buffers and having multiple buffers open (even if not visible) is one of the things I really like about autosaving sessions. That said, I can know the other workflow where visibility and buffers are tied together is common. Another option would be to just have the replace_session option and then also allow a callback to be passed in for auto_save_enabled so someone could define their own function and then we could give the two examples in the documentation.

cameronr commented 1 week ago

@rmagatti I'd like to get your thoughts on args handling based on what i have so far

args_allow_single_directory: if nvim is launched with a single directory, load the session from that directory. handles nvim . (#303) as well as nvim some/dir. i don't see much (any?) downside to this option so could potentially be a default true option

args_handling: with possible values of replace_session (don't load a session but replace it on exit) or replace_session_only_if_multiple_buffers (don't load a session but replace it on exit if there at least two file backed buffers loaded).

So questions are:

  1. How do you feel about supporting a single directory argument?
  2. For file arguments, what do you think about having some kind of args_handling config that allows replacing the session in various scenarios?
  3. For those scenarios, would you rather they be specified by autosession (e.g. replace_session, replace_session_only_if_multiple_buffers, etc) or to allow a callback of some kind (e.g. should_save_session_with_args) and then cover some possibilities in the documentation?

FWIW, If it were just up to me, i'd allow single directory by default (after sufficient testing) and then have an enable_file_args option that's false by default but when enabled would replace the session if there was more than one loaded, file backed buffer. I'd let that function be overridden with a should_save_session_with_args callback and include the windows based one discussed above in the documentation.

rmagatti commented 1 week ago

@cameronr thanks for all the thought put into this. I'll strive to give this a good look over this weekend. Cheers!

powerman commented 1 week ago

I tend to use few windows and more buffers and having multiple buffers open (even if not visible)

Well, my point isn't about one workflow vs another, it's about predictable UX. If user sees multiple windows/tabs when it exits vim it's easy for him to be sure "this session will be restored next time I run vi without args", and if we doesn't like to overwrite (possible - he may not even be sure it is exists) previous session he can just close all-but-one windows/tabs before exiting vim. If there are some hidden buffers and just one windows - how he can be sure is this session will be saved or not (i.e. is his previous session for cwd will be overwritten)?

To me your use case with valuable hidden buffers sounds like you always wanna save current session, no matter what. This is valid use case and it requires much less code to implement. :smile: Probably no code - I believe it's easy to setup current rmagatti/auto-session this way. Of course I may be wrong about it, especially if you always do :ls before exiting vim to know for sure or there are some other tricks for buffer-oriented workflow I don't know about (maybe always show amount of buffers in statusline etc.). But in any case I believe there should be some UI sign which let user know is this session will be saved (possibly overwriting existing one) or not.

someone could define their own function and then we could give the two examples in the documentation.

Probably for discussed common use cases it's better to provide predefined functions to make it easier to setup one of them.

cameronr commented 1 week ago

I still think this is a workflow difference and a version of the classic buffers vs tabs debate.

for those that use tabs, a tab and a buffer are tightly coupled and this is the common GUI editor pattern.

for those that are buffer oriented, there isn't a direct a connection between where a file is being viewed and it being open, although some folks use the "tabline" at the top to list open buffers. Telescope has made it even easier to see and manage which buffers are open.

I understand what you're saying about not wanting to be surprised about a session being saved. for tab folks, the tab tells them there are two files and that a session would be saved. for buffer folks, they're checking the buffer list all the time for switching anyway (or they know there are multiple files open when using jumps) so that's how they'd know that a session would be saved.

i don't think one workflow is "more correct" than the other which is why i suggested a way for the user to configure it to work how they'd like.

rmagatti commented 1 week ago

I have a better picture of this now.

So questions are:

  1. How do you feel about supporting a single directory argument?
  2. For file arguments, what do you think about having some kind of args_handling config that allows replacing the session in various scenarios?
  3. For those scenarios, would you rather they be specified by autosession (e.g. replace_session, replace_session_only_if_multiple_buffers, etc) or to allow a callback of some kind (e.g. should_save_session_with_args) and then cover some possibilities in the documentation?
  1. Sounds good by me. No real downside, you're right. 👍
  2. & 3. I actually think we should keep things simple. The behaviour being asked for would add a pretty arbitrary rule (more than one buffer visible, why not more than 2,3,4,10?). That said, adding a new kind of hook that can influence auto-session's decision to save a session for a matching cwd in cases where it didn't load a session in the first place is an acceptable solution to this. It would keep auto-session simple, but allow its behaviour to be extended by users like @powerman for their own workflow. This way, any user can just do their own checks and decide whether they want to save a session that wasn't going to be automatically saved by auto-session otherwise with their own self-defined rules.
powerman commented 6 days ago

Testing results for v2.4.1:

The only issue is a bug in a README example (probably in both examples, but I've tested only one with tabs/windows):

-if vim.fn.filereadable(file_name) then supported = supported + 1 end
+if file_name ~= '' and vim.fn.filereadable(file_name) then supported = supported + 1 end

Thanks!

cameronr commented 6 days ago

Good catch! I think the underlying issue is that 'vim.fn.filereadable()' returns 0 (FALSE) if the file isn't readable or 1 (TRUE) if it does but 0 is still true in lua. I'll fix the docs

Line should be:

if vim.fn.filereadable(file_name) ~= 0 then supported = supported + 1 end