Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The parse() API #3

Open
mathiasbynens opened this issue Nov 3, 2013 · 8 comments
Open

The parse() API #3

mathiasbynens opened this issue Nov 3, 2013 · 8 comments

Comments

@mathiasbynens
Copy link
Collaborator

Currently the API is parse(string), meaning parse accepts a single string representing a regular expression.

It should be possible to specify which flags apply to the regular expression, since they might influence parsing (as is the case for the ES6 /u flag). Any ideas on the best way to extend the API, moving forward? parse(string, flags) where flags is an object such as { g: true, i: false, m: false, u: true }?

Or would we at some point need to pass more than just flags to parse, e.g. options? In that case we’d be better off using parse(string, options) where options.flags represents the flags.

@jviereck
Copy link
Owner

jviereck commented Nov 3, 2013

Or would we at some point need to pass more than just flags to parse, e.g. options?

Only the future knows if we need options, but I think it is something reasonable to add. Therefore, I prefer your proposal with options.flags. You pass in an object like { g: true, i: false, m: false, u: true }. I find it more natural to pass in a string, like the second argument to the RegExp constructor (like. new RegExp('foo', 'i')).

Maybe the API could also look like this:

// No options and no flags. Use the default options with no flags.
parse(string)

// Pass in flags and use the default options.
parse(string, string /* flags */); // example: parse('foo', 'i')

// Pass in a specific set of options.
parse(string, object); // example: parse('foo', { flags: 'i' })

That introduces polymorphism, but I think that is not too bad here and if it makes the API easier to use, I personally think it is worth it.

Opinions?

@mathiasbynens
Copy link
Collaborator Author

I love the polymorphic approach!

@jviereck
Copy link
Owner

As the flag influences the parse tree as discussed in #11, should the flags/options be stored on the final parse tree?

@mathiasbynens
Copy link
Collaborator Author

Yes, definitely!

@jviereck
Copy link
Owner

Thinking more about the parse API, I would love to see parsing support for the "whole" RegExp. Assuming there is a RegExp /a/u, the user passes only the inner parts to the parse function:

parse('a', 'u');

As it's a common case to look at the regexp as a whole, it should also be possible todo:

parse('/a/u', true /* isCompleteRegExp */);

Sounds good?

@mathiasbynens
Copy link
Collaborator Author

Let’s avoid the boolean trap, though: the second (optional) parameter should either be a string with flags or an object with settings (one of which could be flags, another could be isCompleteRegExp) IMHO.

@jviereck
Copy link
Owner

Let’s avoid the boolean trap

good point.

Maybe we can also shift the defaults - e.g.

parse('/a/u') // implies isCompleteRegExp=true. Currently this is isCompleteRegExp=false.

and

parse('a', 'u') // implies isCompleteRegExp=false, flags='u'

in addition, there could also be new in the game:

parse(/a/u) // Pass an actual RegExp object in here

This is then a backwards-incompatible change.

Thinking about how new RegExp(str, flags) is called, the call pattern similar to the RegExp constructor looks like this:

- parse(/a/u)     -> isCompleteRegExp=true,  flags='u'
- parse('a')      -> isCompleteRegExp=false, flags=''
- parse('a', 'u') -> isCompleteRegExp=false, flags='u'

This CAN be implemented backwards compatible.

Just leaving out the discussion without backwards compatibility, I think the last proposal is the best due to the consistency with the RegExp constructor.

@mathiasbynens, any thoughts?

@mathiasbynens
Copy link
Collaborator Author

That last proposal sounds reasonable to me.

We should be careful about adding sugar unless there is a strong need for it, though (generally speaking).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants