Could CloseTagCodeAction offer option to close and result with an empty element.

scottkurz commented 1 year ago

Environment

Eclipse for Java Developers v4.27 (2023-03)
Wild Web Developer XML tools 1.0.2.202301201156 org.eclipse.wildwebdeveloper.xml.feature.feature.group Eclipse Wild Web Developer project
- This seems to include Lemminx v0.23.0
Liberty Tools 0.9.1.202305311908 (includes Lemminx extension)

Example Data

With XML (server.xml) like this:

<server>
     <!-- ... -->
    <webApplication id="my-app" location="my-app.war" name="my-app">
    <applicationManager autoExpand="true"/>
    <applicationMonitor dropinsEnabled="false"/>

    <!--Default SSL configuration enables trust for default certificates from the Java runtime -->
    <ssl id="defaultSSLConfig" trustDefaultCerts="true" />
</server>

I get a quick fix / code action option to close the webApplication element </webApplication> like this:

Problem Result

If I click to apply the quick fix the </webApplication> gets inserted "at the end", i.e. before the close tag of the parent element (<server>) containing <webApplication>.

    </webApplication>
</server>

Desired Behavior

In this case, I want to simply close out the start tag and produce an empty element: <webApplication ... <attrs>.../>. I don't want to include the other elements as content.

Now, one could argue, if I see the red X and know this is what the problem is, it'd be almost as quick to click the cursor and insert the / char myself instead of relying on the tool.

But..imagine it's scrolled off the screen. Or, maybe I didn't know what the problem was until I hovered over the quick fix suggestion, and at this point I expect now to be able to resolve the issue with this action (even though I could do it myself with a single char now).

Note on Code

Stepping through in the debugger, it seems like this behavior is controlled by the CloseTagCodeAction#doCodeActionsForStartTagClosed method. I don't see that it has logic to perform the behavior I'm describing, but I could be missing something.

Note on Extension

The Liberty Tools Lemminx extension works with server.xml, which is governed by an XSD (similar to this. I don't see this CloseTagCodeAction impl taking this into account either, but I mention this fact in case this is somehow relevant but I'm not aware.

angelozerr commented 1 year ago

The basic idea of close tag action like you have debuged it is to insert the end tag according the fault tolerant parser which set following elements of an element which is not closed as children. You can see that in the outline:

In this case, I want to simply close out the start tag and produce an empty element: <webApplication ... .../>. I don't want to include the other elements as content.

If I understand, you would like to have a new code action which inserts end tag just after the start tag. Are you interested to implement this kind of code action?

The Liberty Tools Lemminx extension works with server.xml, which is governed by an XSD (similar to this. I don't see this CloseTagCodeAction impl taking this into account either

Indeed close code action doesn't take care of XSD (in lemminx we call that CMDocument which is an abstract of XSD, DTD, RNG, etc), it could be nice to takes care of XSD.

In conclusion I think it could be nice to have:

startegy 1: a code action which insert after all children (like today)
startegy 2: a code action which insert teh end tag just after the start tag (your requirement)
if XML is mapped with a XSD, DTD, try to the proper startegy.

@scottkurz are you interested to work on it?

scottkurz commented 1 year ago

The basic idea of close tag action like you have debuged it is to insert the end tag according the fault tolerant parser which set following elements of an element which is not closed as children. You can see that in the outline:

If I understand, you would like to have a new code action which inserts end tag just after the start tag. Are you interested to implement this kind of code action?

@scottkurz are you interested to work on it?

Thank you for your comment and reply. As a start, it's a big help just to hear you confirm I'm understanding this correctly and Lemminx isn't already (supposedly) doing this. I am interested in working on this though I'm not sure exactly when, so I guess we can leave this issue open for now.

In conclusion I think it could be nice to have:

* startegy 1: a code action which insert after all children (like today)

* startegy 2: a code action which insert teh end tag just after the start tag (your requirement)

* if XML is mapped with a XSD, DTD, try to the proper startegy.

Yeah, the last one sounds like to me like a whole other level of complexity, so I would try to start just with 'strategy 2' here. Thx again for your explanation.

angelozerr commented 1 year ago

'm not sure exactly when, so I guess we can leave this issue open for now.

Don't hesitate to ask me if you need some assistance. We have a lot of test with code action, it can be a good start to play with them and write some tests with your future feature.

eclipse / lemminx