Add a globalPrevCapture that persists into nested parses

[FuegoFro]

This adds an additional globalPrevCapture that is "global" state for the parses and persist into nested invocations of nestedParse. This allows rules to determine if they are truly at the beginning of a line. The existing prevCapture resets at each nesting, so it only allows you to determine if you're at the beginning of the nested parse, but not necessarily the line.

[FuegoFro]

Note that this adds a new globalPrevCapture rather than changing the behavior of prevCapture to prevent changing the semantics of the existing APIs, though if you think it would be best to change prevCapture to be global I can change this to do that instead 🙂

[ariabuckles]

Hi @FuegoFro and thanks for sending this!

Work has been super busy so it'll take me a bit to get back to you more thoroughly, but this looks like a great addition.

[ariabuckles]

This looks good to me, but I think we could maybe simplify the whole prevCapture / globalPrevCapture thing by moving prevCapture to be on state, and then defining it to be global (which state is by default).

Then we'd get:

1. the more helpful semantics you propose
2. backwards compatibility with existing code that relies on prevCapture not being global (honestly, not a use case I'm particularly inclined to support unless someone shows me something relying on it)
3. the ability to deprecate prevCapture (since we won't need anyone to rely on it's position in the arguments list, which they would need to if they wanted to use globalPrevCapture here).
4. We can maybe clean up the prevCapture semantics a bit; it's a bit odd that it's named prevCapture but it's only capture[0]

Would that satisfy your use case @FuegoFro and @night ?

I think the change would mostly be:

         // ensure they don't match arbitrary '- ' or '* ' in inline
         // text (see the list rule for more information).
         var prevCapture = "";
+        state.prevCapture = null;
         while (source) {
             // store the best match, it's rule, and quality:
             var ruleType = null;

             }
 
             prevCapture = capture[0];
+            state.prevCapture = capture;
             source = source.substring(prevCapture.length);
         }
         return result;

     },
     list: {
         order: currOrder++,
-        match: function(source, state, prevCapture) {
+        match: function(source, state) {
             // We only want to break into a list if we are at the start of a
             // line. This is to avoid parsing "hi * there" with "* there"
             // becoming a part of a list.
            // You might wonder, "but that's inline, so of course it wouldn't
            // start a list?". You would be correct! Except that some of our
             // lists can be inline, because they might be inside another list,
             // in which case we can parse with inline scope, but need to allow
             // nested lists inside this inline scope.
-            var isStartOfLineCapture = LIST_LOOKBEHIND_R.exec(prevCapture);
+            var prevCaptureStr = state.prevCapture == null ? "" : state.prevCapture[0];
+            var isStartOfLineCapture = LIST_LOOKBEHIND_R.exec(prevCaptureStr);
             var isListBlock = state._list || !state.inline;
 
             if (isStartOfLineCapture && isListBlock) {

If that works for you, would you like to make that (happy to have you get credit for this!), or would it be easier for you if I did?

Thanks again for submitting this and very sorry for the delays!

[FuegoFro]

That makes sense, I'm happy to move it onto the state! I think we can't quite use the exact diffs you have here since that'll still reset state.prevCapture to null at the beginning of each new level of nesting, but I believe that effectively changing each of the places I have globalPrevCapture to state.prevCapture would do the trick. I've updated the PR to do that 🙂

I wasn't sure if I should fully remove the prevCapture argument to the MatchFunction or quality functions now, or if there's a process for deprecation and eventual removal.

[FuegoFro]

If we wanted to maintain the legacy behavior here we could add an || i === 0 to the condition, which would reset it at the beginning of each nesting like it used to.

[ariabuckles]

Makes sense. I think I'll do that so we don't break any legacy code, but will encourage state.prevCapture in the future.

[ariabuckles]

Thanks; this looks great!

I think we can't quite use the exact diffs you have here since that'll still reset state.prevCapture to null at the beginning of each new level of nesting

Oops! Thanks for catching and implementing that correctly!

I wasn't sure if I should fully remove the prevCapture argument to the MatchFunction or quality functions now, or if there's a process for deprecation and eventual removal.

I have some things deprecated, but given how removing any deprecation would technically be a breaking change, I have tried to preserve legacy semantics as much as possible. I think that's possible here too, so I'll eventually update the docs but keep supporting the legacy prevCaptureStr in the code.

Sorry about the delays again, I'll publish a new version with this tonight.

[ariabuckles]

Makes sense. I think I'll do that so we don't break any legacy code, but will encourage state.prevCapture in the future.

[ariabuckles]

Published v0.6.0 with this change <3!

[FuegoFro]

Yay, thank you! 😀

[ariabuckles]

Thank you!