Incorrect (potentially) indentation of function arguments spanning multiple lines #11629
Comments
|
@nomaed thank you for opening this issue. @dbaeumer can I get some other eyes on this? I think I actually prefer this result with auto formatting: But that might simply be my preference. Possibly there should be an option for OP to customize the auto formatting for his preference? |
|
@bowdenk7 if you have a thought on this too that would be helpful. Don't know if this has come up much in your user studies... |
|
From @nomaed on October 14, 2016 15:8 It definitely shouldn't be the only option. I personally grew to like it a lot (it's the default setting in JetBrains' IDEs) and since it's a default for TSLint (https://github.com/palantir/tslint/blob/master/src/rules/alignRule.ts), I am thinking that it's not uncommon to use this setting. |
|
Moving this to TS language service to address adding the setting. Thank you for your feedback @nomaed. It is valued very highly. |
waderyan
added
VS Code Tracked
|
I have a related question about good practice in formatting: what about the returned type? It should be indent, in next line, or how? class Example {
public method(arg1: string,
arg2: number,
arg3: boolean)
: ReturnType {
const result = doSomething();
return new ReturnType(result);
}
}or maybe: |
|
@19majkel94 Option no. 2 probably. I think that these should all be valid: /**
* #1
* If first arg is on the same line as the function decl/name,
* all args are lined up with the first one.
*/
function funcName(arg1: string,
─────────────────┤arg2: number,
─────────────────┤arg3: any): ReturnType {
⇢ console.log('Hello world');
}
/**
* #2
* If first arg is newlined, it is double-indented so it remains 1 indent
* level in front of impl., and all args are lined up.
*/
function funcName(
⇢ ⇢ arg1: string,
⇢ ⇢ arg2: number,
⇢ ⇢ arg3: any): ReturnType {
⇢ console.log('Hello world');
}
/**
* #3
* In case first arg is newlined like in #2, allow closing parenthesis
* to be newlined too (outdent once) with return type following it
*/
function funcName(
⇢ ⇢ arg1: string,
⇢ ⇢ arg2: number,
⇢ ⇢ arg3: any
⇢ ): ReturnType {
⇢ console.log('Hello world');
}The reason why I don't like the way it is now is that the argument aren't aligned and it's decreasing readability. |
|
In case no. 3, the return type indent once is decreasing readability. So I prefer my option no. 2 but it's |
|
My prefered style for large class Example {
public method(
arg1: string,
arg2: number,
arg3: boolean
): ReturnType {
const result = doSomething();
return new ReturnType(result);
}
} |
waderyan
added
the
Awaiting More Feedback
|
I'd like to add that the current indentation rule appear to work incorrectly when combined wth multiple nesting levels. In a naked VSCode install, auto format produces this: instead of: |
|
Maybe I am missing something, but my project has the following {
"defaultSeverity": "error",
"extends": [
"tslint:recommended"
],
"jsRules": {},
"rules": {
"quotemark": [
"single"
],
"no-console": [
false
],
"arrow-parens": false,
"no-string-literal": false,
"no-namespace": [
"allow-declarations"
],
"interface-name": [
"never-prefix"
],
"no-trailing-whitespace": [true, "ignore-comments"]
},
"rulesDirectory": []
}And TSLint does complain about parameters not being aligned after I auto format (using default settings, I am pretty sure since I am not overriding anything with |
mhegazy
added
Suggestion
the tslint rule for aligning blocks of code and typescript-formatter are in conflict because the formatter doesn't like aligning parameters of a function, see microsoft/TypeScript#11629
the tslint rule for aligning blocks of code and typescript-formatter are in conflict because the formatter doesn't like aligning parameters of a function, see microsoft/TypeScript#11629
* refactor(file name): implement project layout defined in #126 BREAKING CHANGE: everyone need to update what they are working on in order to follow the new project structure fix #144 * chore(tslint): turn off align rule due to conflict the tslint rule for aligning blocks of code and typescript-formatter are in conflict because the formatter doesn't like aligning parameters of a function, see microsoft/TypeScript#11629
|
It would be best to provide a configuration so a user can choose which way to go. |
I see quite a of bit of inconsistency in the formatting. Let's add a
.editorconfig file, so the code we write uses the right whitespacing
regardless of the editor we use. I just copied the one I wrote earlier
for Theia to get started, but it doesn't have to be identical.
I used Theia to auto-format all ts and json files and then tslint --fix
to fixup any warnings that it would have inserted.
For example, I the Typescript formatter handles quite badly (IMO) wrapping long
parameter lists, and produces code that result in a warning according to
our current tslint config. The Typescript formatter will do this:
```
export class Bob {
constructor(a: number, b: number, c: number, d: number,
e: number, f: number) {
console.log('Hello');
}
}
```
where the second line of parameters is indented with 4 spaces. I think
is not very readable, because it's easily confused with the first line
of code. I prefer our current tslint setting, which requires it to be
aligned like this:
```
export class Bob {
constructor(a: number, b: number, c: number, d: number,
i e: number, f: number) {
console.log('Hello');
}
}
```
This means that formatting entire documents will always change the code
to look like the first snippet above. It's a minor annoyance, and
hopefully there will be a setting for that one day in the Typescript
formatter. There is an issue about that here:
microsoft/TypeScript#11629
Signed-off-by: Simon Marchi <simon.marchi@ericsson.com>
I see quite a of bit of inconsistency in the formatting. Let's add a
.editorconfig file, so the code we write uses the right whitespacing
regardless of the editor we use. I just copied the one I wrote earlier
for Theia to get started, but it doesn't have to be identical.
I used Theia to auto-format all ts and json files and then tslint --fix
to fixup any warnings that it would have inserted.
For example, I the Typescript formatter handles quite badly (IMO) wrapping long
parameter lists, and produces code that result in a warning according to
our current tslint config. The Typescript formatter will do this:
```
export class Bob {
constructor(a: number, b: number, c: number, d: number,
e: number, f: number) {
console.log('Hello');
}
}
```
where the second line of parameters is indented with 4 spaces. I think
is not very readable, because it's easily confused with the first line
of code. I prefer our current tslint setting, which requires it to be
aligned like this:
```
export class Bob {
constructor(a: number, b: number, c: number, d: number,
i e: number, f: number) {
console.log('Hello');
}
}
```
This means that formatting entire documents will always change the code
to look like the first snippet above. It's a minor annoyance, and
hopefully there will be a setting for that one day in the Typescript
formatter. There is an issue about that here:
microsoft/TypeScript#11629
Signed-off-by: Simon Marchi <simon.marchi@ericsson.com>
I see quite a of bit of inconsistency in the formatting. Let's add a
.editorconfig file, so the code we write uses the right whitespacing
regardless of the editor we use. I just copied the one I wrote earlier
for Theia to get started, but it doesn't have to be identical.
I used Theia to auto-format all ts and json files and then tslint --fix
to fixup any warnings that it would have inserted.
For example, I the Typescript formatter handles quite badly (IMO) wrapping long
parameter lists, and produces code that result in a warning according to
our current tslint config. The Typescript formatter will do this:
```
export class Bob {
constructor(a: number, b: number, c: number, d: number,
e: number, f: number) {
console.log('Hello');
}
}
```
where the second line of parameters is indented with 4 spaces. I think
is not very readable, because it's easily confused with the first line
of code. I prefer our current tslint setting, which requires it to be
aligned like this:
```
export class Bob {
constructor(a: number, b: number, c: number, d: number,
i e: number, f: number) {
console.log('Hello');
}
}
```
This means that formatting entire documents will always change the code
to look like the first snippet above. It's a minor annoyance, and
hopefully there will be a setting for that one day in the Typescript
formatter. There is an issue about that here:
microsoft/TypeScript#11629
Signed-off-by: Simon Marchi <simon.marchi@ericsson.com>
I see quite a of bit of inconsistency in the formatting. Let's add a
.editorconfig file, so the code we write uses the right whitespacing
regardless of the editor we use. I just copied the one I wrote earlier
for Theia to get started, but it doesn't have to be identical.
I used Theia to auto-format all ts and json files and then tslint --fix
to fixup any warnings that it would have inserted.
For example, I the Typescript formatter handles quite badly (IMO) wrapping long
parameter lists, and produces code that result in a warning according to
our current tslint config. The Typescript formatter will do this:
```
export class Bob {
constructor(a: number, b: number, c: number, d: number,
e: number, f: number) {
console.log('Hello');
}
}
```
where the second line of parameters is indented with 4 spaces. I think
is not very readable, because it's easily confused with the first line
of code. I prefer our current tslint setting, which requires it to be
aligned like this:
```
export class Bob {
constructor(a: number, b: number, c: number, d: number,
i e: number, f: number) {
console.log('Hello');
}
}
```
This means that formatting entire documents will always change the code
to look like the first snippet above. It's a minor annoyance, and
hopefully there will be a setting for that one day in the Typescript
formatter. There is an issue about that here:
microsoft/TypeScript#11629
Signed-off-by: Simon Marchi <simon.marchi@ericsson.com>
I see quite a of bit of inconsistency in the formatting. Let's add a
.editorconfig file, so the code we write uses the right whitespacing
regardless of the editor we use. I just copied the one I wrote earlier
for Theia to get started, but it doesn't have to be identical.
I used Theia to auto-format all ts and json files and then tslint --fix
to fixup any warnings that it would have inserted.
For example, I the Typescript formatter handles quite badly (IMO) wrapping long
parameter lists, and produces code that result in a warning according to
our current tslint config. The Typescript formatter will do this:
```
export class Bob {
constructor(a: number, b: number, c: number, d: number,
e: number, f: number) {
console.log('Hello');
}
}
```
where the second line of parameters is indented with 4 spaces. I think
is not very readable, because it's easily confused with the first line
of code. I prefer our current tslint setting, which requires it to be
aligned like this:
```
export class Bob {
constructor(a: number, b: number, c: number, d: number,
i e: number, f: number) {
console.log('Hello');
}
}
```
This means that formatting entire documents will always change the code
to look like the first snippet above. It's a minor annoyance, and
hopefully there will be a setting for that one day in the Typescript
formatter. There is an issue about that here:
microsoft/TypeScript#11629
Signed-off-by: Simon Marchi <simon.marchi@ericsson.com>
|
This is still an open issue in VS code 1.30.1 - the TS autoformatter removes leading whitespace, even if the tslint config is: "rules": {
"align": [
true,
"parameters",
"arguments",
"statements",
"members",
"elements"
],The comments above RE "I prefer a single tab indent when continuing the parameter list on a newline" aren't particularly useful - this is still a bug. |
|
What about this issue? Same problem here. |
|
+1 for adding this formatting option in settings |
|
+1 for this. Just migrating from Intellij to VS Code and this is one of the things that bothers me a lot. There are so many ways to configure the editor, but this simple logic requirement is not configurable. Any progress about that? |
|
FWIW, we have been using prettier now for a couple of years. Really helped with this and other code formatting consistency issues across our teams/products. |
|
+1 for adding this formatting option in settings |
|
+1. I would like to be able to set it the same way as my TSLint is configured. |
Prettier moves all the args on the same line... So it's not helpful at all with this. |
prettier is opinionated. We need to fix this in vscode if possible. |
|
Related Stack Overflow question: VS Code document formatter to indent multi line arguments vertically aligned to the opening bracket/brace of the argument list |
|
I'd also like to +1 this. Our team uses parenthesis aligned parameters for all other programming languages and it's a pain to work with TS due to this issue. We need a way to control parameter alignment on newlines. |
|
hi, did this issue fixed? |
|
Has anyone figured out a workaround until this can get resolved? The biggest issue, IMO, is the fact that following lines also get their indentation messed up. So even if you indent things manually, you end up fighting the editor for the next line. |
From @nomaed on October 14, 2016 10:40
Steps to Reproduce:
Auto-formatting produces this result:
I would expect to see this result instead though:
Also, when manually formatting the arguments to appear in the same column (note: this is also the default/recommended setting in
tslintand maybe other linters), then further lines will start with wrong indentation:Copied from original issue: microsoft/vscode#13748
The text was updated successfully, but these errors were encountered: