Conversation
|
Copy-pasting comment from @osener
refmt --parse=ml --print=re source.ml | refmt --parse=re --print=ml | ocamlformat |
|
@jberdine, do you want to merge this based on the comment above ? |
|
I'm wary of the reason support being something like false advertising. Basically I don't currently have a use for the Reason conversion, and don't use it, and don't have time to fix the bugs that must be in it. On the other hand, it might be more approachable to someone who wants to get it working to leave the code there. Maybe it would be enough to be more explicit that it is there, but untested and largely unsupported? |
|
(Replying to #289 (comment) but keeping the discussion here.) It is not so much work to keep the existing code in the repo, and to keep it compiling. It would be a lot of work to test it. I think it would also likely take some work to keep up to date with changes to refmt, which is a moving target, and ocamlformat's Reason to OCaml conversion support relies tightly on one of refmt's internal representations. (The way ocamlformat_reason works is to pipe a binary serialized internal representation from refmt to ocamlformat.) The last I used ocamlformat_reason, it handled comments and things like explicit_arity reasonably well, much better than outputting ocaml from refmt (which uses Pprintast from ocaml's compiler-libs) and then reformatting that ocaml. But ok, it seems that the best plan is to not remove this code for now. |
|
I'm willing to help at least with the testing side of it. I think it's better for both Reason and OCaml to have a reasonably clean two way conversion process. Having Reason -> OCaml provides an escape hatch for OCaml users who experiment with Reason (lowering the risk involved in trying out Reason's syntax). @jordwalke who may be interested in chiming in and/or may know someone who could help with maintenance. |
|
@hcarty it would be great to have some help testing! Even just if someone pays attention to whether it gets broken would be helpful. I agree about lowering the risk to try Reason, I just can't dedicate the time to it. |
|
We are thinking again about removing the Reason code. |
As discussed in ocaml-ppx#254, keeping this functionality around is not very useful: - it has never been released on opam - it has no known users - `refmt` can already emit ocaml code There are limitations with `refmt`, but workarounds should be implemented there, not as an unrelated tool in `ocamlformat`.
As discussed in ocaml-ppx#254, keeping this functionality around is not very useful: - it has never been released on opam - it has no known users - `refmt` can already emit ocaml code There are limitations with `refmt`, but workarounds should be implemented there, not as an unrelated tool in `ocamlformat`.
As discussed in ocaml-ppx#254, keeping this functionality around is not very useful: - it has never been released on opam - it has no known users - `refmt` can already emit ocaml code There are limitations with `refmt`, but workarounds should be implemented there, not as an unrelated tool in `ocamlformat`.
As discussed in ocaml-ppx#254, keeping this functionality around is not very useful: - it has never been released on opam - it has no known users - `refmt` can already emit ocaml code There are limitations with `refmt`, but workarounds should be implemented there, not as an unrelated tool in `ocamlformat`.
As discussed in #254, keeping this functionality around is not very useful: - it has never been released on opam - it has no known users - `refmt` can already emit ocaml code There are limitations with `refmt`, but workarounds should be implemented there, not as an unrelated tool in `ocamlformat`.
CHANGES: #### New features + Add an option `--margin-check` to emit a warning if the formatted output exceeds the margin (ocaml-ppx/ocamlformat#1110) (Guillaume Petiot) + Preserve comment indentation when `wrap-comments` is unset (ocaml-ppx/ocamlformat#1138, ocaml-ppx/ocamlformat#1159) (Jules Aguillon) + Improve error messages (ocaml-ppx/ocamlformat#1147) (Jules Aguillon) + Display standard output in the emacs plugin even when ocamlformat does not fail (ocaml-ppx/ocamlformat#1189) (Guillaume Petiot) #### Removed + Remove `ocamlformat_reason` (ocaml-ppx/ocamlformat#254, ocaml-ppx/ocamlformat#1185) (Etienne Millon). This tool has never been released to opam, has no known users, and overlaps with what `refmt` can do. + Remove `ocamlformat-diff` (ocaml-ppx/ocamlformat#1205) (Guillaume Petiot) This tool has never been released to opam, has no known users, and overlaps with what `merge-fmt` can do. #### Packaging + Work with base v0.13.0 (ocaml-ppx/ocamlformat#1163) (Jules Aguillon) #### Bug fixes + Fix placement of comments just before a '|' (ocaml-ppx/ocamlformat#1203) (Jules Aguillon) + Fix build version detection when building in the absence of a git root (ocaml-ppx/ocamlformat#1198) (Anil Madhavapeddy) + Fix wrapping of or-patterns in presence of comments with `break-cases=fit` (ocaml-ppx/ocamlformat#1167) (Jules Aguillon) This also fixes an unstable comment bug in or-patterns + Fix an unstable comment bug in variant declarations (ocaml-ppx/ocamlformat#1108) (Jules Aguillon) + Fix: break multiline comments (ocaml-ppx/ocamlformat#1122) (Guillaume Petiot) + Fix: types on named arguments were wrapped incorrectly when preceding comments (ocaml-ppx/ocamlformat#1124) (Guillaume Petiot) + Fix the indentation produced by max-indent (ocaml-ppx/ocamlformat#1118) (Guillaume Petiot) + Fix break after Psig_include depending on presence of docstring (ocaml-ppx/ocamlformat#1125) (Guillaume Petiot) + Remove some calls to if_newline and break_unless_newline and fix break before closing brackets (ocaml-ppx/ocamlformat#1168) (Guillaume Petiot) + Fix unstable cmt in or-pattern (ocaml-ppx/ocamlformat#1173) (Guillaume Petiot) + Fix location of comment attached to the underscore of an open record (ocaml-ppx/ocamlformat#1208) (Guillaume Petiot) + Fix parentheses around optional module parameter (ocaml-ppx/ocamlformat#1212) (Christian Barcenas) + Fix grouping of horizontally aligned comments (ocaml-ppx/ocamlformat#1209) (Guillaume Petiot) + Fix dropped comments around module pack expressions (ocaml-ppx/ocamlformat#1214) (Jules Aguillon) + Fix regression of comment position in list patterns (ocaml-ppx/ocamlformat#1141) (Josh Berdine) + Fix: adjust definition of Location.is_single_line to reflect margin (ocaml-ppx/ocamlformat#1102) (Josh Berdine) #### Documentation + Fix documentation of option `version-check` (ocaml-ppx/ocamlformat#1135) (Wilfred Hughes) + Fix hint when using `break-separators=after-and-docked` (ocaml-ppx/ocamlformat#1130) (Greta Yorsh)
As discussed in ocaml-ppx#254, keeping this functionality around is not very useful: - it has never been released on opam - it has no known users - `refmt` can already emit ocaml code There are limitations with `refmt`, but workarounds should be implemented there, not as an unrelated tool in `ocamlformat`.
This PR is a request for comments from potential users of the Reason conversion support. The support for converting Reason to OCaml has not been actively used or maintained for a year. Much has changed in that time, not to mention that ocamlformat depends on an old version of reason. Unless someone steps forward to actively maintain this functionality, it makes more sense to remove it for the time being.