From dfc97db32325ed86ff86e51f6091714aa21dda65 Mon Sep 17 00:00:00 2001 From: Orta Therox Date: Thu, 11 Jul 2019 14:26:03 -0400 Subject: [PATCH 1/3] Don't add extra indentation for objects inside function parameters --- src/services/formatting/smartIndenter.ts | 3 ++- ...dentionsOfObjectsInAListAfterFormatting.ts | 2 +- .../formatMultipleFunctionArguments.ts | 26 +++++++++++++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) create mode 100644 tests/cases/fourslash/formatMultipleFunctionArguments.ts diff --git a/src/services/formatting/smartIndenter.ts b/src/services/formatting/smartIndenter.ts index 0c9e7b186f..62f4c3c556 100644 --- a/src/services/formatting/smartIndenter.ts +++ b/src/services/formatting/smartIndenter.ts @@ -482,7 +482,6 @@ namespace ts.formatting { case SyntaxKind.ArrayLiteralExpression: case SyntaxKind.Block: case SyntaxKind.ModuleBlock: - case SyntaxKind.ObjectLiteralExpression: case SyntaxKind.TypeLiteral: case SyntaxKind.MappedType: case SyntaxKind.TupleType: @@ -524,6 +523,8 @@ namespace ts.formatting { return rangeIsOnOneLine(sourceFile, child!); } return true; + case SyntaxKind.CallExpression: + return childKind !== SyntaxKind.ObjectLiteralExpression case SyntaxKind.DoStatement: case SyntaxKind.WhileStatement: case SyntaxKind.ForInStatement: diff --git a/tests/cases/fourslash/consistenceOnIndentionsOfObjectsInAListAfterFormatting.ts b/tests/cases/fourslash/consistenceOnIndentionsOfObjectsInAListAfterFormatting.ts index 14ada20602..bbcdaec9ca 100644 --- a/tests/cases/fourslash/consistenceOnIndentionsOfObjectsInAListAfterFormatting.ts +++ b/tests/cases/fourslash/consistenceOnIndentionsOfObjectsInAListAfterFormatting.ts @@ -8,4 +8,4 @@ format.document(); goTo.marker("1"); verify.currentLineContentIs("}, {"); goTo.marker("2"); -verify.currentLineContentIs(" });"); \ No newline at end of file +verify.currentLineContentIs("});"); diff --git a/tests/cases/fourslash/formatMultipleFunctionArguments.ts b/tests/cases/fourslash/formatMultipleFunctionArguments.ts new file mode 100644 index 0000000000..1e3909305a --- /dev/null +++ b/tests/cases/fourslash/formatMultipleFunctionArguments.ts @@ -0,0 +1,26 @@ +/// + +//// +//// someRandomFunction({ +//// prop1: 1, +//// prop2: 2 +//// }, { +//// prop3: 3, +//// prop4: 4 +//// }, { +//// prop5: 5, +//// prop6: 6 +//// }); + +format.document(); +verify.currentFileContentIs(` +someRandomFunction({ + prop1: 1, + prop2: 2 +}, { + prop3: 3, + prop4: 4 +}, { + prop5: 5, + prop6: 6 +});`); From 59d5585814afee67d162228c9b5f702ab1caf8ab Mon Sep 17 00:00:00 2001 From: Orta Therox Date: Fri, 12 Jul 2019 15:21:57 -0400 Subject: [PATCH 2/3] Don't indent properties if an object literal follows directly from another object on the same line --- src/services/formatting/formatting.ts | 3 +++ src/services/formatting/smartIndenter.ts | 23 +++++++++++++++++-- .../formatMultipleFunctionArguments.ts | 20 +++++++++++++++- 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index 01c09a4989..e4b6459a31 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -490,6 +490,9 @@ namespace ts.formatting { else if (SmartIndenter.childStartsOnTheSameLineWithElseInIfStatement(parent, node, startLine, sourceFile)) { return { indentation: parentDynamicIndentation.getIndentation(), delta }; } + else if (SmartIndenter.childStartsInlineWithPreviousObject(parent, node, startLine, sourceFile)) { + return { indentation: parentDynamicIndentation.getIndentation(), delta }; + } else { return { indentation: parentDynamicIndentation.getIndentation() + parentDynamicIndentation.getDelta(node), delta }; } diff --git a/src/services/formatting/smartIndenter.ts b/src/services/formatting/smartIndenter.ts index 62f4c3c556..766228c1f0 100644 --- a/src/services/formatting/smartIndenter.ts +++ b/src/services/formatting/smartIndenter.ts @@ -322,6 +322,26 @@ namespace ts.formatting { return false; } + export function childStartsInlineWithPreviousObject(parent: Node, child: TextRangeWithKind, childStartLine: number, sourceFile: SourceFileLike): boolean { + if (parent.kind === SyntaxKind.CallExpression) { + const parentCallExpression = parent; + const currentNode = find(parentCallExpression.arguments, arg => arg.pos === child.pos); + if (!currentNode) return false; // Shouldn't happen + + const currentIndex = parentCallExpression.arguments.indexOf(currentNode); + if (currentIndex === 0) return false; // Can't look at previous node if first + + const previousNode = parentCallExpression.arguments[currentIndex - 1]; + const lineOfPreviousNode = getLineAndCharacterOfPosition(sourceFile, previousNode.getEnd()).line; + + if (childStartLine === lineOfPreviousNode) { + return true; + } + } + + return false; + } + export function getContainingList(node: Node, sourceFile: SourceFile): NodeArray | undefined { return node.parent && getListByRange(node.getStart(sourceFile), node.getEnd(), node.parent, sourceFile); } @@ -482,6 +502,7 @@ namespace ts.formatting { case SyntaxKind.ArrayLiteralExpression: case SyntaxKind.Block: case SyntaxKind.ModuleBlock: + case SyntaxKind.ObjectLiteralExpression: case SyntaxKind.TypeLiteral: case SyntaxKind.MappedType: case SyntaxKind.TupleType: @@ -523,8 +544,6 @@ namespace ts.formatting { return rangeIsOnOneLine(sourceFile, child!); } return true; - case SyntaxKind.CallExpression: - return childKind !== SyntaxKind.ObjectLiteralExpression case SyntaxKind.DoStatement: case SyntaxKind.WhileStatement: case SyntaxKind.ForInStatement: diff --git a/tests/cases/fourslash/formatMultipleFunctionArguments.ts b/tests/cases/fourslash/formatMultipleFunctionArguments.ts index 1e3909305a..32e6d0eea8 100644 --- a/tests/cases/fourslash/formatMultipleFunctionArguments.ts +++ b/tests/cases/fourslash/formatMultipleFunctionArguments.ts @@ -11,6 +11,15 @@ //// prop5: 5, //// prop6: 6 //// }); +//// +//// someRandomFunction( +//// { prop7: 1, prop8: 2 }, +//// { prop9: 3, prop10: 4 }, +//// { +//// prop11: 5, +//// prop2: 6 +//// } +//// ); format.document(); verify.currentFileContentIs(` @@ -23,4 +32,13 @@ someRandomFunction({ }, { prop5: 5, prop6: 6 -});`); +}); + +someRandomFunction( + { prop7: 1, prop8: 2 }, + { prop9: 3, prop10: 4 }, + { + prop11: 5, + prop2: 6 + } +);`); From 1d78218053332e0a7de3cadcea53a64f3b0beff1 Mon Sep 17 00:00:00 2001 From: Orta Therox Date: Mon, 15 Jul 2019 10:48:10 -0400 Subject: [PATCH 3/3] Handle feedback from #32359 --- src/services/formatting/formatting.ts | 2 +- src/services/formatting/smartIndenter.ts | 13 ++++++------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index e4b6459a31..ed6b71efb7 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -490,7 +490,7 @@ namespace ts.formatting { else if (SmartIndenter.childStartsOnTheSameLineWithElseInIfStatement(parent, node, startLine, sourceFile)) { return { indentation: parentDynamicIndentation.getIndentation(), delta }; } - else if (SmartIndenter.childStartsInlineWithPreviousObject(parent, node, startLine, sourceFile)) { + else if (SmartIndenter.argumentStartsOnSameLineAsPreviousArgument(parent, node, startLine, sourceFile)) { return { indentation: parentDynamicIndentation.getIndentation(), delta }; } else { diff --git a/src/services/formatting/smartIndenter.ts b/src/services/formatting/smartIndenter.ts index 766228c1f0..7a8bfb5f5d 100644 --- a/src/services/formatting/smartIndenter.ts +++ b/src/services/formatting/smartIndenter.ts @@ -322,16 +322,15 @@ namespace ts.formatting { return false; } - export function childStartsInlineWithPreviousObject(parent: Node, child: TextRangeWithKind, childStartLine: number, sourceFile: SourceFileLike): boolean { - if (parent.kind === SyntaxKind.CallExpression) { - const parentCallExpression = parent; - const currentNode = find(parentCallExpression.arguments, arg => arg.pos === child.pos); - if (!currentNode) return false; // Shouldn't happen + export function argumentStartsOnSameLineAsPreviousArgument(parent: Node, child: TextRangeWithKind, childStartLine: number, sourceFile: SourceFileLike): boolean { + if (isCallOrNewExpression(parent)) { + if (!parent.arguments) return false; - const currentIndex = parentCallExpression.arguments.indexOf(currentNode); + const currentNode = Debug.assertDefined(find(parent.arguments, arg => arg.pos === child.pos)); + const currentIndex = parent.arguments.indexOf(currentNode); if (currentIndex === 0) return false; // Can't look at previous node if first - const previousNode = parentCallExpression.arguments[currentIndex - 1]; + const previousNode = parent.arguments[currentIndex - 1]; const lineOfPreviousNode = getLineAndCharacterOfPosition(sourceFile, previousNode.getEnd()).line; if (childStartLine === lineOfPreviousNode) {