Ensure github-linguist compatibility for the syntaxes #1659
Conversation
huwaireb
changed the title
|
Could you take a look at this? @mnxn |
|
@mnxn, there is a slight (non-blocking) issue with the linguist PR, I'd love to hear what you think on. github-linguist/linguist#7126 (comment) could we have a Edit: See this comment to how we can do it, it'll be zero maintenance. |
huwaireb
force-pushed
the
linguist-compat
branch
from
f97f4cc to
675eae7
Compare
huwaireb
force-pushed
the
linguist-compat
branch
from
675eae7 to
1c979bf
Compare
| @@ -279,7 +279,7 @@ | |||
| }, | |||
| { | |||
| "comment": "destructured semantic value capture", | |||
| "begin": "(?<![[:word:]][[:space:]]*)\\(", | |||
| "begin": "(?<!\\w)\\(", | |||
There was a problem hiding this comment.
Not sure if this is a correct fix.
It resolves
- Invalid regex in grammar: `source.ocaml.menhir` (in `syntaxes/menhir.json`)
contains a malformed regex (regex "`(?<![[:word:]][[:space:]]*)\(`":
lookbehind assertion is not fixed length (at offset 26))
caused by [[:space:]]*.
There was a problem hiding this comment.
Previously, the regex would not match either a(b, c) = x or a (b, c) = x. But now the second string matches.
I don't know what the ideal solution is here, but at the very least, both of the strings above should not match.
Also, the regex should stick with [[:word:]] instead of \\w.
There was a problem hiding this comment.
It seems like it accepts this (?<![[:word:]])(?<![[:space:]])\\(, can we do this instead? Rubular parses it fine, i can't seem to get it to make the vsix to try it out.
There was a problem hiding this comment.
hi @mnxn, sorry for the ping. but is there any way we can sort this out? This is the only remaining blocker afaik
There was a problem hiding this comment.
It doesn't look like that regex works. I think it might be fundamentally impossible to achieve the same behavior while staying fixed length.
I'm okay with relaxing the constraint a little bit and recognizing only 0 or 1 spaces like the examples I posted above. This regex should accomplish that: (?<![[:word:]]|[[:word:]][[:space:]])\(
BTW: you can try the extension by opening the repo in VS Code and pressing the run button. No need to make a .vsix or even build the JS if you're just testing the syntaxes.
| @@ -279,7 +279,7 @@ | |||
| }, | |||
| { | |||
| "comment": "destructured semantic value capture", | |||
| "begin": "(?<![[:word:]][[:space:]]*)\\(", | |||
| "begin": "(?<!\\w)\\(", | |||
There was a problem hiding this comment.
Previously, the regex would not match either a(b, c) = x or a (b, c) = x. But now the second string matches.
I don't know what the ideal solution is here, but at the very least, both of the strings above should not match.
Also, the regex should stick with [[:word:]] instead of \\w.
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request.
Files not reviewed (9)
- package.json: Language not supported
- syntaxes/atd.json: Language not supported
- syntaxes/cram.json: Language not supported
- syntaxes/dune-all.json: Language not supported
- syntaxes/dune.json: Language not supported
- syntaxes/menhir.json: Language not supported
- syntaxes/ocaml.json: Language not supported
- syntaxes/ocamlbuild.json: Language not supported
- syntaxes/ocamllex.json: Language not supported
blocking github-linguist/linguist#7126
p.s ensured formatting using biome.