Spaces in brackets "hybrid" setting broken scenario
Closed this issue · 11 comments
if (!foo ){
// code
}I believe it's the spaces_around_operators setting messing with this one.
It also seems to be when they're nested:
// | missing space
JSON.parse( JSON.stringify( queries ));
Actually, the nested one is intentional. It prevents things like:
foo( {
} );And stuff like that.
Hm, OK. That wasn't obvious or expected when I tried it.
It looks quite odd in my eyes that it formats it on the left hand side but not the right. I might look into having a fourth option then (only reason I didn't use "true" was empty literals like [ ] and { }, which I thought "hybrid" was).
Anyway, I won't clutter this issue with that if it's expected behaviour :)
I agree that it's odd, but if you want it spaced on both sides just use the true setting. I'm becoming less and less fond of the hybrid settings anyway and I've been rethinking a lot about how Code Painter even works in the first place. I kinda' took the project over and made it more fully-developed, but now, after this learning experience, I have ideas for how to do it better "next time".
Yeah, the hybrid settings aren't very well defined. I could make you happy and make a bunch others not happy in the process, so making a 4th option would definitely be a better idea, but then what would it be named?
Absolutely, didn't mean to put blame on you in any way. I agree that changing hybrid isn't the best idea. Regarding naming, nothing super-clear comes to mind. jQuery has a few explanations how they approach this (under Arrays and Objects). Maybe noEmptyFillerSpace or something like that?
This might be the right time to tell you my idea of how the new way might work and perhaps get some useful feedback.
The idea is that you could define new rules much more easily by first defining tokens mapped to predefined token names and then saying what you're looking for and with what to replace it. For the spaces_in_brackets rule, it would look something like this, in JSON format:
{
"rule": "spaces_in_brackets",
"setting": false,
"define": {
"(": "OpenBracketPunctuator",
")": "CloseBracketPunctuator",
" ": "WhiteSpacesSansNewline"
},
"find": [
["(", " "],
[" ", ")"]
],
"replace": [
["$1", null],
[null, "$2"]
]
}The OpenBracketPuncuator, CloseBracketPunctuator and WhiteSpacesSansNewline tokens would be predefined by me, so you could easily set them in this JSON file.
I think this would be a huge improvement to Code Painter and I hope to make it language agnostic also, so it can use other parsers for different languages.
What do you think?
It sounds good but pretty limiting:
- What if a rule needs some logic to it (maybe conditional, only whitespace in brackets if it isn't nested in another brackets)?
- In your example, how would you achieve one empty space filler (e.g.
function( ) { };)? If I understand the syntax correctly I would get double spaces inside those brackets.
I like the approach taken in nzakas/eslint with pluggable rules with access to the esprima nodes.
Gotta head to bed but I'll gladly continue a discussion tomorrow.
Thanks for the feedback. Actually, I think we're talking about the same thing. There would be a separate folder that handles the logic, which would be like a tokens folder, defining what those keys on the right actually mean. So, to handle your case more thoroughly, it would look something like this:
{
"rule": "spaces_in_brackets",
"define": {
"(": "OpenBracketPunctuator",
")": "CloseBracketPunctuator",
"!(": "NonOpenBracketPunctuator",
"!)": "NonCloseBracketPunctuator",
" ": "WhiteSpacesSansNewline",
"! ": "NonWhiteSpaces",
"! !(": "NonWhiteSpacesNonOpenBracketPunctuator",
"! !)": "NonWhiteSpacesNonCloseBracketPunctuator"
},
"settings": {
"true" : {
"find": [
["(", "! "],
["! ", ")"]
],
"replace": [
["$1", " ", "$2"],
["$1", " ", "$2"]
]
},
"false": {
"find": [
["(", " "],
[" ", ")"]
],
"replace": [
["$1", null],
[null, "$2"]
]
},
"hybrid": {
"find": [
["(", "! !("],
["! !)", ")"],
["(", " ", "!("],
["!)", " ", ")"]
],
"replace": [
["$1", " ", "$2"],
["$1", " ", "$2"],
["$1", null, "$2"],
["$1", null, "$2"]
]
}
}
}The NonWhiteSpacesNonOpenBracketPunctuator token would be defined in a separate tokens folder, where the logic would be, but this JSON file would give you a logic-less way to define rules, which is great because now rules can be defined, inferred and executed in a very consistent way.
It might also not hurt to use regular expression syntax for the keys, even though they won't actually be parsed as regular expressions, it still conveys what the rule is. Then, it would look like this:
{
"rule": "spaces_in_brackets",
"define": {
"(": "OpenBracketPunctuator",
")": "CloseBracketPunctuator",
"[^(]": "NonOpenBracketPunctuator",
"[^)]": "NonCloseBracketPunctuator",
" +": "WhiteSpacesSansNewline",
"[^\\s]+": "NonWhiteSpaces",
"[^\\s(]+": "NonWhiteSpacesNonOpenBracketPunctuator",
"[^\\s)]+": "NonWhiteSpacesNonCloseBracketPunctuator"
},
"settings": {
"true" : {
"find": [
["(", "[^\\s]+"],
["[^\\s]+", ")"]
],
"replace": [
["$1", " ", "$2"],
["$1", " ", "$2"]
]
},
"false": {
"find": [
["(", " +"],
[" +", ")"]
],
"replace": [
["$1", null],
[null, "$2"]
]
},
"hybrid": {
"find": [
["(", "[^\\s(]+"],
["[^\\s)]+", ")"],
["(", " +", "[^(]"],
["[^)]", " +", ")"]
],
"replace": [
["$1", " ", "$2"],
["$1", " ", "$2"],
["$1", null, "$2"],
["$1", null, "$2"]
]
}
}
}Or just don't define keys at all and make it more readable, like this:
{
"rule": "spaces_in_brackets",
"settings": {
"true" : {
"find": [
["OpenBracketPunctuator", "NonWhiteSpaces"],
["NonWhiteSpaces", "CloseBracketPunctuator"]
],
"replace": [
["$1", " ", "$2"],
["$1", " ", "$2"]
]
},
"false": {
"find": [
["OpenBracketPunctuator", "WhiteSpacesSansNewline"],
["WhiteSpacesSansNewline", "CloseBracketPunctuator"]
],
"replace": [
["$1", null],
[null, "$2"]
]
},
"hybrid": {
"find": [
["OpenBracketPunctuator", "NonWhiteSpacesNonOpenBracketPunctuator"],
["NonWhiteSpacesNonCloseBracketPunctuator", "CloseBracketPunctuator"],
["OpenBracketPunctuator", "WhiteSpacesSansNewline", "NonOpenBracketPunctuator"],
["NonCloseBracketPunctuator", "WhiteSpacesSansNewline", "CloseBracketPunctuator"]
],
"replace": [
["$1", " ", "$2"],
["$1", " ", "$2"],
["$1", null, "$2"],
["$1", null, "$2"]
]
}
}
}You have to also consider the fact that the more rules you define, the more chance there is for them to collide with each other, which then brings in the need for priority on rules.