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

Elm syntax does not compile on master anymore #211

Closed
Keats opened this issue Oct 2, 2018 · 8 comments
Closed

Elm syntax does not compile on master anymore #211

Keats opened this issue Oct 2, 2018 · 8 comments

Comments

@Keats
Copy link
Contributor

Keats commented Oct 2, 2018

The syntaxes are located at https://github.com/elm-community/SublimeElmLanguageSupport/tree/master/Syntaxes . I added the repo to the packages folder of master for trying it quickly.

Error message while running make packs in syntect master

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: ParseSyntax(RegexCompileError("$+(?!^ {4})", Error(-114, target of repeat operator is invalid)), Some("testdata/Packages/SublimeElmLanguageSupport/Syntaxes/Elm Compile Messages.sublime-syntax"))', libcore/result.rs:1009:5

Offending line: https://github.com/elm-community/SublimeElmLanguageSupport/blob/master/Syntaxes/Elm%20Compile%20Messages.sublime-syntax#L67

@keith-hall
Copy link
Collaborator

I wonder if this is related to the regex rewriting for no newlines mode, as it seems to be converting the \n to $, and as it is followed by a +, it's seen as invalid by Oniguruma.

@keith-hall
Copy link
Collaborator

Of course, as Sublime Text syntax grammars only operate on one line at a time, that regex pattern is fairly nonsense anyway and maybe should be fixed in the Elm repo?

@robinst
Copy link
Collaborator

robinst commented Oct 3, 2018

Yeah looks like nonewlines mode. We could tweak the \n to $ rewriting some more, but fixing the grammar would also be good.

Looks like the line to tweak for the rewriting is here: https://github.com/trishume/syntect/blob/master/src/parsing/yaml_load.rs#L561

@Keats
Copy link
Contributor Author

Keats commented Oct 3, 2018

So the line in the grammar should be match: '(?!^ {4})' ?

@robinst
Copy link
Collaborator

robinst commented Oct 4, 2018

@Keats Yeah that would make more sense I think.

robinst added a commit that referenced this issue Oct 4, 2018
Using \n+ in a regex doesn't make any sense for Sublime Text syntaxes,
but let's not produce an invalid regex in that case.
@robinst
Copy link
Collaborator

robinst commented Oct 4, 2018

I raised #212 to tweak the regex rewriting in syntect.

trishume added a commit that referenced this issue Oct 5, 2018
@trishume
Copy link
Owner

trishume commented Oct 5, 2018

Can someone confirm the syntax compiles now that the fix is merged?

@Keats
Copy link
Contributor Author

Keats commented Oct 5, 2018

It does work

@trishume trishume closed this as completed Oct 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants