add Pp.verbatimf

[mbarbin]

Follow the same naming scheme as existing functions textf and paragraphf.

This function is meant to allow simplifying occurrences of the pattern Pp.verbatim (Printf.sprintf ...).

As a motivating example, you may refer to this file (although it is taken from a toy project).

Thank you for pp!

[Alizter]

Thanks for the PR! Looks like I got the type of Pp.paragraphf a little off and I was missing a changelog so I've gone ahead and added those in #19.

There I also added a format_string internal type which should allow us to enforce the correct type internally somewhat. Depending on which gets merged first we should do this for Pp.verbatimf also.

This seems like a sensible addition to me.

[mbarbin]

Hi @Alizter,

Thank you for looking into it ! 😄

There I also added a format_string internal type which should allow us to enforce the correct type internally somewhat.

It's been a while since I looked at these highly polymorphic format parametrized types... 🧐

My impression is that the added format_string type annotation doesn't add new information. I'm basing this hunch on the evidence that when I highlight the parameter fmt from vscode using Merlin in a version of the tree where it is not annotated with your added format_string internal type, it still has the same type as format_string (('a, unit, string, 'tag t) format4). 🤔

Do you believe this type is not inferred from the fact that Printf.ksprintf is applied to a function that returns an 'a t?

[Alizter]

@mbarbin It will enforce the type of the format4 argument but not the return type. Previously my implementation of paragraphf was incorrect so annotating the arg can be an extra internal check. It should be inferred from Printf.ksprintf and your implementation is totally correct. It's just an extra safety measure.

[mbarbin]

It will enforce the type of the format4 argument but not the return type.

In fact, I don't think this is this the case. The returned type of the full invocation is the last parameter of the format4, and it is correctly inferred as being 'a t.

I just looked again here.

I think the added annotation doesn't add any safety in that it only coerces the parameter with the same type as already inferred.

[Alizter]

@mbarbin Yes, you're totally correct. I think what is needed is to add that type to the mli, then it would be able to enforce the correct type, but that has the side effect of polluting the mli which isn't desirable.

[mbarbin]

Previously my implementation of paragraphf was incorrect so annotating the arg can be an extra internal check.

Ah, I think I now understand what you mean. OK, I believe having the correct type in the mli is enough to get a compile error on the previous implementation, so it should be all you need (and not need the added annotation in the ml).

[Alizter]

I think a better way to enforce the correct type is just to add some simple tests like you've done.

[mbarbin]

I think a better way to enforce the correct type is just to add some simple tests like you've done.

I agree yes. It's too easy to get it wrong otherwise!

Thanks for the follow through and for #19