barrettotte / vscode-ibmi-languages

Syntax highlighting for IBM i languages such as RPG, CL, DDS, MI, and RPGLE fixed/free.
https://marketplace.visualstudio.com/items?itemName=barrettotte.ibmi-languages
MIT License
34 stars 13 forks source link

Invalid regex in grammar: lookbehind assertion is not fixed length #124

Closed lildude closed 1 year ago

lildude commented 1 year ago

👋 I'm the lead maintainer of the https://github.com/github/linguist library which is used for language detection and providing the syntax highlighting for languages on GitHub.com, and we use this grammar.

Our grammar compiler has found a problem with your grammar which I thought I'd let you know about.

This regex is invalid as the lookbehind assertion is not a fixed length (offset 24):

https://github.com/barrettotte/vscode-ibmi-languages/blob/7978d28bcb223246d58ea8fdfaf275a1760cb4db/syntaxes/pnlgrp.tmLanguage.json#L738

https://regex101.com/r/g5HFXE/1

This is the error our compiler reported:

Invalid regex in grammar: source.pnlgrp (in syntaxes/pnlgrp.tmLanguage.json) contains a malformed regex (regex "(?i)(?<=(:(MENUI|MI)[ ]))": lookbehind assertion is not fixed length (at offset 24))

barrettotte commented 1 year ago

Thanks for finding that, will fix sometime soon.

But, was there a change to the grammars.yml of the linguist project? I thought the linguist should only be pulling syntax for rpgle as in my original pull request

lildude commented 1 year ago

But, was there a change to the grammars.yml of the linguist project?

Yes, and now I look at your PR, I suspect you manually edited the file in your PR to have only single grammar instead of using the results from running the add-grammar script which would have added a reference to all of the grammars in your repo.

The very first release after your PR was merged, v7.19.0, is what then pulled in all the other grammars not included in your PR as you can see here.

For a bit of background, when a grammar is added to a repo with add-grammar, and whenever a new release is made, the grammar submodule is updated and all grammar files within the repo are then analyzed and compiled and added to the grammars.yml file, if not already there. This is to correctly handle support for variants of a language and refactoring where the grammar author has chosen to breakup their grammar into different grammars which they reference from the parent grammar. It's also to allow other grammars to use grammars from other authors without having to include them in their grammar.

This approach leads to better syntax highlighting overall, keeps behaviour as close to what users experience in their editors and tries to reduce duplication. For example, your rpgle.tmLanguage.json grammar benefits from this behaviour here where you include source.sql but you don't have that grammar in your repo. This highlighting works on GitHub.com because we include this grammar.

barrettotte commented 1 year ago

@chrjorgensen I apologize, I'm not familiar with PNLGRP and no longer have access to an IBM i box. Do you have a minimal example of what is mentioned here so I can use it to test/fix the invalid regex mentioned?

chrjorgensen commented 1 year ago

@barrettotte Don't apologize... Here is an example taken from the manual (I've changed one :MENUI tag to :MI for you):

uim-menu-manual-page-599.pnlgrp.txt

Btw, you can sign up for an account on the public and free IBM i at www.pub400.com. And you can find the manual used for the PNLGRP tags here: Application Display Programming. Part 3 of the manual describes the PNLGRP language, and appendix A lists all the tags.

I was not aware of the look-behind rule in regex, when I wrote the code - and VS Code does not complain and the code works as expected. The tag :MI is a shorthand for :MENUI, and I didn't want to repeat the code for :MENUI - but maybe that will be the solution?

Best regards, Christian

barrettotte commented 1 year ago

@lildude Apologize for the late fix on this, time flew by. According to regexr, I think I fixed the regex issue in commit https://github.com/barrettotte/vscode-ibmi-languages/commit/90728cd5acffee8974d62ba5a30bc3b934cb7f72.

This is part of latest tag v0.6.11

Thanks again for reporting this issue