modelica-tools / atom-language-modelica

Modelica language support for the atom editor
https://atom.io/packages/language-modelica
Other
8 stars 4 forks source link

replace Raphael's grammar with Boris' grammar #13

Closed thorade closed 9 years ago

thorade commented 9 years ago

Opening this Pull Request so we can have discussion about it. Please merge only once discussion has finished.

Boris' grammar seems to be working more reliable, so I think we should use it as a starting point. Someone who understands these grammars should then add everything back that was better in Raphael's grammar.

For comparison, we can use Lightshow and some large packages from the MSL:

thorade commented 9 years ago

Boris' grammar is also not completly error free, so if we decide to merge this PR then we would have to fix the same issues that are present in Boris' issue tracker: https://github.com/BorisChumichev/modelicaSublimeTextPackage/issues

tshort commented 9 years ago

I'm good with merging this. I tried several files. I found one case where @BorisChumichev's grammar is wrong:

http://bit.ly/1PuWY19

Annotations are tricky to get right. Some options for fixing this one are:

  1. Make the regex look for a trailing ); instead of just ;.
  2. Have the grammar look for nested Icon( -- ). I think that's how Boris avoids tagging semicolons in the HTML section.
thorade commented 9 years ago

Is that fix sufficient? Does it work if the annotation ends with ) ; (note the space between parantheses and semicolon)?

tshort commented 9 years ago

Here's what was in the original grammar:

'\\)\\s*;\\s*$'
thorade commented 9 years ago

OK, that line is reverted to Raphael's version. http://bit.ly/1LvqD8O

thorade commented 9 years ago

@lochel if it looks good to you also, feel free to press the Merge button.

thorade commented 9 years ago

Here is another case where Raphael's grammar works better: http://bit.ly/1GeI3G1 versus http://bit.ly/1RJZD6j but I don't know how to fix it. Probably enumeration should appear as keyword somewhere in the grammar file!?

tshort commented 9 years ago

I think this can be merged. The issues with Boris' grammar can be added separately.

thorade commented 9 years ago

Merged the PR and published a new minor version. I will close some of the issues and open new ones this weekend.