-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Show expanded parameter lists in signature help #30299
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
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 probably add a fourslash test this affects.
@weswigham Yes, except I've spent enough time in fourslash hell to last a lifetime. |
Guess this #30084 is no longer needed then :( @ahejlsberg |
@@ -574,7 +574,7 @@ namespace ts.SignatureHelp { | |||
printer.writeList(ListFormat.TypeParameters, args, sourceFile, writer); | |||
} | |||
}); | |||
const parameters = candidateSignature.parameters.map(p => createSignatureHelpParameterForParameter(p, checker, enclosingDeclaration, sourceFile, printer)); | |||
const parameters = checker.getExpandedParameters(candidateSignature).map(p => createSignatureHelpParameterForParameter(p, checker, enclosingDeclaration, sourceFile, printer)); |
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.
Not sure if this matters or not, but isVariadic
will be true for functions with expanded parameters even though the expanded signature will not always be. Not sure if this will be an issue, in my similar PR, I reevaluated whether the signature is variadic based on the last argument. I found this with the fourslash test I added, don't know if any clients of the API will suffer from isVariadic
not being entirely acurate in this case.
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 don't use the isVariadic
property ourselves but it is part of the public API so we should make sure it is set correctly.
@dragomirtitian Sorry, I completely missed that you already had created a PR for this issue. Yours has tests and handles the |
@ahejlsberg No problem these things happen :). I'm happy if you think I can still contribute in some way. I will fix the PR shortly. |
The PR fixes signature help to show expanded parameter lists for rest parameters of tuple types (reported here #30215 (comment)). For example, instead of
(...args: [string]) => string
we now show(a: string) => string
. This is similar to what we already do in quick info.