From 7222963923e2610a9c59b7e899858cb349454d93 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sun, 5 Oct 2014 21:17:13 -0700 Subject: [PATCH 1/3] Tweak how we populate NavBarItems. Now we will always place child items of a node in the right bar, even if we're also placing it on the 'top level nodes' list on the left. This makes things clearer for users by making it clear that any time you have a node selected on the left, you can always find its children on the right. And, it still preserves the behavior we want where the left list acts as a high level 'table of contents' for the file. --- src/compiler/parser.ts | 2 +- src/services/navigationBar.ts | 83 ++++++++++++------- .../scriptLexicalStructureFunctions.ts | 2 +- .../scriptLexicalStructureFunctionsBroken.ts | 2 +- .../scriptLexicalStructureFunctionsBroken2.ts | 2 +- 5 files changed, 57 insertions(+), 34 deletions(-) diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index 422d133d5d0..a74ceb4e024 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -3639,7 +3639,7 @@ module ts { } return finishNode(node); } - + function parseAndCheckEnumDeclaration(pos: number, flags: NodeFlags): EnumDeclaration { function isIntegerLiteral(expression: Expression): boolean { function isInteger(literalExpression: LiteralExpression): boolean { diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index 5cf2dd90eea..144c0e5786b 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -22,7 +22,8 @@ module ts.NavigationBar { // to say it only has a single layer of depth do { current = current.parent; - } while (current.kind === SyntaxKind.ModuleDeclaration); + } + while (current.kind === SyntaxKind.ModuleDeclaration); // fall through case SyntaxKind.ClassDeclaration: @@ -37,14 +38,19 @@ module ts.NavigationBar { return indent; } - + function getChildNodes(nodes: Node[]): Node[] { var childNodes: Node[] = []; for (var i = 0, n = nodes.length; i < n; i++) { var node = nodes[i]; - if (node.kind === SyntaxKind.FunctionDeclaration) { + if (node.kind === SyntaxKind.ClassDeclaration || + node.kind === SyntaxKind.EnumDeclaration || + node.kind === SyntaxKind.InterfaceDeclaration || + node.kind === SyntaxKind.ModuleDeclaration || + node.kind === SyntaxKind.FunctionDeclaration) { + childNodes.push(node); } else if (node.kind === SyntaxKind.VariableStatement) { @@ -52,7 +58,7 @@ module ts.NavigationBar { } } - return childNodes; + return sortNodes(childNodes); } function getTopLevelNodes(node: SourceFile): Node[] { @@ -63,8 +69,27 @@ module ts.NavigationBar { return topLevelNodes; } - + + function sortNodes(nodes: Node[]): Node[] { + return nodes.slice(0).sort((n1: Declaration, n2: Declaration) => { + if (n1.name && n2.name) { + return n1.name.text.localeCompare(n2.name.text); + } + else if (n1.name) { + return 1; + } + else if (n2.name) { + -1; + } + else { + return n1.kind - n2.kind; + } + }); + } + function addTopLevelNodes(nodes: Node[], topLevelNodes: Node[]): void { + nodes = sortNodes(nodes); + for (var i = 0, n = nodes.length; i < n; i++) { var node = nodes[i]; switch (node.kind) { @@ -97,9 +122,10 @@ module ts.NavigationBar { return functionDeclaration.kind === SyntaxKind.FunctionDeclaration && functionDeclaration.body && functionDeclaration.body.kind === SyntaxKind.FunctionBlock && - forEach((functionDeclaration.body).statements, s => s.kind === SyntaxKind.FunctionDeclaration); + forEach((functionDeclaration.body).statements, + s => s.kind === SyntaxKind.FunctionDeclaration && !isEmpty((s).name.text)); } - + function getItemsWorker(nodes: Node[], createItem: (n: Node) => ts.NavigationBarItem): ts.NavigationBarItem[] { var items: ts.NavigationBarItem[] = []; @@ -161,31 +187,26 @@ module ts.NavigationBar { function createChildItem(node: Node): ts.NavigationBarItem { switch (node.kind) { case SyntaxKind.Parameter: - var parameter = node; if ((node.flags & NodeFlags.Modifier) === 0) { return undefined; } - return createItem(node, getTextOfNode(parameter.name), ts.ScriptElementKind.memberVariableElement); + return createItem(node, getTextOfNode((node).name), ts.ScriptElementKind.memberVariableElement); case SyntaxKind.Method: - var method = node; - return createItem(node, getTextOfNode(method.name), ts.ScriptElementKind.memberFunctionElement); + return createItem(node, getTextOfNode((node).name), ts.ScriptElementKind.memberFunctionElement); case SyntaxKind.GetAccessor: - var getAccessor = node; - return createItem(node, getTextOfNode(getAccessor.name), ts.ScriptElementKind.memberGetAccessorElement); + return createItem(node, getTextOfNode((node).name), ts.ScriptElementKind.memberGetAccessorElement); case SyntaxKind.SetAccessor: - var setAccessor = node; - return createItem(node, getTextOfNode(setAccessor.name), ts.ScriptElementKind.memberSetAccessorElement); + return createItem(node, getTextOfNode((node).name), ts.ScriptElementKind.memberSetAccessorElement); case SyntaxKind.IndexSignature: return createItem(node, "[]", ts.ScriptElementKind.indexSignatureElement); case SyntaxKind.EnumMember: - var enumMember = node; - return createItem(node, getTextOfNode(enumMember.name), ts.ScriptElementKind.memberVariableElement); + return createItem(node, getTextOfNode((node).name), ts.ScriptElementKind.memberVariableElement); case SyntaxKind.CallSignature: return createItem(node, "()", ts.ScriptElementKind.callSignatureElement); @@ -194,19 +215,13 @@ module ts.NavigationBar { return createItem(node, "new()", ts.ScriptElementKind.constructSignatureElement); case SyntaxKind.Property: - var property = node; - return createItem(node, getTextOfNode(property.name), ts.ScriptElementKind.memberVariableElement); + return createItem(node, getTextOfNode((node).name), ts.ScriptElementKind.memberVariableElement); case SyntaxKind.FunctionDeclaration: - var functionDeclaration = node; - if (!isTopLevelFunctionDeclaration(functionDeclaration)) { - return createItem(node, getTextOfNode(functionDeclaration.name), ts.ScriptElementKind.functionElement); - } - break; + return createItem(node, getTextOfNode((node).name), ts.ScriptElementKind.functionElement); case SyntaxKind.VariableDeclaration: - var variableDeclaration = node; - return createItem(node, getTextOfNode(variableDeclaration.name), ts.ScriptElementKind.variableElement); + return createItem(node, getTextOfNode((node).name), ts.ScriptElementKind.variableElement); case SyntaxKind.Constructor: return createItem(node, "constructor", ts.ScriptElementKind.constructorImplementationElement); @@ -219,7 +234,15 @@ module ts.NavigationBar { } } + function isEmpty(text: string) { + return !text || text.trim() === ""; + } + function getNavigationBarItem(text: string, kind: string, kindModifiers: string, spans: TypeScript.TextSpan[], childItems: ts.NavigationBarItem[] = [], indent: number = 0): ts.NavigationBarItem { + if (isEmpty(text)) { + return undefined; + } + return { text: text, kind: kind, @@ -290,7 +313,7 @@ module ts.NavigationBar { function createFunctionItem(node: FunctionDeclaration) { if (node.name && node.body && node.body.kind === SyntaxKind.FunctionBlock) { - var childItems = getItemsWorker((node.body).statements, createChildItem); + var childItems = getItemsWorker(getChildNodes((node.body).statements), createChildItem); return getNavigationBarItem(node.name.text, ts.ScriptElementKind.functionElement, @@ -335,7 +358,7 @@ module ts.NavigationBar { ? constructor.parameters.concat(node.members) : node.members; - var childItems = getItemsWorker(nodes, createChildItem); + var childItems = getItemsWorker(sortNodes(nodes), createChildItem); } return getNavigationBarItem( @@ -348,7 +371,7 @@ module ts.NavigationBar { } function createEnumItem(node: EnumDeclaration): ts.NavigationBarItem { - var childItems = getItemsWorker(node.members, createChildItem); + var childItems = getItemsWorker(sortNodes(node.members), createChildItem); return getNavigationBarItem( node.name.text, ts.ScriptElementKind.enumElement, @@ -359,7 +382,7 @@ module ts.NavigationBar { } function createIterfaceItem(node: InterfaceDeclaration): ts.NavigationBarItem { - var childItems = getItemsWorker(node.members, createChildItem); + var childItems = getItemsWorker(sortNodes(node.members), createChildItem); return getNavigationBarItem( node.name.text, ts.ScriptElementKind.interfaceElement, diff --git a/tests/cases/fourslash/scriptLexicalStructureFunctions.ts b/tests/cases/fourslash/scriptLexicalStructureFunctions.ts index a42a226c2c0..dd1f1571875 100644 --- a/tests/cases/fourslash/scriptLexicalStructureFunctions.ts +++ b/tests/cases/fourslash/scriptLexicalStructureFunctions.ts @@ -20,4 +20,4 @@ test.markers().forEach((marker) => { verify.getScriptLexicalStructureListContains(marker.data.itemName, marker.data.kind, marker.fileName, marker.data.parentName); }); -verify.getScriptLexicalStructureListCount(5); // 4 functions + global +verify.getScriptLexicalStructureListCount(9); // 4 functions + global. Note: there are 9 because of the functions show up at the top level and as child items. diff --git a/tests/cases/fourslash/scriptLexicalStructureFunctionsBroken.ts b/tests/cases/fourslash/scriptLexicalStructureFunctionsBroken.ts index 687b9b87296..7b28acf22a5 100644 --- a/tests/cases/fourslash/scriptLexicalStructureFunctionsBroken.ts +++ b/tests/cases/fourslash/scriptLexicalStructureFunctionsBroken.ts @@ -9,4 +9,4 @@ test.markers().forEach((marker) => { verify.getScriptLexicalStructureListContains(marker.data.itemName, marker.data.kind, marker.fileName, marker.data.parentName); }); -verify.getScriptLexicalStructureListCount(1); // 1 function - no global since the inner function thinks it has a declaration. +verify.getScriptLexicalStructureListCount(3); // , 'f' in the top level list, and 'f' as a child of '' diff --git a/tests/cases/fourslash/scriptLexicalStructureFunctionsBroken2.ts b/tests/cases/fourslash/scriptLexicalStructureFunctionsBroken2.ts index 1ac7ed09ad8..155de5dfc9e 100644 --- a/tests/cases/fourslash/scriptLexicalStructureFunctionsBroken2.ts +++ b/tests/cases/fourslash/scriptLexicalStructureFunctionsBroken2.ts @@ -10,4 +10,4 @@ test.markers().forEach((marker) => { verify.getScriptLexicalStructureListContains(marker.data.itemName, marker.data.kind, marker.fileName, marker.data.parentName); }); -verify.getScriptLexicalStructureListCount(1); // 1 function with no global - the broken declaration adds nothing for us at the global scope. +verify.getScriptLexicalStructureListCount(3); // and 'f' as a child of 'global' and as as top level function. \ No newline at end of file From 3a6c328f00dca3b957e0565907e0bebb97b98620 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sun, 5 Oct 2014 21:58:37 -0700 Subject: [PATCH 2/3] Update tests. --- tests/cases/fourslash/scriptLexicalStructureFunctionsBroken.ts | 2 +- tests/cases/fourslash/scriptLexicalStructureFunctionsBroken2.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/cases/fourslash/scriptLexicalStructureFunctionsBroken.ts b/tests/cases/fourslash/scriptLexicalStructureFunctionsBroken.ts index 7b28acf22a5..f8d5f0bf27a 100644 --- a/tests/cases/fourslash/scriptLexicalStructureFunctionsBroken.ts +++ b/tests/cases/fourslash/scriptLexicalStructureFunctionsBroken.ts @@ -9,4 +9,4 @@ test.markers().forEach((marker) => { verify.getScriptLexicalStructureListContains(marker.data.itemName, marker.data.kind, marker.fileName, marker.data.parentName); }); -verify.getScriptLexicalStructureListCount(3); // , 'f' in the top level list, and 'f' as a child of '' +verify.getScriptLexicalStructureListCount(2); // and 'f'. \ No newline at end of file diff --git a/tests/cases/fourslash/scriptLexicalStructureFunctionsBroken2.ts b/tests/cases/fourslash/scriptLexicalStructureFunctionsBroken2.ts index 155de5dfc9e..cab6a055a5b 100644 --- a/tests/cases/fourslash/scriptLexicalStructureFunctionsBroken2.ts +++ b/tests/cases/fourslash/scriptLexicalStructureFunctionsBroken2.ts @@ -10,4 +10,4 @@ test.markers().forEach((marker) => { verify.getScriptLexicalStructureListContains(marker.data.itemName, marker.data.kind, marker.fileName, marker.data.parentName); }); -verify.getScriptLexicalStructureListCount(3); // and 'f' as a child of 'global' and as as top level function. \ No newline at end of file +verify.getScriptLexicalStructureListCount(2); // and 'f' \ No newline at end of file From 1a48da9c94caef0eb9e908b58964af8407d011d5 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sun, 5 Oct 2014 23:47:34 -0700 Subject: [PATCH 3/3] Don't show locals as children of functions. --- src/services/navigationBar.ts | 2 +- tests/cases/fourslash/scriptLexicalStructureFunctions.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index 144c0e5786b..53eed356ceb 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -313,7 +313,7 @@ module ts.NavigationBar { function createFunctionItem(node: FunctionDeclaration) { if (node.name && node.body && node.body.kind === SyntaxKind.FunctionBlock) { - var childItems = getItemsWorker(getChildNodes((node.body).statements), createChildItem); + var childItems = getItemsWorker(sortNodes((node.body).statements), createChildItem); return getNavigationBarItem(node.name.text, ts.ScriptElementKind.functionElement, diff --git a/tests/cases/fourslash/scriptLexicalStructureFunctions.ts b/tests/cases/fourslash/scriptLexicalStructureFunctions.ts index dd1f1571875..298cfc657f0 100644 --- a/tests/cases/fourslash/scriptLexicalStructureFunctions.ts +++ b/tests/cases/fourslash/scriptLexicalStructureFunctions.ts @@ -20,4 +20,4 @@ test.markers().forEach((marker) => { verify.getScriptLexicalStructureListContains(marker.data.itemName, marker.data.kind, marker.fileName, marker.data.parentName); }); -verify.getScriptLexicalStructureListCount(9); // 4 functions + global. Note: there are 9 because of the functions show up at the top level and as child items. +verify.getScriptLexicalStructureListCount(7); // 4 functions + global. Note: there are 7 because of the functions show up at the top level and as child items.