Skip to content

Ignore trailing comma when resolving signature for quick info #25841

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
2 commits merged into from
Jul 31, 2018

Conversation

ghost
Copy link

@ghost ghost commented Jul 20, 2018

Fixes #25807

@ghost ghost requested a review from sheetalkamat July 28, 2018 00:04
Copy link
Member

@sheetalkamat sheetalkamat left a comment

Choose a reason for hiding this comment

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

Can you please add test case for hovering over as in the issue.

@ghost
Copy link
Author

ghost commented Jul 30, 2018

quickInfoSignatureWithTrailingComma.ts should do that -- verify.quickInfoAt("" goes to the /**/ marker in the file and gets quick info.

@sheetalkamat
Copy link
Member

sorry for some reason i thought you had test case for signature help and not quickInfo, Can you please add test case for signature help?

@ghost ghost force-pushed the quickInfoSignatureHelpTrailingComma branch from 5446836 to 1a62233 Compare July 30, 2018 23:31
tags: [{ name: "param", text: "radix The radix" }],
});
verify.signatureHelp(
{
Copy link
Author

Choose a reason for hiding this comment

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

Previously this test was wrong -- the edit.insert call applied to the start of the file, changing the first line to str(2,|function str(n: number): string; and eliminating that overload. The correct behavior still has two overloads, but shows the one with higher arity first.

As for the second test showing {} instead of number, that's because in signature help, we assume a trailing comma indicates trying to write another argument, which means chooseOverload fails and we go to getCandidateForOverloadFailure which doesn't infer type arguments.

@ghost ghost merged commit 0d1a49c into master Jul 31, 2018
@ghost ghost deleted the quickInfoSignatureHelpTrailingComma branch July 31, 2018 18:39
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants