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.

Sign up for GitHub

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

Jump to bottom

lsp: Fill root_uri property on Initialize again #25264

Merged
merged 3 commits into from
Feb 20, 2025
Merged

lsp: Fill root_uri property on Initialize again #25264

merged 3 commits into from
Feb 20, 2025

Conversation

osiewicz
Copy link
Contributor

Closes #ISSUE

Release Notes:

  • Fix some language servers (elixir-ls, tailwindcss, phpactor) failing to start up due to an unfilled root_uri property in the InitializeParams

@osiewicz @Anthony-Eid
lsp: Fill root_uri property on Initialize again
d20c65f 
Co-authored-by: Anthony Eid <hello@anthonyeid.me>
@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Feb 20, 2025
@osiewicz osiewicz enabled auto-merge (squash) February 20, 2025 16:20
osiewicz and others added 2 commits February 20, 2025 18:52
@osiewicz @Anthony-Eid
clippy
83a4231 
Co-authored-by: Anthony Eid <hello@anthonyeid.me>
@osiewicz
clippy for windows
190acbb
@osiewicz osiewicz merged commit 3a222f0 into main Feb 20, 2025
12 checks passed
@osiewicz osiewicz deleted the lsp-root-uri-reintroduction branch February 20, 2025 21:55
@osiewicz
Copy link
Contributor Author

/cherry-pick v0.175.x

gcp-cherry-pick-bot bot pushed a commit that referenced this pull request Feb 20, 2025
@osiewicz @Anthony-Eid
lsp: Fill root_uri property on Initialize again (#25264)
55fdd23 
Closes #ISSUE

Release Notes:

- Fix some language servers (elixir-ls, tailwindcss, phpactor) failing
to start up due to an unfilled root_uri property in the InitializeParams

Co-authored-by: Anthony Eid <hello@anthonyeid.me>
@gcp-cherry-pick-bot gcp-cherry-pick-bot bot mentioned this pull request Feb 20, 2025
Merged
osiewicz added a commit that referenced this pull request Feb 20, 2025
@gcp-cherry-pick-bot @osiewicz @Anthony-Eid
lsp: Fill root_uri property on Initialize again (cherry-pick #25264) (#…
52b9d0e 
…25290)

Cherry-picked lsp: Fill root_uri property on Initialize again (#25264)

Closes #ISSUE

Release Notes:

- Fix some language servers (elixir-ls, tailwindcss, phpactor) failing
to start up due to an unfilled root_uri property in the InitializeParams

Co-authored-by: Anthony Eid <hello@anthonyeid.me>

Co-authored-by: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com>
Co-authored-by: Anthony Eid <hello@anthonyeid.me>
@timfjord
Copy link
Contributor

@osiewicz There are still issues with next-ls Elixir LSP (I am not sure whether it is related to this one though)
Starting from yesterday, I've been getting ** (KeyError) key :elixir_bin_path not found in:, and I assume this is because there is still some data missing.

@osiewicz
Copy link
Contributor Author

osiewicz commented Feb 21, 2025 •
edited
Loading

@timfjord Thanks for the report! It looks like next-ls does not handle us setting workspaceFolders to null, even though that's permitted by spec. I think it's fine to work around it on our side.
tl;dr: not directly related to this particular PR, but adjacent for sure.
Are you willing to build Zed from source? I have a fix ready, but I don't have a full-blown elixir setup to test it myself (other than the fact that it makes the error you've shared go away in next-ls repo)

@osiewicz osiewicz mentioned this pull request Feb 21, 2025
Merged
osiewicz added a commit that referenced this pull request Feb 21, 2025
@osiewicz
lsp: Send non-null workspaceFolders in initialize (#25337)
a97a2eb 
This is a ~workaround for next-ls not handling null workspace folders in
initialize request
Related to #25264
/cc @timfjord
Closes #ISSUE

Release Notes:

- Changed how workspace folders are shared with language servers, fixing
a startup issue with `next-ls` in the process.
gcp-cherry-pick-bot bot pushed a commit that referenced this pull request Feb 21, 2025
@osiewicz
lsp: Send non-null workspaceFolders in initialize (#25337)
496c04c 
This is a ~workaround for next-ls not handling null workspace folders in
initialize request
Related to #25264
/cc @timfjord
Closes #ISSUE

Release Notes:

- Changed how workspace folders are shared with language servers, fixing
a startup issue with `next-ls` in the process.
@gcp-cherry-pick-bot gcp-cherry-pick-bot bot mentioned this pull request Feb 21, 2025
Merged
osiewicz added a commit that referenced this pull request Feb 21, 2025
@gcp-cherry-pick-bot @osiewicz
lsp: Send non-null workspaceFolders in initialize (cherry-pick #25337) (
1863f0b 
#25344)

Cherry-picked lsp: Send non-null workspaceFolders in initialize (#25337)

This is a ~workaround for next-ls not handling null workspace folders in
initialize request
Related to #25264
/cc @timfjord
Closes #ISSUE

Release Notes:

- Changed how workspace folders are shared with language servers, fixing
a startup issue with `next-ls` in the process.

Co-authored-by: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com>
@timfjord
Copy link
Contributor

Thanks for fixing this, @osiewicz
Unfortunately, I don't have everything ready to build Zed from source (I might set it up later)
But I will let you know once I test it in Zed Preview.

@osiewicz
Copy link
Contributor Author

No worries! The build with a fix is already out, give newest Preview a go and let me know if it fixes the issue for you :)

@timfjord
Copy link
Contributor

@osiewicz, I can confirm that the issue has been fixed.
Thanks!

@tuzemec tuzemec mentioned this pull request Feb 24, 2025
Open
@osiewicz osiewicz mentioned this pull request Feb 25, 2025
Open
@filipwiech filipwiech mentioned this pull request Feb 26, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@osiewicz @timfjord