From 9088920efd1681a9845b59dd1daeb3da7114f2fd Mon Sep 17 00:00:00 2001 From: wukongO-O Date: Wed, 12 Mar 2025 02:04:32 -0400 Subject: [PATCH 1/5] Added test case for formatting nested ternary operators --- tests/cases/fourslash/formattingTernaryOperator.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 tests/cases/fourslash/formattingTernaryOperator.ts diff --git a/tests/cases/fourslash/formattingTernaryOperator.ts b/tests/cases/fourslash/formattingTernaryOperator.ts new file mode 100644 index 0000000000000..af43fc14d22bf --- /dev/null +++ b/tests/cases/fourslash/formattingTernaryOperator.ts @@ -0,0 +1,14 @@ +/// + +////var v = +/////*1*/0 ? 1 : +/////*2*/2 ? 3 : +/////*3*/4; + +format.document(); +goTo.marker('1'); +verify.indentationIs(4); +goTo.marker('2'); +verify.indentationIs(4); +goTo.marker('3'); +verify.indentationIs(4); From e1adde677befeab64a64ef909206d85044f10fad Mon Sep 17 00:00:00 2001 From: wukongO-O Date: Wed, 12 Mar 2025 02:23:41 -0400 Subject: [PATCH 2/5] Added a solution to address the aggressive indentation issue, but it causes test formattingConditionals to fail --- src/services/formatting/smartIndenter.ts | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/services/formatting/smartIndenter.ts b/src/services/formatting/smartIndenter.ts index 7f41a2701b8c3..a1bb8f0a3befa 100644 --- a/src/services/formatting/smartIndenter.ts +++ b/src/services/formatting/smartIndenter.ts @@ -438,6 +438,7 @@ export namespace SmartIndenter { // branch beginning on the line that the whenTrue branch ends. export function childIsUnindentedBranchOfConditionalExpression(parent: Node, child: TextRangeWithKind, childStartLine: number, sourceFile: SourceFileLike): boolean { if (isConditionalExpression(parent) && (child === parent.whenTrue || child === parent.whenFalse)) { + const conditionEndLine = getLineAndCharacterOfPosition(sourceFile, parent.condition.end).line; if (child === parent.whenTrue) { return childStartLine === conditionEndLine; @@ -450,8 +451,20 @@ export namespace SmartIndenter { // ? 1 : ( L1: whenTrue indented because it's on a new line // 0 L2: indented two stops, one because whenTrue was indented // ); and one because of the parentheses spanning multiple lines + + // However, when condition and whenTrue are on the same line, whenFalse should not be indented: + // + // const z = + // 0 ? 1 : L1: whenTrue indented because it's on a new line + // 2 ? 3 : L2: not indented + // 4; L3: not indented const trueStartLine = getStartLineAndCharacterForNode(parent.whenTrue, sourceFile).line; const trueEndLine = getLineAndCharacterOfPosition(sourceFile, parent.whenTrue.end).line; + + if (conditionEndLine === trueStartLine && trueStartLine === trueEndLine && trueEndLine !== childStartLine) { + return true; + } + return conditionEndLine === trueStartLine && trueEndLine === childStartLine; } } @@ -654,7 +667,6 @@ export namespace SmartIndenter { case SyntaxKind.VariableStatement: case SyntaxKind.ExportAssignment: case SyntaxKind.ReturnStatement: - case SyntaxKind.ConditionalExpression: case SyntaxKind.ArrayBindingPattern: case SyntaxKind.ObjectBindingPattern: case SyntaxKind.JsxOpeningElement: @@ -695,6 +707,14 @@ export namespace SmartIndenter { return true; } break; + case SyntaxKind.ConditionalExpression: + if (child && sourceFile) { + const childStartCharacter = sourceFile.getLineAndCharacterOfPosition(skipTrivia(sourceFile.text, child.pos)).character; + if (!rangeIsOnOneLine(sourceFile, parent) && childStartCharacter !== SyntaxKind.QuestionToken) { + return false; + } + } + return true; case SyntaxKind.DoStatement: case SyntaxKind.WhileStatement: case SyntaxKind.ForInStatement: From 9de5680bc0c4f3d619853fb2af5d5c7a392396fd Mon Sep 17 00:00:00 2001 From: wukongO-O Date: Thu, 13 Mar 2025 23:51:37 -0400 Subject: [PATCH 3/5] Updated additional formatting conditionals test cases --- ...tingConditionalsWithNestedFalseBranches.ts | 43 +++++++++++++++++++ .../fourslash/formattingTernaryOperator.ts | 14 ------ 2 files changed, 43 insertions(+), 14 deletions(-) create mode 100644 tests/cases/fourslash/formattingConditionalsWithNestedFalseBranches.ts delete mode 100644 tests/cases/fourslash/formattingTernaryOperator.ts diff --git a/tests/cases/fourslash/formattingConditionalsWithNestedFalseBranches.ts b/tests/cases/fourslash/formattingConditionalsWithNestedFalseBranches.ts new file mode 100644 index 0000000000000..0e118c9a50eb0 --- /dev/null +++ b/tests/cases/fourslash/formattingConditionalsWithNestedFalseBranches.ts @@ -0,0 +1,43 @@ +/// + +////var v = +/////*1*/0 ? 1 : +/////*2*/2 ? 3 : +/////*3*/4; + +////var x = +/////*4*/a ? b : +/////*5*/c ? d : +/////*6*/e; + +////var opacity = +/////*7*/depth == 0 ? 1 : +/////*8*/depth == 1 ? .7 : +/////*9*/depth == 2 ? .5 : +/////*10*/depth == 3 ? .4 : .3; + +format.document(); +goTo.marker('1'); +verify.indentationIs(4); +goTo.marker('2'); +verify.indentationIs(4); +goTo.marker('3'); +verify.indentationIs(4); + +format.document(); +goTo.marker('4'); +verify.indentationIs(4); +goTo.marker('5'); +verify.indentationIs(4); +goTo.marker('6'); +verify.indentationIs(4); + +format.document(); +goTo.marker('7'); +verify.indentationIs(4); +goTo.marker('8'); +verify.indentationIs(4); +goTo.marker('9'); +verify.indentationIs(4); +goTo.marker('10'); +verify.indentationIs(4); \ No newline at end of file diff --git a/tests/cases/fourslash/formattingTernaryOperator.ts b/tests/cases/fourslash/formattingTernaryOperator.ts deleted file mode 100644 index af43fc14d22bf..0000000000000 --- a/tests/cases/fourslash/formattingTernaryOperator.ts +++ /dev/null @@ -1,14 +0,0 @@ -/// - -////var v = -/////*1*/0 ? 1 : -/////*2*/2 ? 3 : -/////*3*/4; - -format.document(); -goTo.marker('1'); -verify.indentationIs(4); -goTo.marker('2'); -verify.indentationIs(4); -goTo.marker('3'); -verify.indentationIs(4); From 018051bf3ee5da323aadcb86a729f50ba026ac44 Mon Sep 17 00:00:00 2001 From: wukongO-O Date: Sat, 15 Mar 2025 13:39:54 -0400 Subject: [PATCH 4/5] Added a solution to fix aggressive identation for conditional expression's false branch --- src/services/formatting/smartIndenter.ts | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/src/services/formatting/smartIndenter.ts b/src/services/formatting/smartIndenter.ts index a1bb8f0a3befa..3548e5e16720f 100644 --- a/src/services/formatting/smartIndenter.ts +++ b/src/services/formatting/smartIndenter.ts @@ -451,20 +451,8 @@ export namespace SmartIndenter { // ? 1 : ( L1: whenTrue indented because it's on a new line // 0 L2: indented two stops, one because whenTrue was indented // ); and one because of the parentheses spanning multiple lines - - // However, when condition and whenTrue are on the same line, whenFalse should not be indented: - // - // const z = - // 0 ? 1 : L1: whenTrue indented because it's on a new line - // 2 ? 3 : L2: not indented - // 4; L3: not indented const trueStartLine = getStartLineAndCharacterForNode(parent.whenTrue, sourceFile).line; const trueEndLine = getLineAndCharacterOfPosition(sourceFile, parent.whenTrue.end).line; - - if (conditionEndLine === trueStartLine && trueStartLine === trueEndLine && trueEndLine !== childStartLine) { - return true; - } - return conditionEndLine === trueStartLine && trueEndLine === childStartLine; } } @@ -709,8 +697,9 @@ export namespace SmartIndenter { break; case SyntaxKind.ConditionalExpression: if (child && sourceFile) { - const childStartCharacter = sourceFile.getLineAndCharacterOfPosition(skipTrivia(sourceFile.text, child.pos)).character; - if (!rangeIsOnOneLine(sourceFile, parent) && childStartCharacter !== SyntaxKind.QuestionToken) { + const childStartLine = sourceFile.getLineAndCharacterOfPosition(skipTrivia(sourceFile.text, child.pos)).line; + const childEndLine = sourceFile.getLineAndCharacterOfPosition(skipTrivia(sourceFile.text, child.end)).line; + if (!rangeIsOnOneLine(sourceFile, parent) && childStartLine !== childEndLine && childKind !== SyntaxKind.ParenthesizedExpression) { return false; } } From 322c6970070b31d123551f2b7ff7447df07f0e45 Mon Sep 17 00:00:00 2001 From: wukongO-O Date: Sat, 15 Mar 2025 13:50:23 -0400 Subject: [PATCH 5/5] Cleaned up the solution --- src/services/formatting/smartIndenter.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/services/formatting/smartIndenter.ts b/src/services/formatting/smartIndenter.ts index 3548e5e16720f..9f2564d3fd688 100644 --- a/src/services/formatting/smartIndenter.ts +++ b/src/services/formatting/smartIndenter.ts @@ -438,7 +438,6 @@ export namespace SmartIndenter { // branch beginning on the line that the whenTrue branch ends. export function childIsUnindentedBranchOfConditionalExpression(parent: Node, child: TextRangeWithKind, childStartLine: number, sourceFile: SourceFileLike): boolean { if (isConditionalExpression(parent) && (child === parent.whenTrue || child === parent.whenFalse)) { - const conditionEndLine = getLineAndCharacterOfPosition(sourceFile, parent.condition.end).line; if (child === parent.whenTrue) { return childStartLine === conditionEndLine;