hudochenkov / stylelint-order

A plugin pack of order related linting rules for Stylelint.
MIT License
916 stars 61 forks source link

Merge properties-specified-order, properties-flexible-order, and property-groups-structure into new rule #17

Closed hudochenkov closed 7 years ago

hudochenkov commented 7 years ago

The more I think the more I realise that dividing stylelint's declaration-block-properties-order into more specific rules creates more confusion then clear state of thing. Rule's tidy couple with each other, share lots of code and big pieces of README.

Currently we have:

declaration-block-properties-specified-order rule is basically limited declaration-block-properties-flexible-order rule.

declaration-block-property-groups-structure should support flexible order as well as strict order. So I have to run declaration-block-properties-flexible-order from declaration-block-property-groups-structure as well as declaration-block-properties-specified-order.

See what a mess it became?

declaration-block-properties-alphabetical-order works great and won't be affected.

I was thinking it's better to merge properties-specified-order, properties-flexible-order, and property-groups-structure into one rule. It would be like declaration-block-properties-order in stylelint 6.5.0. Merged rule will support strict order, flexible order, and emptyLineBefore.

Config would be like:

[
    [
        "height",
        "width",
        {
            emptyLineBefore: "always",
            properties: [
                "color",
                "font-size",
                "font-weight",
            ],
        },
        "flex",
        "display",
        {
            emptyLineBefore: "never",
            order: "flexible",
            properties: [
                "margin-top",
                "margin-bottom",
            ],
        },
    ],
    {
        unspecified: "bottom"
    }
]

Strict order applies for main array and to objects without order: "flexible". emptyLineBefore is optional.

I think the new rule should have name declaration-block-properties-structure. Because it's not about order, but also about emptyLineBefore. I believe there could be a better name, because properties-structure it's not clear enough.

/cc @cascornelissen @osk @swernerx @evilebottnawi what do you think about all of this?

alexander-akait commented 7 years ago

@hudochenkov sgtm and the configuration looks pretty simple

swernerx commented 7 years ago

That looks excellent for me.

cascornelissen commented 7 years ago

As I've stated in #15 I agree it's becoming a bit of a mess. This solution would definitely work for us as well.

What would the default for emptyLineBefore be? I assume never?

osk commented 7 years ago

Clear and covers the use cases we have 👍🏽✨

hudochenkov commented 7 years ago

What would the default for emptyLineBefore be? I assume never?

By default it won't be set at all.

hudochenkov commented 7 years ago

Thank you for your response! Any idea for a better name?

alexander-akait commented 7 years ago

@hudochenkov maybe declaration-block-properties-order :smile:

swernerx commented 7 years ago

as it's already namespaced by the plugin name, why not just name it "properties-order"?

hudochenkov commented 7 years ago

@evilebottnawi :) I like it, but as I said before, this rule concerned not only about order, but about emptyLineBefore also.

alexander-akait commented 7 years ago

@hudochenkov I think this can be omitted, primary function of this rule is order

hudochenkov commented 7 years ago

Just have released 0.4.0 with properties-order! Strict order, flexible order, and emptyLineBefore in one rule :)

Thank you, everyone, for discussion!