rgrove / crass

A Ruby CSS parser that's fully compliant with the CSS Syntax Level 3 specification.
MIT License
139 stars 14 forks source link

Add #stringify_inline to output cleaner style attributes #4

Closed Fustrate closed 6 years ago

Fustrate commented 6 years ago

https://github.com/Fustrate/crass/commit/8345aca2a1b2224a9a94d8cfeb3e267d7c7b309c

This is the replacement (with tests) for my earlier PR on Sanitize. The idea is that Sanitize::CSS#properties would use #stringify_inline instead of #stringify. My earlier thought was to have #stringify try to detect if it was stringifying inline or stylesheet CSS, but there's never really an ambiguity to where it originally came from.

I'm not submitting it as a PR yet because I'd like to know if this is on the right path, and if there are any other tests I should write that I'm just not well versed enough in this gem to know are needed.

rgrove commented 6 years ago

Thanks for suggesting this and asking for feedback!

After thinking about this for a while, I'm not convinced that whitespace collapsing is something Crass should do. One of Crass's selling points (and in fact one of the main reasons I wrote it in the first place) is its ability to preserve all original whitespace in the parse tree and stringify a perfect representation of the tree.

That said, I've always thought it would be cool if there were additional libraries that could consume a Crass parse tree and manipulate it in interesting ways before passing it along to a stringifier. Whitespace normalization, selector coalescing, etc. are all interesting things that I think individual libraries could do well, but that probably don't belong in Crass itself.

Would you be interested in implementing this feature as a standalone library?

Fustrate commented 6 years ago

Understood, thanks for the quick response.

Since CSS prettifiers are pretty simple with a pre-existing tree, I think I'm up to the task. Do you have any thoughts on what the external interface for Sanitize/Crass users would look like? I'm thinking the same as Sanitize's transformers would be a consistent way to do it - the entire tree is passed to the transformer(s). The only thing that might be complicated is knowing which format to use, inline or stylesheet. Maybe the parent node would be passed in as well, and the transformer would decide what to do based on if it's a style element or if the node has a style attribute. Or it could look at the tree and see if it starts with a property (inline) or anything else, disregarding comments and whitespace.

I'll do some preliminary work this weekend and see what I can come up with.

rgrove commented 6 years ago

I'm thinking the same as Sanitize's transformers would be a consistent way to do it - the entire tree is passed to the transformer(s). The only thing that might be complicated is knowing which format to use, inline or stylesheet. Maybe the parent node would be passed in as well, and the transformer would decide what to do based on if it's a style element or if the node has a style attribute.

That sounds like a good approach to me. I'm looking forward to seeing what you come up with!

Closing this issue for now since there's no Crass action item, but feel free to keep discussing here if you want.