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

Proposal for change of the indent options #92

Closed
kizu opened this issue Oct 17, 2013 · 18 comments
Closed

Proposal for change of the indent options #92

kizu opened this issue Oct 17, 2013 · 18 comments
Assignees
Milestone

Comments

@kizu
Copy link
Contributor

kizu commented Oct 17, 2013

Current options for setting the indent of blocks, block-indent, rule-indent and stick-brace, are complete nonsense.

Those names are hard to understand and their behaviour is not logical in any way.

tl;dr: in this issue I propose that some new options should be addeed, block-indent should be changed to only change the indent of the whole block, rule-indent shouldn't be able to set newlines and stick-brace should be trashed and replaced by options to set up the whitespace around curly braces.

tab-size option

There should be a main option like tab-size, that should set the global indent size. Without any other settings this shouldn't do anything. It could be one of the following: [0-9]*|[ \t]*

When CSSComb sees the \t symbol (or an actual tab) inside other indent options, it should replace them with the value of tab-size option.

    <tr>
        <th>Original</th>
        <td>
            <pre><code>.foo {
color: red;

}
.bar { color: lime; }

    <tr>
        <th>
            <pre><code>{
"tab-size": 2,
"rule-indent": "\t"

}


    <tr>
        <th>
            <pre><code>{
"tab-size": "        ",
"rule-indent": "\t"

}



Value Result

.foo {
color: red;
}
.bar { color: lime; }


.foo {
color: red;
}
.bar { color: lime; }

root-indent option

This option should set the starting indent of each line. Who knows why it should be used, but there could be some cases where it would be logical.

    <tr>
        <th>Original</th>
        <td>
            <pre><code>.foo {
color: red;

}
.bar { color: lime; }

    <tr>
        <th>
            <pre><code>{
"root-indent": "    "

}


    <tr>
        <th>
            <pre><code>{
"ident": "  ",
"root-indent": "\t"

}



Value Result

    .foo {
color: red;
}
.bar { color: lime; }


  .foo {
color: red;
}
.bar { color: lime; }

block-indent option

The only thing the block-indent option should do is to tell how the blocks of rules should be placed inside of things like media queries etc. It shouldn't change the way the whitespaces are placed around curly braces.

    <tr>
        <th>Original</th>
        <td>
            <pre><code>.foo {
color: red;

}
@media print {
.foo {
color: blue;
}
.bar { color: lime; }
}

    <tr>
        <th>
            <pre><code>{
"block-indent": "  "

}



Value Result

.foo {
color: red;
}
@media print {
.foo {
color: blue;
}
.bar { color: lime; }
}

rule-indent option

Ok, that's the only option that could be saved as it is now I guess. Ah, except for it shouldn't create newlines, only tell how much whitespace should be there before rules. Ah, and it shouldn't do anything when there is no newline, like with rule-delimiter which don't have \n in it.

    <tr>
        <th>Original</th>
        <td>
            <pre><code>.foo {
color: red;

}
.bar { color: lime; }

    <tr>
        <th>
            <pre><code>{
"rule-indent": "  "

}



Value Result

.foo {
color: red;
}
.bar { color: lime; }

declaration-delimiter option

That should tell should which whitespace should be there between the declarations. If it contains at least one \n symbol, then the rule-indent comes into play, otherwise it would be a oneliner.

If there are no second value of block-start-space and/or first value of block-end-space set, then the declaration-delimiter is placed before the first rule and opening brace and after the last rule and closing brace.

    <tr>
        <th>Original</th>
        <td>
            <pre><code>.foo {
color: red;
background: lime;

}
.bar { color: lime; background: red; }

    <tr>
        <th>
            <pre><code>{
"declaration-delimiter": "\n"

}


    <tr>
        <th>
            <pre><code>{
"declaration-delimiter": "\n",
"rule-indent": "    "

}


    <tr>
        <th>
            <pre><code>{
"declaration-delimiter": " "

}



Value Result

.foo {
color: red;
background: lime;
}
.bar {
color: lime;
background: red;
}


.foo {
color: red;
background: lime;
}
.bar {
color: lime;
background: red;
}


.foo { color: red; background: lime; }
.bar { color: lime; background: red; }

block-start-space and block-end-space options

Those options should tell which whitespace should be there around the opening and closing curly braces.

If there is only one value (usual true|false|[ \t\n]*|[0-9] thingy), it should tell which whitespace should be there before the braces and if there would be an array of two values, then it would set the whitespace before and after the curly braces.

One thing to mention is that the second value of block-start-space should override the whitespace (rule-indent) between the opening curly brace and first rule when the declaration-delimiter is set to be empty.

    <tr>
        <th>Original</th>
        <td>
            <pre><code>.foo {
color: red;

}
.bar { color: lime; }

    <tr>
        <th>
            <pre><code>{
"block-start-space": ""

}


    <tr>
        <th>
            <pre><code>{
"block-start-space": "\n"

}


    <tr>
        <th>
            <pre><code>{
"block-start-space": [" ", " "]

}



Value Result

.foo{
color: red;
}
.bar{ color: lime; }


.foo
{
color: red;
}
.bar
{ color: lime; }


.foo { color: red;
}
.bar { color: lime; }

    <tr>
        <th>Original</th>
        <td>
            <pre><code>.foo {
color: red;

}
.bar { color: lime; }

    <tr>
        <th>
            <pre><code>{
"block-end-space": "    "

}



Value Result

.foo {
color: red;
}
.bar { color: lime; }

rule-delimiter option

This option should set the whitespace around the rules. So there would be both ways to set things like a{…} b{…}, a{…}\n\na{…} and others.

This option should override the trailing newlines of the second value of block-end-space option.

    <tr>
        <th>Original</th>
        <td>
            <pre><code>.foo {
color: red;

}
.bar { color: lime; }

    <tr>
        <th>
            <pre><code>{
"rule-delimiter": "\n\n"

}

    <tr>
        <th>
            <pre><code>{
"rule-delimiter": " "

}



Value Result

.foo {
color: red;
}

.bar { color: lime; }



.foo {
color: red;
} .bar { color: lime; }


Those changes would make it possible to recreate almost any code style, also it would be a base for allowing usage of smart (equal to the indent) \t in other whitespace rules, so we could align things in any possible way.

The current available options are not capable of that, for example, tell me how to use them to create things like those (and it doesn't matter how stupid such code style would look, we should be agnostic to it).

.foo {
    bar: baz;
    baz: raz;
    }
.foo {
    bar: baz;
    baz: raz; }
.foo { bar: baz; baz: raz; }

etc.

Legacy

Yep, I want to change a lot of stuff, so things could break, so it maybe should be a major version number update. However, there could be made at least some fallbacks, like if there is a stick-brace option in settings, we could treat the other options in an old way (they would be actually just aliases to the proper options), and we could treat this way other old values that are not there in new proposal (like true value for block-indent).


While someone could find those options a bit oververbose, it is better to have a way to fully describe the code style. Also, the current block-indent, rule-indent and stick-brace are no better: I could understand how they work only after reading the code and tests.

And those changes would allow us to detect the codestyle properly, and when #30 would be implemented, there wouldn't be a need for users to set almost any of the options manually, they could just use the CSS-templates for it.

And another thing — those changes would be a great base for future improvements of the overall whitespacing, like placing the multiple values on new lines with equal indents, having different settings for different cases (like allowing different whitespace settings for different kinds of blocks: uniform arrays of modifiers, keyframes etc).


Discuss :)

@tonyganch
Copy link
Member

Can you please provide examples of all those options in action?
Something like before vs. after.
I have troubles understanding the difference between some of them.

@kizu
Copy link
Contributor Author

kizu commented Nov 7, 2013

Updated the issue's text with examples. If it is still unclear, I could add more, just tell me which options or their combinations seem strange.

@tonyganch
Copy link
Member

@kizu, great work, thank you.

If I understand everything correctly, those options should be renamed:

  • indent -> tab-size
  • rule-delimiter -> declaration-delimiter
  • declaration-delimiter -> rule-delimiter

Reference links: css3 spec, css2 spec.

block-start-space and block-end-space

Does "block-start-space": true mean "block-start-space": " "?

I think that "block-start-space": [" ", " "] is something strange:

  • Array's meaning is not obvious
  • Why someone will ever need this?
  • Why is it one option instead of two?
  • Why is there no "block-end-space": [" ", " "]?

Also block-start-space and block-end-space mean different things: the first one is about spaces outside {...} block while the latter is about inner spaces.
This is confusing.

@kizu
Copy link
Contributor Author

kizu commented Nov 8, 2013

  • indent -> tab-size
  • rule-delimiter -> declaration-delimiter
  • declaration-delimiter -> rule-delimiter

Yes, of course, I'll change it in the text later, thanks.

Does "block-start-space": true mean "block-start-space": " "?

Yes.

On the second though I find current block-start-space and block-end-space to be confusing too: while block-end-space can have second argument, it would rarely do anything helpful.

And the only case when the block-start's second value would mean anything is when you have a oneliner, but in that case it could already be covered by declaration-delimiter (not fully, but it would cover the most cases).

So, I guess we could just throw out the arrays and say those block-start-space and block-end-space are setting the whitespace before the opening and closing curly brackets, as those are the 99.9% of all cases.

However, I now don't like those names much, so if anyone would find something better — it would be nice.

@tonyganch
Copy link
Member

BTW, here is an interesting tweet proposing replacing 4 spaces with 1 tab: https://twitter.com/Guldmand/status/402380200949723136

@kizu
Copy link
Contributor Author

kizu commented Nov 20, 2013

That should be tab-size: "\t", I guess. So, if there is a tab-size set, but no other options (like block-indent, block-indent etc.), we should detect the current tab style used in the stylesheet first and then replace it everywhere. However, it won't be trivial, 'cause there could be things like vendor-prefix-align and other similar users' aligns using spaces, that shouldn't be touched.

@tonyganch
Copy link
Member

I vote for this issue to be done next, before adding sass support.
Current options are too confusing and I don't know how to explain them to users if I can't even understand them myself.

@tonyganch
Copy link
Member

I have plans to start work on this issue, because it's a blocker for adding sass support.
If anyone has anything to add, speak now or forever hold your peace.

@kizu
Copy link
Contributor Author

kizu commented Dec 17, 2013

@tonyganch Woot. BTW, what do you think about renaming existing options like I suggested here?

@mishanga
Copy link
Contributor

@tonyganch @kizu I agree with this issue but first I'd like to publish 2.0.0 version.

@kizu
Copy link
Contributor Author

kizu commented Dec 21, 2013

Some more thoughts on the options names. The main ideas for those renames:

  1. We should split all the options that now have the arrays for before/after values to different options. In “handlers” those could be a single option, so it would be easier to code, but in the options they could be represented as separate ones.

  2. The names should be consistent, so all the indent options should start with the indent-, all the space options should start with space- etc.

  3. Questionable thing: should the space options that can have [before|after] in them have those as modifiers at the end of the name, or in the middle? The end is preferable, because the options would be sorted better then. But the overall names would be more bulky. Also, in non-symmetrical cases, like with commas or colons the before value would be very rarely used, so if they won’t be sorted near the after options, it would be ok. Overall, I’m for space-after-colon than space-colon-after.

  4. In cases the options are symmetrical, we could have one option for both sides, as for space-around-combinator, that could have possible value as {Array} or {Object} to set the individual options.

  5. There could be cases when we would want to setup options based on some conditions, like the space-after-comma, so we could set different values in selectors, in media queries conditions, in values etc. We could make it using {Object} values, with the keys like those:

    "space-after-comma": {
    "all": " ",
    "selectors": "\n",
    "values": "\n\t"
    }
    

I again invite everyone to join me in discussing the names for the options: as this is one of the rare open API in CSSComb.js, the better those names would be the better, so we could find the nest possible way of naming things, so we won’t need to rename anything later. So it is better to think everything straight away and then use those rules on naming things for all future options.

The options

Tab-size

The one option to control the behavior of \t in all the other options:

  • tab-size.

Available values: {Number} of spaces or {String} consisting of [\t ].

Indents

Options that set the size of the indents before different entities:

  • indent-at-root;
  • indent-at-block;
  • indent-at-rule (or indent-at-declaration?).

Available values: {Number} of spaces or {String} consisting of [\t ]. If the tab-size option is defined, the \t would be replaced with its value.

Spacings

Options that set the space around different entities:

I’m not sure about those names; should we have options that would set the whitespace after those braces?

  • space-at-block-start — space before the opening curly brace;
  • space-at-block-end — space before the closing curly brace.

And more similar options could be added/renamed, as those (there is a question on the naming order of the [before|after] part):

  • space-before-colon;
  • space-after-colon;
  • space-before-comma;
  • space-after-comma;
  • space-around-combinator.
  • space-between-values;
  • space-between-blocks;
  • space-between-rules;

Available values: {String} consisting of [\n\t ]. If the tab-size option is defined, the \t would be replaced with its value.

@tonyganch
Copy link
Member

That's my current proposal.

Options removed:

  • colon-space
  • rule-indent
  • stick-brace

Options added:

I believe in two things:

  • less options = less bugs
  • more obvious = less confusing

All names are readable and self-explaining and all values are consistent:

  • spaces/indent accept either Number of spaces or String with spaces/tabs/newlines;
  • tab-size accepts only Number of spaces;
  • others accept Boolean or String with predefined value.

No arrays and no objects.
Clean, simple, not so much space to add bugs.

Those options are enough to reproduce current code styles without adding new functional.
If someone needs indent-at-root or space-after-comma, it can be included later as new features.

@kizu
Copy link
Contributor Author

kizu commented Dec 30, 2013

Those options are enough to reproduce current code styles without adding new functional.
If someone needs indent-at-root or space-after-comma, it can be included later as new features.

Ok, agree.

@tonyganch
Copy link
Member

Ready. Waiting for all 2.x.x-related issues to be closed first.

@kizu
Copy link
Contributor Author

kizu commented Jan 17, 2014

I'm in a process of writing a space-after-comma option and I have a question regarding possible Object option value.

The thing is commas could be in differen places, between:

  1. simple selectors;
  2. values;
  3. arguments of “functions” like rgba.

And as those are really different places, it would make sense to provide a way of setting them independently. Like someone would want to have newlines between selectors, space after commas in values and no space in rgba(0,0,0,0.5).

I see a two ways of doing this:

  1. Allow Object as a value, where the key would be the context of the option, and the value would be the value:

    {
        "space-after-comma": {
            "selector": "\n",
            "value": " ",
            "function": ""
        }
    }

    And we still would need to have single string value as a shortcut to all of them.

  2. We could split those options to multiple ones, so we would have:

    • space-after-comma-in-selector
    • space-after-comma-in-value
    • space-after-comma-in-function

    A question is should we have also a generic space-after-comma that would set all three options? I'd say no.

At first I thought about the first variant, but now I'm fond more of splitting this one option to the multiple ones.

However, there is another thing: some people would want to set different option to some specific functions. So they would want no spaces in rgba and hsla, but single spaces in linear-gradient.

Or even it would make sense to wish different options between values for font-family (one space would be ok) and background-image (newline with a tab).

With the Object solution it would be not that hard to implement any number of such specific options, like this:

{
    "space-after-comma": {
        "default": " ",
        "selector": "\n",
        "value": {
            "backgrounds": "\n\t",
        },
        "function": {
            "colors": "",
            "gradients": "\n"
        }
    }
}

Such object is nice and readable, but it would be hard to detect and some of the keys could be unguessable, so people would need to look deep into docs.

But the splitted options for such cases would be even worse: we would need to create dozens of them and for some cases (like functions) we would need to add “shortcuts” for all of them and then play with the overrides.


So, what'd you say, @tonyganch, @mishanga? Is there any other way to provide people with such variety of options, do you thing it is overengeneering (but I would really like to use different whitespaces for gradients/functions and newlines for backgrounds).

Or maybe there is a simple way to provide people with some simple API to add their own options along the ones we described? Actually, I would see it would make to have an API for user-added options.

@tonyganch
Copy link
Member

@kizu, I'd say that objects and default values are evil because they are potentially confusing.

I totally love the idea of space-after-comma-in-selector and space-after-comma-in-value: those names are readable and easy to understand.
Users should not spend a few hours reading docs to write the config they need.

@kizu
Copy link
Contributor Author

kizu commented Feb 14, 2014

Ok, then space-after-comma-in-selector and space-after-comma-in-value. And when there would be the need for more specific settings for some contexts we could just use the same naming — space-after-comma-in-rgba, space-after-comma-in-backgrounds etc.

@tonyganch
Copy link
Member

I'm closing the issue as it's become too broad.
I encourage everyone to open a separate issue for every new option that makes sense to add.

@kizu, thank you for ideas and great discussion.

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

No branches or pull requests

3 participants