meerk40t / svgelements

SVG Parsing for Elements, Paths, and other SVG Objects.
MIT License
132 stars 29 forks source link

Improve __repr__ and Viewbox #117

Closed Sophist-UK closed 3 years ago

Sophist-UK commented 3 years ago
  1. Viewbox init now handles various forms of arguments so it can recreate from a repr:
    • "x y w h"
    • x=x etc.
  2. Viewbox creates a correct better repr.
  3. SVGImage adds missing attributes, puts the href (which is potentially huge) last and (I hope this is correct) puts the href attribute in quotations so that the end is easily parseable.
  4. Various objects parse preserve_aspect_ratio as an attribute when the SVG attribute is preserveAspectRatio . Backwards compatibility is preserved.
Sophist-UK commented 3 years ago

I believe this is ready for merging subject to review.

tatarize commented 3 years ago
  1. the internal values are always pythonic snake-case. The names as text strings use the exact svg values. See other examples, this is consistent throughout.
SVG_ATTR_PATTERN_CONTENT_UNITS = "patternContentUnits"
self.pattern_content_units = values[SVG_ATTR_PATTERN_CONTENT_UNITS]

SVG_ATTR_CLIP_UNIT_TYPE = "clipPathUnits"
self.unit_type = SVG_UNIT_TYPE_USERSPACEONUSE
Sophist-UK commented 3 years ago

Apologies for breaking backward compatibility. Hopefully this is now OK.

Sophist-UK commented 3 years ago

Ok - hopefully this should be backwards compatible and addresses all concerns (which have also had unit tests added for).

Sophist-UK commented 3 years ago

I thought that conformance to svg was more important.

It could potentially have been backwards incompatible, but:

  1. The repr for Viewbox was new, so no backwards to be incompatible with.
  2. So long as input was backwards compatible, repr seems to be something where backwards compatibility is less important.
  3. If we were going to make backwards compatibility inviolate, then any change would be backwardly incompatible.

Now you have brought my attention to there being other examples, I will put making similar "fixes" on my to-do list. 😉 But before you get too worried, getting to the top of the list seems unlikely.

Sophist-UK commented 3 years ago

stroke-width is not handled properly.

some attribute name are hard-coded in repr rather than referring to the attribute contant.

but this is low priority so not aiming to address either of these any time soon.