syntect icon indicating copy to clipboard operation
syntect copied to clipboard

Don't Panic!

Open Canop opened this issue 11 months ago • 2 comments

(in large friendly letters)

Syntect can panic when highlighting on a syntax file containing a regular expression with a feature not supported by the regex engine.

This isn't theoretical and leads to panics in programs using syntect:

  • https://github.com/Canop/broot/issues/967#issuecomment-2566925354
  • https://github.com/sharkdp/bat/issues/3156

With this fork, the behavior on a syntax having an invalid rule isn't to panic anymore, and is tuned with options:

let options = HighlightOptions {
    ignore_errors: true,
};
HighlightLines::new(syntax, theme, options)

With ignore_errors: false, a faulty regex in a syntax, detected when highlighting some lines, will result in an error.

With ignore_errors: true, the rule containing the faulty regex won't be applied but rest of the highlighting process will proceed.

So it never panics but the calling program can opt for the old behavior by specifying ignore_errors: false and panicking on a returned error.

Canop avatar Jan 03 '25 07:01 Canop

Thanks for taking the time to create a PR. For the record I'm in favor of preventing panics. Any chance you can split this into smaller self-contained PRs for easier digestion and review? The smaller the better.

Enselic avatar Jan 03 '25 08:01 Enselic

I don't think it can really get smaller. Changes are just consequences of regex::is_match and regex::search returning a Result rather than just a bool.

But if it's welcome (I'm also waiting for the bat people to check), I can make a clean PR without the changes in Cargo.toml.

Canop avatar Jan 03 '25 08:01 Canop