microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
163.57k stars 29.02k forks source link

semanticHighlighting: Support numbers in "constant" properties #92469

Closed christianalfoni closed 4 years ago

christianalfoni commented 4 years ago

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[bot] commented 4 years ago

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

IllusionMH commented 4 years ago

/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[bot] commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

/cc @aeschli

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

Unfortunately after couple of tests this token dissapeared.

christianalfoni commented 4 years ago

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 commented 4 years ago

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

aeschli commented 4 years ago

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:

image

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

christianalfoni commented 4 years ago

@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 commented 4 years ago

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

christianalfoni commented 4 years ago

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

aeschli commented 4 years ago

It's in the latest insiders.

vscodebot[bot] commented 4 years ago

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 commented 4 years ago

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!