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

Do not skip whitespace while reading quotient #8000

Merged
merged 1 commit into from
Feb 13, 2024

Conversation

sloriot
Copy link
Member

@sloriot sloriot commented Jan 24, 2024

Fixes issue mentioned in #7927

sentry also eats end-of-line and arrangement formatter has a skip-until-eol after reading vertex coordinates

We decide to not allow whitespaces between num / dum

@lrineau lrineau requested a review from mglisse January 24, 2024 15:08
@mglisse
Copy link
Member

mglisse commented Jan 24, 2024

We decide to not allow whitespaces between num / dum

IIUC, after the patch, you accept whitespace before num, before den, but not between num and '/'.
This seems to match what Gmpq (more generally read_float_or_quotient) does, and is less strict than gmpxx (which also refuses a space after '/'), so it shouldn't break much.
Ideally there would be a bit of testing and/or documentation to go with it, but I can understand if you don't think it is worth the trouble.

@sloriot
Copy link
Member Author

sloriot commented Jan 24, 2024

Actually it's doc with no space at all around /. I can force no space after if you think it makes sense.

@mglisse
Copy link
Member

mglisse commented Jan 24, 2024

I can force no space after if you think it makes sense.

Bah, as long as it isn't causing problems, it can wait...

@sloriot
Copy link
Member Author

sloriot commented Feb 13, 2024

Successfully tested in CGAL-6.0-Ic-169

@lrineau lrineau self-assigned this Feb 13, 2024
@lrineau lrineau added this to the 6.0-beta milestone Feb 13, 2024
@lrineau lrineau added the rm only: ready for master For the release team only: that indicates that a PR is about to be merged in 'master' label Feb 13, 2024
@lrineau lrineau merged commit 554bec8 into CGAL:master Feb 13, 2024
8 checks passed
@lrineau lrineau removed the rm only: ready for master For the release team only: that indicates that a PR is about to be merged in 'master' label Feb 13, 2024
@lrineau lrineau deleted the NT-fix_quotient_reading branch February 13, 2024 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants