Parse \p{…} and \P{…} in Unicode mode

[mathiasbynens]

Ref. #77.

[mathiasbynens]

We probably don’t want to enable this by default. How do we go about this? regjsparser.parse(pattern, flags, { experimental: true })?

[mathiasbynens]

I went with regjsparser.parse(pattern, flags, { experimental: true }). What do you think, @jviereck and @demoneaux?

[jviereck]

Thanks for providing this PR!

  } else if (isExperimental && hasUnicodeFlag && (res = matchReg(/^([pP])\{([^\}]+)\}/))) {

Is that part of the spec, that this feature only becomes active when the unicode flag is set? (Did not read it, just wondering if having this implicit dependency is a good idea / necessary)

Beside this, the implementation LGTM. I am wondering more what's the best way to support the { experimental: true } of the current PR.

Adding a third argument to regjsparser.parse is good in my opinion. However, I would prefer a solution that scales better. More precise: if another features/experiment comes along, that we don't have to start numbering them as { experimental_2: true }.

Therefore, how about making the third argument to regjsparser.parse a general 'feature' argument? This could then look like regjsparser.parse(pattern, flags, { unicodePropertyEscape: true }). Also, this would allow to enable a group-toggle: Assume your proposal gets shipped in ES2018, another feature gets implemented in ES2019 and developers want to activate all features that ship until ES2019, then they could call regjsparser.parse(pattern, flags, 'es2019'). (I hope you get the idea).

@mathiasbynens, what do you think?

PS: An orthogonal way to your PR would be to extend regjsparser by a plugin system (maybe similar to the babel.js parser). Though, I doubt the complexity of introducing/maintaining a plugin system is worth it. In case more features boil up over the next years, we might want to revisit this point, but let's keep things simple for now and add features/experiments/extensions into the codebase directly.

[mathiasbynens]

Thanks for taking a look!

Is that part of the spec, that this feature only becomes active when the unicode flag is set? (Did not read it, just wondering if having this implicit dependency is a good idea / necessary)

There is no spec yet, but once a spec for this feature is written it will only allow \p{…} in u regexps. It’s both a good idea and necessary :) Here’s why: the ES6 spec prohibits use of \p{…} since it throws when \p or any other unneeded simple escape sequence occurs. Without the u flag this doesn’t happen, so enabling \p{…} in non-u regexps wouldn’t be backwards compatible. https://mathiasbynens.be/notes/es6-unicode-regex#impact-syntax

I like your idea of using a feature argument (much better indeed!) and have updated the patch.

[jviereck]

This looks great! Thanks for the quick update @mathiasbynens -> merge!

[jviereck]

@mathiasbynens: Do you want me to create a new npm release or do you have anything else in flight that you want to get in before cutting a new version?

Also: The new version should be 0.2.0 because the API changed instead of 0.1.6, right? (From the SemVer page: "MINOR version when you add functionality in a backwards-compatible manner, and")

[mathiasbynens]

Yeah, v0.2.0 sounds good! Let’s publish once you’ve been able to review the CLI --flags PR in #79?