Skip to content

Conversation

@Drizin
Copy link

@Drizin Drizin commented Dec 4, 2016

e.g.: [[[This %0 allows %1 to have %2|||regex|||nuggets|||(((recursive %0|||(((parameters///comment3)))///comment2)))///comment1]]] -> This regex allows nuggets to have recursive parameters

e.g.: [[[This %0 allows %1 to have %2|||regex|||nuggets|||(((recursive %0|||(((parameters///comment3)))///comment2)))///comment1]]]  -> This regex allows nuggets to have recursive parameters
@turquoiseowl
Copy link
Owner

Would you elaborate on the use-case for this.

A section for the README will help me too.

Thanks.

…de other nuggets): README, Unit test and bug fix.
@Drizin
Copy link
Author

Drizin commented Dec 4, 2016

I just added a small example to README, created a sample unit test, and also fixed a bug - now all tests are passing.

@turquoiseowl
Copy link
Owner

I'm still not clear on the actual need to this. Can you help explain further?

@turquoiseowl
Copy link
Owner

What are the performance implications of the updated regular expression, if any?

I don't fully understand what you are doing with it TBH, though looks clever!

@Drizin
Copy link
Author

Drizin commented Dec 5, 2016

The regex has 2 major changes:

  1. Changed the captures to be named capture groups. Just for clarity.
    I think there is a small performance overhead (in favour of readability). I wasn't really focused in performance. If you'd like to focus on performance we can go back to positional captures, and make some additional improvements on performance like making the regex compiled and making m_regexNuggetBreakdown static like the others.
  2. The regex is using balancing groups, so that the number of parameter begin-tokens ((( must match the number of paramater end-tokens ))). That means that (((Argument1 %0|||(((Argument2)))))) will be correctly matched. If we don't use capture groups whenever the first end-token is found the regex is matched. In other words the number of opening blocks must match the number of closing blocks, instead of just returning the match when the first closing block is found.
    As far as I understand the performance impact is also insignificant, because on the regular case parameters will always have one begin-token and one end-token and the match will work fine.

@Drizin Drizin mentioned this pull request Dec 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants