Skip to content

Conversation

wildmaples
Copy link
Contributor

Motivation

Inconsistent highlighting in constants:

Screen Shot 2022-06-08 at 8 39 31 PM

Implementation

Add new semantic token type, namespace, and add them as a token for constants.

Irrelevant to the primary issue but I also rearranged some of the visit methods in alphabetical order.

Automated Tests

  • Fix the existing tests
  • Add a new test file const.exp
  • Fix the integration test -- the code contains a constant, so the assertion needed to be updated

Manual Tests

  1. Checkout this branch
  2. Open test/fixtures/const.rb
  3. See consistent highlighting:

Screen Shot 2022-06-08 at 8 44 15 PM

@wildmaples wildmaples requested a review from a team June 9, 2022 11:09
@wildmaples wildmaples self-assigned this Jun 9, 2022
TOKEN_TYPES = [
:variable,
:method,
:namespace,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we can't differentiate between a module and class reference, should we use namespace or class?

Copy link
Contributor Author

@wildmaples wildmaples Jun 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought calling both modules and classes a class token may be confusing, and so settled with namespace. Any opinions?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just checked and all classes and modules are considered modules. So, indeed namespace would be the more appropriate one.

@wildmaples wildmaples force-pushed the sh-consistent-constants branch from 0d52ef7 to 64fa643 Compare June 9, 2022 18:13

response = make_request("textDocument/semanticTokens/full", { textDocument: { uri: "file://#{__FILE__}" } })
assert_empty(response[:result][:data])
assert_equal([0, 6, 3, 2, 0], response[:result][:data])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should reconsider how we test this (in another PR) because this is surprisingly cryptic.

@wildmaples wildmaples force-pushed the sh-consistent-constants branch from 64fa643 to 7ecb07f Compare June 10, 2022 18:20
This is an expected failure since the source code contains a constant.
@wildmaples wildmaples force-pushed the sh-consistent-constants branch from 7ecb07f to 11dc40e Compare June 10, 2022 18:27
@wildmaples wildmaples merged commit 75eeb3a into main Jun 10, 2022
@wildmaples wildmaples deleted the sh-consistent-constants branch June 10, 2022 18:29
@shopify-shipit shopify-shipit bot temporarily deployed to production June 15, 2022 15:46 Inactive
andyw8 referenced this pull request in andyw8/ruby-lsp Mar 2, 2024
klaaspieter pushed a commit to klaaspieter/ruby-lsp that referenced this pull request Feb 25, 2025
* Fix spacing between constants and sigs

* Use HTTP response classes instead of code
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.

4 participants