endroid / qr-code

QR Code Generator
https://endroid.nl
MIT License
4.45k stars 726 forks source link

Rethink SvgWriter::WRITER_OPTION_EXCLUDE_XML_DECLARATION #312

Closed ThomasLandauer closed 2 years ago

ThomasLandauer commented 3 years ago

Loosely following up on https://github.com/endroid/qr-code/issues/151, some thoughts about this:

  1. For readability, it would be better to avoid the double negation and rename it to include_xml_declaration
  2. According to https://stackoverflow.com/a/38172170/1668200, for inline SVG, it's always better to exclude the XML declaration; so this should be the default for qr_code_data_uri() of the Symfony bundle: https://github.com/endroid/qr-code-bundle#generate-via-twig
  3. Generally, omitting the XML declaration for an external SVG doesn't harm anybody - since the declaration you're adding (<?xml version="1.0"?>) is the default anyway ;-) On the other hand, including an XML declaration on an inline SVG might irritate some parsers. Conclusion: When in doubt, it's safer to exclude it, so this should be the default always.
  4. If somebody really wants to have it, it's very easy to just add this line manually above the outputted QR code. On the other hand, removing it manually is not so easy.

So in total I'd say: Drop this option completely, and always exclude the XML declaration, and add a short note to README that people might want to add this single line manually for external SVG's. I think this would make it easier for anybody :-)

endroid commented 2 years ago

Cleaning up, planned for nex major https://github.com/endroid/qr-code/issues/386