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

Token patterns with flags are broken #7

Closed
oxc opened this issue Nov 10, 2017 · 4 comments
Closed

Token patterns with flags are broken #7

oxc opened this issue Nov 10, 2017 · 4 comments

Comments

@oxc
Copy link

oxc commented Nov 10, 2017

Since 0e6eb41, Tokens defined by a Pattern or RegEx object are converted to a Token defined by a pattern String.

This breaks use-cases where the pattern object was created with flags set
(e.g. token("^--".toRegex(RegexOption.MULTILINE)))

Currently, this can only be worked around by specifying the flags inside the pattern string
(e.g. token("(?m)^--"))

@h0tk3y
Copy link
Owner

h0tk3y commented Nov 14, 2017

The aim of that commit was supporting custom tokenizers that do not depend on Java regex implementation. I'll take a look into options how regex flags can be returned to work with the default tokenizers.

@oxc
Copy link
Author

oxc commented Nov 14, 2017

I understand that the goal of that change was not to break patterns, but if the Pattern/RegEx parameters are internally passed as Strings, there's not too much use for the overloaded Token constructors anymore, especially if they lose information in the process.

I think in this case I personally would have preferred if the overloaded versions taking a Pattern or a RegEx would have been removed completely.

@h0tk3y
Copy link
Owner

h0tk3y commented Dec 2, 2017

Fixed in 0.3.2. Now, if a Token is created with a Regex or a Pattern, it is saved as a Regex inside the Token, so the Tokenizer can use the regex as well. Default tokenizer uses the Regex if it is present, and creates it from the string pattern otherwise.

@h0tk3y h0tk3y closed this as completed Dec 2, 2017
@oxc
Copy link
Author

oxc commented Dec 6, 2017

Thanks! Works great ;)

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

No branches or pull requests

2 participants