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

Add a little bit more whitespace removal by trimming. #83

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rodrigoaguilera
Copy link

When a svg with whitespaces or linebreaks at the end is passed those are preserved. I thought it would be better to remove them

@rodrigoaguilera
Copy link
Author

I forgot to mention that when you sanitize a file with no newline at the end the process adds an unnecessary newline to the output. I think is a bug in libxml.

For me this generates a diff between the sanitized and unsanitized version of the file that I hope it can be solved upstream.

@verdy-p
Copy link

verdy-p commented Sep 8, 2024

The minification of the "sanitized" SVG actually breaks it: it uses a basic replacement of all whitespace sequences into a single SPACE. This is not correct: this should not affect the whitespaces present within attribute values that take a quoted delimited string in one or several part of their values.

As well it does not minify the superfluous whitespaces present around braces, or around the colon separating an property name from its value, and does not remove the superfluous semicolon that terminates the last property within braces.

This should not be performed using a new lexical/syntaxic parsing of the result (or with the existing basic regexp substitution), but should be done instead and safely by generating the XML directly from the already loaded and sanitized DOM-like structure, where the "pretty-printing" generating whitespaces (and the extra semi-colon at end of property lists in braces) can be done (if not generating minified code), or suppressed entirely (if minifying).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants