Add command line argument for generating reason#150
Add command line argument for generating reason#150aantron merged 9 commits intocamlworks:masterfrom
Conversation
Also add a reason version of the `w-template-files` example that uses this new argument
aantron
left a comment
aantron
left a comment
There was a problem hiding this comment.
Thanks!
The majority of the comments I left are either nits or have to do with the example text. Feel free to ignore those, as I often touch up example text after examples are merged anyway (whether written by me or others). The important comments are under src/eml/.
src/eml/main.ml
Outdated
| module Command_line : | ||
| sig | ||
| val parse : unit -> (string * string) list | ||
| val parse : unit -> (string * string * bool) list |
There was a problem hiding this comment.
With the way the code is currently written, there is a global syntax flag for all the files, so the result type should be
(string * string) list * boolHowever, I think it would be even better to move the code from process_file in eml.ml, which checks the extension, into the command-line parser, and decide here what syntax each file will use. Then, the result type should be either what this PR has now, or, even more helpfully
(string * string * [ `OCaml | `Reason ]) listMove logic that decides whether to use Reason into the cli, and pass this information as a variant.
|
Thank you for the exhaustive feedback, I completely forgot to update the new |
|
Please don't resolve any conversations startred by the reviewer — it's for the reviewer to keep track of those conversations, and whether whatever issues are in them have been addressed. Otherwise, it can get hard for the reviewer to keep track of what has actually been "resolved" and what hasn't. In general, as a first approximation, the person who started the conversation decides if/when it is resolved. |
|
Many thanks! |
2 participants
Closes #95