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

Rethink SvgWriter::WRITER_OPTION_EXCLUDE_XML_DECLARATION #312

Closed
ThomasLandauer opened this issue Apr 8, 2021 · 1 comment
Closed

Rethink SvgWriter::WRITER_OPTION_EXCLUDE_XML_DECLARATION #312

ThomasLandauer opened this issue Apr 8, 2021 · 1 comment
Assignees

Comments

@ThomasLandauer
Copy link
Contributor

Loosely following up on #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
Copy link
Owner

endroid commented Oct 4, 2022

Cleaning up, planned for nex major #386

@endroid endroid closed this as completed Oct 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants