facelessuser / ColorHelper

Sublime plugin that provides helpful color previews and tooltips
https://facelessuser.github.io/ColorHelper/
MIT License
256 stars 31 forks source link

ColorHelper 3.0 Showing Previews for SCSS/SASS variables that are the same as an HTML color name #170

Closed mrwweb closed 3 years ago

mrwweb commented 3 years ago

Description

After upgraded to ColorHelper 3.0, I see the following in a .scss file:

An SCSS variable $blue includes has the HTML color blue's swatch after the $

$blue is an SCSS/SASS variable for a color and is not a reference to the HTML color "blue":

Definition of $blue variable shows correct color

This was correctly not highlighted in previous versions. It would be awesome if the correct color was shown (#142), but I'd be more than happy to just not have this highlighted.

Support Info

Steps to Reproduce Issue

Use any SCSS variable with a name that matches a named HTML color as a CSS property value.

color: $blue;
color: $tomato;
color: $rebeccapurple;
color: $darkgoldenrod;
facelessuser commented 3 years ago

@mrwweb Please provide what syntax package you use for your SCSS. As you can see below, it can vary depending on what SCSS file you use:

Screen Shot 2021-03-12 at 3 15 54 PM

I didn't go through and test every package out there that does SCSS, and the ones I did test, I probably only made a passing glance to make sure it was responding to colors.

facelessuser commented 3 years ago

Oh, never mind, I see we were talking about the variables on the right hand side.

facelessuser commented 3 years ago

I have a fix 🙁.

Screen Shot 2021-03-12 at 3 21 29 PM

We just need to make sure our trigger for color words doesn't have a $ before it:

(?:\b(?<![-#&$])[\w]{3,}(?!\()\b
mrwweb commented 3 years ago

Glad you figured it out! That change makes sense to me and I can't think of any obvious reason why that would have unintended consequences. In fact, I assume the old version must have worked that way?

facelessuser commented 3 years ago

Glad you figured it out! That change makes sense to me and I can't think of any obvious reason why that would have unintended consequences. In fact, I assume the old version must have worked that w

Yeah, it is one of those things that I missed in the rewrite. We want to exclude it from looking like SCSS variables, CSS variables, HTML entities, functions calls, etc. Part of that gets filtered out by scope rules, but some syntax files don't have a resolution that makes that easy, so we also try to do a few lookbehinds and lookaheads on color words. Color words are honestly the trickiest. Things like rgba(...) are far less likely to give you a false positive.

This made me double-check, and I put in a fix for color words with a trailing - as they may be part of a CSS variable and such.

mrwweb commented 3 years ago

Awesome. That all makes sense! Glad this was an easy fix :) Congrats on getting 3.0 out the door! It seems pretty great so far.

facelessuser commented 3 years ago

Thanks! If all the missed bugs are just like this, I'll be pretty happy.