indent: support size and fix #2723
Conversation
|
Thanks for your interest in palantir/tslint, @filipesilva! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request. |
src/rules/indentRule.ts
Outdated
|
|
||
| A second optional argument specifies indentation size, defaulting to 4: | ||
|
|
||
| * \`${OPTION_INDENT_SIZE_2.toString()}\` enforces 2 space indentation. |
There was a problem hiding this comment.
is .toString() really needed here?
There was a problem hiding this comment.
Yeah that surprised me as well! I thought it wasn't necessary but TS is telling me it is:
src/rules/indentRule.ts
Outdated
|
|
||
| constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) { | ||
| super(sourceFile, options); | ||
|
|
||
| if (this.hasOption(OPTION_INDENT_SIZE_2)) { |
There was a problem hiding this comment.
the docs say it has to be the second option. so you can just use this.getOptions().ruleArguments[1] instead of this.hasOption(...)
There was a problem hiding this comment.
Sgtm, will also remove the hasOption change then.
src/rules/indentRule.ts
Outdated
| } else if (this.hasOption(OPTION_INDENT_SIZE_4)) { | ||
| this.size = OPTION_INDENT_SIZE_4; | ||
| } else { | ||
| this.size = OPTION_INDENT_SIZE_DEFAULT; |
There was a problem hiding this comment.
how do I tell the rule not to mess with my indentation level if it defaults to 4?
I'd suggest not to fix anything if there is no indentSize specified.
There was a problem hiding this comment.
I agree, not fixing anything is a better default now that I think about it.
|
|
||
| A second optional argument specifies indentation size: | ||
|
|
||
| * \`${OPTION_INDENT_SIZE_2.toString()}\` enforces 2 space indentation. |
There was a problem hiding this comment.
ts requires toString here because the dedent function takes an array of strings. We can probably make it take in (string | number)[] later
src/rules/indentRule.ts
Outdated
| size = OPTION_INDENT_SIZE_2; | ||
| } else if (this.getOptions()[1] === OPTION_INDENT_SIZE_4) { | ||
| size = OPTION_INDENT_SIZE_4; | ||
| } |
There was a problem hiding this comment.
probably shouldn't do fixes if 2nd arg is an invalid number or type
src/rules/indentRule.ts
Outdated
| }, | ||
| optionExamples: [[true, "spaces"]], | ||
| optionExamples: [ | ||
| [true, "spaces", 4], |
There was a problem hiding this comment.
can replace "string" with const
|
@filipesilva thanks! |
|
Awesome! We this change in, we're thinking of using TSLint in Angular CLI to enable quotes/indentation auto fixing on file generation. It's something our users have been asking for a while. |
PR checklist
Overview of change:
This PR adds indent size support to the
indentrule (defaults to 4), and also allows auto fixing.Is there anything you'd like reviewers to focus on?
Correctness of the auto-fix.
CHANGELOG.md entry:
[new-rule-option][new-fixer]
indentsupport size and fix