From ce133ec2dd88a7d5fdfe0d5019f6fc0704a36436 Mon Sep 17 00:00:00 2001 From: Armando Aguirre Date: Thu, 1 Aug 2019 17:21:29 -0700 Subject: [PATCH 1/5] Fixed issue for navbar when having multiline string literals --- src/services/navigationBar.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index 60f1283f7e6ea..1e60ac824f3d2 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -194,7 +194,7 @@ namespace ts.NavigationBar { // Handle named bindings in imports e.g.: // import * as NS from "mod"; // import {a, b as B} from "mod"; - const {namedBindings} = importClause; + const { namedBindings } = importClause; if (namedBindings) { if (namedBindings.kind === SyntaxKind.NamespaceImport) { addLeafNode(namedBindings); @@ -660,7 +660,16 @@ namespace ts.NavigationBar { else if (isCallExpression(parent)) { const name = getCalledExpressionName(parent.expression); if (name !== undefined) { - const args = mapDefined(parent.arguments, a => isStringLiteralLike(a) ? a.getText(curSourceFile) : undefined).join(", "); + const args = mapDefined(parent.arguments, a => { + if (isStringLiteralLike(a)) { + const line = curSourceFile.getLineAndCharacterOfPosition(a.pos).line; + const endOfLine = getEndLinePosition(line, curSourceFile); + + return a.getText(curSourceFile).substring(0, endOfLine - a.pos); + } + + return undefined; + }).join(", "); return `${name}(${args}) callback`; } } From 98729b44627c315fc14f20bba42099a6a701f442 Mon Sep 17 00:00:00 2001 From: Armando Aguirre Date: Wed, 7 Aug 2019 13:50:18 -0700 Subject: [PATCH 2/5] Truncate to 150 chars and added unit tests --- src/services/navigationBar.ts | 43 +++++++--- ...ionBarItemsMultilineStringIdentifiers1.ts} | 30 ++++--- ...tionBarItemsMultilineStringIdentifiers2.ts | 85 +++++++++++++++++++ ...tionBarItemsMultilineStringIdentifiers3.ts | 40 +++++++++ 4 files changed, 171 insertions(+), 27 deletions(-) rename tests/cases/fourslash/{navigationBarItemsMultilineStringIdentifiers.ts => navigationBarItemsMultilineStringIdentifiers1.ts} (85%) create mode 100644 tests/cases/fourslash/navigationBarItemsMultilineStringIdentifiers2.ts create mode 100644 tests/cases/fourslash/navigationBarItemsMultilineStringIdentifiers3.ts diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index 1e60ac824f3d2..6d85f76a83024 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -15,6 +15,12 @@ namespace ts.NavigationBar { */ const whiteSpaceRegex = /\s+/g; + /** + * Maximum amount of characters to return + * The amount was choosen arbitrarily. + */ + const maxLength = 150; + // Keep sourceFile handy so we don't have to search for it every time we need to call `getText`. let curCancellationToken: CancellationToken; let curSourceFile: SourceFile; @@ -74,7 +80,7 @@ namespace ts.NavigationBar { } function nodeText(node: Node): string { - return node.getText(curSourceFile); + return cleanText(node.getText(curSourceFile)); } function navigationBarNodeKind(n: NavigationBarNode): SyntaxKind { @@ -434,13 +440,13 @@ namespace ts.NavigationBar { function getItemName(node: Node, name: Node | undefined): string { if (node.kind === SyntaxKind.ModuleDeclaration) { - return getModuleName(node); + return cleanText(getModuleName(node)); } if (name) { const text = nodeText(name); if (text.length > 0) { - return text; + return cleanText(text); } } @@ -636,11 +642,11 @@ namespace ts.NavigationBar { function getFunctionOrClassName(node: FunctionExpression | FunctionDeclaration | ArrowFunction | ClassLikeDeclaration): string { const { parent } = node; if (node.name && getFullWidth(node.name) > 0) { - return declarationNameToString(node.name); + return cleanText(declarationNameToString(node.name)); } // See if it is a var initializer. If so, use the var name. else if (isVariableDeclaration(parent)) { - return declarationNameToString(parent.name); + return cleanText(declarationNameToString(parent.name)); } // See if it is of the form " = function(){...}". If so, use the text from the left-hand side. else if (isBinaryExpression(parent) && parent.operatorToken.kind === SyntaxKind.EqualsToken) { @@ -658,18 +664,15 @@ namespace ts.NavigationBar { return ""; } else if (isCallExpression(parent)) { - const name = getCalledExpressionName(parent.expression); + let name = getCalledExpressionName(parent.expression); if (name !== undefined) { - const args = mapDefined(parent.arguments, a => { - if (isStringLiteralLike(a)) { - const line = curSourceFile.getLineAndCharacterOfPosition(a.pos).line; - const endOfLine = getEndLinePosition(line, curSourceFile); + name = cleanText(name); - return a.getText(curSourceFile).substring(0, endOfLine - a.pos); - } + if (name.length > maxLength) { + return `${name} callback`; + } - return undefined; - }).join(", "); + const args = cleanText(mapDefined(parent.arguments, a => isStringLiteralLike(a) ? a.getText(curSourceFile) : undefined).join(", ")); return `${name}(${args}) callback`; } } @@ -700,4 +703,16 @@ namespace ts.NavigationBar { return false; } } + + function cleanText(text: string): string { + // Truncate to maximum amount of characters as we don't want to do a big replace operation. + text = text.length > maxLength ? text.substring(0, maxLength) + "..." : text; + + // Replaces ECMAScript line terminators and removes the trailing `\` from each line: + // \n - Line Feed + // \r - Carriage Return + // \u2028 - Line separator + // \u2029 - Paragraph separator + return text.replace(/\\?(\r?\n|\u2028|\u2029)/g, ''); + } } diff --git a/tests/cases/fourslash/navigationBarItemsMultilineStringIdentifiers.ts b/tests/cases/fourslash/navigationBarItemsMultilineStringIdentifiers1.ts similarity index 85% rename from tests/cases/fourslash/navigationBarItemsMultilineStringIdentifiers.ts rename to tests/cases/fourslash/navigationBarItemsMultilineStringIdentifiers1.ts index 793844ac970d1..e5b6c56bdcbd1 100644 --- a/tests/cases/fourslash/navigationBarItemsMultilineStringIdentifiers.ts +++ b/tests/cases/fourslash/navigationBarItemsMultilineStringIdentifiers1.ts @@ -7,6 +7,10 @@ ////} ////declare module "MultilineMadness" {} //// +////declare module "Multiline\ +////Madness2" { +////} +//// ////interface Foo { //// "a1\\\r\nb"; //// "a2\ @@ -29,17 +33,17 @@ verify.navigationTree({ "kind": "script", "childItems": [ { - "text": "\"Multiline\\\nMadness\"", + "text": "\"MultilineMadness\"", "kind": "module", "kindModifiers": "declare" }, { - "text": "\"Multiline\\r\\nMadness\"", + "text": "\"MultilineMadness2\"", "kind": "module", "kindModifiers": "declare" }, { - "text": "\"MultilineMadness\"", + "text": "\"Multiline\\r\\nMadness\"", "kind": "module", "kindModifiers": "declare" }, @@ -52,7 +56,7 @@ verify.navigationTree({ "kind": "property" }, { - "text": "'a2\\\n \\\n b'", + "text": "'a2 b'", "kind": "method" } ] @@ -66,7 +70,7 @@ verify.navigationTree({ "kind": "property" }, { - "text": "\"a2\\\n \\\n b\"", + "text": "\"a2 b\"", "kind": "method" } ] @@ -80,17 +84,17 @@ verify.navigationBar([ "kind": "script", "childItems": [ { - "text": "\"Multiline\\\nMadness\"", + "text": "\"MultilineMadness\"", "kind": "module", "kindModifiers": "declare" }, { - "text": "\"Multiline\\r\\nMadness\"", + "text": "\"MultilineMadness2\"", "kind": "module", "kindModifiers": "declare" }, { - "text": "\"MultilineMadness\"", + "text": "\"Multiline\\r\\nMadness\"", "kind": "module", "kindModifiers": "declare" }, @@ -105,19 +109,19 @@ verify.navigationBar([ ] }, { - "text": "\"Multiline\\\nMadness\"", + "text": "\"MultilineMadness\"", "kind": "module", "kindModifiers": "declare", "indent": 1 }, { - "text": "\"Multiline\\r\\nMadness\"", + "text": "\"MultilineMadness2\"", "kind": "module", "kindModifiers": "declare", "indent": 1 }, { - "text": "\"MultilineMadness\"", + "text": "\"Multiline\\r\\nMadness\"", "kind": "module", "kindModifiers": "declare", "indent": 1 @@ -131,7 +135,7 @@ verify.navigationBar([ "kind": "property" }, { - "text": "'a2\\\n \\\n b'", + "text": "'a2 b'", "kind": "method" } ], @@ -146,7 +150,7 @@ verify.navigationBar([ "kind": "property" }, { - "text": "\"a2\\\n \\\n b\"", + "text": "\"a2 b\"", "kind": "method" } ], diff --git a/tests/cases/fourslash/navigationBarItemsMultilineStringIdentifiers2.ts b/tests/cases/fourslash/navigationBarItemsMultilineStringIdentifiers2.ts new file mode 100644 index 0000000000000..8c3fe8b4c090e --- /dev/null +++ b/tests/cases/fourslash/navigationBarItemsMultilineStringIdentifiers2.ts @@ -0,0 +1,85 @@ + +//// function f(p1: () => any, p2: string) { } +//// f(() => { }, `line1\ +//// line2\ +//// line3`); +//// +//// class c1 { +//// const a = ' ''line1\ +//// line2'; +//// } + +verify.navigationTree({ + "text": "", + "kind": "script", + "childItems": [ + { + "text": "c1", + "kind": "class", + "childItems": [ + { + "text": "a", + "kind": "property" + }, + { + "text": "'line1 line2'", + "kind": "property" + } + ] + }, + { + "text": "f", + "kind": "function" + }, + { + "text": "f(`line1line2line3`) callback", + "kind": "function" + } + ] +}); + +verify.navigationBar([ + { + "text": "", + "kind": "script", + "childItems": [ + { + "text": "c1", + "kind": "class" + }, + { + "text": "f", + "kind": "function" + }, + { + "text": "f(`line1line2line3`) callback", + "kind": "function" + } + ] + }, + { + "text": "c1", + "kind": "class", + "childItems": [ + { + "text": "a", + "kind": "property" + }, + { + "text": "'line1 line2'", + "kind": "property" + } + ], + "indent": 1 + }, + { + "text": "f", + "kind": "function", + "indent": 1 + }, + { + "text": "f(`line1line2line3`) callback", + "kind": "function", + "indent": 1 + } +]); diff --git a/tests/cases/fourslash/navigationBarItemsMultilineStringIdentifiers3.ts b/tests/cases/fourslash/navigationBarItemsMultilineStringIdentifiers3.ts new file mode 100644 index 0000000000000..f04ce5284936d --- /dev/null +++ b/tests/cases/fourslash/navigationBarItemsMultilineStringIdentifiers3.ts @@ -0,0 +1,40 @@ + +//// declare module 'MoreThanOneHundredAndFiftyCharacters\ +//// MoreThanOneHundredAndFiftyCharacters\ +//// MoreThanOneHundredAndFiftyCharacters\ +//// MoreThanOneHundredAndFiftyCharacters\ +//// MoreThanOneHundredAndFiftyCharacters\ +//// MoreThanOneHundredAndFiftyCharacters\ +//// MoreThanOneHundredAndFiftyCharacters' { } + +verify.navigationTree({ + "text": "", + "kind": "script", + "childItems": [ + { + "text": "'MoreThanOneHundredAndFiftyCharactersMoreThanOneHundredAndFiftyCharactersMoreThanOneHundredAndFiftyCharactersMoreThanOneHundredAndFiftyCharacter...", + "kind": "module", + "kindModifiers": "declare" + } + ] +}); + +verify.navigationBar([ + { + "text": "", + "kind": "script", + "childItems": [ + { + "text": "'MoreThanOneHundredAndFiftyCharactersMoreThanOneHundredAndFiftyCharactersMoreThanOneHundredAndFiftyCharactersMoreThanOneHundredAndFiftyCharacter...", + "kind": "module", + "kindModifiers": "declare" + } + ] + }, + { + "text": "'MoreThanOneHundredAndFiftyCharactersMoreThanOneHundredAndFiftyCharactersMoreThanOneHundredAndFiftyCharactersMoreThanOneHundredAndFiftyCharacter...", + "kind": "module", + "kindModifiers": "declare", + "indent": 1 + } +]); From 209ca0749faccfc7f3dd04a49acf5a83c78e1267 Mon Sep 17 00:00:00 2001 From: Armando Aguirre Date: Thu, 8 Aug 2019 14:29:03 -0700 Subject: [PATCH 3/5] Fixed lint issues --- src/services/navigationBar.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index 6d85f76a83024..8b7afce9e1ccd 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -15,8 +15,8 @@ namespace ts.NavigationBar { */ const whiteSpaceRegex = /\s+/g; - /** - * Maximum amount of characters to return + /** + * Maximum amount of characters to return * The amount was choosen arbitrarily. */ const maxLength = 150; @@ -713,6 +713,6 @@ namespace ts.NavigationBar { // \r - Carriage Return // \u2028 - Line separator // \u2029 - Paragraph separator - return text.replace(/\\?(\r?\n|\u2028|\u2029)/g, ''); + return text.replace(/\\?(\r?\n|\u2028|\u2029)/g, ""); } } From 428e8d0cb16e9474fd554e77ebf60083972ac955 Mon Sep 17 00:00:00 2001 From: Armando Aguirre Date: Thu, 15 Aug 2019 13:49:11 -0700 Subject: [PATCH 4/5] Fixed navigationbar regex --- src/services/navigationBar.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index 8b7afce9e1ccd..57bf910efdafb 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -713,6 +713,6 @@ namespace ts.NavigationBar { // \r - Carriage Return // \u2028 - Line separator // \u2029 - Paragraph separator - return text.replace(/\\?(\r?\n|\u2028|\u2029)/g, ""); + return text.replace(/\\?(\r?\n|\r|\u2028|\u2029)/g, ""); } } From f76e3b59b2dc44c975df346a6fc1cb898ca9fe63 Mon Sep 17 00:00:00 2001 From: Armando Aguirre Date: Fri, 23 Aug 2019 13:51:53 -0700 Subject: [PATCH 5/5] Make trailing slash required on cleanText regex --- src/services/navigationBar.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index 57bf910efdafb..5f826a1a78f9c 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -713,6 +713,6 @@ namespace ts.NavigationBar { // \r - Carriage Return // \u2028 - Line separator // \u2029 - Paragraph separator - return text.replace(/\\?(\r?\n|\r|\u2028|\u2029)/g, ""); + return text.replace(/\\(\r?\n|\r|\u2028|\u2029)/g, ""); } }