Space after keywords superset rule (nested required/allowed)

[nschonni]

DO NOT MERGE

Sample aggregate rule example from #136

"spaceAfterKeywords": {
  "required": ["if"],
  "disallowed": ["return"]
}

TODO:

• How to map old rule back in given current export pattern
• How to deal with conflicting keys? i.e.: keyword that is in both allowed and disallowed

/cc @markelog

[markelog]

This guy – https://github.com/marklog must be really surprised :-). Will check this out in the weekend, but it looks like we should move it to another branch.

[mikesherov]

Thanks as always for contributing! Yeah, definitely move to a branch, but also, a PR that would be acceptable here has to be the base implementation of the translation layer. Meaning:

1. It need to delete the rules it's a duplicate of.
2. It needs to be backwards compatible.
3. It need to be a generic solution.
4. @mdevils and I and @markelog need to agree it's the right format.

[mdevils]

I suggest another approach: mapping.

What we get:
— rules are left as is, small and separate.
— mapping is just a layer on top.
— we have backwards compatibility.

Flawbacks:
— we should keep mapping up to date.

(I'm about implementation)

[mikesherov]

Another drawback is that lots of the sniffs are basically duplicated code of each other, creating more maintenance work. 

Mike

On Fri, Jan 17, 2014 at 4:35 PM, Марат Дулин notifications@github.com
wrote:

I suggest another approach: mapping.
What we get:
— rules are left as is, small and separate.
— mapping is just a layer on top.
— we have backwards compatibility.
Flawbacks:
— we should keep mapping up to date.

(I'm about implementation)

Reply to this email directly or view it on GitHub:
#181 (comment)

[nschonni]

I actually prefer the format in #182, but both of them are half-baked examples to discuss how the old rules can be mapped back.

[mikesherov]

I like 182 as well. I assume true is require, false is disallow, and no value (or null) is no preference?

Speaking of which, we need ability to turn off a rule. Imagine user wants to use jQuery preset except one rule. Null value for a rule should disable it.  Thoughts?

Mike

On Fri, Jan 17, 2014 at 8:47 PM, Nick Schonning notifications@github.com
wrote:

I actually prefer the format in #182, but both of them are half-baked examples to discuss how the old rules can be mapped back.

Reply to this email directly or view it on GitHub:
#181 (comment)

[mdevils]

Another drawback is that lots of the sniffs are basically duplicated code of each other, creating more maintenance work.

Depends. In most of cases we can share some code into separate modules. But having little modules for rules, one for each little behavior, makes development way easier.

[nschonni]

I've updated this and #182 with a map back to the old rules but using require.

---

I assume true is require, false is disallow, and no value (or null) is no preference?

💯

[markelog]

we need ability to turn off a rule

+1

@nschonni i appreciate the effort but we really need to agree on the new format (if we would do that) first, i would hate if all of time spent on this would be for nothing.

Original ticket was not only about changing value of existing rules but also to make them more consistent with each other. Also i wouldn't want to deprecated anything at this point, but provide layer on top, but again, it would depend on the new format, maybe we even should create a different documentation for it.

[nschonni]

This is throw away proof of concept code, so feel free to close it if you want.

[markelog]

@nschonni perhaps i sounded a bit dismissive, we might go with this approach, i don't know frankly, but in any case it's good to provide a proof of concept

[mikesherov]

@nschonni thanks again for doing this work way back when. We're going to take config in a new direction, so closing this for now. Once a new issue is published for it, we'll ping you so you can help us bikeshed :-)