Improve text formatting for experimental formatter

[JessicaJHee]

Fixes #1331

Signed-off-by: Jessica He [email protected]

[datho7561]

I am good with this change. Thanks Jessica! I think we discussed waiting until after the release to make this change though.

[datho7561]

@angelozerr what do you think about this change? Jessica wanted to wait until after the release before merging it, but I think it should be good to go now.

[angelozerr]

I'm really sorry @JessicaJHee @datho7561 to not have reviewed the PR -(

When I see tests, it seems we are different from Oxygen formatter, right? It doesn't follow the behavior described in https://github.com/redhat-developer/vscode-xml/blob/main/docs/Formatting.md#xmlformatexperimental ?

[angelozerr]

I wonder if this PR should cover the usecase:

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

to indent correctly bar

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

but perhaps teh Oxygen behavior doesn't allow that?

[JessicaJHee]

When I see tests, it seems we are different from Oxygen formatter, right? It doesn't follow the behavior described in https://github.com/redhat-developer/vscode-xml/blob/main/docs/Formatting.md#xmlformatexperimental ?

Yes it is a bit different when it comes to text formatting since oxygen will always join text content/ normalize the space between text content by default. It is more similar to IntelliJ. I waned to see how you feel about the changes since it is quite different from before. There are a lot of cases to consider but I think with this PR it would be easier to discuss how we want the behavior for text formatting to be like!

I wonder if this PR should cover the usecase:

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

to indent correctly bar

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

but perhaps teh Oxygen behavior doesn't allow that?

I wasn't sure because in this case we normalize the space if the content is empty (contains only white space and new lines). Unfortunately I don't have access to try oxygen anymore.
https://github.com/eclipse/lemminx/blob/037589a61986a348f3acd5849f900406e28211cb/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/services/format/experimental/XMLFormatterPreserveEmptyContentTest.java#L45

[angelozerr]

Yes it is a bit different when it comes to text formatting since oxygen will always join text content/ normalize the space between text content by default. It is more similar to IntelliJ. I waned to see how you feel about the changes since it is quite different from before. There are a lot of cases to consider but I think with this PR it would be easier to discuss how we want the behavior for text formatting to be like!

Indeed we need to specify more teh behavior of the formatting. My wish is to have the IJ and Oxygen behavior both (with a new settings).

I think it is important to preserve Oxygen behavior, because it fixed #594 (I suggest that you read all comments). If we could provide a new settings which format ala IJ, it should be very nice.

I wasn't sure because in this case we normalize the space if the content is empty (contains only white space and new lines).

IMHO I think we should take care of this usecase (setting a la IJ).

[JessicaJHee]

After reading the previous issue on this, I think it makes sense to set joinContentLines = true by default so we have the same behavior as oxygen. We could have the behavior I have in this PR when joinContentLines = false (more similar to IJ)? We still have preserveEmptyContent for when the user does not want the spaces modified within tags.

I have added a few tests that relate to the sample provided in the issue mentioned in XMLFormatterMaxLineWithTest.java - mixedText()

For the case empty tags, perhaps we could add a setting (something like 'normalizeSpaceInEmptyElement') if necessary?

[datho7561]

joinContentLines = true by default so we have the same behavior as oxygen

I think that this is a good idea. The Oxygen formatter handles mixed content very well, so using this behaviour by default is a good idea.

IMHO I think we should take care of this usecase (setting a la IJ). (angelo)
For the case empty tags, perhaps we could add a setting (something like 'normalizeSpaceInEmptyElement') if necessary? (jessica)

What should the default behaviour be for empty tags? I think it would be nice to completely remove the content (eg. <aaa></aaa>) unless preserveEmptyContent is enabled. I don't know what oxygen's default behaviour is, but this is what Intellij does.

[angelozerr]

Impressive @JessicaJHee ! Thanks so much. Now we should update vscode-xml doc.