add comparison function for Pp.Ast.t #9
Conversation
src/pp.ml
Outdated
| let compare_triple f g h (x, y, z) (a, b, c) = | ||
| compare_both f (compare_both g h) (x, (y, z)) (a, (b, c)) | ||
|
|
||
| let compare compare_tag = |
There was a problem hiding this comment.
You can define it directly on Pp.t
There was a problem hiding this comment.
I can't compare Format so we can just use this together with to_ast for Pp.
There was a problem hiding this comment.
No. that's unnecessarily slow. Just raise if you get Format. We should just get rid of that constructor
There was a problem hiding this comment.
I'll remove Format in another PR. I've made Pp.compare which will raise Invalid_argument if two Pp.of_fmt values are provided.
There was a problem hiding this comment.
To clarify, I made a mistake not listening to Jeremie here: #1
What I think we can do is introduce a Pp.Fmt submodule that allows to inject this Format constructor. The core Pp.t should stay serializable, comparable, and all that good stuff.
There was a problem hiding this comment.
That seems sensible, but it is beyond the scope of what I am willing to do at the moment. We should definitely take another look at that in the future.
There was a problem hiding this comment.
Ok, could you add a CHANGES entry?
Alizter
force-pushed
the
ps/branch/add_comparison_function_for_pp_ast_t
branch
2 times, most recently
from
f01591d
to
438153f
Compare
Signed-off-by: Ali Caglayan <[email protected]>
Alizter
force-pushed
the
ps/branch/add_comparison_function_for_pp_ast_t
branch
from
438153f
to
fee774d
Compare
|
@rgrinberg Added a CHANGES entry. (Before I also added a test). |
CHANGES: - Add `Pp.compare` (ocaml-dune/pp#9, @Alizter)
CHANGES: - Add `Pp.compare` (ocaml-dune/pp#9, @Alizter)
No description provided.