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

Add naming rules doc #13

Merged
merged 4 commits into from
May 8, 2015
Merged

Add naming rules doc #13

merged 4 commits into from
May 8, 2015

Conversation

jeddy3
Copy link
Member

@jeddy3 jeddy3 commented May 7, 2015

Ha, this was harder than I thought! :)

I'm not happy with how I've described the difference between "normal" rules and *-no-* rules. Any ideas?


## Before and after

If the rule is *specifying what whitespace is allowed* around something then use `*-before/after` and use `coma`, `colon`, `semicolon`, `opening-brace`, `closing-brace`, `opening-parenthesis`, `closing-parenthesis`, `operator` and `range-operator` to identify that something.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two m's in comma :)

@davidtheclark
Copy link
Contributor

How about a note regarding single vs multiple line blocks? There was some discussion around that. I actually started a little work yesterday for one such rule, and found the following intuitive:

block-opening-brace-before: string/object

  • string: a string containing the whitespace that is expected before the block's opening brace (whether the block has one or many lines)
  • object: an options object
    • single: a string containing the whitespace expected before single line blocks' opening brace
    • multiple: a string containing the whitespace expected before multi-line blocks' opening brace

What do you think?

@MoOx
Copy link
Contributor

MoOx commented May 7, 2015

Maybe we should look at eslint for that handle single/multilines http://eslint.org/docs/rules/comma-dangle
That said, some complicated rules might need object http://eslint.org/docs/rules/indent

Do you think we should also handle 0,1,2 (disabled, warning, error) ?

@davidtheclark
Copy link
Contributor

Yeah, ESLint seems to have that pattern of using keywords as options. But I think for a whitespace rule like block-opening-brace-before it makes more sense to have an object so that you could theoretically specify different whitespace strings for the different options, right? And I think most of our intended rules where single-/multi-line might differ are about whitespace?

Sometimes keywords will definitely be the way to go, though.

As for 0, 1, 2 --- I think we don't have a use for 0 if we do not intend to have any defaults turned on, right? 1 and 2 I'm pretty indifferent to myself because I always just use 2 :) If somebody really wants 1 and builds it in, that's perfectly fine with me.

@jeddy3
Copy link
Member Author

jeddy3 commented May 8, 2015

Perhaps, once we've implemented a couple more rules we can refine the options (keywords vs object). I think I'll try to implement declaration-no-important this weekend as 1. it is probably the easiest rule 2. it'll be the 1st *-no-* rule. We have a "normal" rule already, and with you working on the whitespace-based block-opening-brace-before, we'll then have a "normal", a *-no-*, and a whitespace rule to look at. Sound OK?

In the meantime, I've added a little bit to the doc about consistently using single-line and multi-line. These seemed a little clearer than "single" and "multiple" e.g. it's not absolutely clear in something like function-comma-after if "multiple" refers to the number of commas or the number of lines. We can change this again once we know if keyword or object-based options are the best way to go. What do you think?

If somebody really wants 1 and builds it in, that's perfectly fine with me.

Same for me.

@MoOx
Copy link
Contributor

MoOx commented May 8, 2015

For 0,1, 2 I think it can make sense if we implement some defaults (see eslint/eslint#2100 (comment))

@MoOx
Copy link
Contributor

MoOx commented May 8, 2015

Also what about "simple" rules like indent ? Should we add something in this doc about that ?

@davidtheclark
Copy link
Contributor

It's fine with me to implement some limited defaults. The 1 vs 2 adds some complexity -- how would you envision that playing out?

@jeddy3
Copy link
Member Author

jeddy3 commented May 8, 2015

I've added a line about simple rules. We've only thought of 3 so far (eol-no-whitespace, eof-newline and indentation) and I couldn't think of what to call them as a group. Is that line good enough for now and we can come back to it later?

@jeddy3
Copy link
Member Author

jeddy3 commented May 8, 2015

Phew, that eslint/eslint#2100 is a long read. @MoOx I somehow came away with completely the opposite impression than you though! Not sure how that happened...

From how I understood it, they originally started with defaults but now they are trying to undo that for version 1.0.0. as it had caused them a lot of issues.

So, if I did understand nzakas' reasoning in that issue correctly then I suggest we do not have any defaults.

@davidtheclark
Copy link
Contributor

Actually after reading that I'm also thinking in agreement with nzakas and @jeddy3, and would prefer not to have defaults. I would rather have a "preset" that people could turn on, or "recommended defaults that people can choose to use" (as nzakas called it). I myself have been bothered by ESLint defaults in the past, and the thread shows that I'm not alone. (This does bring up that we need to ensure syntax errors caught by PostCSS are registered in a meaningful way, since that's the bare default for running the plugin (with no other configuration).)

@MoOx
Copy link
Contributor

MoOx commented May 8, 2015

I am ok with no default but with a fast way (like extends:stylelint-recommanded) to enable a bunch of good practises.

@davidtheclark
Copy link
Contributor

Sweet

@MoOx
Copy link
Contributor

MoOx commented May 8, 2015

Do we merge this ?

@davidtheclark
Copy link
Contributor

Oh sure. And then we just add to and modify as needed. Great start @jeddy3 .

MoOx added a commit that referenced this pull request May 8, 2015
@MoOx MoOx merged commit d46532a into stylelint:master May 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants