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

multiple fixes for of_string functions #78

Merged
merged 4 commits into from
Oct 20, 2020

Conversation

hhugo
Copy link
Contributor

@hhugo hhugo commented Oct 13, 2020

  • Z.of_string should accept string with a 0_ prefix.
  • Q.of_string should not accept a sign after a dot (aka Q.of_string ".-0" should fail)
  • Q.of_string should accept leading underscore after a dot, even if the integer part is empty (aka Q.of_string "._123" should succeed)

@hhugo hhugo changed the title fix underscore after leading zero multiple fixes for of_string functions Oct 19, 2020
@hhugo
Copy link
Contributor Author

hhugo commented Oct 19, 2020

This is ready for review

@hhugo
Copy link
Contributor Author

hhugo commented Oct 19, 2020

@antoinemine, would you have time to review this PR ?

@antoinemine
Copy link
Collaborator

Does first_digit_after_dot works if the string ends with a dot?
Maybe find_in_string should return None when pos >= last, to be on the safe side.

@hhugo
Copy link
Contributor Author

hhugo commented Oct 20, 2020

Does first_digit_after_dot works if the string ends with a dot?

I think it does

Maybe find_in_string should return None when pos >= last, to be on the safe side.

I've done that

@antoinemine
Copy link
Collaborator

OK. It seems good to me.

@antoinemine antoinemine merged commit 8cf737b into ocaml:master Oct 20, 2020
@hhugo hhugo deleted the fix-underscore branch October 20, 2020 17:36
@hhugo
Copy link
Contributor Author

hhugo commented Oct 20, 2020

Thanks
@antoinemine , How do you feel about a new release ?

@antoinemine
Copy link
Collaborator

There are still a couple of MR and issues we could look into before a release.

@hhugo
Copy link
Contributor Author

hhugo commented Oct 22, 2020

I've opened #82 to track the new release and make sure we don't drop it by accident.

antoinemine added a commit that referenced this pull request Nov 11, 2020
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