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

syntax: Highlight block label as "enumMember" & highlight unquoted labels #943

Merged
merged 5 commits into from
Feb 24, 2022

Conversation

radeksimko
Copy link
Member

This is to complement hashicorp/terraform-ls#802 by aligning the scope of block labels in the grammar with semantic token type assigned by the language server.

It may seem counter-intuitive or wrong from semantic perspective to use variable.other. alongside entity.name. (used for known block type) for labels but as explained in the commit log - this is mostly a pragmatic decision made to provide better UX which otherwise would be hard/impossible to achieve due to the scopes used by default VSCode themes.

As mentioned in #802 we explored a number of other token types which VSCode mostly ends up just mapping down to available scope names per https://github.com/microsoft/vscode/blob/de9ab35ce9da7cb3c850a1b570876263a472174b/src/vs/platform/theme/common/tokenClassificationRegistry.ts#L540-L570

So these are the limitations we have to work within. Our future hope is possibly addition of some more token types into LSP as described in microsoft/language-server-protocol#1067

Before

Screenshot 2022-02-23 at 20 30 18 Screenshot 2022-02-23 at 20 30 35

After

Screenshot 2022-02-23 at 20 35 34 Screenshot 2022-02-23 at 20 35 21


Future Enhancements

@dbanck rightly pointed out that the LS/extension alignment may do a dis-service to the user in case of "invalid" labels. This is tracked under hashicorp/terraform-ls#804

There was also point made in #710 about distinguishing "dependent labels" from "freeform-text labels". This is tracked under hashicorp/terraform-ls#803

HLC considers the quotes to be effectively part of the same token (label), i.e. it treats both quoted or unqouted labels the same, although quoted labels are considered best practice.

We reflect the equivalence here and assume that quotes (if present) will be highlighted the same as the string between the quotes.
The two character classes are (AFAIK) equivalent and this just ensures we use one style throughout the grammar.
In LS we assign "enummember" token type to block labels which paints them blue in the default VSCode themes.
To provide better UX without flipping colours we aim to match what LS does with 'variable.other.enummember'
Sadly themes do not support entity.name.enummember which would make more sense semantically.
@radeksimko radeksimko added enhancement New feature or request syntax labels Feb 23, 2022
@radeksimko radeksimko requested a review from a team February 23, 2022 21:40
Copy link
Member

@dbanck dbanck left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@radeksimko radeksimko merged commit bb95e28 into main Feb 24, 2022
@radeksimko radeksimko deleted the b-label-highlighting branch February 24, 2022 12:03
@dbanck dbanck added this to the 2.20.0 milestone Feb 28, 2022
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request syntax
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants