Skip to content

Fixed issue for navbar when having multi-line string literals #32672

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Aug 23, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 32 additions & 8 deletions src/services/navigationBar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -194,7 +200,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);
Expand Down Expand Up @@ -434,13 +440,13 @@ namespace ts.NavigationBar {

function getItemName(node: Node, name: Node | undefined): string {
if (node.kind === SyntaxKind.ModuleDeclaration) {
return getModuleName(<ModuleDeclaration>node);
return cleanText(getModuleName(<ModuleDeclaration>node));
}

if (name) {
const text = nodeText(name);
if (text.length > 0) {
return text;
return cleanText(text);
}
}

Expand Down Expand Up @@ -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 "<expr> = function(){...}". If so, use the text from the left-hand side.
else if (isBinaryExpression(parent) && parent.operatorToken.kind === SyntaxKind.EqualsToken) {
Expand All @@ -658,9 +664,15 @@ namespace ts.NavigationBar {
return "<class>";
}
else if (isCallExpression(parent)) {
const name = getCalledExpressionName(parent.expression);
let name = getCalledExpressionName(parent.expression);
if (name !== undefined) {
const args = mapDefined(parent.arguments, a => isStringLiteralLike(a) ? a.getText(curSourceFile) : undefined).join(", ");
name = cleanText(name);

if (name.length > maxLength) {
return `${name} callback`;
}

const args = cleanText(mapDefined(parent.arguments, a => isStringLiteralLike(a) ? a.getText(curSourceFile) : undefined).join(", "));
return `${name}(${args}) callback`;
}
}
Expand Down Expand Up @@ -691,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|\r|\u2028|\u2029)/g, "");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
////}
////declare module "MultilineMadness" {}
////
////declare module "Multiline\
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change looks good. Just for future if you could number each Madness so result shows exactly which one is turned into which one would be great. Thank you.

That is would prefer this instead of current test

////declare module "Multiline\r\nMadness1" {
////}
////
////declare module "Multiline\
////Madness2" {
////}
////declare module "MultilineMadness3" {}
////
////declare module "Multiline\
////Madness4" {
////}
////

////Madness2" {
////}
////
////interface Foo {
//// "a1\\\r\nb";
//// "a2\
Expand All @@ -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"
},
Expand All @@ -52,7 +56,7 @@ verify.navigationTree({
"kind": "property"
},
{
"text": "'a2\\\n \\\n b'",
"text": "'a2 b'",
"kind": "method"
}
]
Expand All @@ -66,7 +70,7 @@ verify.navigationTree({
"kind": "property"
},
{
"text": "\"a2\\\n \\\n b\"",
"text": "\"a2 b\"",
"kind": "method"
}
]
Expand All @@ -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"
},
Expand All @@ -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
Expand All @@ -131,7 +135,7 @@ verify.navigationBar([
"kind": "property"
},
{
"text": "'a2\\\n \\\n b'",
"text": "'a2 b'",
"kind": "method"
}
],
Expand All @@ -146,7 +150,7 @@ verify.navigationBar([
"kind": "property"
},
{
"text": "\"a2\\\n \\\n b\"",
"text": "\"a2 b\"",
"kind": "method"
}
],
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@

//// function f(p1: () => any, p2: string) { }
//// f(() => { }, `line1\
//// line2\
//// line3`);
////
//// class c1 {
//// const a = ' ''line1\
//// line2';
//// }

verify.navigationTree({
"text": "<global>",
"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": "<global>",
"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
}
]);
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@

//// declare module 'MoreThanOneHundredAndFiftyCharacters\
//// MoreThanOneHundredAndFiftyCharacters\
//// MoreThanOneHundredAndFiftyCharacters\
//// MoreThanOneHundredAndFiftyCharacters\
//// MoreThanOneHundredAndFiftyCharacters\
//// MoreThanOneHundredAndFiftyCharacters\
//// MoreThanOneHundredAndFiftyCharacters' { }

verify.navigationTree({
"text": "<global>",
"kind": "script",
"childItems": [
{
"text": "'MoreThanOneHundredAndFiftyCharactersMoreThanOneHundredAndFiftyCharactersMoreThanOneHundredAndFiftyCharactersMoreThanOneHundredAndFiftyCharacter...",
"kind": "module",
"kindModifiers": "declare"
}
]
});

verify.navigationBar([
{
"text": "<global>",
"kind": "script",
"childItems": [
{
"text": "'MoreThanOneHundredAndFiftyCharactersMoreThanOneHundredAndFiftyCharactersMoreThanOneHundredAndFiftyCharactersMoreThanOneHundredAndFiftyCharacter...",
"kind": "module",
"kindModifiers": "declare"
}
]
},
{
"text": "'MoreThanOneHundredAndFiftyCharactersMoreThanOneHundredAndFiftyCharactersMoreThanOneHundredAndFiftyCharactersMoreThanOneHundredAndFiftyCharacter...",
"kind": "module",
"kindModifiers": "declare",
"indent": 1
}
]);