Hello, Jackson!

Thanks for the report. I never noticed this issue myself, since the
theme I'm using (Leuven) styles the css-property face. We should
definitely fix it.

Like you say, two possible solutions are:

1. Highlight variables with font-lock-constant-face instead of
   font-lock-variable-name-face. A downside is that this is backwards
   wrt. the intended meaning of the faces, so users will see variables
   highlighted in a face that's usually used for constants. An upside is
   that it will probably look nice with existing themes, since they are
   likely to already style font-lock-variable-name-face.

2. Change the css-property face. It doesn't mean it has to inherit from
   another one of the standard Font Lock faces, we could also just
   change the default foreground color, for instance. A downside of this
   approach is that users may be startled that the face that they were
   used to changed, but for themes that already style the css-property,
   everything will be like before. The upside is that
   font-lock-variable-name-face remains used for variables, like it's
   intended to.

I'm not sure which solution is best.

Either way, we should also add a defface for variables and use it for
Sass variables (and also CSS variables later on).

-- Simen

On Fri, Nov 13, 2015 at 7:37 AM, Jackson Hamilton 
<jackson@jacksonrayhamilton.com> wrote:
> I'd like to propose the following change to the scss-mode on master: 
> Use font-lock-constant-face for SCSS variables.
> 
> This may not seem intuitive from a naming perspective, but 
> font-lock-variable-name-face is already used for CSS properties. That 
> makes it harder to distinguish between properties and variables.
> 
> AFAIK, Sass doesn't even have constants, so I don't see much harm in 
> using this face. It'd be a less dramatic change for those who have 
> grown used to variable coloring for CSS properties.
> 
> I guess the alternative would be to inherit the property face from 
> something else, to free up the face for real variables. But then what 
> do we use for properties? (Inheriting from nothing doesn't look good 
> IMO.)
> 
> Attached is the proposed patch.