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

Name optional in WorkspaceFolder? #741

Closed
cdietrich opened this issue Jun 12, 2023 · 4 comments · Fixed by #767
Closed

Name optional in WorkspaceFolder? #741

cdietrich opened this issue Jun 12, 2023 · 4 comments · Fixed by #767
Milestone

Comments

@cdietrich
Copy link
Contributor

Name is optional in WorkspaceFolder constructor, but not in the spec.
is this intentional or an oversight?

https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#workspaceFolder

@pisv
Copy link
Contributor

pisv commented Jun 12, 2023

It does not seem to be intentional.

It got introduced via #133 when this feature was still a proposal.

Perhaps at that stage the name was optional.

But since the time this feature was added to the spec (LSP 3.6), the name was required:
microsoft/language-server-protocol@39b1181

@jnt0r
Copy link
Contributor

jnt0r commented Sep 12, 2023

@pisv would you like to add the name to the constructor, or what solution would you prefer? Would it be an incompatible change or not, as it throws errors if not set?

@pisv
Copy link
Contributor

pisv commented Sep 12, 2023

@jnt0r Thanks for the reminder.

I think that the name should be made required, its javadoc changed to match the current wording in the spec, and the constructor WorkspaceFolder(String uri) removed. Also, the breaking change should be mentioned in the change log.

Would you like to submit a PR for this? Otherwise, I'll handle it, no problem at all.

@jnt0r
Copy link
Contributor

jnt0r commented Sep 12, 2023

Alright. Yes, I can submit a PR for that.

jnt0r added a commit to jnt0r/lsp4j that referenced this issue Sep 12, 2023
@pisv pisv added this to the 0.22.0 milestone Sep 12, 2023
@pisv pisv closed this as completed in #767 Sep 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants