Skip to content

Commit

Permalink
Fix jsx-indent to not check lines starting by literals (fixes #900)
Browse files Browse the repository at this point in the history
  • Loading branch information
yannickcr committed Oct 12, 2016
1 parent c056489 commit 798f2ca
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 2 deletions.
3 changes: 1 addition & 2 deletions lib/rules/jsx-indent.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ module.exports = {
function getFixerFunction(node, needed, gotten) {
if (needed > gotten) {
var spaces = indentChar.repeat(needed - gotten);

return function(fixer) {
return fixer.insertTextBeforeRange([node.range[0], node.range[0]], spaces);
};
Expand Down Expand Up @@ -169,7 +168,7 @@ module.exports = {
var token = node;
do {
token = sourceCode.getTokenBefore(token);
} while (token.type === 'JSXText');
} while (token.type === 'JSXText' && /^\s*$/.test(token.value));
var startLine = node.loc.start.line;
var endLine = token ? token.loc.end.line : -1;

Expand Down
11 changes: 11 additions & 0 deletions tests/lib/rules/jsx-indent.js
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,17 @@ ruleTester.run('jsx-indent', rule, {
].join('\n'),
parserOptions: parserOptions,
options: [2]
}, {
// Literals indentation is not touched
code: [
'<div>',
'bar <div>',
' bar',
' bar {foo}',
'bar </div>',
'</div>'
].join('\n'),
parserOptions: parserOptions
}],

invalid: [{
Expand Down

5 comments on commit 798f2ca

@ljharb
Copy link
Member

@ljharb ljharb commented on 798f2ca Oct 12, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would we want to not touch literals indentation? That should be fixed too (or at least, reported).

@yannickcr
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixing it require to modify the literal and I did not find a good way to do it yet. And reporting it is problematic too, since line breaks are considered part of the Literal.

So for now I chose to ignore them.

@ljharb
Copy link
Member

@ljharb ljharb commented on 798f2ca Nov 1, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's wrong with reporting it and leaving it unfixed? I'd rather have some errors unfixable than have them suppressed :-/

@yannickcr
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since line breaks are considered part of the Literal the error will be reported on the wrong line.

<div>
bar </div>

Here the literal is \nbar and the error will be reported on line 1. Also just detecting indentation errors on literals is not so easy.

@ljharb
Copy link
Member

@ljharb ljharb commented on 798f2ca Nov 2, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i see. tbh i think an error on the wrong line is better than no error, but i understand a bit better now.

Please sign in to comment.