support backrefs in included contexts #288
Conversation
|
I can confirm that this fixes sharkdp/bat#914 - thank you very much! |
|
I pushed a new commit because I noticed that it didn't support nested includes which use backreferences - so I added a test case for it. Please could you review the new changes @robinst? Maybe you can think of a better way of doing it than the "loop multiple times through each context to check if it needs to be marked as using backrefs until no new included contexts are found" approach that I have implemented... |
There was a problem hiding this comment.
Looks good to me!
I streamlined the match statement even more and added a comment that I benchmarked the non-fancy graph flood fill locally and it was negligibly slower than linking previously.
However you don't seem to have enabled the maintainers can push to the branch for this PR so I can't push the commits. If you're around you can see if you can enable that otherwise I'll just manually merge and the Github UI may just look like I closed this PR instead of merging it.
|
Another question: Should we regenerate pack files? That is, does this change actually affect anything in the default syntax sets or do no default syntaxes include a context with backreferences? I notice that when I try regenerating the pack files no syntax tests change. |
sorry about that, but as far as I can tell, GitHub is no longer offering this option - because my fork is owned by a "group" maybe? I certainly used to have this option set. Anyway, I have offered you write access to my fork to work around this. The default packages aren't affected at our current submodule version as I understand it, but in more recent commits, syntaxes like Ruby are affected by this. So no need to regenerate packs as part of this PR. |
|
Thank you all for your continued work on @trishume If you find the time, a (bugfix) release with this PR included would be very much appreciated. |
|
@sharkdp I released |
|
Thank you very much! |
fixes #104 - when building the syntax set, after the contexts are linked to direct references, it now checks if any included contexts use backreferences in their match patterns, and if so, marks the base context as using backreferences.