New: vue/html-indent rule (fixes #46)
#145
Conversation
mysticatea
changed the title
vue/html-indent rule (fixes #46)vue/html-indent rule (fixes #46)
|
Thank you for your patience. I think I have done. Please review and suggest lacking tests! |
.vscode/settings.json
Outdated
| @@ -1,3 +1,4 @@ | |||
| { | |||
| "editor.tabSize": 2 | |||
| "editor.tabSize": 2, | |||
There was a problem hiding this comment.
seems not related to the vue/html-indent. I think it can be in another PR?
package.json
Outdated
| @@ -10,7 +10,8 @@ | |||
| "lint": "eslint .", | |||
| "pretest": "npm run lint", | |||
| "preversion": "npm run update && npm test", | |||
| "update": "node ./tools/update-rules.js" | |||
| "update": "node ./tools/update-rules.js", | |||
| "watch": "warun lib/rules/html-indent.js tests/lib/rules/html-indent.js -- nyc --reporter lcov -- mocha tests/lib/rules/html-indent.js" | |||
There was a problem hiding this comment.
what is the warun? I didn't see it in deps.
There was a problem hiding this comment.
Oops, it was added unintentionally. I was using it to test.
Thank you.
| * @returns {{firstToken:Token,lastToken:Token}} The gotten tokens. | ||
| */ | ||
| function getFirstAndLastTokens (node, borderOffset) { | ||
| borderOffset |= 0 |
There was a problem hiding this comment.
could you explain it a little? :)
There was a problem hiding this comment.
its js magic assign 0 if value is non numeric
There was a problem hiding this comment.
hah.. got it! amazing! lol~
There was a problem hiding this comment.
Yes, it is "casting to signed integer".
Bitwise operators (&, |, "^", ~, <<, >>, and >>>) casts both operands to 32bits signed integer, then the logical OR with 0 does nothing. As the result, the x | 0 expression casts x to 32bits signed integer.
There was a problem hiding this comment.
@mysticatea like i said magic
| return | ||
| } | ||
|
|
||
| // Debug log |
There was a problem hiding this comment.
this debug log seems no longer required. 😄
There was a problem hiding this comment.
Yeah!
But I'd like to leave it to investigate for bug reports, since the debug code is long...
| <div | ||
| aaa | ||
| bbb | ||
| `, |
There was a problem hiding this comment.
not a valid template, is is intentional?
and why the parser not reporting an error?
There was a problem hiding this comment.
Yes, it's intentional. The HTML spec includes recovering process for invalid syntax, so vue-eslint-parser reports syntax errors with the programNode.templateBody.errors property instead of throwing the error. Then vue/no-parsing-error rule would report those syntax errors.
The purpose of this test case is to check whether vue/html-indent rule works properly on an incomplete code. Especially, the expected indentation of closing brackets (>) does not apply to the last attribute even if the closing bracket is lacking.
docs/rules/html-indent.md
Outdated
| "vue/html-indent": ["error", type, { | ||
| "attribute": 1, | ||
| "closeBracket": 0, | ||
| "switchCase": 0, |
There was a problem hiding this comment.
could you please add an example for the switchCase option?
There was a problem hiding this comment.
Or maybe I should remove the option from this rule since switch is a prohibited keyword.
| bar(); | ||
| " | ||
| > | ||
| {{ |
There was a problem hiding this comment.
maybe we need some more tests for {{}}.
and I didn't see tests for comma, e.g:
<template>
{{
foo,
bar,
qux
}}
</template>There was a problem hiding this comment.
In technically, mustaches ({{foo}}) and directive values ("foo" of directives) are completely same for indentation. Both are VExpressionContainer. So I don't think we need many tests for mustaches. I added some tests.
There was a problem hiding this comment.
Wow, impressive work @mysticatea ! 🚀 I wasn't expecting this rule to be that complex, now I see how wrong I was.. I think the best way to properly check this rule is to merge it and beta test on real projects as we still have some time before official release. Feel free to merge it :)
|
|
||
| // Collect related tokens. | ||
| // Commas between this and the previous, and the first token of this node. | ||
| if (lastToken != null) { |
There was a problem hiding this comment.
Do you actually need to check for != null everywhere? From what I can tell it would make sense if the value could be also 0 and it would've been expected. But since those tokens are either Objects or null, simple if(lastToken) would be enough, wouldn't it?
There was a problem hiding this comment.
I apology that I'm inactive during a long time.
It, ensuring boolean type for conditional expression, is just my favorite style 😄
Fixes #46.
This PR adds
vue/html-indentrule.The strategy of the
vue/html-indentrule is the same as coreindentrule. I.e., it makes the base token and offset for every token in the first traversing, then at leaving the root element, it calculate and validate expected indentation for each line.Currently, about inside of directives and mustaches, this
vue/html-indentrule supports only the syntax ECMAScript 2017 standard includes. It just ignores other syntaxes. The indentation rule is one of most complex rules. I think this is a good start point.