deeplook / svglib

Read SVG files and convert them to other formats.
GNU Lesser General Public License v3.0
311 stars 80 forks source link

Refs #343 - Don't forget to render tail of text nodes #344

Closed claudep closed 2 years ago

claudep commented 2 years ago

The patch may need some more work.

lgtm-com[bot] commented 2 years ago

This pull request fixes 6 alerts when merging b96e17e8b6bd87682cbb697c79443a1ce2fe4aee into d1531420d48c10229eb6224c4254c9aebf908ecb - view on LGTM.com

fixed alerts:

lgtm-com[bot] commented 2 years ago

This pull request fixes 6 alerts when merging e2b9cc2fce671d80b309fb3004b5b57e5ae1ad54 into d1531420d48c10229eb6224c4254c9aebf908ecb - view on LGTM.com

fixed alerts:

lgtm-com[bot] commented 2 years ago

This pull request fixes 6 alerts when merging 2ef3517df3748646010a3dba9e0d6698f7294747 into d1531420d48c10229eb6224c4254c9aebf908ecb - view on LGTM.com

fixed alerts:

lgtm-com[bot] commented 2 years ago

This pull request fixes 6 alerts when merging da481091e9e84bd7d9dfbbbe98f0491264c7e4e9 into d1531420d48c10229eb6224c4254c9aebf908ecb - view on LGTM.com

fixed alerts:

claudep commented 2 years ago

@jspobst, if you want to test this PR, you are welcome!

jspobst commented 2 years ago

I just checked it out, and tested the example code I put in the issue. I can confirm that all of the text shows, but the formatting (ie, italics and bold) still doesn't work. The result:

image

If this PR was meant only to address some of the text missing before, I'd say it looks good (but doesn't fully close the corresponding issue). Admittedly, I might not understand all of what goes into 'testing this PR', so if that's more than simply generating one pdf, further testing by someone who understands best practices might be needed.

claudep commented 2 years ago

I thought you might have other real SVGs that you might test.

The style issue is a completely different thing. svglib doesn't yet support the shorthand font, but only the more specific font-* properties.