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

fix: parsing scientific notation in SVG path #365

Merged
merged 1 commit into from
Aug 14, 2024

Conversation

GabrielDertoni
Copy link
Contributor

Before this change

The SVG parsing code failed to parse scientific notation in BezPath. For example M 0 0 L 1e-123 -4E+5 would fail to parse with SvgParseError::Wrong even though it is a valid SVG path. This error was introduced by 8d10d4e which forgot to negate the conditional, causing the parser to fail when it should succeed.

After this change

The change fixes the error and adds a test for it.

Copy link
Contributor

@raphlinus raphlinus left a comment

Choose a reason for hiding this comment

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

Thanks! I actually have a local branch called "fix_svg_exp" with the exact change and a commit comment saying "WIP, really needs a test." So thanks for getting this in.

We have a policy of "author commits" for substantive changes. This is a smaller one (but one that fixes a real bug) so it's up to you whether you'd like us to merge or whether you'd like write access.

@GabrielDertoni
Copy link
Contributor Author

No need to give me write access yet (if I end up wanting to make larger contributions we can revisit). I am happy to just get this in whichever way is faster.

@raphlinus raphlinus added this pull request to the merge queue Aug 14, 2024
@raphlinus
Copy link
Contributor

Thanks! I just added it to the merge queue.

Merged via the queue into linebender:main with commit 1cd126b Aug 14, 2024
15 checks passed
@GabrielDertoni GabrielDertoni deleted the fix-get-number branch August 14, 2024 21:59
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