swift-html icon indicating copy to clipboard operation
swift-html copied to clipboard

Even further with Tag...

Open bgisme opened this issue 2 years ago • 8 comments

This PR goes further than my previous one with changes to Tag

And I'll admit it may be a bridge too far. Not sure. But I thought I'd put it out there.

It tries to address some things that have always struck me as odd or confusing about the library...

Tag appears like the base class. And then GroupTag is a derivation. But it should really be the other way around. GroupTag is more foundational. It's just a renderless collection of child Tags. Every other kind of Tag is a specialization: name, attributes and even more children. So this PR eliminates GroupTag and makes Tag renderless by default. And then provides options and subclasses to specialize. TagBuilder just puts components inside another Tag.

Node has always confused me. And why its properties should be separate. Maybe there's a good reason, but I don't see it in the library. So I moved them all into Tag. And I think it makes how to use the class more apparent. I also eliminated .group and made the type property on Tag optional. Because that's essentially what the class is... a nothing, invisible container.

• Lots of empty arrays get allocated. Tag.children and Node.attributes are[] by default. And while it might not seem like a lot memory allocation, I wonder if it adds up in large hierarchies. So I made them optional.

bgisme avatar Jul 12 '23 16:07 bgisme

I reviewed the changes, if you can resolve the conflicts I'd like to merge this branch (and the other PR) to main. Thank you very much for your contribution. 🙏

tib avatar Jul 19 '23 05:07 tib

I think it's all set now.

bgisme avatar Jul 23 '23 19:07 bgisme

Thanks, hopefully I'll have time to merge these PRs during the second half of this week.

tib avatar Jul 25 '23 06:07 tib

Not sure if you started working on the merge yet.

But I added a few more things.

Like handling the "lang" attribute

bgisme avatar Jul 27 '23 16:07 bgisme