-
Notifications
You must be signed in to change notification settings - Fork 33
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
feat(spxls): add support for retrieving package documentation in hover #1373
base: dev
Are you sure you want to change the base?
Conversation
@@ -26,6 +27,13 @@ type SpxResourceRef struct { | |||
Node gopast.Node | |||
} | |||
|
|||
// SpxReferencePkg is a reference to an imported package. | |||
type SpxReferencePkg struct { |
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.
这个 spx_resource.go
主要是用来组织跟 spx 中的资源系统相关的概念,比如精灵和声音之类的。我的理解 SpxReferencePkg
属于是 go+ 的概念了,放这个文件里感觉不太合适。
Range: result.rangeForNode(rpkg.Node), | ||
}, nil | ||
} | ||
|
||
ident := result.identAtASTFilePosition(astFile, params.Position) |
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.
其实我倾向于认为这种对于在 import "fmt"
语句中包名的位置拿不到 ast.Ident
的情况属于是 go+ 那边的 bug。换句话讲按照我们已有的实现,它应该出现在 typesutil.Info
的 Defs
字段中,且对应的 types.Object
是一个 types.PkgName
才合理。如果那样的话,我们应该可以尝试在 go+ 那边修复这个 bug。
当然不排除 go 那边的处理方式也是类似的,如果是那样就不算 go+ 的 bug 了。
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.
我最早也是考虑在 typesutil 里面处理,typestuil 里有一个 Implicits,ast.ImportSpec 在这个字段中,不过 ImportSpec 具体的pkg 是一个BasicLit 不是 ident,所以就没在这里处理, 看了 gopls 也是单独分支处理,所以就单独放出来
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.
好的。不过感觉这个逻辑是不是可以放进 ident == nil
里去?我的理解它算是在找不到 ident 时的特殊处理。
pkg := imp.Path.Value | ||
unquoted, err := strconv.Unquote(pkg) | ||
if err == nil { | ||
pkg = unquoted |
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.
我的理解正常情况下 strconv.Unquote(imp.Path.value)
应该总是返回 nil error 才对?如果是出于防御编程目的,建议 err != nil
时直接 continue
,包括下面 pkgdata.GetPkgDoc
的错误处理也一样。
This PR has been deployed to the preview environment. You can explore it using the preview URL. Warning Please note that deployments in the preview environment are temporary and will be automatically cleaned up after a certain period. Make sure to explore it before it is removed. For any questions, contact the Go+ Builder team. |
} | ||
pkgDoc, err := pkgdata.GetPkgDoc(unquoted) | ||
if err == nil { | ||
rpkg = &SpxReferencePkg{ |
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.
话说这里是不是就该 return
了?感觉没必要再继续遍历下去了
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.
哦对了,可能需要补个单测,往 hover_test.go
里的 TestServerTextDocumentHover
加个 subtest 就好
fix: #1361