Skip to content

Basic Reason support#445

Closed
vasilisp wants to merge 5 commits into
masterfrom
reason
Closed

Basic Reason support#445
vasilisp wants to merge 5 commits into
masterfrom
reason

Conversation

@vasilisp

Copy link
Copy Markdown
Contributor

This PR provides very rough and mostly untested Reason support.

I added a -reason flag to our compiler wrappers. It is implemented by calling refmt as a preprocessor.

There is a new basic.reason template that uses Reason syntax.

@vasilisp

vasilisp commented Dec 12, 2016

Copy link
Copy Markdown
Contributor Author

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.

Comment thread src/tools/utils.ml Outdated
let preprocess_opt ?(ocaml = false) ?kind opts =
let refmt () =
if !use_refmt then
[ "-pp" ; "refmt -parse re -print ml" ]

Copy link
Copy Markdown
Member

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 ?

@vasilisp vasilisp Dec 13, 2016

Copy link
Copy Markdown
Contributor Author

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.

@Drup

Drup commented Dec 13, 2016

Copy link
Copy Markdown
Member

Nice work!

@vasilisp

Copy link
Copy Markdown
Contributor Author

@Drup, does e706f1d look line a sane approach?

@vasilisp

Copy link
Copy Markdown
Contributor Author

See #504.

@vasilisp vasilisp closed this Jun 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants