nvim-neotest / nvim-nio

A library for asynchronous IO in Neovim
MIT License
288 stars 8 forks source link

[BUG] `uv.fs_scandir` is wrong and does not have `uv.fs_scandir_next`. #3

Closed pysan3 closed 8 months ago

pysan3 commented 8 months ago

NeoVim Version

NVIM v0.10.0-dev-2066+g1813661a6
Build type: RelWithDebInfo
LuaJIT 2.1.1703358377
Run "nvim -V1 -v" for more info

Describe the bug The type definition says that nio.uv.fs_scandir returns nio.uv.DirEntry[] (which is correct in libc) but the lua implementation returns a uv_fs_t which is a handler that should be passed to vim.loop.fs_scandir_next which gives the next unscanned entry in the directory.

The help page of vim.loop states as follows.

uv.fs_scandir({path} [, {callback}])                           *uv.fs_scandir()*

                Parameters:
                - `path`: `string`
                - `callback`: `callable`
                  - `err`: `nil` or `string`
                  - `success`: `uv_fs_t userdata` or `nil`

                Equivalent to `scandir(3)`, with a slightly different API.
                Returns a handle that the user can pass to
                |uv.fs_scandir_next()|.

                Note: This function can be used synchronously or
                asynchronously. The request userdata is always synchronously
                returned regardless of whether a callback is provided and the
                same userdata is passed to the callback if it is provided.

                Returns: `uv_fs_t userdata` or `fail`

uv.fs_scandir_next({fs})                                  *uv.fs_scandir_next()*

                Parameters:
                - `fs`: `uv_fs_t userdata`

                Called on a |uv_fs_t| returned by |uv.fs_scandir()| to get the
                next directory entry data as a `name, type` pair. When there
                are no more entries, `nil` is returned.

                Note: This function only has a synchronous version. See
                |uv.fs_opendir()| and its related functions for an
                asynchronous version.

                Returns: `string, string` or `nil` or `fail`

To Reproduce

local cwd = vim.fn.getcwd()
local p = vim.print

local data = nio.run(function()
  local err, dir_iter = nio.uv.fs_scandir(cwd)
  assert(not err and dir_iter, err)
  p(dir_iter)       -- uv_fs_t: 0x7f7d921433c8
  p(type(dir_iter)) -- userdata
end)

Expected behavior

  1. Fix type annotation of nio.uv.fs_scandir.
  2. Add a corresponding api for vim.loop.fs_scandir_next.

Alternatively, if nio.uv.fs_scandir returned an Iterator<nio.uv.DirEntry> by resolving fs_scandir_next internally and returning an iterator, it will be very useful and will remove much boilerplate.

rcarriga commented 8 months ago

I've fixed the docs to show the correct return type, thanks for pointing out!

Since there is no asynchronous version of fs_scandir_next, there's no way to provide an interface to it except just having nio.uv.fs_scandir_next = vim.loop.fs_scandir_next which I don't like as it is misleading and users may think it's async.

pysan3 commented 8 months ago

I see. Thank you for your response.

So that means that I should pass the returned handler from nio.uv.fs_scandir into vim.loop.fs_scandir_next right?

If that's the case, how about returning uv_fs_t (defined here), instead of defining a custom type name (nio.uv.FsT (what does this mean in the first place?))?

rcarriga commented 8 months ago

So that means that I should pass the returned handler from nio.uv.fs_scandir into vim.loop.fs_scandir_next right?

Correct :+1:

If that's the case, how about returning uv_fs_t (defined here), instead of defining a custom type name (nio.uv.FsT

Ah I hadn't realised that those types existed, they didn't when I originally wrote the wrapper :sweat_smile: I've switched all of the type annotations to use the neodev types.

pysan3 commented 8 months ago

If that's the case, how about returning uv_fs_t (defined here), instead of defining a custom type name (nio.uv.FsT

Ah I hadn't realised that those types existed, they didn't when I originally wrote the wrapper 😅 I've switched all of the type annotations to use the neodev types.

Nice! Ok then please close this issue when you added those neodev types.

pysan3 commented 8 months ago

If that's the case, how about returning uv_fs_t (defined here), instead of defining a custom type name (nio.uv.FsT

Ah I hadn't realised that those types existed, they didn't when I originally wrote the wrapper 😅 I've switched all of the type annotations to use the neodev types.

Nice! Ok then please close this issue when you added those neodev types.

Close with https://github.com/nvim-neotest/nvim-nio/commit/48413f4460b1fe2b1efb134b0a5d13e0619bd0bd

Thanks @rcarriga !