search

eclipse-lemminx / lemminx

XML Language Server
Eclipse Public License 2.0
272 stars 93 forks source link

ICompletionParticipant#onXMLContent is not called for CDATA #1699

Closed laeubi closed 2 weeks ago

laeubi commented 2 weeks ago

See

laeubi commented 2 weeks ago

@angelozerr can you review this?

This fixes the testcase for me, without my change testcase fails, with my change test-case succeeds.

angelozerr commented 2 weeks ago

The code looks good, but CI build fails https://github.com/eclipse-lemminx/lemminx/actions/runs/11721131221/job/32647917208?pr=1699#step:4:681

angelozerr commented 2 weeks ago

I wonder if request should have a flag like isInCDATA, because when apply completion is done, it must not remove the CDATA.

Perhaps we don't need it, but I think your test class should have some tests with apply completion with CDATA and without CDATA).

laeubi commented 2 weeks ago

Any hint how to best test that case?

laeubi commented 2 weeks ago

I looked at the failing test and I'm not sure if this needs to be adjusted, what seem to happen is that now a completion (to close the a tag) is suggested because before CDATA section was a "terminating" element, if I change the test to place the cursor before the cdata section then even three completions are suggested (including the close with a tag) so it seems that just the expectation of the test ist wrong.

laeubi commented 2 weeks ago

Now it works for windows but fails for mac/linux...?!?

angelozerr commented 2 weeks ago

Now it works for windows but fails for mac/linux...?!?

We have some tests which uses Timeout and perhaps they failed, I have restarted the CI build and it is working perfectly.

angelozerr commented 2 weeks ago

Ok let's merge the PR and when you will implement the feature with lemminx-maven we will see if there are some trouble.

Thanks @laeubi !

angelozerr commented 2 weeks ago

Fixes https://github.com/eclipse-lemminx/lemminx/issues/1694