compulim / vscode-closetag

Quickly close last opened HTML/XML tag in Visual Studio Code
14 stars 5 forks source link

Don't close void elements #8

Closed druellan closed 7 years ago

druellan commented 7 years ago

Void elements like <img> technically CAN be closed, but I think the extension should avoid that, since in the end becomes confusing.

For example, if I have this code:

<div>
<div>
<img>
</div>

If I try to close that, I end with:

<div>
<div>
<img>
</div>
</img>

Wich is incorrect.

evanwarner commented 7 years ago

Yep, I'd call this critical functionality for an extension that does just this one job. (Why this isn't built in to VSCode anyway I have no idea.)

druellan commented 7 years ago

I see this marked "help wanted". I want to help but I'm not sure what you need. This is the list of void elements: https://www.w3.org/TR/html/syntax.html#void-elements

2017-04-19_16h11_14

Is not a big list, so, perhaps you can match the element being closed to this list, and if the tag is found, ignore and move to the previous tag.

compulim commented 7 years ago

Sorry for late response, thinking what we should do.

Even if the tags are void/self-closing tags (<img />), if we write them in React JSX, it is still valid in non-void style (<img></img>).

Maybe we should skip the unclosed tag (in this case, <img>). The algorithm works up way from the cursor: we should hit the closing tag, i.e. </img> before the opening tag <img>. If we hit something that is not supposed to be there, <img> in this case. We will just skip it.

compulim commented 7 years ago

@druellan Fixed in 1.0.0

compulim commented 7 years ago

@druellan By any chances if you think this and this fix looks good. Could you kindly re-evaluate the rating of this extension?

druellan commented 7 years ago

@druellan By any chances if you think this and this fix looks good. Could you kindly re-evaluate the rating of this extension?

Absolutely! Let me check.

Even if the tags are void/self-closing tags (), if we write them in React JSX, it is still valid in non-void style ().

Yeah, but lets not think about JSX as a valid HTML, since it is a custom implementation of the syntax. I think the extension must cover the standard first, and then we can discuss any special case.

druellan commented 7 years ago

Sorry, but I still don't see any difference. I'm using version 1.1.0 on Code 1.12.0-insider and is still closing voids on plain HTML files:

close_void

Perhaps I'm missing something?

compulim commented 7 years ago

Love your GIF screenshot, makes everything easier.

In the meanwhile, you will need to set configuration "closeTag.ignoreTags" to "html" to enable the HTML void tags. You can customize the set too.

I am thinking to have a document detection feature (#10). If the document type is HTML, it will setup the ignored tags for you automatically.

druellan commented 7 years ago

Oh, I get it, I was confused about how the "IgnoreTags" works.

I am thinking to have a document detection feature (#10). If the document type is HTML, it will setup the ignored tags for you automatically.

I don't think is going to work, there are many languages that can embed standard HTML, like PHP. I still believe that the best you can do is to pre-fill the closeTag.ignoreTags and let user adjust it if they feel is not working on their code, or detect specifically the JSX syntax, and not the other way around.

Remember that some of those tags can have nasty effects if you close them by accident, and it is a mistake that sometimes can be difficult to spot.

BTW, I can confirm this is working as expected!

    "closeTag.ignoreTags": {
        "area": true,
        "base": true,
        "br": true,
        "col": true,
        "embed": true,
        "hr": true,
        "img": true,
        "input": true,
        "keygen": true,
        "link": true,
        "menuitem": true,
        "meta": true,
        "param": true,
        "source": true,
        "track": true,
        "wbr": true
    },