-
Notifications
You must be signed in to change notification settings - Fork 13k
Wire getDefinitionAtPosition using the new compiler implementation #326
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
Conversation
case SyntaxKind.ArrowFunction: | ||
case SyntaxKind.ModuleDeclaration: | ||
case SyntaxKind.ClassDeclaration: | ||
// Label targets can not be accorss function boundries, so do not walk any further |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"across" and "boundaries"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should still cross function boundaries. The compiler already binds them in this manner (and then reports you have an error if it crossed a boundary). This way, one can still do goto-def (esp. after getting that error).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point. will change it.
var type = symbol && typeChecker.getTypeOfSymbol(symbol); | ||
if (type) { | ||
return { | ||
memberName: new TypeScript.MemberNameString(typeChecker.typeToString(type)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't you need the type name in the containeNode context eg. qualifying typeof expression?
* documentsByName to sourceFilesByName * getSymbolOfIdentifierLikeNode to getSymbolInfo
|
||
var declarations = symbol.getDeclarations(); | ||
var symbolName = typeChecker.symbolToString(symbol, node); | ||
var symbolKind = getSymbolKind(symbol); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is symbolKind based on symbol and not declaration? eg. if it is fundule you want the 2 definitions with the correct function and module declaration kind?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cause I was just lazy.. will fix that a different change.
@sheetalkamat, @vladima and @DanielRosenwasser any more comments? |
return true; | ||
} | ||
|
||
return false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon
👍 |
Wire getDefinitionAtPosition using the new compiler implementation
There are some changes in behavior between old and new implementation, namely: