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

[semantic highlighting] improve fallback scope for macro #110150

Closed
georgewfraser opened this issue Nov 7, 2020 · 8 comments
Closed

[semantic highlighting] improve fallback scope for macro #110150

georgewfraser opened this issue Nov 7, 2020 · 8 comments
Assignees
Labels
author-verification-requested Issues potentially verifiable by issue author feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@georgewfraser
Copy link

  • VSCode Version: 1.51.0
  • OS Version: MacOS 10.15.7

Steps to Reproduce:

  1. Run an extension that uses semantic coloring mark a token as "macro"
  2. Observe that no fallback scope is being applied:

Screen Shot 2020-11-07 at 10 47 49 AM

Does this issue occur when all extensions are disabled?: Yes/No

Not applicable, this bug can only occur when an extension is using macro syntax coloring.

@georgewfraser
Copy link
Author

@aeschli
Copy link
Contributor

aeschli commented Nov 9, 2020

entity.name.other.preprocessor.macro is defined as the fallback scope, but non of the built-in theme define any color for it.

Is there a better scope to use?

@Colengms Any suggestions?

@Colengms
Copy link
Contributor

Colengms commented Nov 9, 2020

@aeschli The TextMate scope for macro used in the original C/C++ semantic colorization feature was: entity.name.function.preprocessor, which we still use as a fallback.

https://code.visualstudio.com/docs/cpp/colorization-cpp

It was mentioned here, when discussing adding scopes to plus themes: #80783

The C/C++ extension currently uses the same semantic token for both function macros and non-function macros.

@georgewfraser
Copy link
Author

@aeschli If entity.name.other.preprocessor.macro is the fallback scope, why doesn't it show up in the tooltip? Perhaps that is a separate bug?

@aeschli
Copy link
Contributor

aeschli commented Nov 10, 2020

@georgewfraser The tooltip wants to show how the style at the current position was determined. For semantic highlighting it would show the theming rule if it had matched one. As current theme doesn't have theming rules to neither the token type (macro) nor the fallback scope(s), no such information is shown.
Not so much space in the hover, but I'm happy to improve it if we can do it in a space efficient way.

@aeschli aeschli changed the title macro fallback scope described in docs does not seem to exist [semantic highlighting] improve fallback scope for macro Nov 10, 2020
@aeschli aeschli added the feature-request Request for new features or functionality label Nov 10, 2020
@aeschli aeschli added this to the November 2020 milestone Nov 10, 2020
@aeschli
Copy link
Contributor

aeschli commented Nov 10, 2020

I pushed a fix to change the macro fallback scope to entity.name.function.preprocessor

bors bot added a commit to rust-lang/rust-analyzer that referenced this issue Nov 26, 2020
6496: Use builtin scopes more r=matklad a=georgewfraser

VSCode has added more builtin fallback scopes, so we can remove some of our fallback scopes by aligning with their conventions. 

Note that the macro scope doesn't seem to actually *work* at the moment. I have filed a bug with VSCode: microsoft/vscode#110150

Co-authored-by: George Fraser <george@fivetran.com>
@aeschli aeschli added the verification-needed Verification of issue is requested label Dec 1, 2020
@aeschli
Copy link
Contributor

aeschli commented Dec 1, 2020

To verify open the https://github.com/microsoft/vscode-extension-samples/tree/master/semantic-tokens-sample, add macro as a supported token. Use the 'Inspect token and scopes' command to verify that it is colored with the color associated with the entity.name.function.preprocessor scope

@rzhao271 rzhao271 added the author-verification-requested Issues potentially verifiable by issue author label Dec 3, 2020
@github-actions
Copy link

github-actions bot commented Dec 3, 2020

This bug has been fixed in to the latest release of VS Code Insiders!

@georgewfraser, you can help us out by confirming things are working as expected in the latest Insiders release. If things look good, please leave a comment with the text /verified to let us know. If not, please ensure you're on version 92192ba of Insiders (today's or later - you can use Help: About in the command palette to check), and leave a comment letting us know what isn't working as expected.

Happy Coding!

@JacksonKearl JacksonKearl added the verified Verification succeeded label Dec 4, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Dec 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
author-verification-requested Issues potentially verifiable by issue author feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

6 participants
@georgewfraser @aeschli @rzhao271 @JacksonKearl @Colengms and others