POM XML formatter incorrectly adds newlines before CDATA sections

[garretwilson]

(This was originally filed as eclipse-m2e/m2e-core#1015, but I was told I need to file it with Eclipse Wild Web Developer.)

Using Eclipse 2022-09, I can add a CDATA section inside a Maven POM (pom.xml) property definition like this:

<properties>
    <test.cdata><![CDATA[<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE properties SYSTEM "http://java.sun.com/dtd/properties.dtd">
<properties>
	<entry key="foo">bar</entry>
</properties>]]></test.cdata>
</properties>

The purpose is obvious: I don't want to XML-encode all the XML delimiters in the value.

However when I format the file using Ctrl+Shift+F, Eclipse adds newlines and indents before and after the CDATA section:

<properties>
    <test.cdata>
      <![CDATA[<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE properties SYSTEM "http://java.sun.com/dtd/properties.dtd">
<properties>
	<entry key="foo">bar</entry>
</properties>]]>
    </test.cdata>
<properties>

This corrupts the data! The whole point of using a CDATA section is that I wanted the exact content specified in a property. Now extra whitespace has been added in the value of the property. (It would have been worse to format inside the CDATA section, but this is still not good.)

I would guess the problem is that the formatting algorithm is treating CDATA sections as if they were XML DOM elements. That is, I understand that if I have this XML:

<foo><bar>…</bar></foo>

Then the XML formatter should format it to:

<foo>
  <bar>…</bar>
</foo>

However CDATA sections are not elements! Their purpose is merely to indicate whether content needs escaping or should be interpreted literally. They do not create a new element nesting context!!! They must be left along entirely; no newlines, whitespace, or anything else should be added, neither inside the CDATA section, before the CDATA section, nor after the CDATA section.

[mickaelistria]

Thanks. I could reproduce the issue it in latest snapshots of Wild Web Developer. However, the problem is not "hosted" by Wild Web Developer itself but by the LemMinX language server that is used by Wild Web Developer to provide formatting (among other things).
Now that you're getting familiar with opening the same issue on multiple projects as investigation goes on, would you mind bringing this issue to https://github.com/eclipse/lemminx/issues ;) I really believe it will be the final destination here.

[fbricon]

LemMinX's new experimental formatter already supports this use case. You can check it in VS Code with the XML extension, after setting "xml.format.experimental": true in the settings.

WWD should be updated to expose that setting. So eclipse/wildwebdeveloper#941 still stands

[angelozerr]

WWD should be updated to expose that setting. So #941 still stands

@fbricon we would like to switch the experimental formatter as current formmater ASAP when it will supports all features from the current formatter. @JessicaJHee works hard on this topic and I think we are in the good way.

In other words WWD should not expose this setting which will disappear in few months.

[angelozerr]

@garretwilson it should be really nice if you could install vscode and vscode-xml and configure the formatter as experimental liake @fbricon has explained to play with experimental formatter and give us any feedback.
More we will have feedback more we could fix issues and switch the current formatter to the experimental formatter.

Thanks for your help!

[garretwilson]

I'm a little confused about your request for me to install VSCode and vscode-xml and play with it. Are you asking me to do this just in general, or specifically in reference to this bug report I filed?

I assume you're just asking me that in general … because in relation to this bug specifically, surely you'll be adding this defect test case to part of the unit test and regression test suite, no? Surely?

[garretwilson]

In other words WWD should not expose this setting which will disappear in few months.

I have gut feeling that "in a few months" will get pushed back to "sometime next year" or even later, and in the meantime we have a bug that corrupts content. In an ideal world, you would fix it in the legacy code even before you switch to the new revamp (whenever that will be ready).

[angelozerr]

I assume you're just asking me that in general

Yes in general, just to check that the experimental formatter format XML as your expectation. But if you have no time to do that, I understand, no problem.

I have gut feeling that "in a few months" will get pushed back to "sometime next year" or even later

I'm generally a pessimist, it is the reason why I said "few months", but @JessicaJHee is working hard on this experimental formatter, and perhaps the formatter will be ready in "few weeks", but I don't want to promise a given date to avoid putting pressure on us.

I have read your XML formatter https://github.com/globalmentor/globalmentor-web/blob/main/xml/src/main/java/com/globalmentor/xml/XMLSerializer.java and basicly our current formatter uses the same strategy : it rewrites the full content of the Document or a given part of XML (when format is done with range).

We noticed that it is a bad strategy (for IDE/Editor XML format), because when you have some invalid XML content, you loose the invalid content. The experimental formatter follows a new strategy which keep the existing content and add / remove expected spaces. This strategy provides the capability to support invalid content but another feature like max line lengh, preserve attributes, etc.

You can see doc with experimental formatter at https://github.com/redhat-developer/vscode-xml/blob/main/docs/Formatting.md#xmlformatexperimental and all settings that it is supporting.

[garretwilson]

As requested I filed eclipse-lemminx/lemminx#1360, but there they claimed that lemminx already supports this and that WWD needs to be updated.

@mickaelistria said in that ticket that WWD would wait until lemminx fixes the bug by default (i.e. without needing a setting change), but lemminx in that ticket never said when they would be making the behavior default, or even if they intended to make it default ever! So they closed their ticket, and this ticket is in limbo.

Can someone provide more clarity on the actual status? It would seem that WWD is waiting on something that no one ever promised, on a ticket that has been closed.

[garretwilson]

I have read your XML formatter … and basicly our current formatter uses the same strategy : it rewrites the full content of the Document or a given part of XML (when format is done with range).

Correct. My formatter was written for a different use case (although it could be modified to support the WWD use case). My point in mentioning it was to point out how easy it is to distinguish CDATA from element DOM nodes. I later removed the reference altogether because I didn't want the discussion to get sidetracked.

The experimental formatter follows a new strategy which keep the existing content and add / remove expected spaces. This strategy provides the capability to support invalid content but another feature like max line lengh, preserve attributes, etc.

… which statement brings some irony with #1006 that now the new formatter is throwing out valid blank lines that the old formatter did not, no? 😉

[mickaelistria]

In latest snapshot, Wild Web Developer adopted latest release of LemMinX which has eclipse-lemminx/lemminx#1360 fixed by default I believe. So please just retry with the latest snapshots and maybe this is fixed.
Note that what's called "new formatter" in LemMinX is a new new formatter: there is the legacy formatter (provided by WTP project), the old LemMinX formatter (used by previous LemMinX release and by current release of Wild Web Developer) and the new LemMinX formatter (which is part of latest LemMinX release and latest snapshots of Wild Web Developer). With that last combination, you should now have options to make #1006 work for you, and have fix eclipse-lemminx/lemminx#1360 included without particular option.

[angelozerr]

In latest snapshot, Wild Web Developer adopted latest release of LemMinX which has eclipse-lemminx/lemminx#1360 fixed by default I believe.

Indeed, now the experimental formatter become the current formatter. We give up the old formatter.

So please just retry with the latest snapshots and maybe this is fixed.

Indeed it should fix the issue.

[angelozerr]

I think you can close this issue.

[sebthom]

I cannot reproduce the issue anymore. Feel free to reopen if problem persists.