Fix semantic token defaultLibrary modifier casing#4005
Open
a-lavis wants to merge 1 commit intoShopify:mainfrom
Open
Fix semantic token defaultLibrary modifier casing#4005a-lavis wants to merge 1 commit intoShopify:mainfrom
defaultLibrary modifier casing#4005a-lavis wants to merge 1 commit intoShopify:mainfrom
Conversation
Author
|
Just signed the CLA! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Ruby LSP currently emits the semantic token modifier as
default_library, but the LSP standard modifier name isdefaultLibrary(camelCase). From the LSP specification:The Ruby LSP docs state:
But
default_libraryis not in the default list.I noticed this because I was using Ruby LSP with Zed, which has default semantic token rules to map semantic token types and modifiers to syntax theme styles. But these default semantic token rules do not support
default_library; they only supportdefaultLibrary.This means that, when using Zed, in order to get semantic token syntax highlighting for
self, you need to define your own custom semantic token rules fordefault_library.This would also apply to any other clients or themes expecting the semantic token types/modifiers from the specification in order to determine syntax theme styles.
In short - using the non-standard modifier
default_librarybreaks protocol compatibility for clients/themes expecting the standard modifierdefaultLibrary.Related PR
See: Shopify/vscode-shopify-ruby#754
That PR updates the Spinel themes to apply the same formatting to
variable.defaultLibraryas it does tovariable.default_library.That PR should ideally be merged and released before this PR, so that VSCode users of the Spinel theme don't notice any changes.
Implementation
default_librarytodefaultLibraryin:lib/ruby_lsp/response_builders/semantic_highlighting.rbselftoken emission to use the new modifier symbol:lib/ruby_lsp/listeners/semantic_highlighting.rbjekyll/semantic-highlighting.markdown(variable.defaultLibrary)defaultLibraryand notdefault_library:test/requests/semantic_highlighting_expectations_test.rbAutomated Tests
Yes, I added tests!
Manual Tests
In VSCode, open a Ruby file containing
self, for example:Run Developer: Inspect Editor Tokens and Scopes.
Place cursor on
self.Verify semantic token shows:
variabledefaultLibrarydefault_library.