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

Hotfix Master: Fix overriding default rules #310

Merged
merged 6 commits into from
Oct 17, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 81 additions & 1 deletion docs/options/merge-default-rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,88 @@

Option `merge-default-rules` will determine whether the rules present in a user's external configuration file or passed directly in to Sass Lint will override or merge with the default rules present in Sass Lint. All other configuration options will still be merged.

If an external configuration file has `merge-default-rules` set to `false`, its rules will override the default rules, but rules passed directly in to Sass Lint will be merged with the configuration file rules (unless they, to have `merge-default-rules` set to `false`, in which case they will override).
If an external configuration file has `merge-default-rules` set to `false`, its rules will override the default rules, but rules passed directly in to Sass Lint will be merged with the configuration file rules (unless they too have `merge-default-rules` set to `false`, in which case they will override).

## Options

* `true`/`false` (defaults to `true`)

## Examples

Below is a sample of sass-lints default rules. You can see the full default ruleset [here](https://github.com/sasstools/sass-lint/blob/develop/lib/config/sass-lint.yml).

```yml
options:
formatter: stylish
files:
include: '**/*.s+(a|c)ss'
rules:
# Extends
extends-before-mixins: 1
extends-before-declarations: 1
placeholder-in-extend: 1

# Mixins
mixins-before-declarations: 1
no-color-literals: 1
```

Below is a default user configured ruleset containing only one rule with `merge-default-rules: true`

```yml
options:
formatter: stylish
merge-default-rules: true
files:
include: '**/*.s+(a|c)ss'
rules:

no-color-literals: 2
```

With `merge-default-rules: true` the rules applied to the linter would be as follows. Notice that `no-color-literals` is now set to `2` and the default ruleset has been merged with our user config

```yml
options:
formatter: stylish
merge-default-rules: true

files:
include: '**/*.s+(a|c)ss'
rules:
# Extends
extends-before-mixins: 1
extends-before-declarations: 1
placeholder-in-extend: 1

# Mixins
mixins-before-declarations: 1
no-color-literals: 2
```

Below is a default user configured ruleset containing only one rule with `merge-default-rules: false`

```yml
options:
formatter: stylish
merge-default-rules: false
files:
include: '**/*.s+(a|c)ss'
rules:

no-color-literals: 1
```

With `merge-default-rules: false` the rules applied to the linter would be as follows. Notice that `no-color-literals` is now set to `1` but rules not present in the user configuration are not merged.

```yml
options:
formatter: stylish
merge-default-rules: false

files:
include: '**/*.s+(a|c)ss'
rules:

no-color-literals: 1
```
47 changes: 34 additions & 13 deletions lib/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,15 @@ module.exports = function (options, configPath) {
var meta,
metaPath,
configMerge = false,
configMergeExists = false,
optionsMerge = false,
optionsMergeExists = false,
config = {},
finalConfig = {};

// ensure our inline options and rules are not undefined
options = options ? options : {};
options.rules = options.rules ? options.rules : {};

if (options.options && options.options['config-file']) {
configPath = options.options['config-file'];
Expand All @@ -62,27 +66,44 @@ module.exports = function (options, configPath) {

if (configPath) {
if (fs.existsSync(configPath)) {
config = yaml.safeLoad(fs.readFileSync(configPath, 'utf8'));
config = yaml.safeLoad(fs.readFileSync(configPath, 'utf8')) || {};
config.rules = config.rules ? config.rules : {};
}
}

configMerge = (config.options && config.options['merge-default-rules'] === false) ? false : true;
optionsMerge = (options.options && options.options['merge-default-rules'] === false) ? false : true;
// check to see if user config contains an options property and whether property has a property called merge-default-rules
configMergeExists = (config.options && typeof config.options['merge-default-rules'] !== 'undefined');

finalConfig = defaults;
// If it does then retrieve the value of it here or return false
configMerge = configMergeExists ? config.options['merge-default-rules'] : false;

if (configMerge) {
finalConfig = merge.recursive(defaults, config);
}
else if (configMerge) {
finalConfig.rules = config.rules ? config.rules : {};
// check to see if inline options contains an options property and whether property has a property called merge-default-rules
optionsMergeExists = (options.options && typeof options.options['merge-default-rules'] !== 'undefined');

// If it does then retrieve the value of it here or return false
optionsMerge = optionsMergeExists ? options.options['merge-default-rules'] : false;


// order of preference is inline options > user config > default config
// merge-default-rules defaults to true so each step above should merge with the previous. If at any step merge-default-rules is set to
// false it should skip that steps merge.

finalConfig = merge.recursive(defaults, config, options);

// if merge-default-rules is set to false in user config file then we essentially skip the merging with default rules by overwriting our
// final rules with the content of our user config otherwise we don't take action here as the default merging has already happened
if (configMergeExists && !configMerge) {
finalConfig.rules = config.rules;
}

if (options && optionsMerge) {
finalConfig = merge.recursive(finalConfig, options);
// if merge-default-rules is set to false in inline options we essentially skip the merging with our current rules by overwriting our
// final rules with the content of our user config otherwise we check to see if merge-default-rules is true OR that we have any inline
// rules, if we do then we want to merge these into our final ruleset.
if (optionsMergeExists && !optionsMerge) {
finalConfig.rules = options.rules;
}
else if (options && !optionsMerge) {
finalConfig.rules = options.rules ? options.rules : {};
else if ((optionsMergeExists && optionsMerge) || options.rules && Object.keys(options.rules).length > 0) {
finalConfig.rules = merge.recursive(finalConfig.rules, options.rules);
}

return finalConfig;
Expand Down