Feedback on API + integration

[stevearc]

Continuing the discussion from reddit, I took another stab at integrating netman into oil.nvim. I'm keeping the work in a branch for now. I was able to get further along than I did before, and I have a working proof-of-concept that can do read-only browsing. A couple of notes:

• The format for urls with an absolute path is scp://host///path/to/file.txt. For netrw, the equivalent would be scp://host//path/to/file.txt. That extra / threw me off for a while, because if it's missing I just get an obscure looking error. I personally prefer the netrw format (I don't see what the extra / is for), but if you stick with this format I'd recommend adding some earlier url validation.
• I love the regular structure for the entries returned by providers. I know it's probably too early for this, but when 1.1 is finalized I'd like to see some documentation about which providers can provide which metadata fields, and for any enum fields I'd like to see a list of their possible values. What sparked that second one is I noticed that entries have TYPE = "regular file", whereas I'd expect that to be TYPE = "file" to match libuv. That made me curious what other values are possible in that slot.
• I'd really love to see an async API. Currently it seems like the only API is synchronous and blocking, which makes the UI feel very clunky and unresponsive. It's possible that neovim 0.9 will ship with an async implementation (there's been a lot of chatter on Lua: structured concurrency, Promises, task pipelines neovim/neovim#19624 recently), but there's always good old-fashioned callbacks.

This is probably as far as I'll go with the integration work until v1.1 is more stabilized. Nice work overall! Looking forward to seeing where this goes.

[miversen33]

@stevearc Thank you for the feedback!

The format for urls with an absolute path is scp://host///path/to/file.txt. For netrw, the equivalent would be scp://host//path/to/file.txt. That extra / threw me off for a while, because if it's missing I just get an obscure looking error.

I hate obscure errors! For this event (a URI that is not able to find a match), you should an error that says netman cannot find a matching provider for the uri. I can word this in a better way if you would like, verbiage is quite important and if this error is not clear enough for a respective user of the framework, it needs to be updated to be more clear. That said, this error should crop up basically immediately when you attempt to do a api.read on an "invalid" URI. If you aren't seeing that, there may be a bug that I will have to further investigate.

I love the regular structure for the entries returned by providers. I know it's probably too early for this, but when 1.1 is finalized I'd like to see some documentation about which providers can provide which metadata fields, and for any enum fields I'd like to see a list of their possible values. What sparked that second one is I noticed that entries have TYPE = "regular file", whereas I'd expect that to be TYPE = "file" to match libuv. That made me curious what other values are possible in that slot.

So given that a provider must adhere to what Netman says it can return, the metadata that is returned is all standardized. It can be found here and you can see an example of how it can be read, in a standard way here. I opted to go with the terminology LINK and DESTINATION since Netman isn't bound strictly to files or filesystems. Thus, a LINK can be treated as a directory and a DESTINATION can be treated as a file (for your purposes). I am iterating over this a bit to clean up how this will be returned by api.read as the explore content (which is poorly named) is quite clunky right now. That will be standardized by the merge of v1.1 into main.

I'd really love to see an async API. Currently it seems like the only API is synchronous and blocking, which makes the UI feel very clunky and unresponsive. It's possible that neovim 0.9 will ship with an async implementation (there's been a lot of chatter on Lua: async/await/await_all abstraction, structured concurrency neovim/neovim#19624 recently), but there's always good old-fashioned callbacks.

I am working through this right now actually! Though I made an active decision to not have read be asynchronous (as I am not quite sure how that should be handled by a consumer just yet). I can look into doing that for with v1.1 as I am working through trying to get searching (executing remote find basically) to be async as well.

I had no idea that async was going to be a main feature of Neovim, I am currently working through abusing libuv's spawn function for all of that right now.

Anyway! Thank you for the feedback, and once I get v1.1 merged into Main, that is when I would recommend taking a peek at this again (as I should have the kinks of consuming content from Netman worked out). I made a bit of a poor choice jumping to v1.0 when I did lol, but the decision is already made. If you would like, I can tag this issue when v1.1 is merged and I would be more than happy to help you integrate with Netman at that point (the less code I have to maintain, the better 😄 )

[stevearc]

If you edit a file with two slashes as an absolute path (e.g. scp://root@host//root/test.txt), it produces this error:

Error detected while processing BufReadCmd Autocommands for "scp://*":
Error executing lua callback: ...nv/netman/data/nvim/lazy/netman.nvim/lua/netman/init.lua:51: attempt to index field 'data' (a nil value)
stack traceback:
        ...nv/netman/data/nvim/lazy/netman.nvim/lua/netman/init.lua:51: in function 'read'
        ...env/netman/data/nvim/lazy/netman.nvim/lua/netman/api.lua:247: in function <...env/netman/data/nvim/lazy/netman.nvim/lua/netman/api.lua:243>

I opted to go with the terminology LINK and DESTINATION since Netman isn't bound strictly to files or filesystems

That makes sense for the top-level API that you're trying to get all of your providers to conform to. It looks like there's some additional metadata for each of the entries, though (blksize, group, inode). Since you're already including that filesystem-specific information, I think it makes sense to also include a more detailed file type so we can distinguish a file from a symbolic link from a fifo, etc.

Feel free to ping this issue when v1.1 is released, but I'm following the releases so I should get notified anyway :)

[miversen33]

Oh thats a true neovim error. I will look into that, you should get a netman error instead of the code crashing. Thanks for your help so far! I appreciate you :)

[miversen33]

With v1.15 in testing now, I think I have hit a good point where this issue can be closed out. I wouldn't call this "complete" per say (as has been called out a few times, my documentation is still lacking), but I do think overall netman has moved in a far enough direction that the various feedback points here have been addressed.

@stevearc I may pull down oil and see if I can get netman integrated with it over christmas break. It has been on my list for a while and it seems like a decent start point for a new "thing" for netman. I don't promise anything usable but it is an interesting idea to poke at and spread netman :)