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

match_group_indexes blank array for plain text regex. #12

Open
JernKunpittaya opened this issue Feb 21, 2023 · 4 comments
Open

match_group_indexes blank array for plain text regex. #12

JernKunpittaya opened this issue Feb 21, 2023 · 4 comments
Labels
bug Something isn't working enhancement New feature or request

Comments

@JernKunpittaya
Copy link

With regex /merg(e)/ as plain text trying to match the "merge" in any text, just results in blank match_group_indexes, hence error. Quick fix can be done via writing regex as /merg(e|a)/ , aka just put that e in the group with some other operations, and this result in matching "e" value.

@katat katat added bug Something isn't working enhancement New feature or request labels Feb 22, 2023
@katat
Copy link
Contributor

katat commented Feb 22, 2023

Thanks for reporting.

This is indeed a valid issue. Currently, the regex compiler assumes at least one alternate group presented in the regex string, and use one of the group to calculate related data points for the output signals.

It is possible to support searches without alternate groups and only provide the data points, such as the start index of the match and how many entire matches. This is a potential feature to support.

For now, I think we can do a check and output proper error messages to avoid confusion, as you have helped point out. @Divide-By-0 may have other thoughts on this.

@Divide-By-0
Copy link
Member

We could also potentially just make the final state right before the accept stage of the DAG be the group that is indexed over?

@katat
Copy link
Contributor

katat commented Feb 24, 2023

We could also potentially just make the final state right before the accept stage of the DAG be the group that is indexed over?

That would also be a way to go.

I think it is about structuring the code to allow opt-in for the features that match_group_indexes serves.

@Divide-By-0
Copy link
Member

The current code that checks if there are >= two states within a match group via two incoming nodes, is brittle and seems to bug on other use cases, and also doesn't quite seem like the thing we always want to match. @JernKunpittaya has some ideas on improving this and will try to setup a way for a user to specify this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants