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

Ensure all string enum values are comparable to strings #15

Merged
merged 4 commits into from
Apr 16, 2024

Conversation

jwortmann
Copy link
Member

This is part of a workaround for a problem described at sublimelsp/LSP#2446 (comment)

Currently it would not work on Python 3.8 to import directly from enum, because most of the enums with string values are just regular Enum instead of StrEnum. This means we can't compare values to the strings from a server response. But if they are StrEnum, they could be compared directly (in theory, because StrEnum is not part of Python 3.8 - but this means that the enums inherit from our fake StrEnum class, instead of to the real Enum class from the enum module).

I think then it should work, together with another small change I have in mind.

@predragnikolic
Copy link
Member

In the last commit LanguageKind enum type was also added.

Note, LanguageKind requires additional code changes in the LSP codebase.

6bb8d43#diff-5b016df699e02c42aafd75508568d98c635c0a233622d3f3ff79a92b0e7ddff3L3347-R3464

-    languageId: str
+    languageId: 'LanguageKind'

@predragnikolic predragnikolic merged commit fe5ef32 into sublimelsp:main Apr 16, 2024
1 check passed
@jwortmann jwortmann deleted the str-enum branch April 16, 2024 19:24
@jwortmann
Copy link
Member Author

jwortmann commented Apr 16, 2024

Thanks, and good that you noticed that.

I think we cannot yet update the protocol.py file in LSP, because in the last PR I added this new value
https://github.com/sublimelsp/LSP/blob/5891ff41deaa781783effeb97c978b3e50bd0a26/plugin/core/protocol.py#L219-L220

but it is not yet added upstream in the https://github.com/microsoft/lsprotocol repo. Or alternatively if we want to update, we need to add it back manually.

Forget that, I see now that it is already in the upstream repo. I must have had something else in mind.

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 this pull request may close these issues.

2 participants