djlint / djLint

✨ HTML Template Linter and Formatter. Django - Jinja - Nunjucks - Handlebars - GoLang
https://djLint.com
GNU General Public License v3.0
668 stars 82 forks source link

[BUG] [Formatter] H017 false positive #372

Closed danielniccoli closed 1 year ago

danielniccoli commented 2 years ago

System Info

Issue

djlint considers <meta charset="utf-8"> a problem, although the syntax is correct. I'm going to quote the MDN on this one:

An empty element is an element from HTML, SVG, or MathML that cannot have any child nodes (i.e., nested elements or text nodes).

The HTML, SVG, and MathML specifications define very precisely what each element can contain. Many combinations have no semantic meaning, for example an \ element nested inside an \


element.

In HTML, using a closing tag on an empty element is usually invalid. For example, <input type="text"></input> is invalid HTML

And for convencience, here's a link to the HTML specs that also shows meta being an empty element with some examples: https://html.spec.whatwg.org/multipage/semantics.html#the-head-element

How To Reproduce

<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
christopherpickering commented 2 years ago

@danielniccoli again, this is a preference. You cited a reference saying they are not nessicary. There will most likely turn up someone who will disagree with you... which is why djlint will let you disable codes you don't agree with... like other linters also do 😉

danielniccoli commented 2 years ago

@christopherpickering You can disagree with an opinion, but if you disagree with the specification (a.k.a the facts), then you're being esoteric.

Void elements only have a start tag; end tags must not be specified for void elements.

Source: The HTML Specification

christopherpickering commented 2 years ago

christopherpickering commented 2 years ago

Just to be clear, rule H017 is for self closing tags, suggesting to use <meta /> in place of <meta > or <img /> in place of <img >. I think the text you quoted is saying to not have a separate closing tag like </meta> or </img>, which I'm in agreement with.

Maybe the "self closing" part of the description isn't clear?

khink commented 1 year ago

@christopherpickering How is this a preference? The specs that @danielniccoli provided are quite clear: <meta ...> is the way it should be done. It makes no sense for djlint to complain about that.

I'm afraid that if i add an exception for H017, it will also not complain about tags that should be closed.

christopherpickering commented 1 year ago

I will reopen this.. perhaps we should split meta into their own rule.

cas-- commented 1 year ago

I have noticed this issue for a while and definitely feel it should be resolved. However it's not just meta it's all the void elements that should be a separate rule or just not flagged since there is a note of caution about marking a void element as self-closing:

From: https://html.spec.whatwg.org/#start-tags

Then, if the element is one of the void elements, or if the element is a foreign element, then there may be a single U+002F SOLIDUS character (/), which on foreign elements marks the start tag as self-closing. On void elements it does not mark the start tag as self-closing but instead is unnecessary and has no effect of any kind. For such void elements, it should be used only with caution — especially since, if directly preceded by an unquoted attribute value, it becomes part of the attribute value rather than being discarded by the parser.

christopherpickering commented 1 year ago

@cas-- I don't fully understand what you are recommending, h017 is only for void (self-closing) tags. @khink is suggesting that meta tags be split to their own rule.

What are you proposing as a solution?

cas-- commented 1 year ago

I can see now the list of rules for H017 is void tags with some obsolete tags.

Therefore the rule description is running counter to the HTML spec where self-closing tags don't exist, especially not for void elements. This is likely where the confusion is occurring in this thread since foreign elements can be self-closing due to potential XML elements:

The trailing U+002F (/) in a start tag name can be used only in foreign content to specify self-closing tags. (Self-closing tags don't exist in HTML.) It is also allowed for void elements, but doesn't have any effect in this case.

I think that @khink is highlighting the meta tag but this technically refers to all void elements since none of them should be closed.

What are you proposing as a solution?

Based on this new understanding and HTML spec I would consider the following:

The rule description could be reworded to include 'void elements' but I am not sure that would help. As mentioned by OP I would say that out-of-the-box requiring users to opt-out of something that is counter to the spec is confusing. Including that the spec specifically mentions not to use slash and even use caution if it is used.

christopherpickering commented 1 year ago

Probably the thing to do is make rules opt-in, such as the rule recommended in #571. I'm sure if that no-br rule showed up as a default, there would be a lot of hate mail 🤣

christopherpickering commented 1 year ago

I will let out a release with a few updates that will hopefully resolve this-

  1. Rule H017 has been divided into a rule H017 and H035. H035 is exclusively for meta tags, and H017 is any other void tag.
  2. Rule H017 name has been updated to say "void"
  3. Rule H017 and H035 are disabled by default. They can be enabled with a new --include flag, or through the config file.
christopherpickering commented 1 year ago

:tada: This issue has been resolved in version 1.20.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

khink commented 1 year ago

Thanks for the fix and release! :pray:

christopherpickering commented 1 year ago

Hey @khink, @cas--, @danielniccoli Have any of you formatted w/ prettier before? How do you handle their conversion of <img> to <img />? I don't see a config option to disable it.

image

I'm planning to update the formatter to auto close these tags and wonder how you currently handle this. It's required for react html as well.

christopherpickering commented 1 year ago

https://github.com/prettier/prettier/issues/5246 I guess they don't have a flag :) I will plan to add one if I change this for the formatter.

khink commented 1 year ago

I haven't seen this. In the project i'm working on now, we only run prettier on JS/CSS. Possibly in other projects we'd run it on the React HTML, but not the Django templates.