Skip to content

xmerl: Fix exporting XML comments #9796

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

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

Conversation

ptome
Copy link

@ptome ptome commented May 5, 2025

Fix issue #5697 by exporting #xmlComment elements when calling export/3 or export_simple/3 and similar functions.

If we want to preserve the original document formatting, this will be impossible to solve only from the export side. The xmerl_scan functions strip white space after comments when creating the element records, and the original text elements are lost.

Top level comments like the example in #5697 will only be exported if we create and export as a document. There is no function to export an #xmlDocument directly, could be a possible enhancement.

The callback module xmerl_xml_indented doesn't indent properly if input data has text elements used for indentation. I allow for indentation of comments like the example in test 45 formatter_pass where there is no indentation in the input. I updated the test case to insert the newly exported comment.

I tried to implement an alternative PR that preserves formatting by changing xmerl_scan to not strip white space after comments. These would be read as extra text elements. Unfortunately this breaks more than 60 unit tests, mostly related to DTD validation. It would be a larger PR and introduce more risk of breaking something. I didn't have time to investigate a solution for this alternative to work.

Fix issue erlang#5697 by exporting #xmlComment elements when calling
export/3 or export_simple/3 and similar functions.
@CLAassistant
Copy link

CLAassistant commented May 5, 2025

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

github-actions bot commented May 5, 2025

CT Test Results

Tests are running... https://github.com/erlang/otp/actions/runs/14847703982

Results for commit 4f21437

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

  • No CT logs found
  • No HTML docs found
  • No Windows Installer found

// Erlang/OTP Github Action Bot

@rickard-green rickard-green added the team:PS Assigned to OTP team PS label May 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team:PS Assigned to OTP team PS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants