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

Basic support for opam switch #6

Merged
merged 1 commit into from
Jan 1, 2025
Merged

Conversation

Khady
Copy link
Contributor

@Khady Khady commented Dec 6, 2024

fix #3

@cla-bot cla-bot bot added the cla-signed label Dec 6, 2024
@Khady
Copy link
Contributor Author

Khady commented Dec 22, 2024

Is there anything required on my side to get this PR merged? No pressure, just that I've not contributed to zed's extensions before so I'm not sure of the process.

@notpeter
Copy link
Contributor

notpeter commented Jan 1, 2025

Hi @Khady sorry for the delay. The lack of daily ocaml users on the Zed team plus the holiday means it slipped past most potential reviewers radar, thanks for the ping.

The error if you have opam installed but not ocaml-language-server is a bit opaque in the logs:

[ERROR] Command not found 'ocamllsp'
Screenshot 2025-01-01 at 15 26 41

I don't think it's possible with the current Extension API to execute an arbitrary command and alter behavior (e.g. if opam list --short --installed --field-match=name:ocaml-lsp-server returns false output a custom error message which suggests running opam install -y ocaml-lsp-server). Perhaps a note in the doc is sufficient. I'll take a crack at that.

Otherwise looks good!

@notpeter notpeter changed the title add basic opam switch support Basic support for opam switch Jan 1, 2025
@notpeter notpeter merged commit a4104d5 into zed-extensions:main Jan 1, 2025
1 check passed
notpeter added a commit that referenced this pull request Jan 1, 2025
notpeter added a commit to zed-industries/extensions that referenced this pull request Jan 1, 2025
@Khady
Copy link
Contributor Author

Khady commented Jan 2, 2025

Thanks for your help!

@Khady Khady deleted the louis/opam branch January 6, 2025 16:25
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.

add support for opam
2 participants