-
-
Notifications
You must be signed in to change notification settings - Fork 47
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
[BUG] auto_session_allowed_dirs with a glob entry fails on Windows. #272
Comments
@josh-nz Thanks for the report and fix! I noticed the same code was used in suppress_dirs so unified the code to fix both (and added a unit test for both cases). @rmagatti The fixes are in my url encoding/command overhaul branch which has a ton of changes along with a bunch of fixes (no more losing dashes on windows!). As soon as you merge the unit test PR, I'll submit a PR for that one. it's pretty big so it would be great if we could have a few people try it out. |
This doesn't seem to work on mac as well, here is my config
When I open neovim in any of the subdirectories under |
@rbhanot4739 Hmm, I'm on mac too and globs work as expected for me so I wonder what else is going on. Can you do the following:
Note: make sure there isn't an existing session for the directory you choose in step 2 (currently, the code only checks |
@cameronr I followed above mentioned steps but same issue, here are the debug logs
|
@rbhanot4739 Ah, I see the issue. the There isn't currently a way to only create sessions in a particular directory and every sub-directory below that directory (e.g. there isn't currently a prefix match option). How were you hoping it would work? If you really want to constrain were sessions are created, you can set |
@cameronr I think the behaviour we have here is acceptable for most use cases.
I did try the recursive glob pattern like
So does that mean if I manually create a session in one directory, the subsequent changes will be automatically captured unless I delete the session manually, in which case a new one won't be created automatically ? |
Yup, with auto_create = function()
local cmd = 'git rev-parse --show-cdup'
return vim.fn.system(cmd) == '\n'
end, |
Describe the bug
If there is a glob path entry in the
auto_session_allowed_dirs
table, and the current working directory matches an expanded path other than first in the expanded series, the session will not be restored.To Reproduce
Configure auto-session with more than one allowed dir:
Assume this folder structure:
C:\foo\bar
C:\foo\baz
Inside
C:\foo\baz
, open nvim.Adjust your open buffers.
Quit nvim (which will create a session file).
Open nvim.
Expected behavior
The buffer previously opened will be restored. Instead, nvim is opened with a blank buffer.
Baseline (please complete the following information):
set sessionoptions?
: not relevantuname -a
:windows32 2.6.2 9200 i686-pc Intel unknown MinGW
nvim --version
:NVIM v0.9.5
Additional context
The cause of the issue is in the following function in
auto-session/init.lua
:Let's break down the line causing the issue (as per my comment in the above code):
vim.fn.simplify
expects to take afilename
string as an argument. However, this isn't exactly whatLib.expand
returns.Lib.expand
returns a string in the format<path>\r\n<path>\r\n...
which is in fact multiple paths. On Windows,vim.fn.simplify
doesn't simplify this correctly. Given an input"C:\\foo\\bar\r\nC:\\foo\\baz"
,vim.fn.simplify
will return"C:\\foo\\bar\r\nC:foo\\baz"
. Noticed how in the second path the backslash after the drive letter has been dropped. This causes this pathC:foo\bar
to not match thecwd
C:\foo\bar
and so theis_allowed_dir
function returnsfalse
.Based on the documentation for
vim.fn.simplify
- that it takes afilename
and not filenames - I don't believe this to be a bug in thevim.fn.simplify
function. The fact that it currently works correctly for MacOS and *nix systems I put down to blind luck.The solution, is to
Lib.expand
the glob, then split on the\r\n
, and thenvim.fn.simplify
, so now it is only being passed a singlefilename
.Here is the solution:
You can inline the
simplified_path
andpath_without_trailing_slashes
locals if desired, I left them there for ease of logging the output during debugging. Note also the added---@diagnostic disable-next-line: param-type-mismatch
line. This was a warning thatstring.gmatch
doesn't take astring[]
. I believe this is either an error with the type spec or Lua's type inference unable to correctly resolve the type:vim.fn.expand
(which is called byLib.expand
) states that it returns a string when thelist
argument is nottrue
, which is the case here.In short, the original code was doing:
expand path glob
simplify (the multiple paths as single string)
split on
\r\n
Now, it is:
expand path glob
split on
\r\n
simplify (a single path string)
I have tested this on Windows and MacOS, and it appears to be fine. So I assume *nix will also be fine. From just reasoning about my proposed change, I can't see any reason why this would break anything.
The text was updated successfully, but these errors were encountered: