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

semanticHighlighting: Support numbers in "constant" properties #92469

Closed
christianalfoni opened this issue Mar 11, 2020 · 16 comments
Closed

semanticHighlighting: Support numbers in "constant" properties #92469

christianalfoni opened this issue Mar 11, 2020 · 16 comments
Assignees
Labels
info-needed Issue requires more information from poster

Comments

@christianalfoni
Copy link

christianalfoni commented Mar 11, 2020

With the following code:

const colors = {
  RED: 'red',
  RED_PRIMARY: 'red',
  RED_500: 'red'
}

We get the following behaviour with the semanticHighlighting highlighting:

colors.RED // "RED" is highlighted
colors.RED_PRIMARY // "RED_PRIMARY" is highlighted
colors.RED_500 // "RED_500" is NOT highlighted... look, also Github highlights it :-)

I would expect colors.RED_500 to be highlighted just like the two other properties.

This could maybe be considered a bug, but it could also be considered a lacking feature 😄

@vscodebot
Copy link

vscodebot bot commented Mar 11, 2020

(Experimental duplicate detection)
Thanks for submitting this issue. Please also check if it is already covered by an existing one, like:

@IllusionMH
Copy link
Contributor

/needsMoreInfo

Could you provide screenshot?
What version do you use?
Is it reproducible with all extensions disabled?
You can try this with F1 and >Developer: Reload Window With Extensions Disabled

I see no difference in highlight (VS Code Insiders)
image

@vscodebot vscodebot bot added the info-needed Issue requires more information from poster label Mar 11, 2020
@vscodebot
Copy link

vscodebot bot commented Mar 11, 2020

Thanks for creating this issue! We figured it's missing some basic information or in some other way doesn't follow our issue reporting guidelines. Please take the time to review these and update the issue.

Happy Coding!

@christianalfoni
Copy link
Author

christianalfoni commented Mar 11, 2020

Hi there and thanks for the quick response!

I think I might have been confused by how the different themes does the highlighting. For example:

This is Dracula Soft without the semanticHighlighting:

Screenshot 2020-03-11 at 13 31 08

And this happens when I add the semanticHighlighting:

Screenshot 2020-03-11 at 13 30 55

Though, with Monokai Dimmed, the complete opposite happens. This is without semanticHighlighting:

Screenshot 2020-03-11 at 13 31 44

And this is with semanticHighlighting:

Screenshot 2020-03-11 at 13 31 55

Though this theme seems to color all properties the same way, they do not differentiate "constant" properties from normal properties.

@christianalfoni
Copy link
Author

christianalfoni commented Mar 11, 2020

Btw, I messed around with the config a bit:

  "editor.tokenColorCustomizationsExperimental": {
    "property": {
      "foreground": "#f51b00"
    }
  }

Which results in this strange behaviour:

Screenshot 2020-03-11 at 13 43 40

The underlying theme comes through on RED500 there

@christianalfoni
Copy link
Author

It looks to me that:

		<dict>
			<key>name</key>
			<string>User-defined constant</string>
			<key>scope</key>
			<string>constant.character, constant.other</string>
			<key>settings</key>
			<dict>
				<key>foreground</key>
				<string>#be9af0</string>
			</dict>
		</dict>

User defined constants breaks? Maybe there is no term for that in the semanticHighlighting?

@IllusionMH
Copy link
Contributor

/cc @aeschli

Strange, if you use RED500 (or RED200) there is an extra token
image

Unfortunately after couple of tests this token dissapeared.

@christianalfoni
Copy link
Author

christianalfoni commented Mar 11, 2020

Ah, look there, yeah... but it seems to me that semanticHighlighting does not support differentiating a property from a constant property then?

If not, then that would indeed be my feature request 😄

@aeschli
Copy link
Contributor

aeschli commented Mar 11, 2020

@christianalfoni How do you decide it a constant property? Because it is all uppercase?

@aeschli
Copy link
Contributor

aeschli commented Mar 11, 2020

I don't see any bugs with the example provided. When I try is, all X in colors.X get the same classification (property). Maybe this is right after editing and semantic highlighting has not yet been applied (I think that what @IllusionMH sees in his example)?
Can you also use the inspect tool?

image

The difference to syntax highlighting is:

  • object keys are classified as properties (can be discussed)
  • it's not readonly, because it isn't really. To formally declare it as readonly, you could define an interface where all the properties are marked as readonly:

image

Here I see that we lack a rule to associate property.readonly to variable.other.constant.property. I will add that.

@christianalfoni
Copy link
Author

@aeschli Hi there!

Ah, this is great. Cause:

  1. Yes, I see the semantic "overlay" is a bit delayed turning it on and off, so this is very likely what is being experienced

  2. Adding readonly will actually fix the issue related to the library I work on, so that is supergreat!

And yeah, constant is meant as all uppercase :-)

@aeschli
Copy link
Contributor

aeschli commented Mar 16, 2020

Do you still see RED_500 not highlighted? Is it always reproducible?

@christianalfoni
Copy link
Author

Oh, is this already released? Or how would I approach testing it?

@aeschli
Copy link
Contributor

aeschli commented Mar 16, 2020

It's in the latest insiders.

@vscodebot vscodebot bot closed this as completed Mar 23, 2020
@vscodebot
Copy link

vscodebot bot commented Mar 23, 2020

This issue has been closed automatically because it needs more information and has not had recent activity. See also our issue reporting guidelines.

Happy Coding!

@christianalfoni
Copy link
Author

Hi there!

Very sorry for my late reply, but I found time to check this now and it works! Great 😄 👍 Thanks for looking into this!

@github-actions github-actions bot locked and limited conversation to collaborators May 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
info-needed Issue requires more information from poster
Projects
None yet
Development

No branches or pull requests

3 participants