trishume / syntect

Rust library for syntax highlighting using Sublime Text syntax definitions.
https://docs.rs/syntect
MIT License
1.92k stars 138 forks source link

fancy-regex: problems with patterns in some syntaxes #287

Open sharkdp opened 4 years ago

sharkdp commented 4 years ago

I just tried the fancy-regex version of syntect for the first time (thank you for everyone involved!).

Here are a few of the problems I have encountered so far:

This list continues for quite some time, but I'm not sure if it's worth to list them all. Most of them seem to be related to "unknown group flag" or "invalid escape".

Note: I just wanted to try this out within bat, there is absolutely no "pressure" to get this fixed (as always, of course :smile:). I was just curious and thought this might help.

trishume commented 4 years ago

cc @robinst any thoughts on what might needed to be added to either the rewriter or fancy-regex to fix these and guesses as to how much work it would be?

Keats commented 4 years ago

Ah damn I was about to try it in Zola and rust-onig still hasn't shipped a new version with optional bindgen :(

@sharkdp how do you feel about creating a repository to contain all .sublime-syntaxes? We are kind of duplicating work in Zola and bat (and potentially other tools) having our own repo of syntaxes.

sharkdp commented 4 years ago

@sharkdp how do you feel about creating a repository to contain all .sublime-syntaxes? We are kind of duplicating work in Zola and bat (and potentially other tools) having our own repo of syntaxes.

Sounds like a great idea. Should we move this discussion to bats issue tracker? There are probably a lot of details which would need to be figured out (what to include? what not to include? how to deal with temporary patches, as bat currently does? what kind of tooling do we want to share? etc.)

Update: see https://github.com/sharkdp/bat/issues/919

robinst commented 4 years ago

cc @robinst any thoughts on what might needed to be added to either the rewriter or fancy-regex to fix these and guesses as to how much work it would be?

Yeah, I think some of these should be easy to fix in fancy-regex, e.g. (?# ...) and \u, I'll work on those first. Others are a bit more work like named capture groups, but on the radar: https://github.com/fancy-regex/fancy-regex/issues/34

Keats commented 4 years ago

@robinst any chance you can release a new version of fancy-regex? I want to update some syntaxes and want to know exactly which ones are failing after your latest patches.

robinst commented 4 years ago

@Keats Published version 0.3.4 now: https://github.com/fancy-regex/fancy-regex/blob/master/CHANGELOG.md#034---2020-04-28

Note that it's unlikely that I'll implement \g<...> for subexp calls in the near future. I would recommend replacing them with {{variable}} references instead. Note that that might even make it better for Sublime Text itself, as I don't think sregex implements that syntax either.

robinst commented 4 years ago

Also created a PR to make error messages include what the unknown flag/escape is: https://github.com/fancy-regex/fancy-regex/pull/46

Keats commented 4 years ago

Thanks a lot @robinst , I'll give it a try asap

robinst commented 4 years ago

Released https://github.com/fancy-regex/fancy-regex/pull/46 as 0.3.5

Keats commented 4 years ago

It looks like a lot of syntaxes are using \G (TypeScript like https://github.com/getzola/zola/blob/bc496e61010be1094a9192003ea59506c14d9397/sublime/syntaxes/TypeScript.sublime-syntax#L518) and \g (Elixir Regex https://github.com/princemaple/elixir-sublime-syntax/blob/master/Regular%20Expressions%20(Elixir).sublime-syntax#L52 and probably a few others). I had never heard of subexp calls before, could the linked Elixir file be changed to not use \g?

robinst commented 4 years ago

\G is an interesting one, it will anchor the match at where the current search begins. I wonder if that's necessary for correctness for those syntaxes or if they do it for performance (but not sure it would actually make things better). @keith-hall maybe you have some insight for that?

For \g, it should be possible to replace by repeating the pattern or using variables. E.g. this:

<({{capture_name}})>|'\g<-1>'|\g<-1>

Should be the same as:

<({{capture_name}})>|'({{capture_name}})'|({{capture_name}})

(But captures might need to be extended.)

keith-hall commented 4 years ago

\G is needed for correctness - this was often used in tmLanguage grammars where there was less functionality for working with contexts. Pretty sure it wouldn't have much impact on performance, or it would slow if down if no matches were found and all the patterns in the context use \G, as it would move the string pointer one char along and try again until it finds a match.

robinst commented 4 years ago

Hmm so do you know why none of the built-in syntaxes use \G at all? Seems strange. I wonder if sregex doesn't support it either and so they avoid it.

keith-hall commented 4 years ago

I believe that is correct, yes.

robinst commented 4 years ago

fancy-regex 0.4.0 now supports named groups and backrefs, see changelog. \g and \G are not yet supported.