From 2c395e6819efdbca9c76c3d7182c8d4f91908be4 Mon Sep 17 00:00:00 2001 From: SaschaNaz Date: Thu, 8 Oct 2015 02:48:10 +0900 Subject: [PATCH 1/8] trimWhitespacesInEmptyLineTrivia --- src/services/formatting/formatting.ts | 51 +++++++++++++++++++++++---- 1 file changed, 45 insertions(+), 6 deletions(-) diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index 4a7033c6f3..08a84c8348 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -121,7 +121,7 @@ namespace ts.formatting { function findOutermostParent(position: number, expectedTokenKind: SyntaxKind, sourceFile: SourceFile): Node { let 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, @@ -151,7 +151,7 @@ namespace ts.formatting { return current; } - + // Returns true if node is a element in some list in parent // i.e. parent is class declaration with the list of members and node is one of members. function isListElement(parent: Node, node: Node): boolean { @@ -198,7 +198,7 @@ namespace ts.formatting { if (!errors.length) { return rangeHasNoErrors; } - + // pick only errors that fall in range let sorted = errors .filter(d => rangeOverlapsWithStartEnd(originalRange, d.start, d.start + d.length)) @@ -340,6 +340,12 @@ namespace ts.formatting { let delta = getOwnOrInheritedDelta(enclosingNode, options, sourceFile); processNode(enclosingNode, enclosingNode, startLine, undecoratedStartLine, initialIndentation, delta); } + else { + let leadingTrivia = formattingScanner.readTokenInfo(undefined).leadingTrivia; + if (leadingTrivia) { + trimWhitespacesInEmptyLineTrivia(leadingTrivia, true); + } + } formattingScanner.close(); @@ -445,7 +451,7 @@ namespace ts.formatting { if ((node).asteriskToken) { return SyntaxKind.AsteriskToken; } - // fall-through + // fall-through case SyntaxKind.PropertyDeclaration: case SyntaxKind.Parameter: @@ -555,6 +561,13 @@ namespace ts.formatting { consumeTokenAndAdvanceScanner(tokenInfo, node, nodeDynamicIndentation); } + if (!formattingScanner.isOnToken()) { + let leadingTrivia = formattingScanner.readTokenInfo(node).leadingTrivia; + if (leadingTrivia) { + trimWhitespacesInEmptyLineTrivia(leadingTrivia, true); + } + } + function processChildNode( child: Node, inheritedIndentation: number, @@ -686,6 +699,7 @@ namespace ts.formatting { let indentToken = false; if (currentTokenInfo.leadingTrivia) { + trimWhitespacesInEmptyLineTrivia(currentTokenInfo.leadingTrivia, formattingScanner.lastTrailingTriviaWasNewLine()); processTrivia(currentTokenInfo.leadingTrivia, parent, childContextNode, dynamicIndentation); } @@ -838,8 +852,8 @@ namespace ts.formatting { // We need to trim trailing whitespace between the tokens if they were on different lines, and no rule was applied to put them on the same line trimTrailingWhitespaces = - (rule.Operation.Action & (RuleAction.NewLine | RuleAction.Space)) && - rule.Flag !== RuleFlags.CanDeleteNewLines; + (rule.Operation.Action & (RuleAction.NewLine | RuleAction.Space)) && + rule.Flag !== RuleFlags.CanDeleteNewLines; } else { trimTrailingWhitespaces = true; @@ -949,6 +963,31 @@ namespace ts.formatting { } } + function trimWhitespacesInEmptyLineTrivia(trivia: TextRangeWithKind[], lastTraviaWasNewLine: boolean) { + let targetCandidate: TextRangeWithKind; + + for (let triviaItem of trivia) { + switch (triviaItem.kind) { + case SyntaxKind.WhitespaceTrivia: + targetCandidate = lastTraviaWasNewLine ? triviaItem : undefined; + break; + case SyntaxKind.NewLineTrivia: + if (targetCandidate) { + recordDelete(targetCandidate.pos, targetCandidate.end - targetCandidate.pos); + targetCandidate = undefined; + } + break; + default: + targetCandidate = undefined; + break; + } + lastTraviaWasNewLine = triviaItem.kind === SyntaxKind.NewLineTrivia; + } + if (targetCandidate && targetCandidate.end === sourceFile.end) { + recordDelete(targetCandidate.pos, targetCandidate.end - targetCandidate.pos); + } + } + function newTextChange(start: number, len: number, newText: string): TextChange { return { span: createTextSpan(start, len), newText } } From 790bb21b127261fe06524ff12c8067a35d26cec1 Mon Sep 17 00:00:00 2001 From: SaschaNaz Date: Fri, 9 Oct 2015 16:38:16 +0900 Subject: [PATCH 2/8] trimTrailingWhitespacesForRemainingRange --- src/services/formatting/formatting.ts | 45 +++++----------- .../fourslash/formatDocumentWithTrivia.ts | 51 +++++++++++++++++++ 2 files changed, 63 insertions(+), 33 deletions(-) create mode 100644 tests/cases/fourslash/formatDocumentWithTrivia.ts diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index 08a84c8348..7c81f3c91b 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -340,10 +340,12 @@ namespace ts.formatting { let delta = getOwnOrInheritedDelta(enclosingNode, options, sourceFile); processNode(enclosingNode, enclosingNode, startLine, undecoratedStartLine, initialIndentation, delta); } - else { + + if (!formattingScanner.isOnToken()) { let leadingTrivia = formattingScanner.readTokenInfo(undefined).leadingTrivia; if (leadingTrivia) { - trimWhitespacesInEmptyLineTrivia(leadingTrivia, true); + processTrivia(leadingTrivia, enclosingNode, enclosingNode, undefined); + trimTrailingWhitespacesForRemainingRange(); } } @@ -561,13 +563,6 @@ namespace ts.formatting { consumeTokenAndAdvanceScanner(tokenInfo, node, nodeDynamicIndentation); } - if (!formattingScanner.isOnToken()) { - let leadingTrivia = formattingScanner.readTokenInfo(node).leadingTrivia; - if (leadingTrivia) { - trimWhitespacesInEmptyLineTrivia(leadingTrivia, true); - } - } - function processChildNode( child: Node, inheritedIndentation: number, @@ -699,7 +694,6 @@ namespace ts.formatting { let indentToken = false; if (currentTokenInfo.leadingTrivia) { - trimWhitespacesInEmptyLineTrivia(currentTokenInfo.leadingTrivia, formattingScanner.lastTrailingTriviaWasNewLine()); processTrivia(currentTokenInfo.leadingTrivia, parent, childContextNode, dynamicIndentation); } @@ -852,7 +846,7 @@ namespace ts.formatting { // We need to trim trailing whitespace between the tokens if they were on different lines, and no rule was applied to put them on the same line trimTrailingWhitespaces = - (rule.Operation.Action & (RuleAction.NewLine | RuleAction.Space)) && + (rule.Operation.Action & (RuleAction.NewLine | RuleAction.Space | RuleAction.Ignore)) && rule.Flag !== RuleFlags.CanDeleteNewLines; } else { @@ -963,29 +957,14 @@ namespace ts.formatting { } } - function trimWhitespacesInEmptyLineTrivia(trivia: TextRangeWithKind[], lastTraviaWasNewLine: boolean) { - let targetCandidate: TextRangeWithKind; - - for (let triviaItem of trivia) { - switch (triviaItem.kind) { - case SyntaxKind.WhitespaceTrivia: - targetCandidate = lastTraviaWasNewLine ? triviaItem : undefined; - break; - case SyntaxKind.NewLineTrivia: - if (targetCandidate) { - recordDelete(targetCandidate.pos, targetCandidate.end - targetCandidate.pos); - targetCandidate = undefined; - } - break; - default: - targetCandidate = undefined; - break; - } - lastTraviaWasNewLine = triviaItem.kind === SyntaxKind.NewLineTrivia; - } - if (targetCandidate && targetCandidate.end === sourceFile.end) { - recordDelete(targetCandidate.pos, targetCandidate.end - targetCandidate.pos); + function trimTrailingWhitespacesForRemainingRange() { + let startPosition = previousRange ? previousRange.end : originalRange.pos; + let startLine = sourceFile.getLineAndCharacterOfPosition(startPosition).line; + let endLine = sourceFile.getLineAndCharacterOfPosition(originalRange.end).line; + if (originalRange.end === sourceFile.end) { + endLine++; } + trimTrailingWhitespacesForLines(startLine, endLine, previousRange); } function newTextChange(start: number, len: number, newText: string): TextChange { diff --git a/tests/cases/fourslash/formatDocumentWithTrivia.ts b/tests/cases/fourslash/formatDocumentWithTrivia.ts new file mode 100644 index 0000000000..d51677be81 --- /dev/null +++ b/tests/cases/fourslash/formatDocumentWithTrivia.ts @@ -0,0 +1,51 @@ +/// + +//// +////// whitespace below +//// +////// whitespace above +//// +////let x; +//// +////// abc +//// +////let y; +//// +////// whitespace above again +//// +////while (true) { +//// while (true) { +//// } +//// +//// // whitespace above +////} +//// +////// whitespace above again +//// +//// + +format.document(); + +verify.currentFileContentIs(` +// whitespace below + +// whitespace above + +let x; + +// abc + +let y; + +// whitespace above again + +while (true) { + while (true) { + } + + // whitespace above +} + +// whitespace above again + +`); From 13d2a729c984813cfb9f128dcb122e07f5c41273 Mon Sep 17 00:00:00 2001 From: SaschaNaz Date: Fri, 9 Oct 2015 16:40:47 +0900 Subject: [PATCH 3/8] restore fall-through indentation --- 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 7c81f3c91b..d3555578cb 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -453,7 +453,7 @@ namespace ts.formatting { if ((node).asteriskToken) { return SyntaxKind.AsteriskToken; } - // fall-through + // fall-through case SyntaxKind.PropertyDeclaration: case SyntaxKind.Parameter: From 6580f877e7a44c4dadeddf24923ebe7caf3f0bd1 Mon Sep 17 00:00:00 2001 From: SaschaNaz Date: Sat, 10 Oct 2015 16:38:10 +0900 Subject: [PATCH 4/8] trim the whole last line when it has whitespaces only --- src/services/formatting/formatting.ts | 38 +++++++++++++++++++++------ 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index d3555578cb..48914147e8 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -946,22 +946,44 @@ namespace ts.formatting { continue; } - let pos = lineEndPosition; - while (pos >= lineStartPosition && isWhiteSpace(sourceFile.text.charCodeAt(pos))) { - pos--; - } - if (pos !== lineEndPosition) { - Debug.assert(pos === lineStartPosition || !isWhiteSpace(sourceFile.text.charCodeAt(pos))); - recordDelete(pos + 1, lineEndPosition - pos); + let whitespaceStart = getTrailingWhitespaceStartPosition(lineStartPosition, lineEndPosition); + if (whitespaceStart !== -1) { + recordDelete(whitespaceStart, lineEndPosition + 1 - whitespaceStart); } } } + /** + * @param start The position of the first character in range + * @param end The position of the last character in range + */ + function getTrailingWhitespaceStartPosition(start: number, end: number) { + let pos = end; + while (pos >= start && isWhiteSpace(sourceFile.text.charCodeAt(pos))) { + pos--; + } + if (pos !== end) { + // pos must be out of range or non-whitespace + Debug.assert(pos === start - 1 || !isWhiteSpace(sourceFile.text.charCodeAt(pos))); + return pos + 1; + } + return -1; + } + + /** + * Trimming will be done for lines after the previous range + */ function trimTrailingWhitespacesForRemainingRange() { let startPosition = previousRange ? previousRange.end : originalRange.pos; + let startLine = sourceFile.getLineAndCharacterOfPosition(startPosition).line; let endLine = sourceFile.getLineAndCharacterOfPosition(originalRange.end).line; - if (originalRange.end === sourceFile.end) { + + let endLineStartPosition = getStartPositionOfLine(endLine, sourceFile); + let endLineEndPosition = getEndLinePosition(endLine, sourceFile); + + if (getTrailingWhitespaceStartPosition(endLineStartPosition, endLineEndPosition) === endLineStartPosition) { + // Trim the whole last line when it has whitespaces only endLine++; } trimTrailingWhitespacesForLines(startLine, endLine, previousRange); From 82bed69123ec40430bedf28ea01293c394d8b04a Mon Sep 17 00:00:00 2001 From: SaschaNaz Date: Sat, 10 Oct 2015 16:41:54 +0900 Subject: [PATCH 5/8] last line whitespace test --- tests/cases/fourslash/formatSelectionWithTrivia2.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 tests/cases/fourslash/formatSelectionWithTrivia2.ts diff --git a/tests/cases/fourslash/formatSelectionWithTrivia2.ts b/tests/cases/fourslash/formatSelectionWithTrivia2.ts new file mode 100644 index 0000000000..06b43009a2 --- /dev/null +++ b/tests/cases/fourslash/formatSelectionWithTrivia2.ts @@ -0,0 +1,10 @@ +/// + +/////*begin*/; +//// +/////*end*/ +//// + +format.selection('begin', 'end'); + +verify.currentFileContentIs(";\n\n\n "); From f5587151baca19d60a2610e87ab2fee0e05f5618 Mon Sep 17 00:00:00 2001 From: SaschaNaz Date: Thu, 10 Dec 2015 22:07:01 +0900 Subject: [PATCH 6/8] applying three feedbacks --- src/services/formatting/formatting.ts | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index 64e420f159..32416472f9 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -847,9 +847,7 @@ namespace ts.formatting { } // We need to trim trailing whitespace between the tokens if they were on different lines, and no rule was applied to put them on the same line - trimTrailingWhitespaces = - (rule.Operation.Action & (RuleAction.NewLine | RuleAction.Space | RuleAction.Ignore)) && - rule.Flag !== RuleFlags.CanDeleteNewLines; + trimTrailingWhitespaces = !(rule.Operation.Action & RuleAction.Delete) && rule.Flag !== RuleFlags.CanDeleteNewLines; } else { trimTrailingWhitespaces = true; @@ -950,6 +948,7 @@ namespace ts.formatting { let whitespaceStart = getTrailingWhitespaceStartPosition(lineStartPosition, lineEndPosition); if (whitespaceStart !== -1) { + Debug.assert(whitespaceStart === lineStartPosition || !isWhiteSpace(sourceFile.text.charCodeAt(whitespaceStart - 1))); recordDelete(whitespaceStart, lineEndPosition + 1 - whitespaceStart); } } @@ -965,8 +964,6 @@ namespace ts.formatting { pos--; } if (pos !== end) { - // pos must be out of range or non-whitespace - Debug.assert(pos === start - 1 || !isWhiteSpace(sourceFile.text.charCodeAt(pos))); return pos + 1; } return -1; @@ -981,14 +978,7 @@ namespace ts.formatting { let startLine = sourceFile.getLineAndCharacterOfPosition(startPosition).line; let endLine = sourceFile.getLineAndCharacterOfPosition(originalRange.end).line; - let endLineStartPosition = getStartPositionOfLine(endLine, sourceFile); - let endLineEndPosition = getEndLinePosition(endLine, sourceFile); - - if (getTrailingWhitespaceStartPosition(endLineStartPosition, endLineEndPosition) === endLineStartPosition) { - // Trim the whole last line when it has whitespaces only - endLine++; - } - trimTrailingWhitespacesForLines(startLine, endLine, previousRange); + trimTrailingWhitespacesForLines(startLine, endLine + 1, previousRange); } function newTextChange(start: number, len: number, newText: string): TextChange { From f2cad8be4022be8c878732574b94e41c748791e7 Mon Sep 17 00:00:00 2001 From: SaschaNaz Date: Thu, 10 Dec 2015 23:05:45 +0900 Subject: [PATCH 7/8] getCurrentLeadingTrivia --- src/services/formatting/formatting.ts | 2 +- src/services/formatting/formattingScanner.ts | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index 0e1931f9ee..6efa918d30 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -342,7 +342,7 @@ namespace ts.formatting { } if (!formattingScanner.isOnToken()) { - let leadingTrivia = formattingScanner.readTokenInfo(undefined).leadingTrivia; + let leadingTrivia = formattingScanner.getCurrentLeadingTrivia(); if (leadingTrivia) { processTrivia(leadingTrivia, enclosingNode, enclosingNode, undefined); trimTrailingWhitespacesForRemainingRange(); diff --git a/src/services/formatting/formattingScanner.ts b/src/services/formatting/formattingScanner.ts index 58e2f30448..5d215788d4 100644 --- a/src/services/formatting/formattingScanner.ts +++ b/src/services/formatting/formattingScanner.ts @@ -15,6 +15,7 @@ namespace ts.formatting { advance(): void; isOnToken(): boolean; readTokenInfo(n: Node): TokenInfo; + getCurrentLeadingTrivia(): TextRangeWithKind[]; lastTrailingTriviaWasNewLine(): boolean; close(): void; } @@ -43,9 +44,10 @@ namespace ts.formatting { let lastTokenInfo: TokenInfo; return { - advance: advance, - readTokenInfo: readTokenInfo, - isOnToken: isOnToken, + advance, + readTokenInfo, + isOnToken, + getCurrentLeadingTrivia: () => leadingTrivia, lastTrailingTriviaWasNewLine: () => wasNewLine, close: () => { Debug.assert(scanner !== undefined); @@ -156,7 +158,7 @@ namespace ts.formatting { if (!isOnToken()) { // scanner is not on the token (either advance was not called yet or scanner is already past the end position) return { - leadingTrivia: leadingTrivia, + leadingTrivia, trailingTrivia: undefined, token: undefined }; From d8260b705998e83fc9aafe514306d676b8fd5c59 Mon Sep 17 00:00:00 2001 From: SaschaNaz Date: Fri, 11 Dec 2015 00:24:10 +0900 Subject: [PATCH 8/8] whitespaces --- src/services/formatting/formattingScanner.ts | 30 ++++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/services/formatting/formattingScanner.ts b/src/services/formatting/formattingScanner.ts index 5d215788d4..7a4cb0924d 100644 --- a/src/services/formatting/formattingScanner.ts +++ b/src/services/formatting/formattingScanner.ts @@ -5,12 +5,12 @@ namespace ts.formatting { const standardScanner = createScanner(ScriptTarget.Latest, /*skipTrivia*/ false, LanguageVariant.Standard); const jsxScanner = createScanner(ScriptTarget.Latest, /*skipTrivia*/ false, LanguageVariant.JSX); - + /** * Scanner that is currently used for formatting */ let scanner: Scanner; - + export interface FormattingScanner { advance(): void; isOnToken(): boolean; @@ -20,7 +20,7 @@ namespace ts.formatting { close(): void; } - const enum ScanAction{ + const enum ScanAction { Scan, RescanGreaterThanToken, RescanSlashToken, @@ -38,7 +38,7 @@ namespace ts.formatting { let wasNewLine: boolean = true; let leadingTrivia: TextRangeWithKind[]; let trailingTrivia: TextRangeWithKind[]; - + let savedPos: number; let lastScanAction: ScanAction; let lastTokenInfo: TokenInfo; @@ -51,7 +51,7 @@ namespace ts.formatting { lastTrailingTriviaWasNewLine: () => wasNewLine, close: () => { Debug.assert(scanner !== undefined); - + lastTokenInfo = undefined; scanner.setText(undefined); scanner = undefined; @@ -60,7 +60,7 @@ namespace ts.formatting { function advance(): void { Debug.assert(scanner !== undefined); - + lastTokenInfo = undefined; let isStarted = scanner.getStartPos() !== startPos; @@ -83,7 +83,7 @@ namespace ts.formatting { let t: SyntaxKind; let pos = scanner.getStartPos(); - + // Read leading trivia and token while (pos < endPos) { let t = scanner.getToken(); @@ -124,10 +124,10 @@ namespace ts.formatting { return false; } - + function shouldRescanJsxIdentifier(node: Node): boolean { if (node.parent) { - switch(node.parent.kind) { + switch (node.parent.kind) { case SyntaxKind.JsxAttribute: case SyntaxKind.JsxOpeningElement: case SyntaxKind.JsxClosingElement: @@ -135,7 +135,7 @@ namespace ts.formatting { return node.kind === SyntaxKind.Identifier; } } - + return false; } @@ -144,7 +144,7 @@ namespace ts.formatting { } function shouldRescanTemplateToken(container: Node): boolean { - return container.kind === SyntaxKind.TemplateMiddle || + return container.kind === SyntaxKind.TemplateMiddle || container.kind === SyntaxKind.TemplateTail; } @@ -154,7 +154,7 @@ namespace ts.formatting { function readTokenInfo(n: Node): TokenInfo { Debug.assert(scanner !== undefined); - + if (!isOnToken()) { // scanner is not on the token (either advance was not called yet or scanner is already past the end position) return { @@ -166,7 +166,7 @@ namespace ts.formatting { // normally scanner returns the smallest available token // check the kind of context node to determine if scanner should have more greedy behavior and consume more text. - let expectedScanAction = + let expectedScanAction = shouldRescanGreaterThanToken(n) ? ScanAction.RescanGreaterThanToken : shouldRescanSlashToken(n) @@ -228,7 +228,7 @@ namespace ts.formatting { if (trailingTrivia) { trailingTrivia = undefined; } - while(scanner.getStartPos() < endPos) { + while (scanner.getStartPos() < endPos) { currentToken = scanner.scan(); if (!isTrivia(currentToken)) { break; @@ -263,7 +263,7 @@ namespace ts.formatting { function isOnToken(): boolean { Debug.assert(scanner !== undefined); - + let current = (lastTokenInfo && lastTokenInfo.token.kind) || scanner.getToken(); let startPos = (lastTokenInfo && lastTokenInfo.token.pos) || scanner.getStartPos(); return startPos < endPos && current !== SyntaxKind.EndOfFileToken && !isTrivia(current);