-
Notifications
You must be signed in to change notification settings - Fork 56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Basic Reason support #445
Basic Reason support #445
Conversation
There is a problem (reported by @dannywillems) with type-checking client values, and probably injections. It is because the new preprocessing pipeline introduces unpredictable intermediate filenames, impacting our auto-generated names for client fragments. To be fixed tomorrow. |
src/tools/utils.ml
Outdated
@@ -362,6 +364,12 @@ let get_ppopts ~impl_intf file = | |||
type_opt impl_intf file @ !ppopt | |||
|
|||
let preprocess_opt ?(ocaml = false) ?kind opts = | |||
let refmt () = | |||
if !use_refmt then | |||
[ "-pp" ; "refmt -parse re -print ml" ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we try to provide a clean failure when reason is not installed ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably. We just need to look for refmt
(the only thing we really need) at the time -reason
is provided.
Edit: that turns out to be quite complicated, and the compiler itself prints a reasonable message about refmt
failing. So, not a priority right now.
Nice work! |
When using refmt, we end up with intermediate filenames that break our identifier generation.
See #504. |
This PR provides very rough and mostly untested Reason support.
I added a
-reason
flag to our compiler wrappers. It is implemented by callingrefmt
as a preprocessor.There is a new
basic.reason
template that uses Reason syntax.