search

michaelficarra / CoffeeScriptRedux

:sweat: rewrite of the CoffeeScript compiler with proper compiler design principles and a focus on robustness and extensibility
https://michaelficarra.github.com/CoffeeScriptRedux/
BSD 3-Clause "New" or "Revised" License
1.84k stars 110 forks source link

Respect syntax error offset location #343

Open ef4 opened 9 years ago

ef4 commented 9 years ago

This makes the syntax error formatter respect the error's offset property. Here is a file demonstrating the difference:

class A

class B
  method: ->
    console.log('a', 'b', 
      'c', 'd'
    )

Before this change, the error is reported at line 1, column 1. With this change, it is reported correctly on line 6.

michaelficarra commented 9 years ago

I don't understand why the line number was incorrect before. Is there not a simpler way we can fix this? Obviously there's a bug somewhere if it's reporting an error on line 1 for that program.

ef4 commented 9 years ago

I was interpreting the error as "while disambiguating a structure that starts at line 1, col 1, I hit an error offset bytes ahead of that point.

I haven't looked into the grammar to know if that's really how it's working, but it seems a reasonable guess.

lydell commented 9 years ago

It might be a bug in pegjs. As far as I know the offset and line/column are supposed to point to the same place.

ef4 commented 9 years ago

Yeah, if that's the intent of offset, line, and column then we probably do have a bug in pegjs. Skimming their repo just now it does look like it's supposed to work that way.

michaelficarra commented 9 years ago

We have to remember that we're doing quite a few hacky things to try to get context sensitivity. It could have something to do with either our preprocessor-inserted context markers or the pegcoffee plugin/variant of pegjs we're using.