-
Notifications
You must be signed in to change notification settings - Fork 640
Fix up hover code, mostly temporary #981
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.
Pull Request Overview
The PR refactors hover text generation to use direct string builders and streamlines printer utilities for single-line output, while temporarily bypassing streaming truncation logic and flipping the unknown-symbol check.
- Introduces
GetSingleLineStringWriter
and makes the pool private for more controlled writer reuse. - Replaces
EmitTextWriter
usage inProvideHover
withstrings.Builder
and inverts the unknown-symbol condition. - Removes explicit writer parameters from
typeToStringEx
/symbolToStringEx
and related methods, centralizing them via the new pool API.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
internal/printer/singlelinestringwriter.go | Renamed pool var to private and added GetSingleLineStringWriter /release function. |
internal/ls/hover.go | Switched hover formatting to strings.Builder , removed printer dependency, flipped unknown-symbol logic. |
internal/checker/symbolaccessibility.go | Updated calls to symbolToStringEx to match new signature (dropped writer arg). |
internal/checker/relater.go | Removed writer arg in error-reporting calls to typeToStringEx . |
internal/checker/printer.go | Simplified internal typeToStringEx , symbolToStringEx , etc., to use the new pool API and drop writer args. |
internal/checker/checker.go | Updated calls to typeToStringEx in diagnostic methods to match new signature. |
Comments suppressed due to low confidence (2)
internal/ls/hover.go:20
- [nitpick] The parameter name
b
is ambiguous; consider renaming it tobuilder
for clarity.
func writeTypeParam(c *checker.Checker, tp *checker.Type, file *ast.SourceFile, b *strings.Builder) {
internal/ls/hover.go:20
- strings.Builder is used in this file but the "strings" package is not imported; add
import "strings"
to the import block.
func writeTypeParam(c *checker.Checker, tp *checker.Type, file *ast.SourceFile, b *strings.Builder) {
Last commit optional, that's mainly there per request (presumably to make the markdown easier) |
Otherwise, looks good to me. |
The writing and clearing and passing down of things doesn't seem to be working; for now just flip this all to use the returned string until we can do something faster. (I'm not doing something faster because some of the code does some truncation that is not easily doable via streaming.)
Also the unknown symbol flip.