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

#640 Remove force unwrap in SVGParser.doubleFromString method #641

Merged
merged 1 commit into from
Nov 26, 2019
Merged

#640 Remove force unwrap in SVGParser.doubleFromString method #641

merged 1 commit into from
Nov 26, 2019

Conversation

vox-humana
Copy link
Contributor

There are two other possible fixes:

  1. Add "null" string to this check in the beginning of SVGParser.doubleFromString method
if string == "none" {
   return 0
}
  1. Make SVGParserRegexHelper.unitsIdenitifierPattern regexp stricter

However, I decided to go with the least destructive fix because tbh I don't know enough about SVG standard.

@amarunko
Copy link
Contributor

Hi, @vox-humana, can you attach svg example where force unwrap calls crash or any other issues?

@vox-humana
Copy link
Contributor Author

vox-humana commented Nov 25, 2019

Hi @amarunko. I did it in the ticket #640. SVG Edit creates polygon with “null” attributes 🤦‍♂️

Sent with GitHawk

@vox-humana
Copy link
Contributor Author

Here is a file from SVG Edit
svg-edit-polygon.svg.zip

@amarunko
Copy link
Contributor

sorry, I missed the issue, will look into

Sent with GitHawk

@ystrot ystrot self-assigned this Nov 26, 2019
@ystrot ystrot added this to the 0.9.6 milestone Nov 26, 2019
@ystrot ystrot merged commit 289c753 into exyte:master Nov 26, 2019
@vox-humana
Copy link
Contributor Author

Thank you @amarunko for the quick response here! /me switching our branch to master

Sent with GitHawk

@vox-humana vox-humana deleted the 640-malformed-double-attribute branch November 26, 2019 23:38
@ystrot ystrot modified the milestones: 0.9.6, 0.9.7 Apr 10, 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.

3 participants