Skip to content

Add option to keep epsilon in get_word() #527

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

Merged
merged 2 commits into from
May 12, 2025
Merged

Conversation

jurajsic
Copy link
Member

@jurajsic jurajsic commented May 6, 2025

Needed for Noodler. It should probably be some enum argument, maybe we should have some general argument that says whether epsilon should be handled as normal symbol for all functions that work with epsilons.

@jurajsic jurajsic requested review from Adda0 and koniksedy May 6, 2025 20:12
Copy link
Collaborator

@koniksedy koniksedy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I agree that some enum with potentially more options would look nice.

@Adda0
Copy link
Collaborator

Adda0 commented May 7, 2025

Could you add this general enum in this PR? Not sure about the names? Simply to have all the future bool flags discoverable easily by looking for enum classes, but also so that we can start using this flag. I think there were already a few functions where we were talking about handling epsilons as normal symbols.

If you need this urgently, just please add a TODO to replace this bool flag with an more general epsilon handling enum class.

@jurajsic
Copy link
Member Author

jurajsic commented May 7, 2025

I am also thinking whether instead of bool/enum, we change fist_epsilon to std::option<first_epsilon> and if it has a value, there are epsilons, otherwise not.

@Adda0
Copy link
Collaborator

Adda0 commented May 7, 2025

Yeah, that should be done 👍 It is probably a better approach in this particular case. In the future, with a full bool flags as bit fields redesign, we will see what the best approach will be, but for now, this sounds like a pretty good idea.

@Adda0 Adda0 merged commit b3c3fd2 into devel May 12, 2025
13 of 15 checks passed
@Adda0 Adda0 deleted the get_word_with_epsilon branch May 12, 2025 12:44
@jurajsic jurajsic mentioned this pull request May 12, 2025
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

Successfully merging this pull request may close these issues.

3 participants