properly support lookahead expresions in begin matches

[joshgoebel]

• passes the FULL remaining buffer to processLexeme so
  that when it attemps to detemine the next mode it can
  run the ruleset against the FULL buffer, not just the
  terminator which resulted in a match (which will not
  be sufficient enough since lookaheads aren't included
  in the actual match)

[joshgoebel]

I think we may need to do this same thing for endOfMode to support end matches, can anyone confirm? The use case I cared about was begin matches, and that's what I've tested.

If this looks good there are a few syntaxes that can be updated now that we have PROPER lookahead support - since a few current have HACKS to deal with the prior broken support such as:

XML.js

        /*
        The lookahead pattern (?=...) ensures that 'begin' only matches
        '<style' as a single word, followed by a whitespace or an
        ending braket. The '$' is needed for the lexeme to be recognized
        by hljs.subMode() that tests lexemes outside the stream.
        */
        begin: '<style(?=\\s|>|$)', end: '>',

[joshgoebel]

Can someone tel me the correct way to run tests from console? npm test seems to be confused about where ../../build is supposed to be.

Ok got it now, will get the tests passing. :-)

[joshgoebel]

Ha, now the issue is we have "broken"? rulesets that will actually match over and over if they can see the full data, such as Fortran:

/(?=\b|\+|\-|\.)(?=\.\d|\d)(?:\d+)?(?:\.?\d*)(?:[de][+-]?\d+)?\b\.?/im

This can actually return a 0 length match, which won't push the position forward, then it'll find AGAIN the same 0 length match in an infinite loop. :-)

I'm thinking perhaps we should ignore 0 length matches.

[egor-rogov]

Hey @yyyc514, how is it going with this PR? Haven't heard from you in a while.

[marcoscaceres]

Heh, so this is how it all started 😂

[joshgoebel]

I got a little distracted, it's true. But I haven't forgotten this. :-)

[joshgoebel]

Diff gets really confused around line 490 or so... probably easier to read lines 539 to 644 outside the diff. They should be fairly easy to follow.

I shrunk processLexeme and moved a lot of the function into doBeginMatch and doEndMatch.

The end of compileMode moves into buildModeRegex which builds a 'super matcher' or whatever you'd like to call it. :-) It pretends to be a regex (supporting exec) but also returns metadata reporting WHICH mode matched, etc... so we don't have to recheck.

[joshgoebel]

Broke the browser build somehow, no idea how.

[egor-rogov]

I'm gonna look into this, but do not expect a quick feedback (:

[joshgoebel]

No problem. Nothing too advanced here I don’t think. It’s really just a purer implementation of the goals of the original system. Maybe it didn’t occur to them at the time you could just use capture groups to track which rule matched without the need to re-run the rule sets.

[joshgoebel]

Not saying it's worth the time but so many places where we use returnBegin right now could be replaced with look-aheads with no negative issues, and perhaps even resulting in simplifying the grammar, like in the abnf case. (since most of the time you're replacing nesting with just a single, simple regex)

[egor-rogov]

I wish I had more time to grok this...
Anyway, what I see:

• all tests are passing
• speed remains roughly the same (around 9 sec on my laptop for npm test)
• lookaheads work!

So I can see no reason to delay this change. However I suggest to split it in 2 (maybe more) PRs: one for highlight.js itself, and the other(s) for improving grammar of languages.

@marcoscaceres You may want to have a look either, 'cause it changes the core of the system.

[egor-rogov]

I think we may need to do this same thing for endOfMode to support end matches, can anyone confirm? The use case I cared about was begin matches, and that's what I've tested.

I think it makes perfect sense.
@yyyc514 it would be great if you can add a section to the developer's docs on what regexp features are supported (like lookaheads (thanks to this PR) and backreferences (thanks to #1897)) and what are not (lookbehinds?).

[joshgoebel]

Well, look behinds technically aren't in JS yet, are they?... but when they are added this PR should allow them to work just fine with begin matchers, and we should keep that in mind for end matchers also.

When you say "developer docs" which docs are you referring to exactly?

[joshgoebel]

However I suggest to split it in 2 (maybe more) PRs: one for highlight.js itself, and the other(s) for improving grammar of languages.

I can definitely do that. If we didn't "Always squash" this could stand-alone. I'm not sure I'm a fan of this always squashing. I see how it makes it easier for people new to git, but when you have something like this that's already packaged in nice small commits it's kind of annoying to have to split it out AGAIN.

I see a dropdown, perhaps it's possible to change it to non-squash... in which case I'd just clean up the one weird commit and then leave the history as is, rebase onto master, and then do a clean fast-forward merge.

[egor-rogov]

When you say "developer docs" which docs are you referring to exactly?

The one in docs/ folder (which goes to https://highlightjs.readthedocs.io/en/latest/)

[joshgoebel]

Would you suggest a new document to talk about regex features?

[egor-rogov]

language-guide looks like a proper place for it.

[joshgoebel]

I think I might keep it small and focus on what we DON'T support... I think the idea is its regex... it should just work... If someone wants to learn about regex there are all sorts of resources.

So right now the caveats (after this PR is merged):

• "look-ahead" doesn't work properly in end expressions.
• "look-behind" (when JS supports it) also doesn't work properly in end expressions.

[egor-rogov]

I thought that what we do support (and since what version) may be also important for those who previously stumbled on something unsupported.

[joshgoebel]

Does that addition to docs help?

[joshgoebel]

@egor-rogov We good here yet?

[egor-rogov]

Ok, here we go!
I think we shouldn’t squash to keep separate commits.

[joshgoebel]

Agree. I'll rebase and fixup the last commit and the plain jain merge when ready.