search

dave-kennedy / clean-html

HTML cleaner and beautifier for Node
The Unlicense
47 stars 10 forks source link

Add new remove-empty-attributes feature #17

Closed castastrophe closed 3 years ago

castastrophe commented 5 years ago

This PR adds a new configurable setting that allows for the removal of empty attributes (such as class="". This also wraps logic to prevent printing empty ="" tags and instead will print the attribute as a boolean if no value is provided.

By default this addition will now remove the follow attributes if their value is empty: ['class', 'style', 'id'].

castastrophe commented 5 years ago

@dave-kennedy Let me know how I can update the proposed changes to fit more with your goals/technique for this project. I'm happy to make any changes that would be helpful. I also took a guess at the way you're doing releases and included the version bump in this PR as well. If you prefer to do those separately, just let me know and I'm happy to make changes.

dave-kennedy commented 5 years ago

Great pull request. I think this part could be flattened and simplified:

        if (!isListedInOptions('remove-attributes', attrib) && !(isListedInOptions('remove-empty-attributes', attrib) && node.attribs[attrib] === "")) {
            openTag += ' ' + attrib;
            // If the value exists
            if(node.attribs[attrib] !== "") {
                openTag += '="' + removeExtraSpace(node.attribs[attrib]) + '"';
            }
            // else allow for removal of empty attributes?
        }

In addition to testing for an empty string, I'd also test to see if it's blank. You can see an example of this in the isEmpty function. Maybe something like:

if (shouldRemoveAttribute(attrib)) {
    continue;
}

openTag += ' ' + attrib;

if (isEmptyOrBlankString(node.attribs[attrib])) {
    continue;
}

openTag += '="' + removeExtraSpace(node.attribs[attrib]) + '"';

As for style, please use single quotes instead of double, and avoid ambiguous comments like else allow for removal of empty attributes?.

Hope I'm not being too nitpicky!

castastrophe commented 5 years ago

@dave-kennedy Thanks for the feedback! I'll work on this applying it as soon as I can. My sprint at work got a little busy but code freeze is coming up and I should have a little clean-up time then.

castastrophe commented 5 years ago

Sorry for the delay, I will get back to this soon and make those updates. Thanks for your patience!

dave-kennedy commented 5 years ago

@castastrophe no worries. I'm pretty busy with work and life and all that also.