move-language / move

Apache License 2.0
2.25k stars 684 forks source link

[move-analyzer] Enhancement for supporting MSL and completion suggestions #930

Open 0xOutOfGas opened 1 year ago

0xOutOfGas commented 1 year ago

Motivation

This is a draft PR for #799 from MoveBit, we'd like to let folks know what we're doing, and have a discussion with developers to improve move-analyzer together.

The changelog is below:

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Tested in our dev member's daily work, may not be fully tested.

0xOutOfGas commented 1 year ago

Yes, it's a big PR, we considered splitting it before submitting it but not succeeded. We'd like to try to split it again, and fixing the format issues, and also add more description about the changes/additions/enhancements.

For the splitting work, we may take some time to finish and submit separate PRs. Before that, we appreciate any reviews and comments on this PR.

awelc commented 1 year ago

Yes, it's a big PR, we considered splitting it before submitting it but not succeeded. We'd like to try to split it again, and fixing the format issues, and also add more description about the changes/additions/enhancements.

For the splitting work, we may take some time to finish and submit separate PRs. Before that, we appreciate any reviews and comments on this PR.

Thank you! I would really hope that it can be split considering that it addresses multiple somewhat separate issues. It would really help with the reviewing effort (I am assuming you tried stacked diffs), as would additional documentation/description (ideally in the code itself so that it's easy to find afterwards).

There is also a lot of new functionality but very few tests...

yuyang-ok commented 1 year ago

The toolchain been modified because of I have the error below.

> move-analyzer@0.0.11 compile
> tsc -p ./ && cd ../../ && cargo build --bin move-analyzer

error: the 'cargo' binary, normally provided by the 'cargo' component, is not applicable to the '1.65.0-x86_64-apple-darwin' toolchain

 *  The terminal process "/bin/zsh '-c', 'npm run pretest'" terminated with exit code: 1. 
 *  Terminal will be reused by tasks, press any key to close it. 

It's wired, should impossible, Someone must used this version on Mac. I don't know how to fix this error.

awelc commented 1 year ago

We were admittedly rather tied up with the mainnet launch until recently, but it's time to get back to enhancing the IDE! I was wondering if there is any progress on (any of) the following:

Yes, it's a big PR, we considered splitting it before submitting it but not succeeded. We'd like to try to split it again, and fixing the format issues, and also add more description about the changes/additions/enhancements.

For the splitting work, we may take some time to finish and submit separate PRs. Before that, we appreciate any reviews and comments on this PR.

0xyilu commented 1 year ago

we are still working on that, will keep you posted

awelc commented 1 year ago

we are still working on that, will keep you posted

Do you have some kind of timeline in mind that you could share?

robinlzw commented 1 year ago

We were admittedly rather tied up with the mainnet launch until recently, but it's time to get back to enhancing the IDE! I was wondering if there is any progress on (any of) the following:

Yes, it's a big PR, we considered splitting it before submitting it but not succeeded. We'd like to try to split it again, and fixing the format issues, and also add more description about the changes/additions/enhancements.

For the splitting work, we may take some time to finish and submit separate PRs. Before that, we appreciate any reviews and comments on this PR.

Hello, I am an engineer from Movebit. We will continue to maintain and optimize the sui move analyzer, and would like to merge the features into move analyzer. For this PR, we plan to split and merge it in the following feature order within about 4 weeks.

  1. Goto definition
  2. Find references
  3. Hover
  4. Auto completion
  5. Inlay hints
robinlzw commented 1 year ago

we are still working on that, will keep you posted

Do you have some kind of timeline in mind that you could share? hello, for "Goto definition" feature, I submitted a PR as this: https://github.com/move-language/move/pull/1064

robinlzw commented 1 year ago

we are still working on that, will keep you posted

Do you have some kind of timeline in mind that you could share? We plan to complete the split and merger of the entire PR around July 15th Find references -- 20230710 Hover -- 20230712 Auto completion -- 20230714 Inlay hints -- 20230717

robinlzw commented 1 year ago

we are still working on that, will keep you posted

Do you have some kind of timeline in mind that you could share?

We have already submitted the PR for the first feature, and the PR for the remaining 4 features are also ready. Due to the need to rely on the first PR, please review and merge first, and we will immediately submit the remaining 4 PRs in sequence.