csscomb/csscomb.js — pull request #167 by tonyganch (2014-01-18 19:28:49+00:00)
Title: [WIP] Refactor Comb class

This is the first of three PR's regarding #92.
They all can easily wait for #111 and similar to be finished, so there is no rush in reviewing.
Since there are some serious changes, I invite you to take a look at code and say if anything bothers you, so we can fix it up.

-
#### TL;DR

- This PR fixes the following issues: #81, #122, #145.
- All private methods have been hidden.
- Handlers have become instance variables.
- "Process" and "detect" methods have been separated.
- Deprecated options (see #92) have been removed.
- No more anonymous functions in logs.
- New method: `Comb#getValue(optionName)`.

-

#### Problem with instances

```
> var a = new Comb({ 'element-case': 'upper' });
> a._handlers[0]._value;
'upper'

> var b = new Comb('csscomb');
> a._handlers[0]._value;
'lower'

> var c = new Comb({ 'element-case': 'upper' });
> a._handlers[0]._value;
'upper'
```

See what happens?
Handlers keep being reconfigured with every new instance.
This has been solved by moving some variables and methods to constructor.
Methods like `detectInFile` which do not use any instance variables stay in `prototype` object.

#### Private methods

```
> var a = new Comb({ 'element-case': 'upper' });
> a;

{ SUPPORTED_SYNTAXES: [ 'css', 'scss', 'less' ],
_options:
[ 'remove-empty-rulesets',
'always-semicolon',
'color-case',
'color-shorthand',
'element-case',
'leading-zero',
'quotes',
'strip-spaces',
'eof-newline',
'stick-brace',
'colon-space',
'combinator-space',
'rule-indent',
'block-indent',
'unitless-zero',
'sort-order',
'vendor-prefix-align' ],
_exclude: [],
_detect: false,
_handlers:
[ { setValue: [Function],
process: [Function],
detect: [Function],
_value: 'upper' } ],
processed: 0,
tbchanged: 0,
changed: 0,
_verbose: undefined,
_lint: undefined }
```

I find all this information useless.
The only good property here is `_handlers` array which is still useless because you can't see options' names.
There are also a lot of `this.nani` like properties which can be replaced by `var nani`.
So I used simple `var` and `function` where it could be done and made constructor return public instance methods:

```is
> var a = new Comb({ 'element-case': 'upper' });
> a;

{ configure: [Function: configure],
getValue: [Function: getValue],
processPath: [Function: processPath],
processDirectory: [Function: processDirectory],
processFile: [Function: processFile],
processString: [Function: processString] }
```

Maybe it should return something more meaningful, for example map with configured options and its values.

#### Separate "process" and "detect"

All "detect" methods now live inside `prototype` object while "process" methods have moved to constructor.
This way detection does not mess with instance variables and does not configure anything.
This fixes #145.

#### Best guess

I took a look at "best guess" code inside options (inside `detect` methods) and could not see a reason why we needed it there at all.
So I removed it.
No test failed.
"Best guess" logic stays implemented in one place: `getDetectedOptions()` method inside Comb's prototype.
I think this fixes #122.

#### Common `setValue()` method

I saw @mishanga's [comment](https://github.com/csscomb/csscomb.js/issues/81#issuecomment-24494125) on #81 and liked the idea.
I renamed `config` to `accepts` because too many variable names in our code contain `config` already.
Option now should either have `accepts` property or implement custom `setValue` method.
`accepts` object is convenient for simple cases while `setValue` can be used for options like `sort-order`.
Example:
```js
...
accepts: {
string: /^[ \n\t]*$/, // valid value must be any number of spaces
string: /^lower|upper$/, // valid value must be either "lower" or "upper"
number: true, // valid value must be an integer
boolean: true // valid value must be either true or false
}
...
```

Note that `boolean: true` means both `true` and `false` and if your option accepts only one of those, you need `setValue` method.
I have some doubts now about this implementation and think that @mishanga's original idea about `boolean: [true, false]` is better.

#### Get any option's value

Since `handlers` property is now private and options' objects are not tied with Comb's instances any more, you can not use `this._value` to get option's value for current instance.
But you have a handy `getValue()` method instead:
```js
var spaceValue = this.getValue('space-before-closing-brace');
var blockIndentValue = this.getValue('block-indent');
if (spaceValue !== blockIndentValue) doSomething();
```
It's useful for processing and detecting several options that depend on each other.

![fox](https://f.cloud.github.com/assets/872004/1947988/e6c44e76-8075-11e3-83f8-34af7a23436c.gif)