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

XML CDATA minify issue #722

Closed
amoledwatchfaces opened this issue Jul 3, 2024 · 12 comments
Closed

XML CDATA minify issue #722

amoledwatchfaces opened this issue Jul 3, 2024 · 12 comments

Comments

@amoledwatchfaces
Copy link

amoledwatchfaces commented Jul 3, 2024

<![CDATA[ %d ]]> where empty space is before and after %d results in " %d" - space after %d is stripped.

Can this be improved in the future so CDATA maintains white spaces more properly?

tdewolff added a commit that referenced this issue Jul 4, 2024
@tdewolff
Copy link
Owner

tdewolff commented Jul 4, 2024

In the general case, XML whitespace should be preserved as it is up to the parser/application to decide if it's significant (e.g. whitespace at the beginning or end of a tag). In most cases it is not significant, much like HTML, which has been the default for the XML minifier. You should enable the KeepWhitespace option to keep all whitespace at the start/end of a tag's content.

@amoledwatchfaces
Copy link
Author

amoledwatchfaces commented Jul 4, 2024

@tdewolff

I've tried KeepWhitespace but it still removes empty spaces at the end of the CDATA content.

Can I somehow force this inside of the CDATA?

Unfortunately, my parser accepts empty spaces at the end and this is viable for my usecase.

@tdewolff
Copy link
Owner

tdewolff commented Jul 4, 2024

Do you have a small but complete XML example? This shouldn't happen inside a tag.

@amoledwatchfaces
Copy link
Author

amoledwatchfaces commented Jul 4, 2024

Do you have a small but complete XML example? This shouldn't happen inside a tag.

@tdewolff Here, test.xml, test-minify.xml & test-minify-keepwhitespace
test-xml.zip

test.xml
image

test-minify.xml (removes spaces completely)
image

test-keep-whitespace-minify (removes space at the end)
image

@amoledwatchfaces
Copy link
Author

I would expect from simple minify to output this?
image

@tdewolff
Copy link
Owner

tdewolff commented Jul 4, 2024

I believe that test-minify.xml outputs as expected. We can debate if the KeepWhitespace option should be on or off by default though.

The test-keep-whitespace-minify.xml is also as I expect. There is a whitespace before and after the %s part: before it is a space 0x20, and after it is a newline 0x0A. Both are valid whitespace. Perhaps we shouldn't collapse multiple whitespace as we do in HTML?

The original content between <Template> and <Parameter ... is: [space]%d[space][newline][tab][tab][tab][tab][tab][tab]. You say you only expect the spaces to be kept but all other whitespace to be removed, or do you wish to remove all whitespace? Then what is the purpose of minifying?

@amoledwatchfaces
Copy link
Author

amoledwatchfaces commented Jul 4, 2024

You say you only expect the spaces to be kept but all other whitespace to be removed

This. Application that parses this xml needs to maintain all spaces correctly inside CDATA. This is used in a watch face renderer.
So after minifying CDATA should keep all spaces correctly [0x20][%d][0x20].

Newline renders wrong result unfortunately. Perhaps there could be --xml-keep-cdata ?

minify --xml-keep-cdata which will remove all whitespaces as single 'minify' but keeps CDATA and its spaces correctly:
image

@tdewolff
Copy link
Owner

tdewolff commented Jul 4, 2024

I've added a fix that keeps all whitespace inside CDATA as-is.

@amoledwatchfaces
Copy link
Author

Thanks! Two coffees on me ;)

@tdewolff
Copy link
Owner

tdewolff commented Jul 4, 2024

Awesome, thank you very much!!

@tdewolff
Copy link
Owner

tdewolff commented Jul 8, 2024

I've released v2.20.36 with this fix!

@amoledwatchfaces
Copy link
Author

I've released v2.20.36 with this fix!

Thanks! 🙂

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

No branches or pull requests

2 participants