From 48eb3755d95079669084c172c2fc10fec42abd4b Mon Sep 17 00:00:00 2001 From: Orta Therox Date: Fri, 24 Jan 2020 17:20:07 -0500 Subject: [PATCH 1/5] WIP on making the JSX text node not include whitespace --- src/compiler/scanner.ts | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/compiler/scanner.ts b/src/compiler/scanner.ts index 2ccc8ef270d0f..e98b496c02d8a 100644 --- a/src/compiler/scanner.ts +++ b/src/compiler/scanner.ts @@ -2067,6 +2067,8 @@ namespace ts { // First non-whitespace character on this line. let firstNonWhitespace = 0; + let lastNonWhitespace = -1; + // These initial values are special because the first line is: // firstNonWhitespace = 0 to indicate that we want leading whitespace, @@ -2095,10 +2097,22 @@ namespace ts { else if (!isWhiteSpaceLike(char)) { firstNonWhitespace = pos; } + + // newlines are OK + if (!isWhiteSpaceLike(char)) { + // console.log(text.charAt(pos)); + lastNonWhitespace = pos + 1; + } + pos++; } - tokenValue = text.substring(startPos, pos); + const endPosition = lastNonWhitespace === -1 ? pos : lastNonWhitespace; + tokenValue = text.substring(startPos, endPosition); + pos = endPosition; + + // console.log("one: '" + text.substring(startPos, pos) + "'"); + // console.log("two: '" + tokenValue + "'"); return firstNonWhitespace === -1 ? SyntaxKind.JsxTextAllWhiteSpaces : SyntaxKind.JsxText; } From 627119701e7b03fe8853bd14709f0bec27383fcf Mon Sep 17 00:00:00 2001 From: Orta Therox Date: Tue, 28 Jan 2020 13:49:19 -0500 Subject: [PATCH 2/5] Scans to the last newline for JSX correctly --- src/compiler/scanner.ts | 17 +++++++----- .../fourslash/formatTsxClosingAfterJsxText.ts | 27 +++++++++++++++++++ 2 files changed, 37 insertions(+), 7 deletions(-) create mode 100644 tests/cases/fourslash/formatTsxClosingAfterJsxText.ts diff --git a/src/compiler/scanner.ts b/src/compiler/scanner.ts index e98b496c02d8a..d285378486748 100644 --- a/src/compiler/scanner.ts +++ b/src/compiler/scanner.ts @@ -2073,6 +2073,13 @@ namespace ts { // firstNonWhitespace = 0 to indicate that we want leading whitespace, while (pos < end) { + + // newlines are OK + if (!isWhiteSpaceSingleLine(char)) { + // console.log(text.charAt(pos)); + lastNonWhitespace = pos; + } + char = text.charCodeAt(pos); if (char === CharacterCodes.openBrace) { break; @@ -2085,6 +2092,8 @@ namespace ts { break; } + if (lastNonWhitespace > 0) lastNonWhitespace++; + // FirstNonWhitespace is 0, then we only see whitespaces so far. If we see a linebreak, we want to ignore that whitespaces. // i.e (- : whitespace) //
---- @@ -2098,18 +2107,12 @@ namespace ts { firstNonWhitespace = pos; } - // newlines are OK - if (!isWhiteSpaceLike(char)) { - // console.log(text.charAt(pos)); - lastNonWhitespace = pos + 1; - } - pos++; } const endPosition = lastNonWhitespace === -1 ? pos : lastNonWhitespace; tokenValue = text.substring(startPos, endPosition); - pos = endPosition; + // pos = endPosition; // console.log("one: '" + text.substring(startPos, pos) + "'"); // console.log("two: '" + tokenValue + "'"); diff --git a/tests/cases/fourslash/formatTsxClosingAfterJsxText.ts b/tests/cases/fourslash/formatTsxClosingAfterJsxText.ts new file mode 100644 index 0000000000000..95c9210c2aa24 --- /dev/null +++ b/tests/cases/fourslash/formatTsxClosingAfterJsxText.ts @@ -0,0 +1,27 @@ +/// +//// +////const a = ( +////
+//// text +////
+////) +//// +////const b = ( +////
+//// +////
+////) + +format.document(); +verify.currentFileContentIs(` +const a = ( +
+ text +
+) + +const b = ( +
+ +
+);`); From ff1a1aadd02574ff934464420717269bb4bdf633 Mon Sep 17 00:00:00 2001 From: Orta Therox Date: Fri, 31 Jan 2020 15:47:36 -0500 Subject: [PATCH 3/5] Handle JSX closing element wrapping --- src/compiler/scanner.ts | 7 ++-- src/services/formatting/formatting.ts | 4 ++- src/services/formatting/formattingScanner.ts | 8 ++++- src/services/formatting/smartIndenter.ts | 18 ++++++++++ .../fourslash/formatTsxClosingAfterJsxText.ts | 34 +++++++++++-------- 5 files changed, 49 insertions(+), 22 deletions(-) diff --git a/src/compiler/scanner.ts b/src/compiler/scanner.ts index d285378486748..bc38546825f1c 100644 --- a/src/compiler/scanner.ts +++ b/src/compiler/scanner.ts @@ -2074,9 +2074,9 @@ namespace ts { while (pos < end) { - // newlines are OK + // We want to keep track of the last non-whitespace (but including + // newlines character for hitting the end of the JSX Text region) if (!isWhiteSpaceSingleLine(char)) { - // console.log(text.charAt(pos)); lastNonWhitespace = pos; } @@ -2112,10 +2112,7 @@ namespace ts { const endPosition = lastNonWhitespace === -1 ? pos : lastNonWhitespace; tokenValue = text.substring(startPos, endPosition); - // pos = endPosition; - // console.log("one: '" + text.substring(startPos, pos) + "'"); - // console.log("two: '" + tokenValue + "'"); return firstNonWhitespace === -1 ? SyntaxKind.JsxTextAllWhiteSpaces : SyntaxKind.JsxText; } diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index 6895ddc96b9b7..12efdff108f69 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -501,6 +501,9 @@ namespace ts.formatting { else if (SmartIndenter.argumentStartsOnSameLineAsPreviousArgument(parent, node, startLine, sourceFile)) { return { indentation: parentDynamicIndentation.getIndentation(), delta }; } + else if (SmartIndenter.isJSXClosingWithMultiLineText(parent, node, startLine, sourceFile)) { + return { indentation: parentDynamicIndentation.getIndentation(), delta }; + } else { return { indentation: parentDynamicIndentation.getIndentation() + parentDynamicIndentation.getDelta(node), delta }; } @@ -736,7 +739,6 @@ namespace ts.formatting { const childIndentation = computeIndentation(child, childStartLine, childIndentationAmount, node, parentDynamicIndentation, effectiveParentStartLine); processNode(child, childContextNode, childStartLine, undecoratedChildStartLine, childIndentation.indentation, childIndentation.delta); - if (child.kind === SyntaxKind.JsxText) { const range: TextRange = { pos: child.getStart(), end: child.getEnd() }; indentMultilineCommentOrJsxText(range, childIndentation.indentation, /*firstLineIsIndented*/ true, /*indentFinalLine*/ false); diff --git a/src/services/formatting/formattingScanner.ts b/src/services/formatting/formattingScanner.ts index 98f7adc43aa12..7449cdbb92edb 100644 --- a/src/services/formatting/formattingScanner.ts +++ b/src/services/formatting/formattingScanner.ts @@ -122,7 +122,13 @@ namespace ts.formatting { } function shouldRescanJsxText(node: Node): boolean { - return node.kind === SyntaxKind.JsxText; + const isJSXText = isJsxText(node); + if (isJSXText) { + const containingElement = findAncestor(node.parent, p => isJsxElement(p)); + if (!containingElement) return false; // should never happen + return !isParenthesizedExpression(containingElement.parent); + } + return false; } function shouldRescanSlashToken(container: Node): boolean { diff --git a/src/services/formatting/smartIndenter.ts b/src/services/formatting/smartIndenter.ts index 5539a8e30cb2d..285e48f4db361 100644 --- a/src/services/formatting/smartIndenter.ts +++ b/src/services/formatting/smartIndenter.ts @@ -343,6 +343,24 @@ namespace ts.formatting { return false; } + export function isJSXClosingWithMultiLineText(parent: Node, child: TextRangeWithKind, _childStartLine: number, sourceFile: SourceFileLike): boolean { + if (isJsxElement(parent) && isParenthesizedExpression(parent.parent)) { + const siblings = parent.getChildren(sourceFile); + const currentNode = find(siblings, arg => arg.pos === child.pos); + if (!currentNode) return false; + + const currentIndex = siblings.indexOf(currentNode); + if (currentIndex === 0) return false; + + const previousNode = siblings[currentIndex - 1]; + const startLineOfPreviousNode = getLineAndCharacterOfPosition(sourceFile, previousNode.getStart()).line; + const endLineOfPreviousNode = getLineAndCharacterOfPosition(sourceFile, previousNode.getEnd()).line; + + return startLineOfPreviousNode !== endLineOfPreviousNode; + } + return false; + } + export function getContainingList(node: Node, sourceFile: SourceFile): NodeArray | undefined { return node.parent && getListByRange(node.getStart(sourceFile), node.getEnd(), node.parent, sourceFile); } diff --git a/tests/cases/fourslash/formatTsxClosingAfterJsxText.ts b/tests/cases/fourslash/formatTsxClosingAfterJsxText.ts index 95c9210c2aa24..36c5cd2688416 100644 --- a/tests/cases/fourslash/formatTsxClosingAfterJsxText.ts +++ b/tests/cases/fourslash/formatTsxClosingAfterJsxText.ts @@ -1,27 +1,31 @@ /// +// @Filename: foo.tsx //// ////const a = ( -////
-//// text -////
+////
+//// text +////
////) -//// ////const b = ( -////
-//// -////
+////
+//// text +//// twice +////
////) +//// + format.document(); verify.currentFileContentIs(` const a = ( -
- text -
+
+ text +
) - const b = ( -
- -
-);`); +
+ text + twice +
+) +`); From b5725bbd6ce758e6d145c52fcc7ccd52cc9aa3f3 Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Fri, 31 Jan 2020 16:48:34 -0800 Subject: [PATCH 4/5] Offload all jsx text indentation handling to indentMultilineCommentOrJsxText --- src/services/formatting/formatting.ts | 36 ++++++++++++++++++------ src/services/formatting/smartIndenter.ts | 18 ------------ 2 files changed, 28 insertions(+), 26 deletions(-) diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index 12efdff108f69..f5241d241d70a 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -501,9 +501,6 @@ namespace ts.formatting { else if (SmartIndenter.argumentStartsOnSameLineAsPreviousArgument(parent, node, startLine, sourceFile)) { return { indentation: parentDynamicIndentation.getIndentation(), delta }; } - else if (SmartIndenter.isJSXClosingWithMultiLineText(parent, node, startLine, sourceFile)) { - return { indentation: parentDynamicIndentation.getIndentation(), delta }; - } else { return { indentation: parentDynamicIndentation.getIndentation() + parentDynamicIndentation.getDelta(node), delta }; } @@ -652,6 +649,11 @@ namespace ts.formatting { if (tokenInfo.token.end > node.end) { break; } + if (node.kind === SyntaxKind.JsxText) { + // Intentation rules for jsx text are handled by `indentMultilineCommentOrJsxText` inside `processChildNode`; just fastforward past it here + formattingScanner.advance(); + continue; + } consumeTokenAndAdvanceScanner(tokenInfo, node, nodeDynamicIndentation, node); } @@ -741,7 +743,20 @@ namespace ts.formatting { processNode(child, childContextNode, childStartLine, undecoratedChildStartLine, childIndentation.indentation, childIndentation.delta); if (child.kind === SyntaxKind.JsxText) { const range: TextRange = { pos: child.getStart(), end: child.getEnd() }; - indentMultilineCommentOrJsxText(range, childIndentation.indentation, /*firstLineIsIndented*/ true, /*indentFinalLine*/ false); + if (range.pos !== range.end) { // don't indent zero-width jsx text + const siblings = parent.getChildren(sourceFile); + const currentNode = find(siblings, arg => arg.pos === child.pos); + const currentIndex = siblings.indexOf(currentNode!); + const previousNode = siblings[currentIndex - 1]; + if (previousNode) { + // The jsx text needs no indentation whatsoever if it ends on the same line the previous sibling ends on + if (sourceFile.getLineAndCharacterOfPosition(range.end).line !== sourceFile.getLineAndCharacterOfPosition(previousNode.end).line) { + // The first line is (already) "indented" if the text starts on the same line as the previous sibling element ends on + const firstLineIsIndented = sourceFile.getLineAndCharacterOfPosition(range.pos).line === sourceFile.getLineAndCharacterOfPosition(previousNode.end).line; + indentMultilineCommentOrJsxText(range, childIndentation.indentation, firstLineIsIndented, /*indentFinalLine*/ false, /*jsxStyle*/ true); + } + } + } } childContextNode = node; @@ -1041,7 +1056,7 @@ namespace ts.formatting { return indentationString !== sourceFile.text.substr(startLinePosition, indentationString.length); } - function indentMultilineCommentOrJsxText(commentRange: TextRange, indentation: number, firstLineIsIndented: boolean, indentFinalLine = true) { + function indentMultilineCommentOrJsxText(commentRange: TextRange, indentation: number, firstLineIsIndented: boolean, indentFinalLine = true, jsxTextStyleIndent?: boolean) { // split comment in lines let startLine = sourceFile.getLineAndCharacterOfPosition(commentRange.pos).line; const endLine = sourceFile.getLineAndCharacterOfPosition(commentRange.end).line; @@ -1072,7 +1087,7 @@ namespace ts.formatting { const nonWhitespaceColumnInFirstPart = SmartIndenter.findFirstNonWhitespaceCharacterAndColumn(startLinePos, parts[0].pos, sourceFile, options); - if (indentation === nonWhitespaceColumnInFirstPart.column) { + if (indentation === nonWhitespaceColumnInFirstPart.column && !jsxTextStyleIndent) { return; } @@ -1083,14 +1098,19 @@ namespace ts.formatting { } // shift all parts on the delta size - const delta = indentation - nonWhitespaceColumnInFirstPart.column; + let delta = indentation - nonWhitespaceColumnInFirstPart.column; for (let i = startIndex; i < parts.length; i++ , startLine++) { const startLinePos = getStartPositionOfLine(startLine, sourceFile); const nonWhitespaceCharacterAndColumn = i === 0 ? nonWhitespaceColumnInFirstPart : SmartIndenter.findFirstNonWhitespaceCharacterAndColumn(parts[i].pos, parts[i].end, sourceFile, options); - + if (jsxTextStyleIndent) { + // skip adding indentation to blank lines + if (isLineBreak(sourceFile.text.charCodeAt(getStartPositionOfLine(startLine, sourceFile)))) continue; + // reset delta on every line + delta = indentation - nonWhitespaceCharacterAndColumn.column; + } const newIndentation = nonWhitespaceCharacterAndColumn.column + delta; if (newIndentation > 0) { const indentationString = getIndentationString(newIndentation, options); diff --git a/src/services/formatting/smartIndenter.ts b/src/services/formatting/smartIndenter.ts index 285e48f4db361..5539a8e30cb2d 100644 --- a/src/services/formatting/smartIndenter.ts +++ b/src/services/formatting/smartIndenter.ts @@ -343,24 +343,6 @@ namespace ts.formatting { return false; } - export function isJSXClosingWithMultiLineText(parent: Node, child: TextRangeWithKind, _childStartLine: number, sourceFile: SourceFileLike): boolean { - if (isJsxElement(parent) && isParenthesizedExpression(parent.parent)) { - const siblings = parent.getChildren(sourceFile); - const currentNode = find(siblings, arg => arg.pos === child.pos); - if (!currentNode) return false; - - const currentIndex = siblings.indexOf(currentNode); - if (currentIndex === 0) return false; - - const previousNode = siblings[currentIndex - 1]; - const startLineOfPreviousNode = getLineAndCharacterOfPosition(sourceFile, previousNode.getStart()).line; - const endLineOfPreviousNode = getLineAndCharacterOfPosition(sourceFile, previousNode.getEnd()).line; - - return startLineOfPreviousNode !== endLineOfPreviousNode; - } - return false; - } - export function getContainingList(node: Node, sourceFile: SourceFile): NodeArray | undefined { return node.parent && getListByRange(node.getStart(sourceFile), node.getEnd(), node.parent, sourceFile); } From f07a017cb00f709dadda0de64c09c64b4cbf4b44 Mon Sep 17 00:00:00 2001 From: Orta Therox Date: Mon, 3 Feb 2020 15:51:18 -0500 Subject: [PATCH 5/5] Switch from find node -> find inde in formatting --- src/services/formatting/formatting.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index f5241d241d70a..9d95b84724c99 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -745,8 +745,7 @@ namespace ts.formatting { const range: TextRange = { pos: child.getStart(), end: child.getEnd() }; if (range.pos !== range.end) { // don't indent zero-width jsx text const siblings = parent.getChildren(sourceFile); - const currentNode = find(siblings, arg => arg.pos === child.pos); - const currentIndex = siblings.indexOf(currentNode!); + const currentIndex = findIndex(siblings, arg => arg.pos === child.pos); const previousNode = siblings[currentIndex - 1]; if (previousNode) { // The jsx text needs no indentation whatsoever if it ends on the same line the previous sibling ends on