Skip to content
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

APIReference UI improvement #1239

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from
Open

APIReference UI improvement #1239

wants to merge 6 commits into from

Conversation

nighca
Copy link
Collaborator

@nighca nighca commented Jan 14, 2025

Copy link

qiniu-prow bot commented Jan 14, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Choose a reason for hiding this comment

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

Copilot reviewed 8 out of 15 changed files in this pull request and generated no comments.

Files not reviewed (7)
  • tools/ispx/go.mod: Language not supported
  • tools/spxls/go.mod: Language not supported
  • spx-gui/src/components/editor/ProjectEditor.vue: Evaluated as low risk
  • spx-gui/src/components/editor/code-editor/ui/CodeEditorUI.vue: Evaluated as low risk
  • spx-gui/src/components/editor/code-editor/ui/api-reference/APIReferenceUI.vue: Evaluated as low risk
  • spx-gui/src/components/editor/code-editor/document-base/spx/index.ts: Evaluated as low risk
  • spx-gui/src/components/editor/code-editor/ui/resource-reference/index.ts: Evaluated as low risk
@nighca nighca marked this pull request as ready for review January 14, 2025 01:30
}

// getFuncAndOverloadsType returns the function type and all its overloads.
func getFuncAndOverloadsType(compileResult *compileResult, funIdent *gopast.Ident) (fun *types.Func, overloads []*types.Func) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

基本上是从 https://github.com/goplus/builder/pull/1209/files#diff-f13ba743896019fa1eadba9196fe866428e9616044e188defc8314b9ac2fba86 这里抄的,现在的代码里好像没有对类似逻辑的封装了🤣

//
// NOTE: There are limitations with current implementation:
// 1. Only `LambdaExpr2` (not `LambdaExpr`) is supported.
// 2. Only the last parameter of the lambda is checked.
Copy link
Collaborator Author

@nighca nighca Jan 14, 2025

Choose a reason for hiding this comment

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

如果考虑 lambda 每一个 parameter,尤其是当多个 parameter 可以省略的时候,“检查匹配的 overload”本身就会变复杂不少,而性价比很低;所以这里只处理 lambda 最后一个 parameter 可省略的情况,这已经能覆盖所有已知的 spx API 了

Copy link

qiniu-x bot commented Jan 15, 2025

[Git-flow] Hi @nighca, There are some suggestions for your information:


Rebase suggestions

  • Following commits have duplicated messages

    typo

    typo

Which seems insignificant, recommend to use git rebase command to reorganize your PR.

For other git-flow instructions, recommend refer to these examples.

If you have any questions about this comment, feel free to raise an issue here:

@qiniu-ci
Copy link

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.

@@ -28,7 +27,7 @@ func (s *Server) textDocumentFormatting(params *DocumentFormattingParams) ([]Tex
return nil, err
}

formatted, err := formatSpx(content)
formatted, err := formatSpx(s, spxFile)
Copy link
Member

Choose a reason for hiding this comment

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

这样看来不如直接把 formatSpx 的逻辑合并进 Server.textDocumentFormatting 里来,且用 astFile.Code 作为原始 content 来跟 formatted 做比较来判断是否发生变化

Copy link
Member

Choose a reason for hiding this comment

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

且用 astFile.Code 作为原始 content

这个是由于目前的 content 是实时读取的,因为预期会拿它去格式化。如果改成依赖 compile 后,按理说就不能用实时读取的 content 了。

astFile, err := gopparser.ParseFSEntry(fset, memfs.SingleFile("", "main.spx", string(src)), "main.spx", nil, gopparser.Config{
Mode: gopparser.ParseComments | gopparser.ParseGoPlusClass,
})
compileResult, _ := s.compile()
Copy link
Member

Choose a reason for hiding this comment

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

这里的 error 应该还是要处理下的,不然下面访问 compileResult 可能会 panic。可以参考这里的逻辑:

result, spxFile, astFile, err := s.compileAndGetASTFileForDocumentURI(param.TextDocument.URI)
if err != nil {
if errors.Is(err, errNoValidSpxFiles) || errors.Is(err, errNoMainSpxFile) {
return nil, nil
}
return nil, err
}
if astFile == nil {
return nil, nil
}

return true
}
if pn, overloadId := parseGopFuncName(method.Name()); pn == funIdent.Name && overloadId == nil {
underlineFunType = method
Copy link
Member

Choose a reason for hiding this comment

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

这里应该可以 return false

if underlineFunType == nil {
return
}
return funType, expandGopOverloadableFunc(underlineFunType)
Copy link
Member

Choose a reason for hiding this comment

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

除了 expandGopOverloadableFunc(underlineFunType) 其实还有一种做法就是直接在 walkStruct 里收集所有 pn == funIdent.Name && overloadId != nil 的 methods,拿到的结果和 expandGopOverloadableFunc 应该是一样的

End: Position{Line: 8, Character: 0},
},
NewText: `// A spx game.
onKey [KeyLeft, KeyRight], () => {
Copy link
Member

Choose a reason for hiding this comment

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

像这种会建议写成 onKey [KeyLeft, KeyRight], () => { 还是 onKey [KeyLeft, KeyRight], => {

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.

APIReference UI Improvement Upgrade spx
3 participants