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

Error handling failure with too few arguments in path command #17

Open
bgmort opened this issue Oct 23, 2020 · 2 comments
Open

Error handling failure with too few arguments in path command #17

bgmort opened this issue Oct 23, 2020 · 2 comments

Comments

@bgmort
Copy link

bgmort commented Oct 23, 2020

There are two related issues here. One is a quick fix, and I'll send a pull request for it with tests. The second is more complicated, and I see that you've started a rewrite, so I'll just point it out for you to be aware of.

Apparently the A arc path's two flag arguments, which must be either 0 or 1, do not have to have any spaces after them. So, the following command: a 110 110 0 0 1 55 -7 could be written like this: a 110 110 0 0155-7. You'll get output like that from an optimizer like https://jakearchibald.github.io/svgomg/.

The current parser doesn't take that into account, and parses the latter command as having only 5 arguments. I couldn't find where it was described in the svg spec, but it's clearly widely supported as any SVG renderer I tried doesn't have a problem with it. <path d="M0 0a 100 100 0 00200-0z" /> would be a valid path to test.

Second, this would have been easier to diagnose, but the line of code that should spit out the error has a typo in it, so produces a reference error instead. I'll have a pull request ready to fix that shortly.

bgmort added a commit to bgmort/warpjs that referenced this issue Oct 23, 2020
benjamminf added a commit that referenced this issue Oct 23, 2020
fix arguments error handling message. fixes #17
@benjamminf
Copy link
Owner

benjamminf commented Oct 23, 2020

@bgmort thanks for finding and fixing! There's a lot of corners I cut, particularly with parsing, that I look back on now and wonder what I was thinking. I've merged your PR and will put out a new release this weekend. Really appreciate that you took the time to surface the issue and put out a fix for it :)

I'm actually looking at using a proper parser for the path in the rewrite. Either by forking https://github.com/hughsk/svg-path-parser/blob/master/grammar.peg (which looks like it does parse arc commands correctly) or writing my own, not sure yet. But I'll ensure I support exactly what the spec defines in https://www.w3.org/TR/SVG/paths.html

@bgmort
Copy link
Author

bgmort commented Oct 24, 2020

No problem, thank you for the library! I'm a developer that does my own graphic design sometimes, and was disappointed to find that Affinity Designer has no support for warping text, but I was able to solve my problem using warp.js.

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

No branches or pull requests

2 participants