Prevent exception in DomElementFormatter.formatEndTagElement()?

[enxio]

If I could have made a pull request I would have. Sorry about that. I seem to lack the necessary git-fu skills.

So, in DomElementFormatter.java line 296

if ((element.getLastChild().isElement() || element.getLastChild().isComment()) ...

an exception can occur (and it has indeed over here) when element.getLastChild() becomes null. Well, It does no longer occur with the following change:

if (element.getLastChild() != null && (element.getLastChild().isElement() || element.getLastChild().isComment()) ...

Maybe this matches your style and could be considered? Or something similar.

[vrubezhny]

@enxio Can you provide the steps to reproduce that cause this NPE to happen?

[enxio]

It happens in a rather complex (internal) document. I don't think I can track this one down in a timely manner. The formatter is acting "weird" in that document as well. It seems valid though and its well-formed according to the DTD... If I find the issue I will give notice. Sorry, again.

[vrubezhny]

@enxio Maybe you can provide an XML document (if it's not restricted) for which the formater has weird behavior or at least part of such document that help to reproduce?

[enxio]

It's annoying as hell, believe me :) So, if I find something to reproduce, I will provide it!

[fbricon]

Which version of lemminx are you using, with which client?

[enxio]

The exception was happening with version 0.22.0. The client is VS Code. The issue above was tried out using a compiled version of "latest".

[fbricon]

have you tried setting "xml.format.experimental":true?

[enxio]

Yes I have.

[angelozerr]

have you tried setting "xml.format.experimental":true?

@fbricon the problem comes from the experimental formatter.

Thanks @enxio to have reported the problem.

Thanks @vrubezhny to have created the PR #1362

[angelozerr]

The formatter is acting "weird" in that document as well.

What is weird? The result of formatter for your XML file? For any XML files?

[enxio]

First of all: I like the experimental formatter very much!

But in that one XML file only, the formatter does not seem to do anything. Indents are misaligned, it does not fix excess whitespace. Empty lines are not removed as expected. etc.

As I said, XML is according to the DTD, no red squigglies there.

[fbricon]

if the formatter chokes on that NPE, it's expected it doesn't format anything

[angelozerr]

First of all: I like the experimental formatter very much!

Thanks!

@enxio is your XML is bind with an XSD?

[enxio]

A DTD, actually.

[angelozerr]

Ok I can reproduce it.

Given this DTD:

<!ELEMENT foo (#PCDATA)>

Given this XML:

<?xml-model href="foo.dtd" ?>
<foo></foo>

If you try to format XML, you will have the NPE.

The new experimental formatter is smart and detect that <foo></foo> is not a mixed content. So if you don't associate to the DTD, the format will work.

Now if you bind with the foo.dtd which defines foo as mixed content and you have this settings to true https://github.com/redhat-developer/vscode-xml/blob/main/docs/Formatting.md#xmlformatgrammarawareformatting (by default), the formatter will force the foo element as mixed, and you will have the NPE problem.

@vrubezhny could you write a test with this usecase in your PR please. If you need some help, please ask to @JessicaJHee. Thanks!

[angelozerr]

@enxio for the moment, if you disable https://github.com/redhat-developer/vscode-xml/blob/main/docs/Formatting.md#xmlformatgrammarawareformatting, your XML should be formatted correctly.

Is it working by disable this setting?

[enxio]

Yes, it works as expected when the setting is false. Thanks a lot!!!

[angelozerr]

Yes, it works as expected when the setting is false. Thanks a lot!!!

Glad it works for you. When PR #1362 will be merged, and vscode-xml build will be finished, you could install the pre-release to enjoy with grammar aware formatting settings again.

It should be realy nice if you could test again our formatter, because we want to switch soon our current formatter with experimental formatter.

It should be really nice too when #1359 will be merged that you test max line width settings.

In other words, any feedback are welcome!

[angelozerr]

@enxio could you install the vscode-xml pre-release and give us feedback (your issue should be fixed) and if you could play with maxLineWidth settings to give us some feedback. Thanks!

[enxio]

Hi @angelozerr, it turns out, that version 0.22 and up did change the formatting in a way that is a bit confusing. In 0.21 a file containing

<t>a    b</t>

was simply formatted to

<t>a b</t>

Now with 0.22 and up, this is no longer happening regardless of what the grammar aware setting actually is. I also played around with the maxlinewidth (setting it to something low like 80) which also had no visible change to a document that contains many long lines.

I also noticed that sometimes it takes a VERY long time to format the document (several seconds). I am not sure whether this has anything to do with it, in the console I saw that there were some badlocationexceptions(?) being thrown. Anyway, somtimes after restarting VS Code, the format, of the same document, was instant. I am also using the latest VS Code, so I am not sure where this might come from.

[enxio]

Oh, I'm willing and happy to help tracking down issues :)

[fbricon]

@enxio "xml.format.joinContentLines": true will normalize spaces

[fbricon]

and 0.21.0 behaves the same as 0.22.0 wrt content spaces. You probably had "xml.format.joinContentLines": true at some point

[enxio]

Actually, it seems that "xml.format.preserveEmptyContent": false did the trick here. If set to true, spaces are not normalized. In fact I did have set "xml.format.joinContentLines": true when I wrote the above example.

[angelozerr]

@enxio could you create an issue per problem please (performance, normalized spaces which is not done corectlt, etc) by given us the most detail (ex : attache the file which causes the performance problem), Thanks

[angelozerr]

Actually, it seems that "xml.format.preserveEmptyContent": false did the trick here.

@JessicaJHee have we not remove this settings for experimental formatter?

[angelozerr]

Now with 0.22 and up, this is no longer happening regardless of what the grammar aware setting actually is.

You can disable grammar aware settings if you wish. If you activate grammar aware settings, teh DTD, XSD rule will win, i snot teh expected behavior?

I also played around with the maxlinewidth (setting it to something low like 80) which also had no visible change to a document that contains many long lines.

Please create issue with sample please.

[enxio]

@angelozerr I did create the issue with the preserverEmptContent issue. That one is easily to reproduce.

I could not reproduce the line length "issue", which worked as expected in the sample file I used. I also could not (yet) reproduce the performance issues. Well, FWIW, to me it seems that after a while and editing (insert/delete/format), the responses become quite slow (several seconds for a format). Especially in larger/complex documents. If I re-start VS Code, the performance is usually as expected. If I can reproduce it, I will file an issue.

Thanks for your help!

[angelozerr]

I could not reproduce the line length "issue", which worked as expected in the sample file I used.

Ok, if you find some issues with max line length, please create an issue.

I also could not (yet) reproduce the performance issues. Well, FWIW, to me it seems that after a while and editing (insert/delete/format), the responses become quite slow (several seconds for a format). Especially in larger/complex documents. If I re-start VS Code, the performance is usually as expected. If I can reproduce it, I will file an issue.

I don't like that -( if you can share your XML file it should be nice, if you can share the output traces when you have this problem it should bee nice too. Thanks.

[angelozerr]

Well, FWIW, to me it seems that after a while and editing (insert/delete/format), the responses become quite slow (several seconds for a format). Especially in larger/complex documents. If I re-start VS Code, the performance is usually as expected. If I can reproduce it, I will file an issue.

@enxio is there any chance that you share your XML please?

[enxio]

@angelozerr unfortunately not. Its format and content is proprietary. You mentioned output traces would help. How do I turn them on?

[enxio]

@angelozerr for the record: the performance "issue" was a very naive configuration of another VS Code extension. Once that configuration was corrected, the performance of lemminx during formatting was as expected.

[angelozerr]

@angelozerr for the record: the performance "issue" was a very naive configuration of another VS Code extension. Once that configuration was corrected, the performance of lemminx during formatting was as expected.

You mean your performance problem comes from an another vscode extension?

[enxio]

@angelozerr yes, that looked to me as the most likely cause. I changed the regex culprit in the other extension into something sane and voilà lemminx quick as weasel again.

I did not notice this because I had that insane regex active for a long time and the impact was not noticeable in the past.

[angelozerr]

I wonder why this vscode extension impacted lemminx?