From 1663d01844ee79b9d87ad07008e5c6d7cb2cc28b Mon Sep 17 00:00:00 2001 From: uniqueiniquity Date: Wed, 9 Aug 2017 14:38:26 -0700 Subject: [PATCH 1/6] Fix outlining of objects in array --- src/services/outliningElementsCollector.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/services/outliningElementsCollector.ts b/src/services/outliningElementsCollector.ts index 5099f8b66bdd5..0958892c1e61a 100644 --- a/src/services/outliningElementsCollector.ts +++ b/src/services/outliningElementsCollector.ts @@ -16,6 +16,18 @@ namespace ts.OutliningElementsCollector { } } + function addOutliningForObjectLiteralsInArray(startElement: Node, endElement: Node, autoCollapse: boolean) { + if (startElement && endElement) { + const span: OutliningSpan = { + textSpan: createTextSpanFromBounds(startElement.getStart(), endElement.end), + hintSpan: createTextSpanFromBounds(startElement.getStart(), endElement.end), + bannerText: collapseText, + autoCollapse + }; + elements.push(span); + } + } + function addOutliningSpanComments(commentSpan: CommentRange, autoCollapse: boolean) { if (commentSpan) { const span: OutliningSpan = { @@ -161,6 +173,10 @@ namespace ts.OutliningElementsCollector { case SyntaxKind.CaseBlock: { const openBrace = findChildOfKind(n, SyntaxKind.OpenBraceToken, sourceFile); const closeBrace = findChildOfKind(n, SyntaxKind.CloseBraceToken, sourceFile); + if (n.kind === SyntaxKind.ObjectLiteralExpression && n.parent.kind === SyntaxKind.ArrayLiteralExpression) { + addOutliningForObjectLiteralsInArray(openBrace, closeBrace, autoCollapse(n)); + break; + } addOutliningSpan(n, openBrace, closeBrace, autoCollapse(n)); break; } From df5e1a0f6971a06916be4f6046267c0b72ddedde Mon Sep 17 00:00:00 2001 From: uniqueiniquity Date: Wed, 9 Aug 2017 15:55:02 -0700 Subject: [PATCH 2/6] add fourslash test --- .../getOutliningForObjectsInArray.ts | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 tests/cases/fourslash/getOutliningForObjectsInArray.ts diff --git a/tests/cases/fourslash/getOutliningForObjectsInArray.ts b/tests/cases/fourslash/getOutliningForObjectsInArray.ts new file mode 100644 index 0000000000000..87d2bc96e2e2f --- /dev/null +++ b/tests/cases/fourslash/getOutliningForObjectsInArray.ts @@ -0,0 +1,23 @@ +/// + +////// objects in x should generate outlining spans that do not render in VS +//// const x =[| [ +//// [|{ a: 0 }|], +//// [|{ b: 1 }|], +//// [|{ c: 2 }|] +//// ]|]; +//// +////// objects in y should generate outlining spans that render as expected +//// const y =[| [ +//// [|{ +//// a: 0 +//// }|], +//// [|{ +//// b: 1 +//// }|], +//// [|{ +//// c: 2 +//// }|] +//// ]|]; + +verify.outliningSpansInCurrentFile(test.ranges()); \ No newline at end of file From c7d691dc15550b69cfec59bcb10c8e261e7dd7db Mon Sep 17 00:00:00 2001 From: uniqueiniquity Date: Thu, 10 Aug 2017 13:27:24 -0700 Subject: [PATCH 3/6] Generalize to nested arrays and refactor --- src/services/outliningElementsCollector.ts | 40 +++++++------------ .../getOutliningForObjectsInArray.ts | 19 +++++++++ 2 files changed, 33 insertions(+), 26 deletions(-) diff --git a/src/services/outliningElementsCollector.ts b/src/services/outliningElementsCollector.ts index 0958892c1e61a..edebf98a24bc4 100644 --- a/src/services/outliningElementsCollector.ts +++ b/src/services/outliningElementsCollector.ts @@ -4,25 +4,13 @@ namespace ts.OutliningElementsCollector { const elements: OutliningSpan[] = []; const collapseText = "..."; - function addOutliningSpan(hintSpanNode: Node, startElement: Node, endElement: Node, autoCollapse: boolean) { + function addOutliningSpan(hintSpanNode: Node, startElement: Node, endElement: Node, autoCollapse: boolean, fullStart: boolean) { if (hintSpanNode && startElement && endElement) { const span: OutliningSpan = { - textSpan: createTextSpanFromBounds(startElement.pos, endElement.end), - hintSpan: createTextSpanFromNode(hintSpanNode, sourceFile), - bannerText: collapseText, - autoCollapse, - }; - elements.push(span); - } - } - - function addOutliningForObjectLiteralsInArray(startElement: Node, endElement: Node, autoCollapse: boolean) { - if (startElement && endElement) { - const span: OutliningSpan = { - textSpan: createTextSpanFromBounds(startElement.getStart(), endElement.end), + textSpan: createTextSpanFromBounds(fullStart ? startElement.pos : startElement.getStart(), endElement.end), hintSpan: createTextSpanFromBounds(startElement.getStart(), endElement.end), bannerText: collapseText, - autoCollapse + autoCollapse, }; elements.push(span); } @@ -125,7 +113,7 @@ namespace ts.OutliningElementsCollector { parent.kind === SyntaxKind.WithStatement || parent.kind === SyntaxKind.CatchClause) { - addOutliningSpan(parent, openBrace, closeBrace, autoCollapse(n)); + addOutliningSpan(parent, openBrace, closeBrace, autoCollapse(n), /* fullStart */ true); break; } @@ -133,13 +121,13 @@ namespace ts.OutliningElementsCollector { // Could be the try-block, or the finally-block. const tryStatement = parent; if (tryStatement.tryBlock === n) { - addOutliningSpan(parent, openBrace, closeBrace, autoCollapse(n)); + addOutliningSpan(parent, openBrace, closeBrace, autoCollapse(n), /* fullStart */ true); break; } else if (tryStatement.finallyBlock === n) { const finallyKeyword = findChildOfKind(tryStatement, SyntaxKind.FinallyKeyword, sourceFile); if (finallyKeyword) { - addOutliningSpan(finallyKeyword, openBrace, closeBrace, autoCollapse(n)); + addOutliningSpan(finallyKeyword, openBrace, closeBrace, autoCollapse(n), /* fullStart */ true); break; } } @@ -163,27 +151,27 @@ namespace ts.OutliningElementsCollector { case SyntaxKind.ModuleBlock: { const openBrace = findChildOfKind(n, SyntaxKind.OpenBraceToken, sourceFile); const closeBrace = findChildOfKind(n, SyntaxKind.CloseBraceToken, sourceFile); - addOutliningSpan(n.parent, openBrace, closeBrace, autoCollapse(n)); + addOutliningSpan(n.parent, openBrace, closeBrace, autoCollapse(n), /* fullStart */ true); break; } case SyntaxKind.ClassDeclaration: case SyntaxKind.InterfaceDeclaration: case SyntaxKind.EnumDeclaration: - case SyntaxKind.ObjectLiteralExpression: case SyntaxKind.CaseBlock: { const openBrace = findChildOfKind(n, SyntaxKind.OpenBraceToken, sourceFile); const closeBrace = findChildOfKind(n, SyntaxKind.CloseBraceToken, sourceFile); - if (n.kind === SyntaxKind.ObjectLiteralExpression && n.parent.kind === SyntaxKind.ArrayLiteralExpression) { - addOutliningForObjectLiteralsInArray(openBrace, closeBrace, autoCollapse(n)); - break; - } - addOutliningSpan(n, openBrace, closeBrace, autoCollapse(n)); + addOutliningSpan(n, openBrace, closeBrace, autoCollapse(n), /* fullStart */ true); break; } + case SyntaxKind.ObjectLiteralExpression: + const openBrace = findChildOfKind(n, SyntaxKind.OpenBraceToken, sourceFile); + const closeBrace = findChildOfKind(n, SyntaxKind.CloseBraceToken, sourceFile); + addOutliningSpan(n, openBrace, closeBrace, autoCollapse(n), /* fullStart */ n.parent.kind !== SyntaxKind.ArrayLiteralExpression); + break; case SyntaxKind.ArrayLiteralExpression: const openBracket = findChildOfKind(n, SyntaxKind.OpenBracketToken, sourceFile); const closeBracket = findChildOfKind(n, SyntaxKind.CloseBracketToken, sourceFile); - addOutliningSpan(n, openBracket, closeBracket, autoCollapse(n)); + addOutliningSpan(n, openBracket, closeBracket, autoCollapse(n), /* fullStart */ n.parent.kind !== SyntaxKind.ArrayLiteralExpression); break; } depth++; diff --git a/tests/cases/fourslash/getOutliningForObjectsInArray.ts b/tests/cases/fourslash/getOutliningForObjectsInArray.ts index 87d2bc96e2e2f..207da1997b4c9 100644 --- a/tests/cases/fourslash/getOutliningForObjectsInArray.ts +++ b/tests/cases/fourslash/getOutliningForObjectsInArray.ts @@ -19,5 +19,24 @@ //// c: 2 //// }|] //// ]|]; +//// +////// same behavior for nested arrays +//// const w =[| [ +//// [|[ a: 0 ]|], +//// [|[ b: 1 ]|], +//// [|[ c: 2 ]|] +//// ]|]; +//// +//// const z =[| [ +//// [|[ +//// a: 0 +//// ]|], +//// [|[ +//// b: 1 +//// ]|], +//// [|[ +//// c: 2 +//// ]|] +//// ]|]; verify.outliningSpansInCurrentFile(test.ranges()); \ No newline at end of file From d6ccee67661987373949af9c97bf3c636a22d496 Mon Sep 17 00:00:00 2001 From: uniqueiniquity Date: Fri, 11 Aug 2017 13:42:14 -0700 Subject: [PATCH 4/6] Cleans up and adds nested case to test --- .../getOutliningForObjectsInArray.ts | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/tests/cases/fourslash/getOutliningForObjectsInArray.ts b/tests/cases/fourslash/getOutliningForObjectsInArray.ts index 207da1997b4c9..2d57d3335bdf1 100644 --- a/tests/cases/fourslash/getOutliningForObjectsInArray.ts +++ b/tests/cases/fourslash/getOutliningForObjectsInArray.ts @@ -22,20 +22,34 @@ //// ////// same behavior for nested arrays //// const w =[| [ -//// [|[ a: 0 ]|], -//// [|[ b: 1 ]|], -//// [|[ c: 2 ]|] +//// [|[ 0 ]|], +//// [|[ 1 ]|], +//// [|[ 2 ]|] //// ]|]; //// //// const z =[| [ //// [|[ -//// a: 0 +//// 0 //// ]|], //// [|[ -//// b: 1 +//// 1 //// ]|], //// [|[ -//// c: 2 +//// 2 +//// ]|] +//// ]|]; +//// +////// multiple levels of nesting work as expected +//// const z =[| [ +//// [|[ +//// [|{ hello: 0 }|] +//// ]|], +//// [|[ +//// [|{ hello: 3 }|] +//// ]|], +//// [|[ +//// [|{ hello: 5 }|], +//// [|{ hello: 7 }|] //// ]|] //// ]|]; From 760812f7143ccc311746e0c3eac191d2905722c9 Mon Sep 17 00:00:00 2001 From: uniqueiniquity Date: Mon, 14 Aug 2017 09:27:45 -0700 Subject: [PATCH 5/6] Add explanatory comments, consolidate main body --- src/services/outliningElementsCollector.ts | 18 +++++++++++------- .../fourslash/getOutliningForObjectsInArray.ts | 8 ++++---- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/services/outliningElementsCollector.ts b/src/services/outliningElementsCollector.ts index edebf98a24bc4..cdeba21b32bc4 100644 --- a/src/services/outliningElementsCollector.ts +++ b/src/services/outliningElementsCollector.ts @@ -3,11 +3,17 @@ namespace ts.OutliningElementsCollector { export function collectElements(sourceFile: SourceFile, cancellationToken: CancellationToken): OutliningSpan[] { const elements: OutliningSpan[] = []; const collapseText = "..."; + let depth = 0; + const maxDepth = 20; + + walk(sourceFile); + return elements; - function addOutliningSpan(hintSpanNode: Node, startElement: Node, endElement: Node, autoCollapse: boolean, fullStart: boolean) { + // If useFullStart is true, then the collapsing span includes leading whitespace, including linebreaks. + function addOutliningSpan(hintSpanNode: Node, startElement: Node, endElement: Node, autoCollapse: boolean, useFullStart: boolean) { if (hintSpanNode && startElement && endElement) { const span: OutliningSpan = { - textSpan: createTextSpanFromBounds(fullStart ? startElement.pos : startElement.getStart(), endElement.end), + textSpan: createTextSpanFromBounds(useFullStart ? startElement.pos : startElement.getStart(), endElement.end), hintSpan: createTextSpanFromBounds(startElement.getStart(), endElement.end), bannerText: collapseText, autoCollapse, @@ -82,8 +88,6 @@ namespace ts.OutliningElementsCollector { return isFunctionBlock(node) && node.parent.kind !== SyntaxKind.ArrowFunction; } - let depth = 0; - const maxDepth = 20; function walk(n: Node): void { cancellationToken.throwIfCancellationRequested(); if (depth > maxDepth) { @@ -163,6 +167,9 @@ namespace ts.OutliningElementsCollector { addOutliningSpan(n, openBrace, closeBrace, autoCollapse(n), /* fullStart */ true); break; } + // If the block has no leading keywords and is a member of an array literal, + // we again want to only collapse the span of the block. + // Otherwise, the collapsed section will include the end of the previous line. case SyntaxKind.ObjectLiteralExpression: const openBrace = findChildOfKind(n, SyntaxKind.OpenBraceToken, sourceFile); const closeBrace = findChildOfKind(n, SyntaxKind.CloseBraceToken, sourceFile); @@ -178,8 +185,5 @@ namespace ts.OutliningElementsCollector { forEachChild(n, walk); depth--; } - - walk(sourceFile); - return elements; } } \ No newline at end of file diff --git a/tests/cases/fourslash/getOutliningForObjectsInArray.ts b/tests/cases/fourslash/getOutliningForObjectsInArray.ts index 2d57d3335bdf1..896342248328a 100644 --- a/tests/cases/fourslash/getOutliningForObjectsInArray.ts +++ b/tests/cases/fourslash/getOutliningForObjectsInArray.ts @@ -1,13 +1,13 @@ /// -////// objects in x should generate outlining spans that do not render in VS +// objects in x should generate outlining spans that do not render in VS //// const x =[| [ //// [|{ a: 0 }|], //// [|{ b: 1 }|], //// [|{ c: 2 }|] //// ]|]; //// -////// objects in y should generate outlining spans that render as expected +// objects in y should generate outlining spans that render as expected //// const y =[| [ //// [|{ //// a: 0 @@ -20,7 +20,7 @@ //// }|] //// ]|]; //// -////// same behavior for nested arrays +// same behavior for nested arrays //// const w =[| [ //// [|[ 0 ]|], //// [|[ 1 ]|], @@ -39,7 +39,7 @@ //// ]|] //// ]|]; //// -////// multiple levels of nesting work as expected +// multiple levels of nesting work as expected //// const z =[| [ //// [|[ //// [|{ hello: 0 }|] From e6c1afb4a0b8f04f6202a7dd1f39d7cdffeaf096 Mon Sep 17 00:00:00 2001 From: uniqueiniquity Date: Fri, 18 Aug 2017 15:59:22 -0700 Subject: [PATCH 6/6] Style changes and cleanup --- src/services/outliningElementsCollector.ts | 29 +++++++++++----------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/src/services/outliningElementsCollector.ts b/src/services/outliningElementsCollector.ts index cdeba21b32bc4..e0d99d1d27165 100644 --- a/src/services/outliningElementsCollector.ts +++ b/src/services/outliningElementsCollector.ts @@ -1,20 +1,21 @@ /* @internal */ namespace ts.OutliningElementsCollector { + const collapseText = "..."; + const maxDepth = 20; + export function collectElements(sourceFile: SourceFile, cancellationToken: CancellationToken): OutliningSpan[] { const elements: OutliningSpan[] = []; - const collapseText = "..."; let depth = 0; - const maxDepth = 20; walk(sourceFile); return elements; - // If useFullStart is true, then the collapsing span includes leading whitespace, including linebreaks. + /** If useFullStart is true, then the collapsing span includes leading whitespace, including linebreaks. */ function addOutliningSpan(hintSpanNode: Node, startElement: Node, endElement: Node, autoCollapse: boolean, useFullStart: boolean) { if (hintSpanNode && startElement && endElement) { const span: OutliningSpan = { - textSpan: createTextSpanFromBounds(useFullStart ? startElement.pos : startElement.getStart(), endElement.end), - hintSpan: createTextSpanFromBounds(startElement.getStart(), endElement.end), + textSpan: createTextSpanFromBounds(useFullStart ? startElement.getFullStart() : startElement.getStart(), endElement.getEnd()), + hintSpan: createTextSpanFromNode(hintSpanNode, sourceFile), bannerText: collapseText, autoCollapse, }; @@ -117,7 +118,7 @@ namespace ts.OutliningElementsCollector { parent.kind === SyntaxKind.WithStatement || parent.kind === SyntaxKind.CatchClause) { - addOutliningSpan(parent, openBrace, closeBrace, autoCollapse(n), /* fullStart */ true); + addOutliningSpan(parent, openBrace, closeBrace, autoCollapse(n), /*useFullStart*/ true); break; } @@ -125,13 +126,13 @@ namespace ts.OutliningElementsCollector { // Could be the try-block, or the finally-block. const tryStatement = parent; if (tryStatement.tryBlock === n) { - addOutliningSpan(parent, openBrace, closeBrace, autoCollapse(n), /* fullStart */ true); + addOutliningSpan(parent, openBrace, closeBrace, autoCollapse(n), /*useFullStart*/ true); break; } else if (tryStatement.finallyBlock === n) { const finallyKeyword = findChildOfKind(tryStatement, SyntaxKind.FinallyKeyword, sourceFile); if (finallyKeyword) { - addOutliningSpan(finallyKeyword, openBrace, closeBrace, autoCollapse(n), /* fullStart */ true); + addOutliningSpan(finallyKeyword, openBrace, closeBrace, autoCollapse(n), /*useFullStart*/ true); break; } } @@ -155,7 +156,7 @@ namespace ts.OutliningElementsCollector { case SyntaxKind.ModuleBlock: { const openBrace = findChildOfKind(n, SyntaxKind.OpenBraceToken, sourceFile); const closeBrace = findChildOfKind(n, SyntaxKind.CloseBraceToken, sourceFile); - addOutliningSpan(n.parent, openBrace, closeBrace, autoCollapse(n), /* fullStart */ true); + addOutliningSpan(n.parent, openBrace, closeBrace, autoCollapse(n), /*useFullStart*/ true); break; } case SyntaxKind.ClassDeclaration: @@ -164,21 +165,21 @@ namespace ts.OutliningElementsCollector { case SyntaxKind.CaseBlock: { const openBrace = findChildOfKind(n, SyntaxKind.OpenBraceToken, sourceFile); const closeBrace = findChildOfKind(n, SyntaxKind.CloseBraceToken, sourceFile); - addOutliningSpan(n, openBrace, closeBrace, autoCollapse(n), /* fullStart */ true); + addOutliningSpan(n, openBrace, closeBrace, autoCollapse(n), /*useFullStart*/ true); break; } - // If the block has no leading keywords and is a member of an array literal, - // we again want to only collapse the span of the block. + // If the block has no leading keywords and is inside an array literal, + // we only want to collapse the span of the block. // Otherwise, the collapsed section will include the end of the previous line. case SyntaxKind.ObjectLiteralExpression: const openBrace = findChildOfKind(n, SyntaxKind.OpenBraceToken, sourceFile); const closeBrace = findChildOfKind(n, SyntaxKind.CloseBraceToken, sourceFile); - addOutliningSpan(n, openBrace, closeBrace, autoCollapse(n), /* fullStart */ n.parent.kind !== SyntaxKind.ArrayLiteralExpression); + addOutliningSpan(n, openBrace, closeBrace, autoCollapse(n), /*useFullStart*/ !isArrayLiteralExpression(n.parent)); break; case SyntaxKind.ArrayLiteralExpression: const openBracket = findChildOfKind(n, SyntaxKind.OpenBracketToken, sourceFile); const closeBracket = findChildOfKind(n, SyntaxKind.CloseBracketToken, sourceFile); - addOutliningSpan(n, openBracket, closeBracket, autoCollapse(n), /* fullStart */ n.parent.kind !== SyntaxKind.ArrayLiteralExpression); + addOutliningSpan(n, openBracket, closeBracket, autoCollapse(n), /*useFullStart*/ !isArrayLiteralExpression(n.parent)); break; } depth++;