[move-analyzer] Support document symbol PSL #218
Conversation
yubing744
changed the title
|
@awelc I fixed the problem of vscode v1.69.2, and manually installed and tested move-analyzer.vsix on the mac, can you check if it can be merged? |
There was a problem hiding this comment.
The new plugin and language server binary are now working with VSCode 1.69.2! Thanks!
There are some unanswered/unaddressed comments still. Could you please take a look?
Also, how do you actually use the new functionality in VSCode? I could not figure it out from the tests code and I though it would become clear when I can play with the editor and the new extension, but I am still not sure. I thought it would be a new move-analyzer command but the only one I can see is still just the Move server version one:
If it's something that is immediately useful for the end user, we should emphasize it in the docs published in the README (there is an advanced section in the bottom with a list of new features). If it isn't (e.g., because it requires StarCoin plugin rather than "generic" plugin) than I am not sure how much sense it makes to modify the "generic" plugin.
If we indeed want to publish a new version of the plugin, please bump up plugin version in package.json to 0.0.10 (re-publishing in VSCode marketplace required an increased version number).
| @@ -1,7063 +1,7266 @@ | |||
| { | |||
| "name": "move-analyzer", | |||
There was a problem hiding this comment.
Did you try conversion to Unix-style line endings? The diff for this file is still huge.
| @@ -60,7 +67,7 @@ export class Context { | |||
| * To read more about the messages sent and responses received by this client, such as | |||
| * "initialize," read [the Language Server Protocol specification](https://microsoft.github.io/language-server-protocol/specifications/specification-current/#initialize). | |||
| **/ | |||
| startClient(): void { | |||
| async startClient(): Promise<void> { | |||
There was a problem hiding this comment.
Any additional comment on this? I still do not understand why we need asynchrony. You yourself write that the symbol table parsing happens synchronously so why we need async code here?
|
@awelc The Motivation of this PR explains how starcoin-ide uses this function, to the effect that it identifies the Move file test function and facilitates users to run test cases. |
|
@awelc Implemented the textDocument/DocumentSymbol interface of the LSP protocol, and the outline function will be directly available，I'll revise the documentation later and add function descriptions
|
|
@awelc Regarding the problem of marking functions as asynchronous, this is a grammatical feature of Nodejs. By marking functions as asynchronous functions, synchronous waiting can be achieved through await. |
…port-document-symbol-psl-10
|
@awelc I tried to convert the new line method of package-lock.json, and the changes were still very big. Later, I restored package-lock.json from the main branch, and then regenerated this file, and the changes were still very large. My analysis may be that upgrading @vscode/test-electron will indeed lead to major changes. |
|
@awelc I updated the PR |
There was a problem hiding this comment.
Approved! Thank you for your patience and explanations. Please add the following bullet to the extension's README file (at the end, where the new features are listed) so that I can merge and re-publish the extension with this short new feature description
- outline view showing symbol tree for Move source files
|
@awelc I updated the PR with a detailed feature description in the README |
|
@tnowacki Can you help to review or merge this PR? |
I approved yesterday so it's good to go and the reason it's not merged yet is because one runner does not finish in our CI. |
OK，thanks. |
|
It is merged now (needed an extra change in our CI to get past the test runner problem). |
|
@awelc OK, Any plans for the move-analyzer roadmap? |
There are a few things that we may be doing or that may come as external contributions:
Realistically, only the first bullet will probably happen soon-ish on our side. The rest will have to wait at least a bit unless it's contributed externally. |
|
Also, just re-published the new version of the extension to the VSCode Marketplace. Thank you again for this contribution!!! |
* feat: add release workflow * feat: update release script * feat: update release * feat: fmt release.sh * feat: add lsp.test.ts for move-analyzer * feat: fix copyfiles error * feat: config test for lsp * feat: ok for lsp test * feat: get symbols from table * feat: export symbols * feat: test textDocument/documentSymbol * feat: re impl symbols * feat: parse symbols tree * feat: ok for on_document_symbol_request * feat: auto build move-analyzer and set default for serverPath * feat: fix ci check fail * feat: fix ci check error * feat: fix lint error * fix: fix lsp test fail * feat: fmt move-analyzer code * feat: fix lsp fisrt test error * feat: update timeout for move-analyzer active * feat: support events * fix: fix error for LSP test * feat: fix bug for merge from master * feat: fmt code * feat: update test and igonre package-lock.json for broken url test * feat: change name for ready events * feat: remove unrelate code * feat: remove yarn.lock * feat: ignore yarn.lock and recover configuration.ts * feat: update the comment for OnReady * feat: remove codes * feat: lock file * feat: add waiting code * feat: try lock code * feat: test new way * feat: ok for symbols * feat: fix ci-check issue * feat: update ci-check issues * feat: fix ci-check issues * [Move analyzer] Added support for computing first symbol info synchronously * feat: handle symbols on start * feat: remove unuse code * feat: format code * feat: add test for mac and windows platform * feat: fix bug for uri to path * feat: fixbug for windows platform get_symbols stack overflow * feat: fixbug for reporting lsp diagnostics * feat: fixbug for windows test * feat: fix lint error for language/move-analyzer/src/symbols.rs * feat: remove config for windows stack * feat: update pipeline for move-analyzer * feat: change name for test-move-analyzer-vscode-extension * feat: remove tsfmt for windows check fail * feat: fixbug for stack overflow and lint error * feat: fix lint error * feat: add commit for linebreak-style * feat: fix code review issue * feat: remove linkarg config * feat: update cargo/config fmt * feat: config move-analyzer test vscode version to 1.69.2 * feat: add comments for plugin activate * feat: upgrate ci-test mac system to mac-11 * feat: update style issue * feat: fixbug for user data dir too long for 1.69.2 * feat: fixbug for userDataDir too long * feat: fixbug for vsce package missing files * feat: add feature in README.md * feat: add comment for startClient func in context.ts * feat: update package-lock.json * feat: format context.ts * feat: update package-lock.json for move-analyzer * feat: format context.ts * feat: add outline feature detail descripe * feat: format README
Motivation
starcoin-ide expects that users can select a single test function on the interface, and then execute the test. In order to realize this function, I need to extract the test function entry in the move file. I found that move-analyzer has parsed the AST of Move, and found the textDocument/DocumentSymbol interface of the LSP protocol, which can get all the function information in the move file, but move-analyzer has not implemented this interface yet, so I submitted this PR.
Have you read the Contributing Guidelines on pull requests?
YES
Test Plan
I added a use case to the move-analyzer plugin to test the textDocument/DocumentSymbol protocol.
Test file: https://github.com/yubing744/starcoin-move/blob/support-document-symbol-psl/language/move-analyzer/editors/code/tests/lsp.test.ts