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

Regex matching panic #508

Open
crowlKats opened this issue Dec 22, 2023 · 4 comments
Open

Regex matching panic #508

crowlKats opened this issue Dec 22, 2023 · 4 comments

Comments

@crowlKats
Copy link

crowlKats commented Dec 22, 2023

I am getting a panic on https://github.com/denoland/deno_doc/tree/simplify-syntect (can be reproduced with cargo run --example ddoc --features=html -- --name "" --html ./panic.ts --output gen)

thread 'main' panicked at /Users/crowlkats/.cargo/registry/src/index.crates.io-6f17d22bba15001f/syntect-5.1.0/src/parsing/regex.rs:69:53:
regex string should be pre-tested: CompileError(LookBehindNotConst)

(full panic here).

I tried making a small reproducible example with comrak, however that does not panic, even though the comrak & syntect logic used is identical between it and deno_doc. tried with syntect directly, and still no panic, so unsure what is happening.

I debugged a bit, and for some reason with the deno_doc build, the using keyword does not seem to work, but not an issue with the smaller reproduction. I checked, and the typescript syntax does support the keyword.

I am using the typescript syntax as described in #97 (comment).

@keith-hall
Copy link
Collaborator

tried with syntect directly, and still no panic, so unsure what is happening.

can you clarify this, please? using syntect's default features but with the TypeScriptReact syntax you linked to?

presumably the LookBehindNotConst error is due to this pattern https://github.com/microsoft/TypeScript-TmLanguage/blob/ce43472727bf0fbfe8f08c71c18d4f1f869f87e8/TypeScriptReact.YAML-tmLanguage#L26C77-L26C92

does it make a difference if you use Oniguruma with the ONIG_SYN_DIFFERENT_LEN_ALT_LOOK_BEHIND flag set, instead of fancy regex? https://github.com/denoland/deno_doc/blob/simplify-syntect/Cargo.toml#L43C160-L43C160 (syntect's default features use Oniguruma, not sure about the state of that flag by default though.)

@crowlKats
Copy link
Author

crowlKats commented Dec 23, 2023

can you clarify this, please? using syntect's default features but with the TypeScriptReact syntax you linked to?

my bad, yes, that is what I meant.

does it make a difference if you use Oniguruma with the ONIG_SYN_DIFFERENT_LEN_ALT_LOOK_BEHIND flag set, instead of fancy regex? https://github.com/denoland/deno_doc/blob/simplify-syntect/Cargo.toml#L43C160-L43C160 (syntect's default features use Oniguruma, not sure about the state of that flag by default though.)

Tried (without needing to set the onig flag manually), and seems to work fine, so we'll be using that instead for now, though we don't really want to depend on yet another regex library.

presumably the LookBehindNotConst error is due to this pattern https://github.com/microsoft/TypeScript-TmLanguage/blob/ce43472727bf0fbfe8f08c71c18d4f1f869f87e8/TypeScriptReact.YAML-tmLanguage#L26C77-L26C92

Hm it is still odd that with the small reproducible example this doesn't to not panic, but with the deno_doc project it does, so seems like something weird must happening somewhere

@crowlKats
Copy link
Author

@keith-hall onig has been working for us, however we can't depend on it for longer since depending on onig is problematic for us, and as such best solution for us would be if we could use fancy, but this panic is blocking us from it

@keith-hall
Copy link
Collaborator

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

2 participants