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

do not strip trailing white space in XML elements (bsc#1195910) #1241

Closed
wants to merge 4 commits into from

Conversation

wfeldt
Copy link
Member

@wfeldt wfeldt commented Feb 16, 2022

Task

Surrounding white space in some elements may be desired. Like in scripts or other file content. Be more careful when removing it.

The catch

The AutoYaST profile is processed by YaST to construct the final profile. It's written, modified by scripts, then read again, for example. Ensure that during this processing the CDATA 'attribute' to elements is not lost.

Solution

Strip leading white space when reading XML CDATA blocks. And create CDATA blocks when generating XML if an element contains trailing white space.

Note

Why not also keeping leading white space? - It's common to have a leading newline in script sections. They must be removed.

@coveralls
Copy link

coveralls commented Feb 16, 2022

Coverage Status

Coverage increased (+0.006%) to 41.638% when pulling 6151ff4 on sw_10 into 6b08011 on master.

# Do NOT strip trailing white space in CDATA blocks. Maybe people put
# it intentionally there (bsc#1195910).
text_nodes = node.xpath("text()")
text = text_nodes.map { |n| n.cdata? ? n.content.lstrip : n.content.strip }.join
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there reason for that lstrip in cdata? I expect it should be as it is

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You want to strip leading newlines in scripts.

Copy link
Member

@jreidinger jreidinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general what I miss here is unit testing demonstrating issue.
And I am not sure if we should also check serializer, if to_xml parse_xml pair of operation does not strip newlines, which can be also issue.

@wfeldt
Copy link
Member Author

wfeldt commented Feb 16, 2022

replaced by #1243

@wfeldt wfeldt closed this Feb 16, 2022
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.

3 participants