More PR feedback

Author: JsonFreeman

src/services/services.ts

@@ -2101,24 +2101,6 @@ module ts {
             }
         }
 
-        /** Get a token that contains the position. This is guaranteed to return a token, the position can be in the 
-          * leading trivia or within the token text.
-          */
-        function getTokenAtPosition(sourceFile: SourceFile, position: number) {
-            var current: Node = sourceFile;
-            outer: while (true) {
-                // find the child that has this
-                for (var i = 0, n = current.getChildCount(); i < n; i++) {
-                    var child = current.getChildAt(i);
-                    if (child.getFullStart() <= position && position < child.getEnd()) {
-                        current = child;
-                        continue outer;
-                    }                  
-                }
-                return current;
-            }
-        }
-
         function getContainerNode(node: Node): Node {
             while (true) {
                 node = node.parent;
@@ -3486,9 +3468,8 @@ module ts {
 
             fileName = TypeScript.switchToForwardSlashes(fileName);
             var sourceFile = getSourceFile(fileName);
-            var node = getNodeAtPosition(sourceFile, position);
 
-            return SignatureHelp.getSignatureHelpItems(sourceFile, position, node, typeInfoResolver, cancellationToken);
+            return SignatureHelp.getSignatureHelpItems(sourceFile, position, typeInfoResolver, cancellationToken);
         }
 
         // This is a syntactic operation
@@ -3893,7 +3874,7 @@ module ts {
 
                     // OK, we have found a match in the file.  This is only an acceptable match if
                     // it is contained within a comment.
-                    var token = getTokenAtPosition(sourceFile, matchPosition);
+                    var token = ServicesSyntaxUtilities.getTokenAtPosition(sourceFile, matchPosition);
 
                     if (token.getStart() <= matchPosition && matchPosition < token.getEnd()) {
                         // match was within the token itself.  Not in the comment.  Keep searching

src/services/servicesSyntaxUtilities.ts

@@ -44,6 +44,41 @@ module ts.ServicesSyntaxUtilities {
         return -1;
     }
 
+    /** Get a token that contains the position. This is guaranteed to return a token, the position can be in the 
+      * leading trivia or within the token text.
+      */
+    export function getTokenAtPosition(sourceFile: SourceFile, position: number) {
+        var current: Node = sourceFile;
+        outer: while (true) {
+            // find the child that has this
+            for (var i = 0, n = current.getChildCount(); i < n; i++) {
+                var child = current.getChildAt(i);
+                if (child.getFullStart() <= position && position < child.getEnd()) {
+                    current = child;
+                    continue outer;
+                }
+            }
+            return current;
+        }
+    }
+
+    /**
+      * The token on the left of the position is the token that strictly includes the position
+      * or sits to the left of the cursor if it is on a boundary. For example
+      *
+      *   fo|o               -> will return foo
+      *   foo <comment> |bar -> will return foo
+      *
+      */
+    export function findTokenOnLeftOfPosition(file: SourceFile, position: number): Node {
+        var tokenAtPosition = getTokenAtPosition(file, position);
+        if (position > tokenAtPosition.getStart(file)) {
+            return tokenAtPosition;
+        }
+
+        return findPrecedingToken(position, file);
+    }
+
     export function findNextToken(previousToken: Node, parent: Node): Node {
         return find(parent);
 

src/services/signatureHelp.ts

@@ -333,9 +333,10 @@ module ts.SignatureHelp {
     //}
     var emptyArray: any[] = [];
 
-    export function getSignatureHelpItems(sourceFile: SourceFile, position: number, startingNode: Node, typeInfoResolver: TypeChecker, cancellationToken: CancellationTokenObject): SignatureHelpItems {
+    export function getSignatureHelpItems(sourceFile: SourceFile, position: number, typeInfoResolver: TypeChecker, cancellationToken: CancellationTokenObject): SignatureHelpItems {
         // Decide whether to show signature help
-        var argumentList = getContainingArgumentList(startingNode);
+        var startingToken = ServicesSyntaxUtilities.findTokenOnLeftOfPosition(sourceFile, position);
+        var argumentList = getContainingArgumentList(startingToken);
         cancellationToken.throwIfCancellationRequested();
 
         // Semantic filtering of signature help
@@ -361,6 +362,19 @@ module ts.SignatureHelp {
                 return undefined;
             }
 
+            // There are 3 cases to handle:
+            //   1. The token introduces a list, and should begin a sig help session
+            //   2. The token is either not associated with a list, or ends a list, so the session should end
+            //   3. The token is buried inside a list, and should give sig help
+            //
+            // The following are examples of each:
+            //
+            //    Case 1:
+            //          foo<$T, U>($a, b)    -> The token introduces a list, and should begin a sig help session
+            //    Case 2:
+            //          fo$o<T, U>$(a, b)$   -> The token is either not associated with a list, or ends a list, so the session should end
+            //    Case 3:
+            //          foo<T$, U$>(a$, $b$) -> The token is buried inside a list, and should give sig help
             var parent = <CallExpression>node.parent;
             // Find out if 'node' is an argument, a type argument, or neither
             if (node.kind === SyntaxKind.LessThanToken || node.kind === SyntaxKind.OpenParenToken) {
@@ -380,18 +394,6 @@ module ts.SignatureHelp {
         }
 
         function getContainingArgumentList(node: Node): Node {
-            // We only want this node if it is a token and it strictly contains the current position.
-            // Otherwise we want the previous token
-            var isToken = node.kind < SyntaxKind.Missing;
-            if (!isToken || position <= node.getStart() || position >= node.getEnd()) {
-                node = ServicesSyntaxUtilities.findPrecedingToken(position, sourceFile);
-
-                if (!node) {
-                    return undefined;
-                }
-            }
-
-            var signatureHelpAvailable = false;
             for (var n = node; n.kind !== SyntaxKind.SourceFile; n = n.parent) {
                 if (n.kind === SyntaxKind.FunctionBlock) {
                     return undefined;