feat: add vim.fs.relpath

[dundargoc]

This is needed to replace the nvim-lspconfig function is_descendant that
some lspconfg configurations still use.

[dundargoc]

Alternative to #31702

[dundargoc]

@neovim/core: there's been some discussion in matrix chat without clear resolution whether this should work for target paths outside of the base directory. In other words, whether vim.fs.relpath("/var/lib", "/var/apache") should return ../apache or nil. I think ../apache is technically more correct but that returning nil would make the implementation easier. Other than that I'm not entirely decided. Input welcome if anyone has ideas.

For reference:
golang, zig, java, D, R, dart and realpath tool: returns ../apache
python: returns an error by default, but has an optional flag that allows outside paths in which case it would return ../apache
haskell: does not allow this case (so I guess an error?)
rust: does not have relative path functionality

[echasnovski]

I think ../apache is technically more correct but that returning nil would make the implementation easier.

Returning '../apache' definitely makes relpath() more versatile. My personal opinion is that it comes at somewhat high cost of (significantly) improved complexity of the function itself and the inability to use it to decide whether a path is a descendant of the base (thus requiring a separate function for that). Plus I don't think I can get a good enough practical use case for supporting returning '../apache' like output.

If the requirement is dropped, then implementation is fairly straightforward and allows vim.fs.relpath(path, dir) == nil be used to check whether path is descendant of dir:

vim.fs.relpath = function(from, to)
  from, to = vim.fs.normalize(vim.fs.abspath(from)), vim.fs.normalize(vim.fs.abspath(to))
  to = to .. (to ~= '/' and '/' or '')
  return vim.startswith(from, to) and from:sub(#to + 1) or nil
end

For reference, the relative path function for golang, zig, java, D and dart would return ../apache.

Resorting for what is commonly used in other languages might indeed be the way to go here. You can also add one of R's popular file system library to the list of "... would return ../apache".

[gpanders]

The realpath tool also returns ../apache:

$ realpath --relative-to /var/lib /var/apache
../apache

[justinmk]

If the requirement is dropped, then implementation is fairly straightforward and allows vim.fs.relpath(path, dir) == nil be used to check whether path is descendant of dir:

Let's do that way then. We can add support for the "outside of target directory" case as a optional flag, later, if anyone actually requests it. But the default behavior should allow us to avoid is_relative_to.

[wookayin]

or nil if base is not an ancestor.

What's the main motivation behind this design? Wouldn't base="/var/lib", target="/var/foo" => relpath = "../foo" also make sense (relpath in other programming languages implements such a behavior)?

[wookayin]

Sorry for a duplicate comment, I missed the previous discussion. But I'm still worried about potential breaking changes in the future and the design of the API.

[justinmk]

See #31790 (comment)

The current behavior allows relpath() to also provide "is relative to" functionality.

Wouldn't base="/var/lib", target="/var/foo" => relpath = "../foo" also make sense

We can get that in the future if it's wanted, by adding a opts flag. I think it's uncommon. It also complicates the logic. So if people want it, we can add it later.

[wookayin]

I agree with we can avoid a complication for uncommon scenario, but as a low-level stdlib I believe the correct and reasonable default behavior is to make it behave the same and consistentyl as the common relpath and other programming languages (including python's os.path.relpath).

I believe eventually we will implement the remaining case ("walk_up") sometime in the future, or at least some people will find the behavior of API surprising (arguably). If the function would work correctly only under a certain opt, that would sound weird in my humble opinion.

If the main purpose of adding this low-level vim.fs API is to implement is_descendant (with being performant), why don't we add a dedicated API instead?

So please consider requiring an explicit option such as opt = {walk_up = False} to achieve the (current) behavior, where the default option for the API ({ walk_up = true }) could remain yet-to-be implemented for the time being.

[dundargoc]

I guess we could do that, although adding an option for an unimplemented feature sounds a bit odd. I'm fine with either option, I guess you'd need to convince @justinmk and @echasnovski though.

[justinmk]

I believe the correct and reasonable default behavior is to make it behave the same and consistentyl as the common relpath and other programming languages (including python's os.path.relpath).

I didn't think about that. Though I can see that it would be useful for sibling files. But I have never needed that...

The common use-case for wanting a relative path is for a target that you know is descendant, and you want to avoid an absolute path (e.g. "workspace roots" for LSP or other project-like resolution).

Do you have some real-world examples where a "sibling" relative path is wanted?

[dundargoc]

@wookayin I'd prefer not to delay with this for much longer. If you have any examples in mind let us know, otherwise I'll go ahead and merge this soon.

[echasnovski]

If you have any examples in mind let us know, otherwise I'll go ahead and merge this soon.

Might be worth adding a third opts argument for future use and explicitly say that it is "reserved for future use". Not mandatory, since adding it later seems to follow the API contract for Lua stdlib.

[dundargoc]

OK, add an opts argument

[dundargoc]

I'll merge this, we can change the signature before release if it turns out doing the other way is more useful.