fix: normalizing a relative path returns an absolute path

[itmecho]

This PR is an attempt to fix #250 which came from this issue in telescope: nvim-telescope/telescope.nvim#1171

The issue is that when a file contains ..., it ends up prefixed with a / (probably different on windows but I can't test that). This is a valid filename in frameworks like NextJS (example)

The change I made to path.lua fixes the issue apart from when the paths also has .. segments. I could use some help figuring that part out. I think it's to do with path.root() but I'm not too sure why the behaviour is affected by the filename as I'm on linux and from what I can tell, that will always return /?

Todo

• Properly normalize files containing ..., i.e. dir1/[...test].ts
• Properly normalize files containing both ... and .. segments, i.e. dir1/dir2/../[...test].ts

[itmecho]

OK so it's when it finds paths containing .., it prepends the root to the path here

plenary.nvim/lua/plenary/path.lua

Line 101 in 6bb0e09

out_file = path.root(filename) .. table.concat(parts, path.sep)

[itmecho]

Maybe I misunderstood, I don't think it's actually a problem with files containing ... although that highlights the actual issue.

I think it's whenever a path has a .. segment, it ends up normalizing differently to if it doesn't have a .. segment, even if they should be the same path:

:lua print(require('plenary.path'):new('dir1/test.txt'):normalize())
dir1/text.txt

:lua print(require('plenary.path'):new('dir1/dir2/../test.txt'):normalize())
/dir1/text.txt

Does anyone know why that logic prepends the path.root()?

[itmecho]

OK I moved the is_absolute logic to the top of the file and then use it to check if the filename is absolute. Now it will only return an absolute path is the input filename is absolute.

All tests pass including the 2 new ones I added for testing relative paths

[itmecho]

Also tested it with telescope and it now correctly opens a file named pages/api/auth/[...nextauth].ts which it wasn't doing before

[Conni2461]

I dont know why this was added with this PR #140

Your changes look good to me. Can you fix stylua? Ignore the other CI we dont have a nightly image to pull (again)

[itmecho]

Done!

[Conni2461]

Okay i thought about this again and i think that easy. Example i am in /home/conni and do this little script.

local Path = require"plenary.path"

local p = Path:new("../../usr/local/share/nvim/runtime/lua/../autoload/ada.vim")
print(vim.inspect(p))
print(p:normalize())

I expect normalize to be /usr/local/share/nvim/runtime/autoload/ada.vim (and it is on master) after your patch its usr/local/share/nvim/runtime/autoload/ada.vim

Edit: In the object we have the correct value under _absoulte thanks to uv.fs_realpath but that also resolves symbolic links and we dont want to do that.

[Conni2461]

The whole path lib is long overdue for a refactor and splitup into something different. We shouldnt run fs_realpath on every new path object. It should be more lightweight. But thats a story for another day.

[itmecho]

Ah I didn't think of that =\ I'm not sure how you'd know that it should be absolute in that case without comparing it to _absolute in some way?

[Conni2461]

comparing against _absolute is also not a solution. Imagine we have this file. /home/conni/link which is a symbolic link to a file /usr/bin/bash then _absolute will be /usr/bin/bash. The current path object just has flaws we need to work around right now, to fix the bug and then we can rework everything and take our time to do something better (or worse) 😆

What we could do is count parts of _cwd (which is absolute) and see how many parts are taken away from the beginning. So if _cwd has two parts (/home/conni) and input has ../../ at the beginning (aka two parts or more should also be valid) then we need to append / and remove those parts otherwise if it only have one ../ we only need to remove it.

Does this make sense?

[itmecho]

Got you! I'll have a crack at it

[itmecho]

OK I think I've got it working now! All previous tests pass and I added two more which should cover the cases you found

[itmecho]

@Conni2461 don't want to bother you but is there anything I can do to help this along?

[Conni2461]

Sorry i forgot about it 😬

i think this looks good to me but i am not 100% sure its the most optimal solution. We can just try it out 😆