Add mustache-curly-spacing rule.
#153
Conversation
armano2
force-pushed
the
curly-bracket-spacing
branch
from
587a32f to
435e9d0
Compare
armano2
changed the title
curly-bracket-spacing.curly-bracket-spacing rule.
docs/rules/curly-bracket-spacing.md
Outdated
| @@ -0,0 +1,67 @@ | |||
| # Enforce spacing on the style of curly brackets. (curly-bracket-spacing) | |||
There was a problem hiding this comment.
I think that curly-bracket has too wide meaning. There are object-curly-spacing, template-curly-spacing, and block-spacing in core. How about mustache-curly-spacing or embedded-expression-curly-spacing?
There was a problem hiding this comment.
i prefer mustache-curly-spacing.
docs/rules/curly-bracket-spacing.md
Outdated
|
|
||
| ```html | ||
| <template> | ||
| <div>{{ text }}</div> |
There was a problem hiding this comment.
It looks a correct code to me. It should be covered by no-multi-spaces rule.
| invalid: [ | ||
| { | ||
| filename: 'test.vue', | ||
| code: '<template><div>{{ }}</div></template>', |
There was a problem hiding this comment.
I think that this rule should ignore mustaches which have syntax errors. If syntax errors exist, node.expression is null.
| filename: 'test.vue', | ||
| code: '<template><div>{{ text }}</div></template>', | ||
| options: ['always'] | ||
| } |
There was a problem hiding this comment.
Could you add tests which confirm that this rule doesn't warn directives, especially directives with unquoted attributes?
There was a problem hiding this comment.
@mysticatea yup there was issue here, i forgot to check types of startToken and endToken
|
@mysticatea suggestion requested in code review applied, |
armano2
changed the title
curly-bracket-spacing rule.mustache-curly-spacing rule.
armano2
force-pushed
the
curly-bracket-spacing
branch
from
cf083cb to
48107fb
Compare
docs/rules/mustache-curly-spacing.md
Outdated
| @@ -0,0 +1,67 @@ | |||
| # Enforce spacing on the style of mustache curly brackets. (mustache-curly-spacing) | |||
There was a problem hiding this comment.
Regarding rule name I have another proposition: mustache-interpolation-spacing. We all know what interpolation is, plus mustache interpolation type is well described in official documentation - this way we should avoid any confusion. What do you think @armano2 @mysticatea ?
There was a problem hiding this comment.
@michalsnik I was not sure about the name of this rule, there was many suggestions like: curly-bracket, mustache-curly-spacing, embedded-expression-curly-spacing but i think yours matches at least documentation.
docs/rules/mustache-curly-spacing.md
Outdated
|
|
||
| ## :book: Rule Details | ||
|
|
||
| This rule aims to enforce unified spacing of curly brackets. |
lib/rules/mustache-curly-spacing.js
Outdated
| module.exports = { | ||
| meta: { | ||
| docs: { | ||
| description: 'Enforce spacing on the style of mustache curly brackets.', |
There was a problem hiding this comment.
enforce unified spacing in mustache interpolations
lib/rules/mustache-curly-spacing.js
Outdated
| filter: token => token.type !== 'HTMLWhitespace' // When there is only whitespace between ignore it | ||
| }) | ||
|
|
||
| const startToken = tokens.shift() |
There was a problem hiding this comment.
Do you intentionally mutate tokens here? Using that kind of methods usually introduce more confusion and may lead to unexpected results.
There was a problem hiding this comment.
Yes, i'm taking first and last token from array, checking their types, and i'm using it to compare
lib/rules/mustache-curly-spacing.js
Outdated
| function checkTokens (leftToken, rightToken) { | ||
| if (leftToken.loc.end.line === rightToken.loc.start.line) { | ||
| const spaces = rightToken.loc.start.column - leftToken.loc.end.column | ||
| if (optSpaces === (spaces === 0)) { |
There was a problem hiding this comment.
Don't you think this expression is hard to read?
Can you please keep it simple stupid?
const noSpacesFound = spaces === 0
if (optSpaces && noSpacesFound) {
|
@michalsnik suggestions applied, thank you for code review. |
This PR implements rule proposed in #150.