[samukce]

Hey @robinst / @colinodell , bringing our discussion to here. (I'm committing this code partially for us to have some code to discuss over)

So, based on our initial thoughts in the other PR(#177) I came with this idea in this new PR to us discuss over and see if make senses. The main thing that made me (re)think to give a try in a new approach it was when I studied the InlineParser default impl and I noticed that to inject the idea about "priority extensible nodes" would mean redo the entire logic. Also, I saw better about the text patterns and I thought about how we could provide an extensible way without losing performance avoiding as much as possible regex needs. So I came with a new redesign idea. (totally understandable if it doesn't make senses ;) )

That said, I created a new InlineParser implementation following the @robinst recommendation such as 1. One character to trigger the node process; 2. One way to prioritize the winner nodes;

In high level the idea is based on some points:

1. InlineParserImpl: will run all characters from the line; Once noticed a special trigger character, it will start a watch process by the node which can have multiple watches based on the node setup (TextNodeIdentifierSetup)
2. TextNodeIdentifierSetup: it will provide the text identifier logic, which is called for each character after being started by the InlineParser;
3. Once the Text identifier noticed that found their pattern, they "inform" the inline parser through NodePatternIdentifier
4. Once the node pattern found to raise an event, this one is put in an array which is considered the priority configuration positionsReservedByNodePriority;
5. After running the entire line, we interact in the array positions positionsReservedByNodePriority to check which nodes it was found;
6. So we call the NodeCreator for that give node passing some metadata and the substring for each node found;
7. For this NodeCreator process, we do have a set of common extractor classes that can help to avoid an extra process by the client-side but, it's up to the client to decide how to build their node based on the substring that is provided to them.

Tring to put some more colors, the algorithm is:

Let's assume that we those Nodes setup: 
- ImageLogoNodeSetup  Priority:30     Pattern: ![]()  CharacterTrigger: !
- LinkNodeSetup       Priority:20     Pattern: []()   CharacterTrigger: [

Line example: "Some text ![text](/url.png) and [link](a.com)"

1. Interacting character by character and trigger the watches and found:

Example:
  Some text ![text](/url.png) and [link](a.com)
  ----------^
            ! ImageLogoNodeSetup triggered by this point for each character
  -----------^
             [ LinkNodeSetup triggered by this point for each character
              --------------^
                            ) Found close character that match for both patterns; 
                              So, the priority attribute came to picture and ImageNode wins;
            ![text](/url.png) Positions added to the positionsArray with the a PreNode metadata
                             -----^
                                  [ LinkNodeSetup triggered by this point for each character
                                   ------------^
                                               ) Found close character that match for both patterns; 
                                                 LinkNodeSetup trigger the complete node found
                                  [link](a.com) Positions added to the positionsArray with the a PreNode metadata
                                  

2. Interact in positionsArray that is a kinda line mirror for nodes/no-nodes to create the real Node object;

In this idea, if the client wants to add a new Node configuration, they just need to create a TextNodeIdentifierSetup and to add this one to the parser factory, using the priority property to override any pre-node setup if needed. (i.e: ImageLogoNodeSetup.java and check InlineParserNodeSetupFactory.java) (For sure that we can add a builder class or something else for this flow)

Also, comparing the current way with the new one the performance is really close: (I set up just a few os nodes to be able to do this comparison)

Benchmark                            Mode  Cnt   Score   Error  Units
ParseBenchmark.currentRegularParser    ss   50  56.990 ± 9.272  ms/op
ParseBenchmark.newFashionParser        ss   50  68.604 ± 3.257  ms/op

Benchmark                            Mode  Cnt   Score   Error  Units
ParseBenchmark.currentRegularParser    ss   50  59.101 ± 8.874  ms/op
ParseBenchmark.newFashionParser        ss   50  72.102 ± 3.760  ms/op

Geez, such a huge blog text.. sorry folks. 😅 Thoughts? Ideas? Revert the code? :)

(I've put everything in a package apart to avoid impact the current code, but, if you want to see for example which tests are falling just replace the Parser.getInlineParserFactory() by the new factory InlineParserNodeSetupFactory)

[robinst]

I checked this out and replaced the inline parser used in InlineParserFactory in Parser with it and ran SpecCoreTest.

There are a lot of failures around emphasis parsing for example. I don't know if the new model can implement emphasis parsing according to the spec. In general I'd rather keep the existing logic as it's very close to what the JS reference implementation does and follows the spec.

I might take a stab at making inline parsing extensible based on the current logic (can't promise a timeline though).

[samukce]

Gotcha @robinst. It makes sense. Thanks for your thoughts on this. Do you think that makes sense put any effort into this idea? for instance, trying to see if matches with emphasis patterns for instance? or go by the original one? My time for the next weeks is not so good but I can try to help you somehow.

[robinst]

Hey! I've taken some time to work on this and it's looking really good. I've opened a WIP PR here: #180

[robinst]

Closing this, see #177 (comment)