Open glennsl opened 4 years ago
Here's an initial "thinking" response. None of it reflects any "final" decisions I am making; instead, I just want to reply with my thoughts until now, such as the considerations that went into making the OCaml API how it is. We can discuss further :)
Luv.Loop.run
returns a boolean value whose meaning does not seem to be documented and, judging by the examples, in most cases is not used, but usually justignore
d.
Luv.Loop.run
's return value is documented at uv_run
in the libuv docs, in each of the bullet points of the list. In general, the OCaml docs attempt to be thin, provide links to the C docs, and treat the C docs as the definitive reference. This is true not only with respect to libuv, but especially more so with respect to linking man pages, where there is no hope, for example, of describing all the error conditions exactly, since they differ system by system. For this reason, the docs also link to at least POSIX man pages wherever possible, and the intent is for readers to visit them for a full understanding.
Would it help to more prominently indicate to readers, that the docs should be used in this way?
I would argue that using
ignore
without any type annotation is very error-prone because it sets the trap for accidental partial application. It should be considered bad style IMO, and therefore should be avoided particularly in examples. I also thinkLuv.Loop.run
exacerbates the likelihood of errors because its arguments are all optional except for the terminatingunit
. It's very easy to specify one or more optional arguments but forget the terminating unit. This leads to partial application, and with the use ofignore
will go entirely unnoticed except for the observation that nothing is happening, which is a symptom that doesn't narrow down the possible causes a whole lot.
This is definitely a real ergonomics issue on the OCaml side. I should probably begin with rewriting the examples not to use ignore
. After that, we can look into a higher-level API.
Furthermore, it's a stated goal for Luv to provide "more natural APIs":
where libuv is forced to offer difficult APIs due to the limitations of C, Luv provides more natural APIs
But it's also stated that Luv is "a fairly thin binding", which to a significant extent opposes the above goal. And there's certainly also a documentation and familiarity benefit of having the API closely mirroring the original.
Indeed, but the stated goal is not exactly to provide "more natural APIs" — quoting it that way discards context, and if that context is not discarded, the opposition in goals goes away, too :) As it says, the goal of more natural APIs only applies in places where the libuv API is hampered by C, such as in uv_spawn
, where the huge number of options leads libuv to choose to take a struct
as an argument, with several helper enums, etc. In all other cases, the goal is to keep the API as easy to map to the C API as possible. So, the "more natural APIs" goal only applies where the C API is so thoroughly annoying, that you'd rather not learn it in the first place, and don't want anything to correspond directly to it, but just want a mapping from a more natural API explained in the OCaml docs, in case you need to refer to C :) In other words, we can readily imagine that users and authors of uv_spawn
didn't want it to be the way it is, but they had no better choice.
So there are two parts to this issue I think. One is the more general question of how much deviation from the C API is considered acceptable. Where the line is, or should be, drawn between providing "natural APIs" and being a thin binding. The other [...]
I agree with this. Part of the issue is that I originally saw two "major" use cases for Luv:
Lwt_unix
, to replace the current implementation by libuv.Since I imagined the library would be mostly used in the implementation of other libraries, I thought it's acceptable (even desirable) to trade polish of the API for direct, predictable correspondence with the behavior of libuv, so that Luv stays out of the way of higher-level implementors as much as possible, except where all likely implementors would probably agree that something should just be taken care of as part of the translation done by Luv.
If Luv gets more direct usage, this will probably have to be adjusted in some way. Perhaps a thin wrapper with less sharp corners would need to be written, or there is a compromise on all strange APIs that can be found, that makes Luv directly suitable for both kinds of usage.
Also
- It can be prevented by providing a better, more natural, API (in some future version 2.0).
We can just break Luv all the time, it's in 0.x and has few users :)
The use of
init
everywhere, for example, seems unnecessary in most use cases
Don't we need creation functions for most kinds of objects?
The choice of name init
is not typical for OCaml, but I made it to correspond to the names in the libuv API.
Also, I will add a sentence at Luv.Loop.run
about looking in uv_run
docs for the explanation of the return value.
I added the minimal, initial change of replacing
ignore (Luv.Loop.run ())
by
ignore (Luv.Loop.run () : bool)
in the README and all examples, so that at least it forms a habit that, if practiced, will make partial application less likely. Posted the new docs.
Thanks! I completely agree with the approach of having Luv be thin low-level bindings that correspond closely with its C counterpart, and building higher-level bindings on top of that. I've even used this approach before myself. I guess my expectations were just skewed a bit because despite being low-level and relatively unpolished, this is still the most polished and well-documented API I'm aware of for what it provides!
It might still make sense to make minor alterations like turning the Loop
modes into separate functions, but only as long as the name reflects that they're all variations of uv_run
. I think the API I proposed above does that, despite having other issues.
Would it help to more prominently indicate to readers, that the docs should be used in this way?
Definitely. Although I did look at the docs for uv_run
, but I guess I expected a stand-alone sentence describing the return type and didn't consider that it would depend so much on the "mode". There's still a difference between the int
returned by uv_run
and the bool
returned by Loop.run
though, and the relation between them isn't entirely obvious. I'm guessing 0
here corresponds to false
, but often C APIs return error codes where 0
means success, which might more naturally map to true
.
[...] quoting it that way discards context, and if that context is not discarded, the opposition in goals goes away, too :)
To be fair, I did quote it with context too, although that still didn't stop me from interpreting it wider than intended :)
Don't we need creation functions for most kinds of objects?
The choice of name init is not typical for OCaml, but I made it to correspond to the names in the libuv API.
The APIs I've interacted with so far, mostly just Loop
and FS_event
, don't seem like objects though. In FS_event
, for example, it would in most cases be sufficient for start
and init
to be a single function, returning a handle to be passed to stop
later. I can't imagine a scenario where it makes sense to call init
without immediately calling start
.
Of course arguments to the contrary include consistency across modules and correspondence with the C API, and I think you've already convinced me that it should remain as it is. But if the goal was a more natural API then init
certainly sticks out in this case at least.
It might still make sense to make minor alterations like turning the Loop modes into separate functions, but only as long as the name reflects that they're all variations of
uv_run
. I think the API I proposed above does that, despite having other issues.
It occurs to me that we probably only need one of those, Luv.Loop.run_indefinitely
. The other modes are kind of "advanced." They would probably be only used for people working on an integration or doing something unusual, i.e. experts, and I think we can realistically rely on them looking at the function signature of run
carefully.
I expected a stand-alone sentence describing the return type and didn't consider that it would depend so much on the "mode". There's still a difference between the
int
returned byuv_run
and thebool
returned byLoop.run
though, and the relation between them isn't entirely obvious. I'm guessing0
here corresponds tofalse
, but often C APIs return error codes where0
means success, which might more naturally map totrue
.
Thanks, I added a further commit with a sentence about that.
The APIs I've interacted with so far, mostly just
Loop
andFS_event
, don't seem like objects though.
Indeed, FS_event
would look a bit different in an OCaml API. But it is an object in libuv, because it is an FS_event
handle, and has all the handle functions, etc. A higher-level wrapper of libuv that did filesystem watching would probably hide all that (it's been discussed :)).
Apart from consistency on the level of aesthetics or reading docs, there is a certain subtype relationship between libuv base handles and FS_event
that I'd like the basic Luv API to maintain. For example, among other things, active FS_event
handles can be found through loop querying functions. I suspect this kind of usage of a loop and an FS_event
handle would be extremely rare, and limited to experts only. Nonetheless I wouldn't want Luv itself to become a technical obstacle to it.
It occurs to me that we probably only need one of those, Luv.Loop.run_indefinitely. The other modes are kind of "advanced." They would probably be only used for people working on an integration or doing something unusual, i.e. experts, and I think we can realistically rely on them looking at the function signature of run carefully.
Yep, like me 😬 So maybe not 😉
I'm integrating with Revery, which has its own app loop and offers no control over it. I have to hook into it with what's essentially a setTimeout(..., 0)
and run the loop with `NOWAIT
.
Revery in turn (via another dependency) uses requestAnimationFrame
when compiled to JS, so I don't think it can be made to offer much more control over the loop either. I think this scenario might be relatively common as well.
A higher-level wrapper of libuv that did filesystem watching would probably hide all that (it's been discussed :)).
Cool! We need an API that does globbing as well, or essentially the ability to emulate the fswatch API. So if that aligns with those plans, I'm pretty sure we can offer some manpower to get it going relatively soon.
Apart from consistency on the level of aesthetics or reading docs, there is a certain subtype relationship between libuv base handles and FS_event that I'd like the basic Luv API to maintain. [...] I wouldn't want Luv itself to become a technical obstacle to it.
Yeah that makes complete sense. Thanks for explaining!
Related question that maybe could result in something being added to the docs: When you are using the default mode, and you get true
back, that corresponds to non-zero value from libuv, so that indicates that there are still active handles or requests
. Should this be treated as an error case by the caller, and other calls should be made first to close connections, or is this expected? Does it just indicate that at the time of calling Luv.Handle.close
, there were active connections, but they will all be closed cleanly?
Could you create a little program that creates a long-lived connection to some test server, then stops the default loop. Does this cause run
to return true
? If so, do you observe the connection getting closed, or does it linger on? I will add a note to the docs with your findings.
(Or merge a PR to the docs, as you choose! :) )
Luv.Loop.run
returns a boolean value whose meaning does not seem to be documented and, judging by the examples, in most cases is not used, but usually justignore
d.I would argue that using
ignore
without any type annotation is very error-prone because it sets the trap for accidental partial application. It should be considered bad style IMO, and therefore should be avoided particularly in examples. I also thinkLuv.Loop.run
exacerbates the likelihood of errors because its arguments are all optional except for the terminatingunit
. It's very easy to specify one or more optional arguments but forget the terminating unit. This leads to partial application, and with the use ofignore
will go entirely unnoticed except for the observation that nothing is happening, which is a symptom that doesn't narrow down the possible causes a whole lot.These issues are also in most cases preventable by providing a better API, for example:
I still don't know what the
bool
means of course, so this might still be way off. My assumption here is that it means there's more work to do, or something along those lines. But if it means something else this API probably doesn't make much sense.Furthermore, it's a stated goal for Luv to provide "more natural APIs":
But it's also stated that Luv is "a fairly thin binding", which to a significant extent opposes the above goal. And there's certainly also a documentation and familiarity benefit of having the API closely mirroring the original.
So there are two parts to this issue I think. One is the more general question of how much deviation from the C API is considered acceptable. Where the line is, or should be, drawn between providing "natural APIs" and being a thin binding. The other is the specific question concerning this particular API, its use cases, the meaning of the boolean and what follows as a natural API
To summarize:
bool
return type ofLuv.Loop.run
is not documented.ignore
.ignore
is error-prone and IMO bad style.PS. I'm just starting to get my feet wet with Luv, and so far it seems very solid and well-documented, although in need of some polish in places. I ask the more general question because this isn't the only part of the API that feels unnatural. The use of
init
everywhere, for example, seems unnecessary in most use cases. Anyway, it's certainly still very usable thanks to the great work you've done on documentation, and I want to emphasize how much I personally appreciate that little detail, as well as thank you for the library as a whole.