Skip to content

Added support for embedded rulesets. Resolves #336. #338

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

Merged
merged 3 commits into from
Jan 17, 2013
Merged

Added support for embedded rulesets. Resolves #336. #338

merged 3 commits into from
Jan 17, 2013

Conversation

shannonmoeller
Copy link
Contributor

Implements feature requested in #336. (Sorry for the trailing-whitespace changes. I have vim set to nuke those.)

Update: For posterity, this pull request adds the ability to override csslint rules on a per-file basis by embedding the rules in a special comment. It follows the jslint and jshint convention in style and meaning (defaults to warn, false is ignore, true is error).

/*csslint adjoining-classes:false, box-sizing:false, universal-selector:true */

@nzakas
Copy link
Contributor

nzakas commented Jan 14, 2013

Can you explain a little bit about how this works? Specifically, if you have command-line options, a .csslintrc file, and in-file rules specified, how do those get resolved?

@shannonmoeller
Copy link
Contributor Author

The modifications were limited to the CSSLint.verify(text, ruleset) method. If text contains embedded rules, the ruleset parameter will be ignored. It is assumed that the author of the CSS file knew best which rules to enforce and which to ignore.

In your example use case, if file-level rules are specified, the command-line options and .csslintrc options are ignored for that file.

The included test-case demonstrates this behavior.

@nschonni
Copy link
Member

I believe JSHint uses specificity instead of ignoring them:

  • what is the default value
  • what is the value in .jshintrc (again, has its' own nested specificity based on file path)
  • what is the command line parameters
  • what is the global to the file
  • what is the per-line value

@nzakas
Copy link
Contributor

nzakas commented Jan 14, 2013

Figuring out how the various settings interact is important. We basically
have three sources for the ruleset:

  1. The command line
  2. The .csslintrc file
  3. The in-file settings

I don't think having one completely overrule the others is the correct
behavior. I think a hierarchy like JSHint would make sense, but I'm still
not sure what would be highest priority.

On Mon, Jan 14, 2013 at 1:27 PM, Nick Schonning [email protected]:

I believe JSHint uses specificity instead of ignoring them:

  • what is the default value

  • what is the value in .jshintrc (again, has its' own nested
    specificity based on file path)

  • what is the command line parameters

  • what is the global to the file

  • what is the per-line value


    Reply to this email directly or view it on GitHubhttps://github.com/Added support for embedded rulesets. Resolves #336. #338#issuecomment-12240673.


Nicholas C. Zakas
@SlickNet

Author, Professional JavaScript for Web Developers
Buy it at Amazon.com:
http://www.amazon.com/Professional-JavaScript-Developers-Nicholas-Zakas/dp/1118026691/ref=sr_1_3

@shannonmoeller
Copy link
Contributor Author

Reworking the cascade of csslint rules as a whole is outside the scope of this issue, if needed at all. But I can see the case for having the file-level rules modify, rather than override, the ruleset passed into .verify(). I've made the change and updated the test case.

@mpetrovich
Copy link

+1 This is very useful and necessary to make this a serious tool for our CI environment

@nzakas
Copy link
Contributor

nzakas commented Jan 17, 2013

I think this is a good first step. We can label this feature as experimental and see what feedback we get.

nzakas added a commit that referenced this pull request Jan 17, 2013
Added support for embedded rulesets. Resolves #336.
@nzakas nzakas merged commit 955e8cb into CSSLint:master Jan 17, 2013
@shannonmoeller shannonmoeller deleted the feature/embedded-rulesets branch January 17, 2013 17:58
@shannonmoeller
Copy link
Contributor Author

@nzakas Thank you, sir!

@mpetrovich
Copy link

Awesome turnaround, thanks @nzakas & @shannonmoeller

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

Successfully merging this pull request may close these issues.

4 participants