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

zeroOrMore() and oneOrMore() do not generat correct results #36

Open
Tavio opened this issue Feb 6, 2015 · 4 comments
Open

zeroOrMore() and oneOrMore() do not generat correct results #36

Tavio opened this issue Feb 6, 2015 · 4 comments

Comments

@Tavio
Copy link
Contributor

Tavio commented Feb 6, 2015

The expression:

regex().add("Java").zeroOrMore().build();

should generate the regex: /(?:Java)*/, just as the expression:

regex().maybe("Java").build();

generates the regex: /(?:Java)?/. However, the first expression actually generates the regex: /Java*/, which is not correct. The oneOrMore() method suffers from the similar problem.

I propose to change both zeroOrMore() and oneOrMore() to accept a string, much as the maybe() method does so it can return the correct result.

@lanwen
Copy link
Contributor

lanwen commented Feb 6, 2015

regex().add("Java").zeroOrMore().build(); should generate Java* as expected - you should use grouping to generate (?:Java)*

@lanwen
Copy link
Contributor

lanwen commented Feb 6, 2015

You shouldn't remove current behaviour because this is expected. Just add alias that can return already grouped value. What for we should have 2 maybe methods? :)

@Maaartinus
Copy link

IMHO this is wrong. That Java* parses as (?:Jav)(?:a)* is perfectly fine, but when using a higher level more verbose description, I'd expect, it's structure gets preserved. And there's the word Java occurring as a unit in .add("Java").zeroOrMore(), so it should be handled as a unit rather than being split.

@KamasamaK
Copy link

add is just supposed to append the literal value -- no additional grouping or sanitizing. It should actually be private, but that would be a breaking change. What you want is then/find, which does group the text passed.

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

4 participants