improved pathlib

[jamestrew]

trafficstars

Introduce new pathlib - plenary.path2, with the focus of greatly improving Windows support. This help close several Windows path issues in telescope https://github.com/nvim-telescope/telescope.nvim/pull/3275

There's a number of breaking changes which is why I opted to create a new pathlib over just fixing plenary.path but most of the API remains consistent to help with migration.

Breaking Changes

• Path objects are now "immutable" (as much as lua allows)

  Many of the path operations are done using "metadata" parsed from the args pass on instantiation rather than on the public filename field. This frees us from needlessly parsing filename on each operation (eg. Path:absolute). I don't think users mutating path instances is a common thing anyways.

• Path.new no longer supported -- felt redundant and didn't feel too valuable

• Path:new drops sep table param support (eg. Path:new { "foo", "bar", sep = "/" }) -- this doesn't make sense for posix paths since there's only one valid separator anyways, for Windows see next bullet

• Respects shellslash option on Windows -- provides more consistent experience with the rest of vim

• Drops __concat metamethod -- it was untested, and had a todo comment, wasn't sure what the intent of it was

• Fix Path:make_relative

  With plenary.path:

  Path:new('foo/bar_baz'):make_relative('foo/bar') -- returns 'foo/bar_baz'
  

  This is wrong. With plenary.path2, it will throw an error if the object path (foo/bar_baz) is not a subpath of the target path (foo/bar). But to compensate for this, adds a new walk_up option (default false) to walk up the target path until the two paths reach a common parent. eg.

  Path:new('foo/bar_baz'):make_relative('foo/bar', { walk_up = true }) -- returns '../bar_baz'
  

  This is probably the most dangerous change since at least in telescope, there are lots of calls to make_relative (mostly via normalize) to make paths relative to incompatible paths (eg. make README.md relative to ~/projects/telescope.nvim -- if both were absolute paths, README.md would be considered as a being a subpath of ~/projects/telescope.nvim, but as a relative path, it's not possible to declare that it is, hence make_relative will fail). I'm tempted to add an option to silence errors for these types of conditions.

• Path:normalize:

  • Many call sites in telescope actually depend on the previous "broken" make_relative behavior so for normalize, I'm pcalling the make_relative part and using the original path if make_relative fails -- this largely preserves existing behavior while improving others like https://github.com/nvim-telescope/telescope.nvim/pull/3151/files#r1633252306 eg. with plenary.path

    -- cwd = ~/projects/plenary.nvim
    local p = Path:new "../telescope.nvim/README.md"  -- starts relative to cwd
    print(p:normalize(vim.uv.cwd())) -- telescope.nvim/README.md now no longer relative, BAD
    

    now with plenary.path2

    -- cwd = ~/projects/plenary.nvim
    local p = Path:new "../telescope.nvim/README.md"
    print(p:normalize(vim.uv.cwd())) -- ../telescope.nvim/README.md keeps path relative to cwd
    
    -- also
    print(Path:new("README.md"):normalize(vim.uv.cwd())) -- README.md despite `make_relative` failing interally
    

  • No longer substitute home directory for ~ -- this feels like the exact opposite of what "normalize" style functions do. And generally, paths will be relative and so shouldn't apply in most cases. Maybe add an option or a separate function?

• Path.filename is semi pre-normalized (no extraneous . or path separators) -- with how the path metadata is parsed, this comes for free

• Path:rename

  • new_name is now a positional arg (eg. path:rename('foobar'))
  • returns a new Path instance rather than mutating itself

• Path:copy

  • destination is now a positional arg (eg. path:copy('foobar', { exists_ok = true }))
  • drops interactive mode (mostly was just lazy - can add this back)
  • return value table is pre-flattened
  • return value table value is {success: boolean, err: string?} rather than just boolean

• Error handling is generally a lot more explicit/loud, most noticeably with libuv interactions, raising returned errors rather than silently swallowing them or doing generic assertions (eg, local fd = assert(uv.fs_open(...)))

• Renamed Path:iter to Path:iter_lines for more clarity

• Path:find_upwards returns nil if file not found rather than an empty string

[clason]

If you are rewriting the pathlib module from scratch, you should make sure that you are leveraging the latest relevant Neovim API (in particular, vim.fs). Plenary was written before there was a "Lua stdlib" in Neovim, but today that is very much a thing, and any new implementation should respect that.

Conversely, if the stdlib is missing any functionality (in its scope), consider contributing it there (first).

[clason]

Also, a (strong) API design recommendation:

1. mandatory arguments should be positional;
2. optional arguments should be keyword-style (i.e., part of a final(!) opts table).

(This implies thinking hard about which arguments should be mandatory and implementing them from the start, while optional arguments can -- and should -- be left for later.)

Otherwise you're very quickly painting yourself into a corner and can't add new options without breaking the API.

[jamestrew]

Thanks for the feedback.

I don't think there's much I can use from vim.fs since this new Path class deconstructs a given path into "parts" mostly since Windows paths are so hard to work with as one string. I also took care to keep things nvim0.7 compatible as that's where the existing plenary.path module sits.

There's probably some learnings I can upstream to vim.fs though.

Also, a (strong) API design recommendation:

1. **mandatory** arguments should be **positional**;

2. **optional** arguments should be **keyword-style** (i.e., part of a final(!) `opts` table).

Good idea, I'll take this into consideration.