Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support allow list of self-closing tags. Fixes #15 #16

Merged
merged 1 commit into from
Nov 29, 2018

Conversation

jonathanKingston
Copy link
Contributor

It likely needs some clean-up to simplify those conditionals however, feedback would be great!

@bodil
Copy link
Owner

bodil commented Nov 29, 2018

Are you sure this goes the right way? What I mean is, I think ordinarily the <tag/> way of closing tags will work with any tag, but there are a few edge cases which won't, like, iirc, <script></script>, which would thus need to be treated differently from the norm. I've no idea what the standard says on this issue, but I'm pretty sure eg. <div/> would work? I may be horribly mistaken about this, but thought it best to ask.

@jonathanKingston
Copy link
Contributor Author

Hey, er so my understanding is that only void elements are used to self close like this so the closing tag can be omitted (so can the trailing slash in HTML5).
I think you might be conflating the correction rules on tag ommission: https://html.spec.whatwg.org/multipage/syntax.html#optional-tags
As far as I am aware:

<div /><span /><script>boop()</script>

Would end up being in most browsers:

<div><span><script>boop()</script></span></div>

Where as the correction rules would ignore / and work for <p>:

<p/>text<div/>

Would be:

<p>text</p><div></div>

Would it make sense to codify void elements into a trait? I couldn't work out how I would branch on a marker trait though.

@bodil
Copy link
Owner

bodil commented Nov 29, 2018

No, you can't branch on a marker trait, it doesn't even exist at code generation time, which is why currently things like this need to be hardcoded in the macros package, annoying as it may be.

I think I'll just merge it as is and see if a more elegant solution presents itself later. It'll complicate the adding of custom tags, but I suppose just assuming none of them are self closing is still much better than nothing.

@bodil bodil merged commit e80c8e6 into bodil:master Nov 29, 2018
@jonathanKingston
Copy link
Contributor Author

Thanks @bodil! :)

It'll complicate the adding of custom tags, but I suppose just assuming none of them are self closing is still much better than nothing.

Yeah they currently never support self-closing tags but perhaps the parser could output with the closing tag but support self closing as an input?

you can't branch on a marker trait, it doesn't even exist at code generation time

I was thinking more using the self.traits as a bound to this code rather than the hard coded list. That way the marker traits would express void elements and it would be used in a similar way here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants