Skip to content
This repository has been archived by the owner on May 4, 2024. It is now read-only.

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

Draft
wants to merge 128 commits into
base: sui-move
Choose a base branch
from

Conversation

0xOutOfGas
Copy link

@0xOutOfGas 0xOutOfGas commented Feb 24, 2023

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:

  • 1 Added semantic analysis to the Move language and Sui Move, and enhanced some features of the plug-in, such as go-to-definition, auto-completion, finding references, etc.;
  • 2 Added semantic analysis to MSL (Move Specification Language), supports features such as go-to-definition and auto-completion, and supports go-to-definition from MSL code to Move code;
  • 3 Integrated common Sui development commands into Command Palette, support Sui Code Snippets automatic completion (currently only init function);
  • 4 Support parallel development of multiple projects under the same directory;

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.

Copy link
Collaborator

@awelc awelc left a comment

Choose a reason for hiding this comment

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

It's a really large PR. I would strongly suggest splitting it into separate PRs, dealing with separate issues mentioned in the PR description.

A more detailed description of changes/additions/enhancements would also be greatly appreciated to ease the burden or reviewing the new code. In particular, it would be great to have a better idea of what the enhancements to the existing features are.

It also looks like some differences are solely due to different formatting. It would be great to have those unified with how it was formatted previously to emphasize portions of the code that have really changed.

@0xOutOfGas
Copy link
Author

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
Copy link
Collaborator

awelc commented Feb 28, 2023

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
Copy link

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
Copy link
Collaborator

awelc commented May 18, 2023

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
Copy link

0xyilu commented May 25, 2023

we are still working on that, will keep you posted

@awelc
Copy link
Collaborator

awelc commented Jun 13, 2023

we are still working on that, will keep you posted

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants