Lwt_fmt

[Drup]

This is a prelude to solving #310. This adds a new module Lwt_fmt which exposes some lwt-powered printf function in the style of Format. The functions return promises and are compatible with all the Format/Fmt combinators. The implementation is not complicated, but uses some fairly subtle Format tricks. I added some tests to ensure that the behavior of flushing and composition was correct.

For now, the module is in lwt_unix. I believe this should not be the case, but I expect @aantron to have better ideas on how and what to split where.

cc @Octachron @c-cube (@dbuenzli) as fellow Format fetishists.

[dbuenzli]

{!Format.printf}

[dbuenzli]

{!Format.eprintf}

[dbuenzli]

These toplevel module names could be {!links}.

[Octachron]

I think it would make sense to have (formatter -> 'a) rather than (Format.formatter -> unit Lwt.t -> 'a) for the continuation argument: with write_pending exposed, it is still possible to write fprintf from the formatter -> 'a version and it avoids exposing the underlying Format.formatter of the formatter argument.

[Drup]

Hum, that is right, but I'm pretty sure people will forget to call write_pending if you don't do it for them, which will lead to very weird bugs.

I'm trying to think if exposing the Format.formatter is a bad thing or not. It's not really unsafe, but I'm not sure if it's useful or not.

[Octachron]

In term of coherency of the API, I think that in any case it would be better to expose a formatter rather than Format.formatter otherwise kfprintf provides a strange entry point to features only available for Format.formatter (e.g. modifying margin, tags).

I agree that people will probably forget the call to write_pending, even if this could be mitigated by adding Do not forget to call {!write_pending} to the documentation and the limited amount of people using kprintf without reading the documentation.

[Drup]

Or maybe we can keep the unit Lwt.t and just expose a formatter.

Do you think I need to expose a function to retrieve the Format.formatter too ?

[Octachron]

Yep, I think that formatter -> unit Lwt.t -> 'a is a good compromise.

Do you think I need to expose a function to retrieve the Format.formatter too ?

I think that It would be nice to avoid it. I believe that letting make_formatter handle the configuration of the formatter is fine since safely modifying the underlying formatter is as hard as safely creating a second formatter with the same output channel. Nevertheless, in this case, it might be interesting to add the part of the Format API not exposed to kfprintf, i.e. the tabulation boxes and pp_print_if_newline.

[Drup]

Isn't that point sort of moot, given the existence of %t and %a? Lwt_fmt.fprintf fmt "%t" (fun fmt -> ....) will always work.

[Octachron]

True,%t and %a should be enough for tabulation boxes and friends.

[hakuch]

For what reason would I want the formatting functions to return promises? If I were just reading the documentation for this module, it would not be readily apparent to me that these functions would be blocking.

[Drup]

@hakuch The underlying channel on which the printing is done can be blocking. That is why the Lwt_io module is there.

[hakuch]

Thanks, @Drup. That makes sense!

[aantron]

This seems pretty nice. My only real concern is where it should go. I don't mind ultimately adding a new module to Lwt, but what are people's opinions on that, as opposed to:

• A separate repo,
• Somewhere inside Lwt_io,
• elsewhere?

[c-cube]

maybe add a () argument at the end, so future versions can add optional arguments?

[Drup]

Good idea.

[c-cube]

This looks cool. I think it should go in the core lwt library, provided it's only accessed through module aliases (and thus is of minimal overhead for people who don't use it).

[hcarty]

I like it as a top-level module slightly more than in Lwt_io. There are no extra requirements so it would be nice to keep it in the main Lwt repository.

[aantron]

It currently depends on Lwt_io.stdout and .stderr, so that makes it impossible to put it in the core library (in the exact form as it is now). However, in principle, it should be possible to put it in the core and still have some kind of stdout, etc. I believe stdout (EDIT: from Pervasives) is interpreted by both jsoo and BuckleScript.

[Drup]

I strongly think this should be in Lwt itself, and I'm not going to bother creating a package for that alone. I think leftpad-ifying the OCaml ecosystem by creating lot's of 100-lines packages that would be perfectly at home in other packages is a complete waste of time and is very user hostile.
Also, putting it inside lwt means that people might actually use it, which increases the chance we can one day deprecate all the bad functions in Lwt_io.

As for Lwt_io, the problem is that it already has *printf functions, and wrong ones even. We can't really add new formatting functions to it without a lot of name contortions.

Currently, the functions of Lwt_fmt that depends on Lwt_io are of_channel, stdout, stderr, printf, eprintf.
We could split those out, but I'm not sure what's the best way to organize it.

[aantron]

Very well. Perhaps this is already proposed, but I suggest we merge this as is, putting it into lwt.unix. We can move it to the core lwt if/when Lwt_io is untangled. That change should be backwards-compatible for any users (if we have a release): anyone who uses Lwt_fmt will have to depend on lwt.unix, which depends on lwt, so either way people will be pulling in a package with the Lwt_fmt module.

[Drup]

@aantron That seems like a good direction for me. Let me fix what I discussed with @Octachron above and we will be good to go! :)

[Drup]

I just added a new commit that make the API a bit more regular, as discussed with @Octachron . If CI is happy, this is ready to be merged as far as I'm concerned.

[aantron]

Ok, thank you!

[rgrinberg]

Something that's unrelated but still important:

Is there a plan for Lwt to move away from having multiple top level modules per library? It would be really nice if Lwt switched to using (wrapped true) or made all modules apart from Lwt internal (the way dbuenzli does it).

The reason why I'm raising this here is that this PR adds yet another toplevel module which is a bit unfortunate.

[Drup]

@rgrinberg lwt has multiple distinct libraries. If you were to wrap everything, Lwt_fmt would become Lwt_unix.Fmt, not Lwt.Fmt. In the end, it would be strictly more inconsistent than the current situation, and I don't see which gain you would have (all the modules are already prefixed).

[rgrinberg]

If you were to wrap everything, Lwt_fmt would become Lwt_unix.Fmt, not Lwt.Fmt

To me this is a good thing. Yes, this can look a little less aesthetically pleasing, but it streamlines understanding new code for people as you no longer have to think about which library introduces a particular module.

[aantron]

I want to reorganize the module structure of Lwt eventually, but I have no concrete plan to do it at the moment.

I think putting modules now in lwt.unix under Lwt. can be done with module aliases, but, we may have redesigned the Unix binding by then anyway (#328).