From 4c40c42f56f2d1faa334e9e3c36a1978d86e73c7 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Mon, 19 Jun 2017 20:30:24 -0700 Subject: [PATCH 01/11] format on open curly --- src/services/formatting/formatting.ts | 73 ++++++++++++------- src/services/formatting/formattingContext.ts | 8 -- .../formatting/formattingRequestKind.ts | 1 + src/services/formatting/rules.ts | 2 +- src/services/services.ts | 6 +- 5 files changed, 54 insertions(+), 36 deletions(-) diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index 531b768f6d8..1e40d6e1d0b 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -97,11 +97,23 @@ namespace ts.formatting { } export function formatOnSemicolon(position: number, sourceFile: SourceFile, rulesProvider: RulesProvider, options: FormatCodeSettings): TextChange[] { - return formatOutermostParent(position, SyntaxKind.SemicolonToken, sourceFile, options, rulesProvider, FormattingRequestKind.FormatOnSemicolon); + const outermostParent = findOutermostParentWithinListLevelFromPosition(position, SyntaxKind.SemicolonToken, sourceFile); + return formatOutermostParent(outermostParent, sourceFile, options, rulesProvider, FormattingRequestKind.FormatOnSemicolon); + } + + export function formatOnOpeningCurly(position: number, sourceFile: SourceFile, rulesProvider: RulesProvider, options: FormatCodeSettings): TextChange[] { + const openingCurly = findPrecedingTokenOfKind(position, SyntaxKind.FirstPunctuation, sourceFile); + const block = openingCurly && openingCurly.parent; + if (!(block && isBlock(block))) { + return []; + } + const outermostParent = findOutermostParentWithinListLevel(block); + return formatOutermostParent(outermostParent, sourceFile, options, rulesProvider, FormattingRequestKind.FormatOnOpeningCurlyBrace); } export function formatOnClosingCurly(position: number, sourceFile: SourceFile, rulesProvider: RulesProvider, options: FormatCodeSettings): TextChange[] { - return formatOutermostParent(position, SyntaxKind.CloseBraceToken, sourceFile, options, rulesProvider, FormattingRequestKind.FormatOnClosingCurlyBrace); + const outermostParent = findOutermostParentWithinListLevelFromPosition(position, SyntaxKind.CloseBraceToken, sourceFile); + return formatOutermostParent(outermostParent, sourceFile, options, rulesProvider, FormattingRequestKind.FormatOnClosingCurlyBrace); } export function formatDocument(sourceFile: SourceFile, rulesProvider: RulesProvider, options: FormatCodeSettings): TextChange[] { @@ -121,44 +133,55 @@ namespace ts.formatting { return formatSpan(span, sourceFile, options, rulesProvider, FormattingRequestKind.FormatSelection); } - function formatOutermostParent(position: number, expectedLastToken: SyntaxKind, sourceFile: SourceFile, options: FormatCodeSettings, rulesProvider: RulesProvider, requestKind: FormattingRequestKind): TextChange[] { - const parent = findOutermostParent(position, expectedLastToken, sourceFile); + function formatOutermostParent(parent: Node | undefined, sourceFile: SourceFile, options: FormatCodeSettings, rulesProvider: RulesProvider, requestKind: FormattingRequestKind): TextChange[] { if (!parent) { return []; } + const span = { pos: getLineStartPositionForPosition(parent.getStart(sourceFile), sourceFile), end: parent.end }; + return formatSpan(span, sourceFile, options, rulesProvider, requestKind); } - function findOutermostParent(position: number, expectedTokenKind: SyntaxKind, sourceFile: SourceFile): Node { + function findPrecedingTokenOfKind(position: number, expectedTokenKind: SyntaxKind, sourceFile: SourceFile): Node | undefined { const precedingToken = findPrecedingToken(position, sourceFile); - // when it is claimed that trigger character was typed at given position - // we verify that there is a token with a matching kind whose end is equal to position (because the character was just typed). - // If this condition is not hold - then trigger character was typed in some other context, - // i.e.in comment and thus should not trigger autoformatting - if (!precedingToken || - precedingToken.kind !== expectedTokenKind || - position !== precedingToken.getEnd()) { - return undefined; - } + return precedingToken && precedingToken.kind === expectedTokenKind && position === precedingToken.getEnd() ? + precedingToken : + undefined; + } - // walk up and search for the parent node that ends at the same position with precedingToken. - // for cases like this - // - // let x = 1; - // while (true) { - // } - // after typing close curly in while statement we want to reformat just the while statement. - // However if we just walk upwards searching for the parent that has the same end value - - // we'll end up with the whole source file. isListElement allows to stop on the list element level - let current = precedingToken; + /** + * Validating `expectedLastToken` ensures the token was typed in the context we expect (eg: not a comment). + * @param expectedLastToken The last token constituting the desired parent node. + */ + function findOutermostParentWithinListLevelFromPosition(position: number, expectedLastToken: SyntaxKind, sourceFile: SourceFile) { + const precedingToken = findPrecedingTokenOfKind(position, expectedLastToken, sourceFile); + return precedingToken && findOutermostParentWithinListLevel(precedingToken); + } + + /** + * Finds the outermost parent within the same list level as the token at position. + * + * Consider typing the following + * ``` + * let x = 1; + * while (true) { + * } + * ``` + * Upon typing the closing curly, we want to format the entire `while`-statement, but not the preceding + * variable declaration. + */ + function findOutermostParentWithinListLevel(token: Node): Node { + // If we walk upwards searching for the parent that has the same end value, we'll end up with the whole source file. + // `isListElement` allows to stop on the list element level. + let current = token; while (current && current.parent && - current.parent.end === precedingToken.end && + current.parent.end === token.end && !isListElement(current.parent, current)) { current = current.parent; } diff --git a/src/services/formatting/formattingContext.ts b/src/services/formatting/formattingContext.ts index 043df72f434..fd79481173e 100644 --- a/src/services/formatting/formattingContext.ts +++ b/src/services/formatting/formattingContext.ts @@ -47,14 +47,6 @@ namespace ts.formatting { return this.contextNodeAllOnSameLine; } - public NextNodeAllOnSameLine(): boolean { - if (this.nextNodeAllOnSameLine === undefined) { - this.nextNodeAllOnSameLine = this.NodeIsOnOneLine(this.nextTokenParent); - } - - return this.nextNodeAllOnSameLine; - } - public TokensAreOnSameLine(): boolean { if (this.tokensAreOnSameLine === undefined) { const startLine = this.sourceFile.getLineAndCharacterOfPosition(this.currentTokenSpan.pos).line; diff --git a/src/services/formatting/formattingRequestKind.ts b/src/services/formatting/formattingRequestKind.ts index d0397e4a8d9..6c671e1b888 100644 --- a/src/services/formatting/formattingRequestKind.ts +++ b/src/services/formatting/formattingRequestKind.ts @@ -7,6 +7,7 @@ namespace ts.formatting { FormatSelection, FormatOnEnter, FormatOnSemicolon, + FormatOnOpeningCurlyBrace, FormatOnClosingCurlyBrace } } \ No newline at end of file diff --git a/src/services/formatting/rules.ts b/src/services/formatting/rules.ts index 2c1becb8dd1..7f5d362e796 100644 --- a/src/services/formatting/rules.ts +++ b/src/services/formatting/rules.ts @@ -671,7 +671,7 @@ namespace ts.formatting { // This check is done before an open brace in a control construct, a function, or a typescript block declaration static IsBeforeMultilineBlockContext(context: FormattingContext): boolean { - return Rules.IsBeforeBlockContext(context) && !(context.NextNodeAllOnSameLine() || context.NextNodeBlockIsOnOneLine()); + return Rules.IsBeforeBlockContext(context) && !(context.NextNodeBlockIsOnOneLine()); } static IsMultilineBlockContext(context: FormattingContext): boolean { diff --git a/src/services/services.ts b/src/services/services.ts index b508285b182..bd8bfc81884 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1763,8 +1763,10 @@ namespace ts { function getFormattingEditsAfterKeystroke(fileName: string, position: number, key: string, options: FormatCodeOptions | FormatCodeSettings): TextChange[] { const sourceFile = syntaxTreeCache.getCurrentSourceFile(fileName); const settings = toEditorSettings(options); - - if (key === "}") { + if (key === "{") { + return formatting.formatOnOpeningCurly(position, sourceFile, getRuleProvider(settings), settings); + } + else if (key === "}") { return formatting.formatOnClosingCurly(position, sourceFile, getRuleProvider(settings), settings); } else if (key === ";") { From 0df66a5e6d3b237c3697d61a7475b5f87639e326 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Tue, 20 Jun 2017 11:49:36 -0700 Subject: [PATCH 02/11] format space before single-line blocks --- src/services/formatting/rules.ts | 27 +++++---------------------- 1 file changed, 5 insertions(+), 22 deletions(-) diff --git a/src/services/formatting/rules.ts b/src/services/formatting/rules.ts index 7f5d362e796..d490741d27b 100644 --- a/src/services/formatting/rules.ts +++ b/src/services/formatting/rules.ts @@ -290,15 +290,15 @@ namespace ts.formatting { // Place a space before open brace in a function declaration this.FunctionOpenBraceLeftTokenRange = Shared.TokenRange.AnyIncludingMultilineComments; - this.SpaceBeforeOpenBraceInFunction = new Rule(RuleDescriptor.create2(this.FunctionOpenBraceLeftTokenRange, SyntaxKind.OpenBraceToken), RuleOperation.create2(new RuleOperationContext(Rules.IsFunctionDeclContext, Rules.IsBeforeBlockContext, Rules.IsNotFormatOnEnter, Rules.IsSameLineTokenOrBeforeMultilineBlockContext), RuleAction.Space), RuleFlags.CanDeleteNewLines); + this.SpaceBeforeOpenBraceInFunction = new Rule(RuleDescriptor.create2(this.FunctionOpenBraceLeftTokenRange, SyntaxKind.OpenBraceToken), RuleOperation.create2(new RuleOperationContext(Rules.IsFunctionDeclContext, Rules.IsBeforeBlockContext, Rules.IsNotFormatOnEnter, Rules.IsSameLineTokenOrBeforeBlockContext), RuleAction.Space), RuleFlags.CanDeleteNewLines); // Place a space before open brace in a TypeScript declaration that has braces as children (class, module, enum, etc) this.TypeScriptOpenBraceLeftTokenRange = Shared.TokenRange.FromTokens([SyntaxKind.Identifier, SyntaxKind.MultiLineCommentTrivia, SyntaxKind.ClassKeyword, SyntaxKind.ExportKeyword, SyntaxKind.ImportKeyword]); - this.SpaceBeforeOpenBraceInTypeScriptDeclWithBlock = new Rule(RuleDescriptor.create2(this.TypeScriptOpenBraceLeftTokenRange, SyntaxKind.OpenBraceToken), RuleOperation.create2(new RuleOperationContext(Rules.IsTypeScriptDeclWithBlockContext, Rules.IsNotFormatOnEnter, Rules.IsSameLineTokenOrBeforeMultilineBlockContext), RuleAction.Space), RuleFlags.CanDeleteNewLines); + this.SpaceBeforeOpenBraceInTypeScriptDeclWithBlock = new Rule(RuleDescriptor.create2(this.TypeScriptOpenBraceLeftTokenRange, SyntaxKind.OpenBraceToken), RuleOperation.create2(new RuleOperationContext(Rules.IsTypeScriptDeclWithBlockContext, Rules.IsNotFormatOnEnter, Rules.IsSameLineTokenOrBeforeBlockContext), RuleAction.Space), RuleFlags.CanDeleteNewLines); // Place a space before open brace in a control flow construct this.ControlOpenBraceLeftTokenRange = Shared.TokenRange.FromTokens([SyntaxKind.CloseParenToken, SyntaxKind.MultiLineCommentTrivia, SyntaxKind.DoKeyword, SyntaxKind.TryKeyword, SyntaxKind.FinallyKeyword, SyntaxKind.ElseKeyword]); - this.SpaceBeforeOpenBraceInControl = new Rule(RuleDescriptor.create2(this.ControlOpenBraceLeftTokenRange, SyntaxKind.OpenBraceToken), RuleOperation.create2(new RuleOperationContext(Rules.IsControlDeclContext, Rules.IsNotFormatOnEnter, Rules.IsSameLineTokenOrBeforeMultilineBlockContext), RuleAction.Space), RuleFlags.CanDeleteNewLines); + this.SpaceBeforeOpenBraceInControl = new Rule(RuleDescriptor.create2(this.ControlOpenBraceLeftTokenRange, SyntaxKind.OpenBraceToken), RuleOperation.create2(new RuleOperationContext(Rules.IsControlDeclContext, Rules.IsNotFormatOnEnter, Rules.IsSameLineTokenOrBeforeBlockContext), RuleAction.Space), RuleFlags.CanDeleteNewLines); // Insert a space after { and before } in single-line contexts, but remove space from empty object literals {}. this.SpaceAfterOpenBrace = new Rule(RuleDescriptor.create3(SyntaxKind.OpenBraceToken, Shared.TokenRange.Any), RuleOperation.create2(new RuleOperationContext(Rules.IsOptionEnabledOrUndefined("insertSpaceAfterOpeningAndBeforeClosingNonemptyBraces"), Rules.IsBraceWrappedContext), RuleAction.Space)); @@ -644,25 +644,8 @@ namespace ts.formatting { return context.contextNode.kind === SyntaxKind.ConditionalExpression; } - static IsSameLineTokenOrBeforeMultilineBlockContext(context: FormattingContext): boolean { - //// This check is mainly used inside SpaceBeforeOpenBraceInControl and SpaceBeforeOpenBraceInFunction. - //// - //// Ex: - //// if (1) { .... - //// * ) and { are on the same line so apply the rule. Here we don't care whether it's same or multi block context - //// - //// Ex: - //// if (1) - //// { ... } - //// * ) and { are on different lines. We only need to format if the block is multiline context. So in this case we don't format. - //// - //// Ex: - //// if (1) - //// { ... - //// } - //// * ) and { are on different lines. We only need to format if the block is multiline context. So in this case we format. - - return context.TokensAreOnSameLine() || Rules.IsBeforeMultilineBlockContext(context); + static IsSameLineTokenOrBeforeBlockContext(context: FormattingContext): boolean { + return context.TokensAreOnSameLine() || Rules.IsBeforeBlockContext(context); } static IsBraceWrappedContext(context: FormattingContext): boolean { From 28fce55e1f1845439b0843e29108f8134a612df7 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Mon, 19 Jun 2017 20:30:11 -0700 Subject: [PATCH 03/11] add and update tests --- .../fourslash/autoFormattingOnPasting.ts | 23 +++++++++---------- .../forceIndentAfterNewLineInsert.ts | 23 +++++++++++-------- .../formatOnEnterOpenBraceRemoveNewLine.ts | 14 +++++++++++ .../formatRemoveNewLineAfterOpenBrace.ts | 15 ++++++++++++ .../formattingOfMultilineBlockConstructs.ts | 2 +- .../fourslash/formattingOnSingleLineBlocks.ts | 19 ++++++--------- 6 files changed, 62 insertions(+), 34 deletions(-) create mode 100644 tests/cases/fourslash/formatOnEnterOpenBraceRemoveNewLine.ts create mode 100644 tests/cases/fourslash/formatRemoveNewLineAfterOpenBrace.ts diff --git a/tests/cases/fourslash/autoFormattingOnPasting.ts b/tests/cases/fourslash/autoFormattingOnPasting.ts index 0c7f471d43d..5c9e2c86e37 100644 --- a/tests/cases/fourslash/autoFormattingOnPasting.ts +++ b/tests/cases/fourslash/autoFormattingOnPasting.ts @@ -4,11 +4,11 @@ /////**/ ////} goTo.marker(""); -edit.paste(" class TestClass{\r\n\ -private foo;\r\n\ -public testMethod( )\r\n\ -{}\r\n\ -}"); +edit.paste(` class TestClass{ +private foo; +public testMethod( ) +{} +}`); // We're missing scenarios of formatting option settings due to bug 693273 - [TypeScript] Need to improve fourslash support for formatting options. // Missing scenario ** Uncheck Tools->Options->Text Editor->TypeScript->Formatting->General->Format on paste ** //verify.currentFileContentIs("module TestModule {\r\n\ @@ -19,10 +19,9 @@ public testMethod( )\r\n\ //}\r\n\ //}"); // Missing scenario ** Check Tools->Options->Text Editor->TypeScript->Formatting->General->Format on paste ** -verify.currentFileContentIs("module TestModule {\r\n\ - class TestClass {\r\n\ - private foo;\r\n\ - public testMethod()\r\n\ - { }\r\n\ - }\r\n\ -}"); \ No newline at end of file +verify.currentFileContentIs(`module TestModule { + class TestClass { + private foo; + public testMethod() { } + } +}`); \ No newline at end of file diff --git a/tests/cases/fourslash/forceIndentAfterNewLineInsert.ts b/tests/cases/fourslash/forceIndentAfterNewLineInsert.ts index a076213ebb6..d4c41be01ee 100644 --- a/tests/cases/fourslash/forceIndentAfterNewLineInsert.ts +++ b/tests/cases/fourslash/forceIndentAfterNewLineInsert.ts @@ -1,18 +1,23 @@ /// -////function f() +////function f1() ////{ return 0; } +////function f2() +////{ +////return 0; +////} ////function g() ////{ function h() { ////return 0; ////}} format.document(); verify.currentFileContentIs( - "function f()\n" + - "{ return 0; }\n" + - "function g() {\n" + - " function h() {\n" + - " return 0;\n" + - " }\n" + - "}" - ); +`function f1() { return 0; } +function f2() { + return 0; +} +function g() { + function h() { + return 0; + } +}`); diff --git a/tests/cases/fourslash/formatOnEnterOpenBraceRemoveNewLine.ts b/tests/cases/fourslash/formatOnEnterOpenBraceRemoveNewLine.ts new file mode 100644 index 00000000000..6a1d5937255 --- /dev/null +++ b/tests/cases/fourslash/formatOnEnterOpenBraceRemoveNewLine.ts @@ -0,0 +1,14 @@ +/// + +//// if(true) +//// /**/ + + +format.setOption("PlaceOpenBraceOnNewLineForControlBlocks", false); +goTo.marker(""); +edit.insert("{"); +// TODO: figure out how to get rid of 3 extra spaces on second last line. +verify.currentFileContentIs( + `if (true) {`); + + \ No newline at end of file diff --git a/tests/cases/fourslash/formatRemoveNewLineAfterOpenBrace.ts b/tests/cases/fourslash/formatRemoveNewLineAfterOpenBrace.ts new file mode 100644 index 00000000000..6309ef21805 --- /dev/null +++ b/tests/cases/fourslash/formatRemoveNewLineAfterOpenBrace.ts @@ -0,0 +1,15 @@ +/// + +//// function foo() +//// { +//// } +//// if (true) +//// { +//// } + +format.document(); +verify.currentFileContentIs( +`function foo() { +} +if (true) { +}`); diff --git a/tests/cases/fourslash/formattingOfMultilineBlockConstructs.ts b/tests/cases/fourslash/formattingOfMultilineBlockConstructs.ts index 18f497496d8..fc52df62da3 100644 --- a/tests/cases/fourslash/formattingOfMultilineBlockConstructs.ts +++ b/tests/cases/fourslash/formattingOfMultilineBlockConstructs.ts @@ -47,7 +47,7 @@ verify.currentLineContentIs("enum E {"); goTo.marker('4'); verify.currentLineContentIs("class MyClass {"); goTo.marker('cons'); -verify.currentLineContentIs(" constructor()"); +verify.currentLineContentIs(" constructor() { }"); goTo.marker('5'); verify.currentLineContentIs(" public MyFunction() {"); goTo.marker('6'); diff --git a/tests/cases/fourslash/formattingOnSingleLineBlocks.ts b/tests/cases/fourslash/formattingOnSingleLineBlocks.ts index fc1a6818b95..67990056274 100644 --- a/tests/cases/fourslash/formattingOnSingleLineBlocks.ts +++ b/tests/cases/fourslash/formattingOnSingleLineBlocks.ts @@ -1,16 +1,11 @@ /// -/////*1*/class C -////{}/*2*/ -/////*3*/if (true) -////{}/*4*/ +////class C +////{} +////if (true) +////{} format.document(); -goTo.marker("1"); -verify.currentLineContentIs("class C"); -goTo.marker("2"); -verify.currentLineContentIs("{ }"); -goTo.marker("3"); -verify.currentLineContentIs("if (true)"); -goTo.marker("4"); -verify.currentLineContentIs("{ }"); \ No newline at end of file +verify.currentFileContentIs( +`class C { } +if (true) { }`); \ No newline at end of file From f9592b6479ba79a7fa4e05199591c90ad0cda262 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Thu, 22 Jun 2017 11:17:38 -0700 Subject: [PATCH 04/11] fix and add test --- .../formatOnEnterOpenBraceAddNewLine.ts | 18 ++++++++++++++++++ .../formatOnEnterOpenBraceRemoveNewLine.ts | 4 ---- 2 files changed, 18 insertions(+), 4 deletions(-) create mode 100644 tests/cases/fourslash/formatOnEnterOpenBraceAddNewLine.ts diff --git a/tests/cases/fourslash/formatOnEnterOpenBraceAddNewLine.ts b/tests/cases/fourslash/formatOnEnterOpenBraceAddNewLine.ts new file mode 100644 index 00000000000..63276d3ce7c --- /dev/null +++ b/tests/cases/fourslash/formatOnEnterOpenBraceAddNewLine.ts @@ -0,0 +1,18 @@ +/// + +//// if(true) {/*0*/} +//// if(false)/*1*/{ +//// } + +format.setOption("PlaceOpenBraceOnNewLineForControlBlocks", true); +goTo.marker("0"); +edit.insertLine(""); +goTo.marker("1"); +edit.insertLine(""); +verify.currentFileContentIs( +`if (true) +{ +} +if (false) +{ +}`); \ No newline at end of file diff --git a/tests/cases/fourslash/formatOnEnterOpenBraceRemoveNewLine.ts b/tests/cases/fourslash/formatOnEnterOpenBraceRemoveNewLine.ts index 6a1d5937255..262a96daa42 100644 --- a/tests/cases/fourslash/formatOnEnterOpenBraceRemoveNewLine.ts +++ b/tests/cases/fourslash/formatOnEnterOpenBraceRemoveNewLine.ts @@ -3,12 +3,8 @@ //// if(true) //// /**/ - format.setOption("PlaceOpenBraceOnNewLineForControlBlocks", false); goTo.marker(""); edit.insert("{"); -// TODO: figure out how to get rid of 3 extra spaces on second last line. verify.currentFileContentIs( `if (true) {`); - - \ No newline at end of file From 485927b26aefe4d1719d110b9566057fcbd0b9a9 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Thu, 22 Jun 2017 11:17:55 -0700 Subject: [PATCH 05/11] clarify comment --- src/services/formatting/formatting.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index 1e40d6e1d0b..4ede9da335b 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -1119,7 +1119,7 @@ namespace ts.formatting { return; } - // edit should not be applied only if we have one line feed between elements + // edit should not be applied if we have one line feed between elements const lineDelta = currentStartLine - previousStartLine; if (lineDelta !== 1) { recordReplace(previousRange.end, currentRange.pos - previousRange.end, options.newLineCharacter); From 61af3157789ada079f96f84031f0ca2524d5efae Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Mon, 26 Jun 2017 10:55:04 -0700 Subject: [PATCH 06/11] respond to comments --- src/services/formatting/formatting.ts | 78 +++++++++---------- src/services/formatting/formattingContext.ts | 8 ++ src/services/formatting/rules.ts | 2 +- src/services/textChanges.ts | 2 +- .../formatOnEnterOpenBraceAddNewLine.ts | 6 ++ .../semicolonFormattingNestedStatements.ts | 2 +- 6 files changed, 53 insertions(+), 45 deletions(-) diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index 4ede9da335b..86935fb3984 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -97,23 +97,21 @@ namespace ts.formatting { } export function formatOnSemicolon(position: number, sourceFile: SourceFile, rulesProvider: RulesProvider, options: FormatCodeSettings): TextChange[] { - const outermostParent = findOutermostParentWithinListLevelFromPosition(position, SyntaxKind.SemicolonToken, sourceFile); - return formatOutermostParent(outermostParent, sourceFile, options, rulesProvider, FormattingRequestKind.FormatOnSemicolon); + const semicolon = findImmediatelyPrecedingTokenOfKind(position, SyntaxKind.SemicolonToken, sourceFile); + return formatOutermostNodeWithinListLevel(semicolon, sourceFile, options, rulesProvider, FormattingRequestKind.FormatOnSemicolon); } export function formatOnOpeningCurly(position: number, sourceFile: SourceFile, rulesProvider: RulesProvider, options: FormatCodeSettings): TextChange[] { - const openingCurly = findPrecedingTokenOfKind(position, SyntaxKind.FirstPunctuation, sourceFile); - const block = openingCurly && openingCurly.parent; - if (!(block && isBlock(block))) { - return []; - } - const outermostParent = findOutermostParentWithinListLevel(block); - return formatOutermostParent(outermostParent, sourceFile, options, rulesProvider, FormattingRequestKind.FormatOnOpeningCurlyBrace); + const openingCurly = findImmediatelyPrecedingTokenOfKind(position, SyntaxKind.OpenBraceToken, sourceFile); + const curlyBraceRange = openingCurly && openingCurly.parent; + return curlyBraceRange ? + formatOutermostNodeWithinListLevel(curlyBraceRange, sourceFile, options, rulesProvider, FormattingRequestKind.FormatOnOpeningCurlyBrace) : + []; } export function formatOnClosingCurly(position: number, sourceFile: SourceFile, rulesProvider: RulesProvider, options: FormatCodeSettings): TextChange[] { - const outermostParent = findOutermostParentWithinListLevelFromPosition(position, SyntaxKind.CloseBraceToken, sourceFile); - return formatOutermostParent(outermostParent, sourceFile, options, rulesProvider, FormattingRequestKind.FormatOnClosingCurlyBrace); + const precedingToken = findImmediatelyPrecedingTokenOfKind(position, SyntaxKind.CloseBraceToken, sourceFile); + return formatOutermostNodeWithinListLevel(precedingToken, sourceFile, options, rulesProvider, FormattingRequestKind.FormatOnClosingCurlyBrace); } export function formatDocument(sourceFile: SourceFile, rulesProvider: RulesProvider, options: FormatCodeSettings): TextChange[] { @@ -133,38 +131,21 @@ namespace ts.formatting { return formatSpan(span, sourceFile, options, rulesProvider, FormattingRequestKind.FormatSelection); } - function formatOutermostParent(parent: Node | undefined, sourceFile: SourceFile, options: FormatCodeSettings, rulesProvider: RulesProvider, requestKind: FormattingRequestKind): TextChange[] { - if (!parent) { - return []; - } + /** + * Validating `expectedLastToken` ensures the token was typed in the context we expect (eg: not a comment). + * @param expectedLastToken The last token constituting the desired parent node. + */ + function findImmediatelyPrecedingTokenOfKind(end: number, expectedTokenKind: SyntaxKind, sourceFile: SourceFile): Node | undefined { + const precedingToken = findPrecedingToken(end, sourceFile); - const span = { - pos: getLineStartPositionForPosition(parent.getStart(sourceFile), sourceFile), - end: parent.end - }; - - return formatSpan(span, sourceFile, options, rulesProvider, requestKind); - } - - function findPrecedingTokenOfKind(position: number, expectedTokenKind: SyntaxKind, sourceFile: SourceFile): Node | undefined { - const precedingToken = findPrecedingToken(position, sourceFile); - - return precedingToken && precedingToken.kind === expectedTokenKind && position === precedingToken.getEnd() ? + return precedingToken && precedingToken.kind === expectedTokenKind && end === precedingToken.getEnd() ? precedingToken : undefined; } /** - * Validating `expectedLastToken` ensures the token was typed in the context we expect (eg: not a comment). - * @param expectedLastToken The last token constituting the desired parent node. - */ - function findOutermostParentWithinListLevelFromPosition(position: number, expectedLastToken: SyntaxKind, sourceFile: SourceFile) { - const precedingToken = findPrecedingTokenOfKind(position, expectedLastToken, sourceFile); - return precedingToken && findOutermostParentWithinListLevel(precedingToken); - } - - /** - * Finds the outermost parent within the same list level as the token at position. + * Finds and formats the highest node enclosing `position` whose end does not exceed the given position + * and is at the same list level as the token at `position`. * * Consider typing the following * ``` @@ -175,18 +156,18 @@ namespace ts.formatting { * Upon typing the closing curly, we want to format the entire `while`-statement, but not the preceding * variable declaration. */ - function findOutermostParentWithinListLevel(token: Node): Node { + function formatOutermostNodeWithinListLevel(node: Node, sourceFile: SourceFile, options: FormatCodeSettings, rulesProvider: RulesProvider, formattingRequestKind: FormattingRequestKind) { // If we walk upwards searching for the parent that has the same end value, we'll end up with the whole source file. // `isListElement` allows to stop on the list element level. - let current = token; + let current = node; while (current && current.parent && - current.parent.end === token.end && + current.parent.end === node.end && !isListElement(current.parent, current)) { current = current.parent; } - return current; + return formatNodeLines(current, sourceFile, options, rulesProvider, formattingRequestKind); } // Returns true if node is a element in some list in parent @@ -338,7 +319,7 @@ namespace ts.formatting { } /* @internal */ - export function formatNode(node: Node, sourceFileLike: SourceFileLike, languageVariant: LanguageVariant, initialIndentation: number, delta: number, rulesProvider: RulesProvider): TextChange[] { + export function formatNodeGivenIndentation(node: Node, sourceFileLike: SourceFileLike, languageVariant: LanguageVariant, initialIndentation: number, delta: number, rulesProvider: RulesProvider): TextChange[] { const range = { pos: 0, end: sourceFileLike.text.length }; return formatSpanWorker( range, @@ -353,6 +334,19 @@ namespace ts.formatting { sourceFileLike); } + function formatNodeLines(node: Node, sourceFile: SourceFile, options: FormatCodeSettings, rulesProvider: RulesProvider, requestKind: FormattingRequestKind): TextChange[] { + if (!node) { + return []; + } + + const span = { + pos: getLineStartPositionForPosition(node.getStart(sourceFile), sourceFile), + end: node.end + }; + + return formatSpan(span, sourceFile, options, rulesProvider, requestKind); + } + function formatSpan(originalRange: TextRange, sourceFile: SourceFile, options: FormatCodeSettings, diff --git a/src/services/formatting/formattingContext.ts b/src/services/formatting/formattingContext.ts index fd79481173e..043df72f434 100644 --- a/src/services/formatting/formattingContext.ts +++ b/src/services/formatting/formattingContext.ts @@ -47,6 +47,14 @@ namespace ts.formatting { return this.contextNodeAllOnSameLine; } + public NextNodeAllOnSameLine(): boolean { + if (this.nextNodeAllOnSameLine === undefined) { + this.nextNodeAllOnSameLine = this.NodeIsOnOneLine(this.nextTokenParent); + } + + return this.nextNodeAllOnSameLine; + } + public TokensAreOnSameLine(): boolean { if (this.tokensAreOnSameLine === undefined) { const startLine = this.sourceFile.getLineAndCharacterOfPosition(this.currentTokenSpan.pos).line; diff --git a/src/services/formatting/rules.ts b/src/services/formatting/rules.ts index d490741d27b..51d2c0a7f40 100644 --- a/src/services/formatting/rules.ts +++ b/src/services/formatting/rules.ts @@ -654,7 +654,7 @@ namespace ts.formatting { // This check is done before an open brace in a control construct, a function, or a typescript block declaration static IsBeforeMultilineBlockContext(context: FormattingContext): boolean { - return Rules.IsBeforeBlockContext(context) && !(context.NextNodeBlockIsOnOneLine()); + return Rules.IsBeforeBlockContext(context) && !(context.NextNodeAllOnSameLine() || context.NextNodeBlockIsOnOneLine()); } static IsMultilineBlockContext(context: FormattingContext): boolean { diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index 5aa4ebca7d0..1cd5580ec9d 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -512,7 +512,7 @@ namespace ts.textChanges { lineMap, getLineAndCharacterOfPosition: pos => computeLineAndCharacterOfPosition(lineMap, pos) }; - const changes = formatting.formatNode(nonFormattedText.node, file, sourceFile.languageVariant, initialIndentation, delta, rulesProvider); + const changes = formatting.formatNodeGivenIndentation(nonFormattedText.node, file, sourceFile.languageVariant, initialIndentation, delta, rulesProvider); return applyChanges(nonFormattedText.text, changes); } diff --git a/tests/cases/fourslash/formatOnEnterOpenBraceAddNewLine.ts b/tests/cases/fourslash/formatOnEnterOpenBraceAddNewLine.ts index 63276d3ce7c..c5045717df5 100644 --- a/tests/cases/fourslash/formatOnEnterOpenBraceAddNewLine.ts +++ b/tests/cases/fourslash/formatOnEnterOpenBraceAddNewLine.ts @@ -7,6 +7,12 @@ format.setOption("PlaceOpenBraceOnNewLineForControlBlocks", true); goTo.marker("0"); edit.insertLine(""); +verify.currentFileContentIs( +`if (true) +{ +} +if(false){ +}`); goTo.marker("1"); edit.insertLine(""); verify.currentFileContentIs( diff --git a/tests/cases/fourslash/semicolonFormattingNestedStatements.ts b/tests/cases/fourslash/semicolonFormattingNestedStatements.ts index 20d49a35f47..9e7d0643d08 100644 --- a/tests/cases/fourslash/semicolonFormattingNestedStatements.ts +++ b/tests/cases/fourslash/semicolonFormattingNestedStatements.ts @@ -11,7 +11,7 @@ goTo.marker("innermost"); edit.insert(";"); -// Adding smicolon should format the innermost statement +// Adding semicolon should format the innermost statement verify.currentLineContentIs(' var x = 0;'); // Also should format any parent statement that is terminated by the semicolon From c5f6c4fac086bb73b8e277f9572bbb5b9684c0a5 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Mon, 26 Jun 2017 11:21:16 -0700 Subject: [PATCH 07/11] remove unecessary check --- src/services/formatting/formatting.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index 86935fb3984..f6eaa812f45 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -104,9 +104,7 @@ namespace ts.formatting { export function formatOnOpeningCurly(position: number, sourceFile: SourceFile, rulesProvider: RulesProvider, options: FormatCodeSettings): TextChange[] { const openingCurly = findImmediatelyPrecedingTokenOfKind(position, SyntaxKind.OpenBraceToken, sourceFile); const curlyBraceRange = openingCurly && openingCurly.parent; - return curlyBraceRange ? - formatOutermostNodeWithinListLevel(curlyBraceRange, sourceFile, options, rulesProvider, FormattingRequestKind.FormatOnOpeningCurlyBrace) : - []; + return formatOutermostNodeWithinListLevel(curlyBraceRange, sourceFile, options, rulesProvider, FormattingRequestKind.FormatOnOpeningCurlyBrace); } export function formatOnClosingCurly(position: number, sourceFile: SourceFile, rulesProvider: RulesProvider, options: FormatCodeSettings): TextChange[] { From a5c8a29fa4a64854d4f497c6152f74598487f723 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Wed, 28 Jun 2017 12:48:14 -0700 Subject: [PATCH 08/11] only format opencurly if no intervening tokens --- src/services/formatting/formatting.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index f6eaa812f45..aa127621b58 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -104,7 +104,10 @@ namespace ts.formatting { export function formatOnOpeningCurly(position: number, sourceFile: SourceFile, rulesProvider: RulesProvider, options: FormatCodeSettings): TextChange[] { const openingCurly = findImmediatelyPrecedingTokenOfKind(position, SyntaxKind.OpenBraceToken, sourceFile); const curlyBraceRange = openingCurly && openingCurly.parent; - return formatOutermostNodeWithinListLevel(curlyBraceRange, sourceFile, options, rulesProvider, FormattingRequestKind.FormatOnOpeningCurlyBrace); + const nextToken = curlyBraceRange && findNextToken(openingCurly, curlyBraceRange); + return nextToken && nextToken.kind === SyntaxKind.CloseBraceToken ? + formatOutermostNodeWithinListLevel(curlyBraceRange, sourceFile, options, rulesProvider, FormattingRequestKind.FormatOnOpeningCurlyBrace) : + []; } export function formatOnClosingCurly(position: number, sourceFile: SourceFile, rulesProvider: RulesProvider, options: FormatCodeSettings): TextChange[] { @@ -130,8 +133,8 @@ namespace ts.formatting { } /** - * Validating `expectedLastToken` ensures the token was typed in the context we expect (eg: not a comment). - * @param expectedLastToken The last token constituting the desired parent node. + * Validating `expectedTokenKind` ensures the token was typed in the context we expect (eg: not a comment). + * @param expectedTokenKind The kind of the last token constituting the desired parent node. */ function findImmediatelyPrecedingTokenOfKind(end: number, expectedTokenKind: SyntaxKind, sourceFile: SourceFile): Node | undefined { const precedingToken = findPrecedingToken(end, sourceFile); @@ -142,8 +145,8 @@ namespace ts.formatting { } /** - * Finds and formats the highest node enclosing `position` whose end does not exceed the given position - * and is at the same list level as the token at `position`. + * Finds and formats the highest node enclosing `node` whose end does not exceed the `node.end` + * and is at the same list level as the token at `node`. * * Consider typing the following * ``` From eae234cab2c8f18f4b296c1483eb5366bb036721 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Wed, 28 Jun 2017 12:48:57 -0700 Subject: [PATCH 09/11] disable spaceBeforeOpenCurly if newline rule is enabled --- src/services/formatting/rules.ts | 10 +++++++--- ...wLine.ts => formatOnOpenCurlyBraceRemoveNewLine.ts} | 4 ++-- 2 files changed, 9 insertions(+), 5 deletions(-) rename tests/cases/fourslash/{formatOnEnterOpenBraceRemoveNewLine.ts => formatOnOpenCurlyBraceRemoveNewLine.ts} (84%) diff --git a/src/services/formatting/rules.ts b/src/services/formatting/rules.ts index 51d2c0a7f40..15bbf5041d3 100644 --- a/src/services/formatting/rules.ts +++ b/src/services/formatting/rules.ts @@ -290,15 +290,15 @@ namespace ts.formatting { // Place a space before open brace in a function declaration this.FunctionOpenBraceLeftTokenRange = Shared.TokenRange.AnyIncludingMultilineComments; - this.SpaceBeforeOpenBraceInFunction = new Rule(RuleDescriptor.create2(this.FunctionOpenBraceLeftTokenRange, SyntaxKind.OpenBraceToken), RuleOperation.create2(new RuleOperationContext(Rules.IsFunctionDeclContext, Rules.IsBeforeBlockContext, Rules.IsNotFormatOnEnter, Rules.IsSameLineTokenOrBeforeBlockContext), RuleAction.Space), RuleFlags.CanDeleteNewLines); + this.SpaceBeforeOpenBraceInFunction = new Rule(RuleDescriptor.create2(this.FunctionOpenBraceLeftTokenRange, SyntaxKind.OpenBraceToken), RuleOperation.create2(new RuleOperationContext(Rules.isOptionDisabledOrUndefinedOrTokensOnSameLine("placeOpenBraceOnNewLineForFunctions"), Rules.IsFunctionDeclContext, Rules.IsBeforeBlockContext, Rules.IsNotFormatOnEnter, Rules.IsSameLineTokenOrBeforeBlockContext), RuleAction.Space), RuleFlags.CanDeleteNewLines); // Place a space before open brace in a TypeScript declaration that has braces as children (class, module, enum, etc) this.TypeScriptOpenBraceLeftTokenRange = Shared.TokenRange.FromTokens([SyntaxKind.Identifier, SyntaxKind.MultiLineCommentTrivia, SyntaxKind.ClassKeyword, SyntaxKind.ExportKeyword, SyntaxKind.ImportKeyword]); - this.SpaceBeforeOpenBraceInTypeScriptDeclWithBlock = new Rule(RuleDescriptor.create2(this.TypeScriptOpenBraceLeftTokenRange, SyntaxKind.OpenBraceToken), RuleOperation.create2(new RuleOperationContext(Rules.IsTypeScriptDeclWithBlockContext, Rules.IsNotFormatOnEnter, Rules.IsSameLineTokenOrBeforeBlockContext), RuleAction.Space), RuleFlags.CanDeleteNewLines); + this.SpaceBeforeOpenBraceInTypeScriptDeclWithBlock = new Rule(RuleDescriptor.create2(this.TypeScriptOpenBraceLeftTokenRange, SyntaxKind.OpenBraceToken), RuleOperation.create2(new RuleOperationContext(Rules.isOptionDisabledOrUndefinedOrTokensOnSameLine("placeOpenBraceOnNewLineForFunctions"), Rules.IsTypeScriptDeclWithBlockContext, Rules.IsNotFormatOnEnter, Rules.IsSameLineTokenOrBeforeBlockContext), RuleAction.Space), RuleFlags.CanDeleteNewLines); // Place a space before open brace in a control flow construct this.ControlOpenBraceLeftTokenRange = Shared.TokenRange.FromTokens([SyntaxKind.CloseParenToken, SyntaxKind.MultiLineCommentTrivia, SyntaxKind.DoKeyword, SyntaxKind.TryKeyword, SyntaxKind.FinallyKeyword, SyntaxKind.ElseKeyword]); - this.SpaceBeforeOpenBraceInControl = new Rule(RuleDescriptor.create2(this.ControlOpenBraceLeftTokenRange, SyntaxKind.OpenBraceToken), RuleOperation.create2(new RuleOperationContext(Rules.IsControlDeclContext, Rules.IsNotFormatOnEnter, Rules.IsSameLineTokenOrBeforeBlockContext), RuleAction.Space), RuleFlags.CanDeleteNewLines); + this.SpaceBeforeOpenBraceInControl = new Rule(RuleDescriptor.create2(this.ControlOpenBraceLeftTokenRange, SyntaxKind.OpenBraceToken), RuleOperation.create2(new RuleOperationContext(Rules.isOptionDisabledOrUndefinedOrTokensOnSameLine("placeOpenBraceOnNewLineForControlBlocks"), Rules.IsControlDeclContext, Rules.IsNotFormatOnEnter, Rules.IsSameLineTokenOrBeforeBlockContext), RuleAction.Space), RuleFlags.CanDeleteNewLines); // Insert a space after { and before } in single-line contexts, but remove space from empty object literals {}. this.SpaceAfterOpenBrace = new Rule(RuleDescriptor.create3(SyntaxKind.OpenBraceToken, Shared.TokenRange.Any), RuleOperation.create2(new RuleOperationContext(Rules.IsOptionEnabledOrUndefined("insertSpaceAfterOpeningAndBeforeClosingNonemptyBraces"), Rules.IsBraceWrappedContext), RuleAction.Space)); @@ -585,6 +585,10 @@ namespace ts.formatting { return (context) => !context.options || !context.options.hasOwnProperty(optionName) || !context.options[optionName]; } + static isOptionDisabledOrUndefinedOrTokensOnSameLine(optionName: keyof FormatCodeSettings): (context: FormattingContext) => boolean { + return (context) => !context.options || !context.options.hasOwnProperty(optionName) || !context.options[optionName] || context.TokensAreOnSameLine(); + } + static IsOptionEnabledOrUndefined(optionName: keyof FormatCodeSettings): (context: FormattingContext) => boolean { return (context) => !context.options || !context.options.hasOwnProperty(optionName) || !!context.options[optionName]; } diff --git a/tests/cases/fourslash/formatOnEnterOpenBraceRemoveNewLine.ts b/tests/cases/fourslash/formatOnOpenCurlyBraceRemoveNewLine.ts similarity index 84% rename from tests/cases/fourslash/formatOnEnterOpenBraceRemoveNewLine.ts rename to tests/cases/fourslash/formatOnOpenCurlyBraceRemoveNewLine.ts index 262a96daa42..ad6aa4da1db 100644 --- a/tests/cases/fourslash/formatOnEnterOpenBraceRemoveNewLine.ts +++ b/tests/cases/fourslash/formatOnOpenCurlyBraceRemoveNewLine.ts @@ -1,10 +1,10 @@ /// //// if(true) -//// /**/ +//// /**/ } format.setOption("PlaceOpenBraceOnNewLineForControlBlocks", false); goTo.marker(""); edit.insert("{"); verify.currentFileContentIs( - `if (true) {`); + `if (true) { }`); From 60b78c618f2c65d90dfc15145faee3723c66dd77 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Thu, 29 Jun 2017 11:13:44 -0700 Subject: [PATCH 10/11] only format open curly up to the open curly --- src/services/formatting/formatting.ts | 31 ++++++++++++++++----------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index aa127621b58..809d2103e2f 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -98,21 +98,28 @@ namespace ts.formatting { export function formatOnSemicolon(position: number, sourceFile: SourceFile, rulesProvider: RulesProvider, options: FormatCodeSettings): TextChange[] { const semicolon = findImmediatelyPrecedingTokenOfKind(position, SyntaxKind.SemicolonToken, sourceFile); - return formatOutermostNodeWithinListLevel(semicolon, sourceFile, options, rulesProvider, FormattingRequestKind.FormatOnSemicolon); + return formatNodeLines(findOutermostNodeWithinListLevel(semicolon), sourceFile, options, rulesProvider, FormattingRequestKind.FormatOnSemicolon); } export function formatOnOpeningCurly(position: number, sourceFile: SourceFile, rulesProvider: RulesProvider, options: FormatCodeSettings): TextChange[] { const openingCurly = findImmediatelyPrecedingTokenOfKind(position, SyntaxKind.OpenBraceToken, sourceFile); - const curlyBraceRange = openingCurly && openingCurly.parent; - const nextToken = curlyBraceRange && findNextToken(openingCurly, curlyBraceRange); - return nextToken && nextToken.kind === SyntaxKind.CloseBraceToken ? - formatOutermostNodeWithinListLevel(curlyBraceRange, sourceFile, options, rulesProvider, FormattingRequestKind.FormatOnOpeningCurlyBrace) : - []; + if (openingCurly) { + const curlyBraceRange = openingCurly.parent; + const outermostNode = findOutermostNodeWithinListLevel(curlyBraceRange); + + const textRange: TextRange = { + pos: getLineStartPositionForPosition(outermostNode.getStart(sourceFile), sourceFile), + end: position + }; + + return formatSpan(textRange, sourceFile, options, rulesProvider, FormattingRequestKind.FormatOnOpeningCurlyBrace); + } + return []; } export function formatOnClosingCurly(position: number, sourceFile: SourceFile, rulesProvider: RulesProvider, options: FormatCodeSettings): TextChange[] { const precedingToken = findImmediatelyPrecedingTokenOfKind(position, SyntaxKind.CloseBraceToken, sourceFile); - return formatOutermostNodeWithinListLevel(precedingToken, sourceFile, options, rulesProvider, FormattingRequestKind.FormatOnClosingCurlyBrace); + return formatNodeLines(findOutermostNodeWithinListLevel(precedingToken), sourceFile, options, rulesProvider, FormattingRequestKind.FormatOnClosingCurlyBrace); } export function formatDocument(sourceFile: SourceFile, rulesProvider: RulesProvider, options: FormatCodeSettings): TextChange[] { @@ -145,8 +152,8 @@ namespace ts.formatting { } /** - * Finds and formats the highest node enclosing `node` whose end does not exceed the `node.end` - * and is at the same list level as the token at `node`. + * Finds the highest node enclosing `node` at the same list level as `node` + * and whose end does not exceed `node.end`. * * Consider typing the following * ``` @@ -157,9 +164,7 @@ namespace ts.formatting { * Upon typing the closing curly, we want to format the entire `while`-statement, but not the preceding * variable declaration. */ - function formatOutermostNodeWithinListLevel(node: Node, sourceFile: SourceFile, options: FormatCodeSettings, rulesProvider: RulesProvider, formattingRequestKind: FormattingRequestKind) { - // If we walk upwards searching for the parent that has the same end value, we'll end up with the whole source file. - // `isListElement` allows to stop on the list element level. + function findOutermostNodeWithinListLevel(node: Node) { let current = node; while (current && current.parent && @@ -168,7 +173,7 @@ namespace ts.formatting { current = current.parent; } - return formatNodeLines(current, sourceFile, options, rulesProvider, formattingRequestKind); + return current; } // Returns true if node is a element in some list in parent From 25abf8a9e8fecd7a0364cfc03d2c3b54bb231777 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Thu, 29 Jun 2017 17:31:41 -0700 Subject: [PATCH 11/11] respond ot comments --- src/services/formatting/formatting.ts | 34 ++++++++++++++++++--------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index 809d2103e2f..c55672229ba 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -103,18 +103,30 @@ namespace ts.formatting { export function formatOnOpeningCurly(position: number, sourceFile: SourceFile, rulesProvider: RulesProvider, options: FormatCodeSettings): TextChange[] { const openingCurly = findImmediatelyPrecedingTokenOfKind(position, SyntaxKind.OpenBraceToken, sourceFile); - if (openingCurly) { - const curlyBraceRange = openingCurly.parent; - const outermostNode = findOutermostNodeWithinListLevel(curlyBraceRange); - - const textRange: TextRange = { - pos: getLineStartPositionForPosition(outermostNode.getStart(sourceFile), sourceFile), - end: position - }; - - return formatSpan(textRange, sourceFile, options, rulesProvider, FormattingRequestKind.FormatOnOpeningCurlyBrace); + if (!openingCurly) { + return []; } - return []; + const curlyBraceRange = openingCurly.parent; + const outermostNode = findOutermostNodeWithinListLevel(curlyBraceRange); + + /** + * We limit the span to end at the opening curly to handle the case where + * the brace matched to that just typed will be incorrect after further edits. + * For example, we could type the opening curly for the following method + * body without brace-matching activated: + * ``` + * class C { + * foo() + * } + * ``` + * and we wouldn't want to move the closing brace. + */ + const textRange: TextRange = { + pos: getLineStartPositionForPosition(outermostNode.getStart(sourceFile), sourceFile), + end: position + }; + + return formatSpan(textRange, sourceFile, options, rulesProvider, FormattingRequestKind.FormatOnOpeningCurlyBrace); } export function formatOnClosingCurly(position: number, sourceFile: SourceFile, rulesProvider: RulesProvider, options: FormatCodeSettings): TextChange[] {