Made picosvg ignore <?xpacket?> tags.

[zond]

• Abstracted out "check if tag is a comment" into _is_redundant(tag).
• Added etree.ProcessingInstruction to the set of redundant tags.
• Created function to remove processing instructions.
• Changed topicosvg to remove processing instructions.
• Added test to ensure xpacket tags no longer break picosvg, and are
  actually removed when topicosvg is called.

[anthrotype]

hi! thanks for the PR, the changes look good to me. Just one thing about the test. We don't need to create a whole new test module just for the xpackets, it's sufficient that you add a test inside tests/svg_test.py.

Also, you could assert that the svg no longer contains the xpacket elements.

[anthrotype]

regarding the CI lint job failing, you can fix it by installing the black python auto-formatter (pip install black) and running it on the files that you modified.

[zond]

Hey @anthrotype,

I moved the test to svg_test.py and removed xpacket_test.py.

The thing is - I only ignored ?xpacket the way comments are ignored - and that was evidently enough to not crash a call to simplify(), but the ?xpacket is still there.

WDYT?

• Should it actually be removed?
• Should comments actually be removed? (They don't seem to be now, just ignored)

[anthrotype]

Should comments actually be removed?

they are removed, but not in simplify. The SVG.topicosvg() is the main public entry point, and that one calls a bunch of other methods like remove_comments or remove_nonsvg_content or remove_title_meta_desc (BTW, if your xpacket elements are inside a metadata element, like they seem to be in the test svg file you uploaded, then they should be removed by the latter method).

The simplify method is about removing groups, applying clip-paths and transforms (stuff that needs to be done all at once as opposed to one at a time) and isn't really meant to be called in isolation from the rest, but as part of topicosvg public method, and it's not currently specifically tested on its own, as you can see. But I understand you would like to apply only some picosvg transformations and not all of them, hence your need to call directly the simplify method instead of the entire topicosvg method with all the accompanying transformations.

How about you add a new remove_processing_instructions method (similar to remove_comments, and should be also called inside topicosvg) whichdoes what you want and add a specific unit test for that (instead of calling simplify)?

[zond]

Ah, now it makes more sense. Thank you for explaining.

I updated the PR to add a function to remove processing instructions (like comments are removed), and made topicosvg use it. See what you think!

[zond]

Cool, thanks!

I am happy with this PR now, but I don't have write access, so please merge at will :)

[anthrotype]

Thank you!

[zond]

Likewise! Woohoo!

[anthrotype]

FYI, I made a minor fixup to the tests here

[zond]

Ah, that's much nicer - thanks!