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

Add move language v0.0.1 #1799

Merged
merged 7 commits into from
Dec 17, 2024
Merged

Conversation

Tzal3x
Copy link
Contributor

@Tzal3x Tzal3x commented Dec 14, 2024

Closes #717

Here is an example of the LSP working by hovering over the use sui::dynamic_field as df;

image

@cla-bot cla-bot bot added the cla-signed label Dec 14, 2024
@lispking
Copy link
Contributor

Tzal3x/move-zed-extension#2

@Tzal3x It was found through actual testing that there are some problems with some expressions in the repository. Only after the actual modifications can the grammar display be satisfied. And a pull request (PR) has been submitted to your repository to fix some issues. Please help review it. Thank you.

@Tzal3x
Copy link
Contributor Author

Tzal3x commented Dec 15, 2024

@lispking what are the issues? I am not able to install the extension using the modifications you submitted.

The PR I submitted is functional, so this can be merged. We can make a patch to the extension (if needed) when the exact problem is highlighted. Thanks! 🚀

@lispking
Copy link
Contributor

Alternatively, I'll submit the patch to your repository. That would be more appropriate. @Tzal3x

@Tzal3x
Copy link
Contributor Author

Tzal3x commented Dec 16, 2024

Just saw your update on the PR @lispking. Nice work, merged! ✅

Let's hope zed maintainers will accept this one soon to reach the community.

Update gitmodules for move language

Such a name is more in line with the official naming standards. When I was installing it on my computer, if I used "move-zed-extension", it couldn't show that the installation was successful, and I couldn't find any plugin with the name "move" either. However, after changing the name to "move", it could be installed successfully
@Tzal3x
Copy link
Contributor Author

Tzal3x commented Dec 16, 2024

Hey, @notpeter respectively asking for your review. 🙏

@notpeter notpeter self-assigned this Dec 16, 2024
@notpeter
Copy link
Member

notpeter commented Dec 16, 2024

Excite you were able to get this working!

I know this is a big ask, but would be willing to re-write history for your repo to remove the 60-odd MB of binaries from /target which were accidentally included in-tree prior to Tzal3x/move-zed-extension@d49f35f?

I know it's a PITA, but I would prefer not require another 60MB of disk storage on everyone who recursively checks out the extensions repo. I'd recommend something like git-filter-repo. Here's an example git-filter-repo usage instructions we use extracting extensions from the zed repo.

The zed repo submodule is by far the worst offender (192MB) but we are working to remove that entirely in favor of individual extension repos.

@Tzal3x
Copy link
Contributor Author

Tzal3x commented Dec 16, 2024

That would be a huge PITA indeed, and since this is the very first version of the extension I just squashed all commits. From now on each patch/version will have its own commit and the issue with /target is resolved. Good ol' git rebase -i.

@notpeter
Copy link
Member

notpeter commented Dec 16, 2024

I bumped the submodule commit on your branch.

  1. 176KB is much better than 60MB (yay!)

  2. This change broke tests, I think because you have queries which reference a newer release of the tree-sitter than what is referenced in extensions.toml

A lot has happened since 8bc0d1692caa8763fef54d48068238d9bf3c0264
Compare: tzakian/tree-sitter-move@8bc0d16...main

  1. You should actually .gitignore grammars/ directory altogether too.

@Tzal3x

This comment was marked as resolved.

@Tzal3x
Copy link
Contributor Author

Tzal3x commented Dec 17, 2024

I'm not sure I understand the error of the CI, and I don't have access re-running it to experiment with it. Mind lending a hand here? 😬

@notpeter notpeter changed the title Add move lang support extension Add move language v0.0.1 Dec 17, 2024
@notpeter notpeter merged commit be415d2 into zed-industries:main Dec 17, 2024
3 checks passed
@notpeter
Copy link
Member

That did it! Thanks! And congrats on your first Zed Extension!

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

Successfully merging this pull request may close these issues.

Sui Move-lang support
3 participants