From 0a61334677b839992338af86e8c210307912f076 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Wed, 29 Jun 2016 08:08:46 -0700 Subject: [PATCH 1/4] Do not scan nodes preceding formatted region, just skip over them --- src/compiler/scanner.ts | 15 ++++++++-- src/services/formatting/formatting.ts | 5 +++- src/services/formatting/formattingScanner.ts | 29 +++++++++++++++++--- src/services/formatting/smartIndenter.ts | 2 +- src/services/services.ts | 6 ++-- tests/cases/fourslash/templateStringRange.ts | 9 ++++++ 6 files changed, 54 insertions(+), 12 deletions(-) create mode 100644 tests/cases/fourslash/templateStringRange.ts diff --git a/src/compiler/scanner.ts b/src/compiler/scanner.ts index 82359eacc3e..dde0065a312 100644 --- a/src/compiler/scanner.ts +++ b/src/compiler/scanner.ts @@ -33,6 +33,7 @@ namespace ts { scan(): SyntaxKind; // Sets the text for the scanner to scan. An optional subrange starting point and length // can be provided to have the scanner only scan a portion of the text. + getText(): string; setText(text: string, start?: number, length?: number): void; setOnError(onError: ErrorCallback): void; setScriptTarget(scriptTarget: ScriptTarget): void; @@ -364,6 +365,11 @@ namespace ts { const hasOwnProperty = Object.prototype.hasOwnProperty; + export function isWhiteSpaceLike(ch: number): boolean { + return isWhiteSpace(ch) || isLineBreak(ch); + } + + /** Does not include line breaks. For that, see isWhiteSpaceLike. */ export function isWhiteSpace(ch: number): boolean { // Note: nextLine is in the Zs space, and should be considered to be a whitespace. // It is explicitly not a line-break as it isn't in the exact set specified by EcmaScript. @@ -505,7 +511,7 @@ namespace ts { break; default: - if (ch > CharacterCodes.maxAsciiCharacter && (isWhiteSpace(ch) || isLineBreak(ch))) { + if (ch > CharacterCodes.maxAsciiCharacter && (isWhiteSpaceLike(ch))) { pos++; continue; } @@ -658,7 +664,7 @@ namespace ts { } break; default: - if (ch > CharacterCodes.maxAsciiCharacter && (isWhiteSpace(ch) || isLineBreak(ch))) { + if (ch > CharacterCodes.maxAsciiCharacter && (isWhiteSpaceLike(ch))) { if (result && result.length && isLineBreak(ch)) { lastOrUndefined(result).hasTrailingNewLine = true; } @@ -763,6 +769,7 @@ namespace ts { scanJsxToken, scanJSDocToken, scan, + getText, setText, setScriptTarget, setLanguageVariant, @@ -1789,6 +1796,10 @@ namespace ts { return speculationHelper(callback, /*isLookahead*/ false); } + function getText(): string { + return text; + } + function setText(newText: string, start: number, length: number) { text = newText || ""; end = length === undefined ? text.length : start + length; diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index 5dae6393b99..8efbf90c68e 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -78,7 +78,7 @@ namespace ts.formatting { // 1. the end of the previous line // 2. the last non-whitespace character in the current line let endOfFormatSpan = getEndLinePosition(line, sourceFile); - while (isWhiteSpace(sourceFile.text.charCodeAt(endOfFormatSpan)) && !isLineBreak(sourceFile.text.charCodeAt(endOfFormatSpan))) { + while (isWhiteSpace(sourceFile.text.charCodeAt(endOfFormatSpan))) { endOfFormatSpan--; } // if the character at the end of the span is a line break, we shouldn't include it, because it indicates we don't want to @@ -599,6 +599,9 @@ namespace ts.formatting { // child node is outside the target range - do not dive inside if (!rangeOverlapsWithStartEnd(originalRange, child.pos, child.end)) { + if (child.end < originalRange.pos) { + formattingScanner.skipToEndOf(child); + } return inheritedIndentation; } diff --git a/src/services/formatting/formattingScanner.ts b/src/services/formatting/formattingScanner.ts index babf0db3fd5..a1959194a3b 100644 --- a/src/services/formatting/formattingScanner.ts +++ b/src/services/formatting/formattingScanner.ts @@ -17,6 +17,7 @@ namespace ts.formatting { readTokenInfo(n: Node): TokenInfo; getCurrentLeadingTrivia(): TextRangeWithKind[]; lastTrailingTriviaWasNewLine(): boolean; + skipToEndOf(node: Node): void; close(): void; } @@ -36,12 +37,12 @@ namespace ts.formatting { scanner.setTextPos(startPos); let wasNewLine = true; - let leadingTrivia: TextRangeWithKind[]; - let trailingTrivia: TextRangeWithKind[]; + let leadingTrivia: TextRangeWithKind[] | undefined; + let trailingTrivia: TextRangeWithKind[] | undefined; let savedPos: number; - let lastScanAction: ScanAction; - let lastTokenInfo: TokenInfo; + let lastScanAction: ScanAction | undefined; + let lastTokenInfo: TokenInfo | undefined; return { advance, @@ -49,6 +50,7 @@ namespace ts.formatting { isOnToken, getCurrentLeadingTrivia: () => leadingTrivia, lastTrailingTriviaWasNewLine: () => wasNewLine, + skipToEndOf, close: () => { Debug.assert(scanner !== undefined); @@ -278,5 +280,24 @@ namespace ts.formatting { } return tokenInfo; } + + function skipToEndOf(node: Node): void { + scanner.setTextPos(backUpWhitespace()); + savedPos = scanner.getStartPos(); + lastScanAction = undefined; + lastTokenInfo = undefined; + wasNewLine = false; + leadingTrivia = undefined; + trailingTrivia = undefined; + + function backUpWhitespace(): number { + const text = scanner.getText(); + let end = node.end; + while (end > 0 && isWhiteSpaceLike(text.charCodeAt(end - 1))) { + end--; + } + return end; + } + } } } \ No newline at end of file diff --git a/src/services/formatting/smartIndenter.ts b/src/services/formatting/smartIndenter.ts index 120a5be6e4e..f1686fefadc 100644 --- a/src/services/formatting/smartIndenter.ts +++ b/src/services/formatting/smartIndenter.ts @@ -42,7 +42,7 @@ namespace ts.formatting { let current = position; while (current > 0) { const char = sourceFile.text.charCodeAt(current); - if (!isWhiteSpace(char) && !isLineBreak(char)) { + if (!isWhiteSpaceLike(char)) { break; } current--; diff --git a/src/services/services.ts b/src/services/services.ts index 529ce8a7bf7..4c391f770e7 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -474,8 +474,7 @@ namespace ts { for (; pos < end; pos++) { const ch = sourceFile.text.charCodeAt(pos); - if (!isWhiteSpace(ch) || isLineBreak(ch)) { - // Either found lineBreak or non whiteSpace + if (!isWhiteSpace(ch)) { return pos; } } @@ -494,8 +493,7 @@ namespace ts { function isName(pos: number, end: number, sourceFile: SourceFile, name: string) { return pos + name.length < end && sourceFile.text.substr(pos, name.length) === name && - (isWhiteSpace(sourceFile.text.charCodeAt(pos + name.length)) || - isLineBreak(sourceFile.text.charCodeAt(pos + name.length))); + isWhiteSpaceLike(sourceFile.text.charCodeAt(pos + name.length)); } function isParamTag(pos: number, end: number, sourceFile: SourceFile) { diff --git a/tests/cases/fourslash/templateStringRange.ts b/tests/cases/fourslash/templateStringRange.ts new file mode 100644 index 00000000000..20654f3f963 --- /dev/null +++ b/tests/cases/fourslash/templateStringRange.ts @@ -0,0 +1,9 @@ +/// + +////`${1}`; +////` +////`;/**/1 + +goTo.marker(); +edit.insert('\n'); +verify.currentLineContentIs("1"); From 9daa800c6af0abc342b0d65947d3272ffde2adff Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Thu, 30 Jun 2016 06:46:49 -0700 Subject: [PATCH 2/4] Respond to PR comments --- src/compiler/scanner.ts | 2 +- src/services/formatting/formattingScanner.ts | 11 +---------- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/src/compiler/scanner.ts b/src/compiler/scanner.ts index dde0065a312..13d26b23e50 100644 --- a/src/compiler/scanner.ts +++ b/src/compiler/scanner.ts @@ -31,9 +31,9 @@ namespace ts { scanJsxToken(): SyntaxKind; scanJSDocToken(): SyntaxKind; scan(): SyntaxKind; + getText(): string; // Sets the text for the scanner to scan. An optional subrange starting point and length // can be provided to have the scanner only scan a portion of the text. - getText(): string; setText(text: string, start?: number, length?: number): void; setOnError(onError: ErrorCallback): void; setScriptTarget(scriptTarget: ScriptTarget): void; diff --git a/src/services/formatting/formattingScanner.ts b/src/services/formatting/formattingScanner.ts index a1959194a3b..401794a077b 100644 --- a/src/services/formatting/formattingScanner.ts +++ b/src/services/formatting/formattingScanner.ts @@ -282,22 +282,13 @@ namespace ts.formatting { } function skipToEndOf(node: Node): void { - scanner.setTextPos(backUpWhitespace()); + scanner.setTextPos(node.end); savedPos = scanner.getStartPos(); lastScanAction = undefined; lastTokenInfo = undefined; wasNewLine = false; leadingTrivia = undefined; trailingTrivia = undefined; - - function backUpWhitespace(): number { - const text = scanner.getText(); - let end = node.end; - while (end > 0 && isWhiteSpaceLike(text.charCodeAt(end - 1))) { - end--; - } - return end; - } } } } \ No newline at end of file From fee06acf84ea010a5b58aacebed72ce8a00c3ab3 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Thu, 30 Jun 2016 09:21:08 -0700 Subject: [PATCH 3/4] Better name for test --- .../{templateStringRange.ts => formattingTemplatesWithNewline.ts} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/cases/fourslash/{templateStringRange.ts => formattingTemplatesWithNewline.ts} (100%) diff --git a/tests/cases/fourslash/templateStringRange.ts b/tests/cases/fourslash/formattingTemplatesWithNewline.ts similarity index 100% rename from tests/cases/fourslash/templateStringRange.ts rename to tests/cases/fourslash/formattingTemplatesWithNewline.ts From 2c40fea9f1da904957ab774c23bd1523e255a513 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Fri, 1 Jul 2016 13:57:20 -0700 Subject: [PATCH 4/4] Improve names of whitespace functions --- src/compiler/emitter.ts | 2 +- src/compiler/scanner.ts | 16 ++++++++-------- src/compiler/utilities.ts | 2 +- src/services/formatting/formatting.ts | 6 +++--- src/services/formatting/smartIndenter.ts | 4 ++-- src/services/services.ts | 8 ++++---- 6 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index 30f0e74adb4..2d5acb3b486 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -7686,7 +7686,7 @@ const _super = (function (geti, seti) { } firstNonWhitespace = -1; } - else if (!isWhiteSpace(c)) { + else if (!isWhiteSpaceSingleLine(c)) { lastNonWhitespace = i; if (firstNonWhitespace === -1) { firstNonWhitespace = i; diff --git a/src/compiler/scanner.ts b/src/compiler/scanner.ts index 13d26b23e50..6dd84298008 100644 --- a/src/compiler/scanner.ts +++ b/src/compiler/scanner.ts @@ -365,12 +365,12 @@ namespace ts { const hasOwnProperty = Object.prototype.hasOwnProperty; - export function isWhiteSpaceLike(ch: number): boolean { - return isWhiteSpace(ch) || isLineBreak(ch); + export function isWhiteSpace(ch: number): boolean { + return isWhiteSpaceSingleLine(ch) || isLineBreak(ch); } /** Does not include line breaks. For that, see isWhiteSpaceLike. */ - export function isWhiteSpace(ch: number): boolean { + export function isWhiteSpaceSingleLine(ch: number): boolean { // Note: nextLine is in the Zs space, and should be considered to be a whitespace. // It is explicitly not a line-break as it isn't in the exact set specified by EcmaScript. return ch === CharacterCodes.space || @@ -511,7 +511,7 @@ namespace ts { break; default: - if (ch > CharacterCodes.maxAsciiCharacter && (isWhiteSpaceLike(ch))) { + if (ch > CharacterCodes.maxAsciiCharacter && (isWhiteSpace(ch))) { pos++; continue; } @@ -664,7 +664,7 @@ namespace ts { } break; default: - if (ch > CharacterCodes.maxAsciiCharacter && (isWhiteSpaceLike(ch))) { + if (ch > CharacterCodes.maxAsciiCharacter && (isWhiteSpace(ch))) { if (result && result.length && isLineBreak(ch)) { lastOrUndefined(result).hasTrailingNewLine = true; } @@ -1209,7 +1209,7 @@ namespace ts { continue; } else { - while (pos < end && isWhiteSpace(text.charCodeAt(pos))) { + while (pos < end && isWhiteSpaceSingleLine(text.charCodeAt(pos))) { pos++; } return token = SyntaxKind.WhitespaceTrivia; @@ -1527,7 +1527,7 @@ namespace ts { } return token = getIdentifierToken(); } - else if (isWhiteSpace(ch)) { + else if (isWhiteSpaceSingleLine(ch)) { pos++; continue; } @@ -1696,7 +1696,7 @@ namespace ts { let ch = text.charCodeAt(pos); while (pos < end) { ch = text.charCodeAt(pos); - if (isWhiteSpace(ch)) { + if (isWhiteSpaceSingleLine(ch)) { pos++; } else { diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index e8f2832cd59..480d94912f9 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -2607,7 +2607,7 @@ namespace ts { function calculateIndent(text: string, pos: number, end: number) { let currentLineIndent = 0; - for (; pos < end && isWhiteSpace(text.charCodeAt(pos)); pos++) { + for (; pos < end && isWhiteSpaceSingleLine(text.charCodeAt(pos)); pos++) { if (text.charCodeAt(pos) === CharacterCodes.tab) { // Tabs = TabSize = indent size and go to next tabStop currentLineIndent += getIndentSize() - (currentLineIndent % getIndentSize()); diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index 8efbf90c68e..8a27501e684 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -78,7 +78,7 @@ namespace ts.formatting { // 1. the end of the previous line // 2. the last non-whitespace character in the current line let endOfFormatSpan = getEndLinePosition(line, sourceFile); - while (isWhiteSpace(sourceFile.text.charCodeAt(endOfFormatSpan))) { + while (isWhiteSpaceSingleLine(sourceFile.text.charCodeAt(endOfFormatSpan))) { endOfFormatSpan--; } // if the character at the end of the span is a line break, we shouldn't include it, because it indicates we don't want to @@ -966,7 +966,7 @@ namespace ts.formatting { const whitespaceStart = getTrailingWhitespaceStartPosition(lineStartPosition, lineEndPosition); if (whitespaceStart !== -1) { - Debug.assert(whitespaceStart === lineStartPosition || !isWhiteSpace(sourceFile.text.charCodeAt(whitespaceStart - 1))); + Debug.assert(whitespaceStart === lineStartPosition || !isWhiteSpaceSingleLine(sourceFile.text.charCodeAt(whitespaceStart - 1))); recordDelete(whitespaceStart, lineEndPosition + 1 - whitespaceStart); } } @@ -978,7 +978,7 @@ namespace ts.formatting { */ function getTrailingWhitespaceStartPosition(start: number, end: number) { let pos = end; - while (pos >= start && isWhiteSpace(sourceFile.text.charCodeAt(pos))) { + while (pos >= start && isWhiteSpaceSingleLine(sourceFile.text.charCodeAt(pos))) { pos--; } if (pos !== end) { diff --git a/src/services/formatting/smartIndenter.ts b/src/services/formatting/smartIndenter.ts index f1686fefadc..cd85d84dfea 100644 --- a/src/services/formatting/smartIndenter.ts +++ b/src/services/formatting/smartIndenter.ts @@ -42,7 +42,7 @@ namespace ts.formatting { let current = position; while (current > 0) { const char = sourceFile.text.charCodeAt(current); - if (!isWhiteSpaceLike(char)) { + if (!isWhiteSpace(char)) { break; } current--; @@ -406,7 +406,7 @@ namespace ts.formatting { let column = 0; for (let pos = startPos; pos < endPos; pos++) { const ch = sourceFile.text.charCodeAt(pos); - if (!isWhiteSpace(ch)) { + if (!isWhiteSpaceSingleLine(ch)) { break; } diff --git a/src/services/services.ts b/src/services/services.ts index 4c391f770e7..8c69b66c7d8 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -474,7 +474,7 @@ namespace ts { for (; pos < end; pos++) { const ch = sourceFile.text.charCodeAt(pos); - if (!isWhiteSpace(ch)) { + if (!isWhiteSpaceSingleLine(ch)) { return pos; } } @@ -493,7 +493,7 @@ namespace ts { function isName(pos: number, end: number, sourceFile: SourceFile, name: string) { return pos + name.length < end && sourceFile.text.substr(pos, name.length) === name && - isWhiteSpaceLike(sourceFile.text.charCodeAt(pos + name.length)); + isWhiteSpace(sourceFile.text.charCodeAt(pos + name.length)); } function isParamTag(pos: number, end: number, sourceFile: SourceFile) { @@ -688,7 +688,7 @@ namespace ts { return paramDocComments; function consumeWhiteSpaces(pos: number) { - while (pos < end && isWhiteSpace(sourceFile.text.charCodeAt(pos))) { + while (pos < end && isWhiteSpaceSingleLine(sourceFile.text.charCodeAt(pos))) { pos++; } @@ -5725,7 +5725,7 @@ namespace ts { // Avoid recalculating getStart() by iterating backwards. for (let j = ifKeyword.getStart() - 1; j >= elseKeyword.end; j--) { - if (!isWhiteSpace(sourceFile.text.charCodeAt(j))) { + if (!isWhiteSpaceSingleLine(sourceFile.text.charCodeAt(j))) { shouldCombindElseAndIf = false; break; }