Skip to content
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

[BUG] Cannot parse file (trailing comma) #989

Closed
giltho opened this issue Apr 13, 2020 · 4 comments · Fixed by #1391
Closed

[BUG] Cannot parse file (trailing comma) #989

giltho opened this issue Apr 13, 2020 · 4 comments · Fixed by #1391

Comments

@giltho
Copy link

giltho commented Apr 13, 2020

Describe the bug
I'm getting an error when compiling a projet :

Error: cannot parse file "path/to/harfbuzz.js"
(orig:"path/to/harfbuzz.js" from l:3, c:0)

this file contains

// Provides: rehb_new_face
function rehb_new_face(
  _fontName /*: string */,
) {
  return undefined;
}

// Provides: rehb_shape
// Requires: caml_to_js_string
function rehb_shape(_face /*: fk_face */, text /*: string */) {
  var str = caml_to_js_string(text);
  var ret = str.split("").map(function mapper(_char) {
      return [/* <jsoo_empty> */ 0, /* glyphId */ 0, /* cluster */ 0];
    });

  // Adding the leading `0` to make it a jsoo compatible array
  ret.unshift(0);
  return ret;
}

I'm thinking this might already have been fixed by #980 , so I tried to pin the latest commit (203dc9c), but I get a new error. I cannot even build js_of_ocaml-compiler anymore, and the dependency build fails with

running: 'dune' 'build' '-p' 'js_of_ocaml-compiler' '-j' '4'
           ocaml (internal)
    ocamlfind: Package `graphics' not found
           ocaml (internal) (exit 2)

Versions
ocaml: 4.08.1
js_of_ocaml: 3.5.2

@giltho giltho added the bug label Apr 13, 2020
@hhugo
Copy link
Member

hhugo commented Apr 13, 2020

Well, it seems there is indeed a syntax error. Why do you have trailing comma _fontName /*: string */, ?

@giltho
Copy link
Author

giltho commented Apr 13, 2020

The file comes from here
https://github.com/revery-ui/reason-harfbuzz/blob/master/src/harfbuzz.js

If I was to guess, this looks like the output of a JS formatter like prettier. Trailing commas for multi-line function parameters are useful so you don't have to modify the previous line when adding a new parameter, and it's correct syntax in ES2017.

I don't really know what's the policy of js_of_ocaml here

@giltho
Copy link
Author

giltho commented Apr 13, 2020

I didn't realise it was because of the trailing comma though, I created a PR for reason-harfbuzz to remove the trailing comma

@hhugo
Copy link
Member

hhugo commented Apr 13, 2020

The JavaScript parser inside js_of_ocaml didn't keep up with recent version of ecmascript. One could update the parser. PR welcome.

@hhugo hhugo mentioned this issue Jan 11, 2022
@hhugo hhugo changed the title [BUG] Cannot parse file [BUG] Cannot parse file (trailing comma) Jan 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants