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

add alt_flat function #29

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

EmileTrotignon
Copy link

@EmileTrotignon EmileTrotignon commented Nov 8, 2024

Add an alt_flat function, which is just like alt but for regexps of the same type.

This PR together with #27 and #28 allow to write the following code :

let str_nbsp txt = Tyre.(attach (" " ^ txt) (str " " *> str txt))

let insert_nbsp_in_string txt =
  Result.get_ok
    Tyre.(replace (compile (str_nbsp ":" <||> str_nbsp ";" <||> str_nbsp "!")) txt)

Neat !

@Drup
Copy link
Owner

Drup commented Nov 9, 2024

This is the signature of the traditional alt for an applicative functor .... The problem is that the behaviour, when combined with conv, is not very consistent. In one direction, it "tries both branches" and use the one that works. In the other direction, it just takes the left in an arbitrary fashion. I think it would definitely make for a footgun.

Do you have a use case that can't be solved by using raw Re (and Tyre.regexp) ?

@EmileTrotignon
Copy link
Author

I did not realize it meant eval could return a value that is inconsistent with the input.

I do not need raw Refor this, conv and regular alt are enough to reimplement it.

I still want this in Tyre, but I believe the correct way to go about it is to return an exception in conv's from argument, and update the documentation accordingly.

In a lot of cases, regexps are never going to be evaluated, so I think it is fine if its properly documented. I can also add an explanation that explains that you need one alt_flat per type, handwritten with conv if you need alt_flat and evaluation.

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.

2 participants