[IDE] Move signature help formatting to IDE instead of SourceKit #84403
Conversation
a7medev
requested review from
bnbarham,
hamishknight and
rintaro
as code owners
| struct Parameter { | ||
| /// The offset of the parameter text in the signature text. | ||
| unsigned Offset; | ||
|
|
||
| /// The length of the parameter text in the signature text. | ||
| unsigned Length; | ||
|
|
||
| /// The internal parameter name. | ||
| llvm::StringRef Name; | ||
|
|
||
| Parameter() {} |
There was a problem hiding this comment.
I removed the DocComment field since we now rely on SourceKit-LSP to parse the signature's documentation and extract the parameter documentation.
| // CLOSURE_PARAM: Begin signatures, 1 items | ||
| // CLOSURE_PARAM-NEXT: Signature[Active]: body(<param active>Value</param>) -> Result | ||
| // CLOSURE_PARAM-NEXT: End signatures |
There was a problem hiding this comment.
Switched back to FileCheck from diff for swift-ide-test tests since the output is much nicer now besides that swift-ide-test prints "found code completion token <TOKEN> at offset OFFSET>" which we don't want to match with diff.
| virtual void cancelled() = 0; | ||
| }; | ||
|
|
||
| struct SignatureHelpResponse { |
There was a problem hiding this comment.
We don't need a separate SourceKit::SignatureHelpResponse since swift::ide::FormattedSignatureHelp already includes all the information in a format that could just be output by SourceKit.
|
|
||
| class SignatureHelpFormatter { | ||
| private: | ||
| llvm::BumpPtrAllocator &Allocator; |
There was a problem hiding this comment.
SignatureHelpFormatter doesn't own its allocator to make the lifetime of the results it returns more explicit to the caller.
| } | ||
| ExitCode = doSignatureHelp( | ||
| InitInvok, options::SourceFilename, options::SecondSourceFilename, | ||
| options::CodeCompletionToken, options::CodeCompletionDiagnostics); |
There was a problem hiding this comment.
Should I add separate -signature-help-token and -signature-help-diagnostics flags to avoid confusion or is relying on the existing code completion options fine here? I feel neutral to be honest.
There was a problem hiding this comment.
I'm fine with re-using the completion options for this
|
@swift-ci please test |
|
@swift-ci please test macOS |
| } | ||
| ExitCode = doSignatureHelp( | ||
| InitInvok, options::SourceFilename, options::SecondSourceFilename, | ||
| options::CodeCompletionToken, options::CodeCompletionDiagnostics); |
There was a problem hiding this comment.
I'm fine with re-using the completion options for this
| // RUN: %target-swift-ide-test -signature-help -code-completion-token=CURRY_TOPLEVEL -source-filename=%s | %FileCheck %s --check-prefix=CURRY_TOPLEVEL | ||
| // RUN: %target-swift-ide-test -signature-help -code-completion-token=CURRY_MEMBER_PARTIAL -source-filename=%s | %FileCheck %s --check-prefix=CURRY_MEMBER_PARTIAL | ||
| // RUN: %target-swift-ide-test -signature-help -code-completion-token=CURRY_MEMBER_FULL -source-filename=%s | %FileCheck %s --check-prefix=CURRY_MEMBER_FULL |
There was a problem hiding this comment.
If we end up adding more signature help test cases with multiple places to do the request, it might be worth seeing if we can adapt batch-code-completion to also work for signature help. For now I think this is fine though since there aren't many of them
There was a problem hiding this comment.
I agree. We don't have many such tests at the moment though so I think it's fine for now.
a7medev
force-pushed
the
refactor/signature-help-to-ide
branch
from
c6d547c to
7089473
Compare
|
@swift-ci please smoke test |
a7medev
force-pushed
the
refactor/signature-help-to-ide
branch
from
7089473 to
456a40a
Compare
|
@swift-ci please smoke test |
2 participants
SignatureHelpFormatterclass insideIDE.swift-ide-testand moves most tests totest/IDEinstead oftest/SourceKit.CodeCompletionStringBuilder.hintolib/IDEto be a private implementation detail ofIDEjust likeCodeCompletionResultBuildernow that signature help formatting is also part ofIDE.