Skip to content
This repository was archived by the owner on Aug 17, 2021. It is now read-only.

use ocaml-css-parser lib#80

Closed
tatchi wants to merge 2 commits intodylanirlbeck:masterfrom
tatchi:ocaml-css-parser
Closed

use ocaml-css-parser lib#80
tatchi wants to merge 2 commits intodylanirlbeck:masterfrom
tatchi:ocaml-css-parser

Conversation

@tatchi
Copy link
Copy Markdown
Collaborator

@tatchi tatchi commented May 7, 2020

Fixes #51

I added the ocaml-css-parser so it's not needed anymore to copy/paste the code.

Due to ocaml-css-parser constraints, I had to bump dune to >= 2.4. Meaning that I had also to bump the OCaml version.

I took the opportunity to bump some other deps as well.

For now there are 2 "issues":

  • One test is failing.
  • Errors are not formatted as well as before in the console. I think it's probably due to the Ocaml version which had to be upgraded.

Comment on lines +3 to +20
// Source: https://github.com/reasonml-community/graphql_ppx/blob/master/src/bucklescript_bin/Bin.re
let argv =
switch (Sys.argv |> Array.to_list) {
| [program, ...rest] =>
switch (List.rev(rest)) {
| [output_file, input_file, ...args] =>
List.concat([
[program],
List.rev(args),
[input_file, "-o", output_file, "--dump-ast"],
])
|> Array.of_list
| _ => Sys.argv
}
| _ => Sys.argv
};

let () = Driver.run_main(~argv, ()); No newline at end of file
Copy link
Copy Markdown
Collaborator Author

@tatchi tatchi May 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to use this otherwise there was an error when running on bs side

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. I think this is because the BuckleScript compiler is up-to-date with OCaml 4.06: https://bucklescript.github.io/docs/en/upgrade-to-v7#a-quick-look-under-the-hood.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is because you updated ocaml-migrate-parsetree. It has nothing to do with BuckleScript.

https://github.com/ocaml-ppx/ocaml-migrate-parsetree/blob/master/MANUAL.md#new-registration-interface

@tatchi tatchi force-pushed the ocaml-css-parser branch from c114e3e to 3034606 Compare May 7, 2020 18:01
Copy link
Copy Markdown
Owner

@dylanirlbeck dylanirlbeck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So after reading through some docs, and seeing your addition in bin/Bin.re, I'm wondering if it's even worth using the css-parser package at all, if we have to go through this hacky way of getting BuckleScript to work.

I think the fundamental difference between tailwind-ppx and graphql-ppx, at least right now, is that graphql-ppx is equally targeting BuckleScript and Reason native projects, so it makes sense to have a newer version of the OCaml compiler. On the other hand, I'd like to optimize the experience of tailwind-ppx for BuckleScript projects, since that's how most people are writing web apps (at least until an issue gets opened by a js_of_ocaml user).

Does that make sense? If anything, I think at this point it'd be better to colocate the parser code into it's own directory, instead of using the css-parser library.

Comment on lines +3 to +20
// Source: https://github.com/reasonml-community/graphql_ppx/blob/master/src/bucklescript_bin/Bin.re
let argv =
switch (Sys.argv |> Array.to_list) {
| [program, ...rest] =>
switch (List.rev(rest)) {
| [output_file, input_file, ...args] =>
List.concat([
[program],
List.rev(args),
[input_file, "-o", output_file, "--dump-ast"],
])
|> Array.of_list
| _ => Sys.argv
}
| _ => Sys.argv
};

let () = Driver.run_main(~argv, ()); No newline at end of file
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. I think this is because the BuckleScript compiler is up-to-date with OCaml 4.06: https://bucklescript.github.io/docs/en/upgrade-to-v7#a-quick-look-under-the-hood.

@tatchi
Copy link
Copy Markdown
Collaborator Author

tatchi commented May 11, 2020

Yes, I agree that we should make the bucklescript experience optimal. I tried several things but I wasn't able to keep ocaml 4.06. So let's close this for now :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use the css-parser OPAM package

3 participants