* bug#21898: scss-mode font-lock face for variables
@ 2015-11-13 6:37 Jackson Hamilton
2015-11-13 20:29 ` Simen Heggestøyl
0 siblings, 1 reply; 5+ messages in thread
From: Jackson Hamilton @ 2015-11-13 6:37 UTC (permalink / raw)
To: 21898
[-- Attachment #1.1: Type: text/plain, Size: 747 bytes --]
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.
[-- Attachment #1.2: Type: text/html, Size: 885 bytes --]
[-- Attachment #2: 0001-Use-font-lock-constant-face-for-scss-variables.patch --]
[-- Type: text/x-patch, Size: 959 bytes --]
From 72fb4670228324b59531a19409618277b62ad230 Mon Sep 17 00:00:00 2001
From: Jackson Ray Hamilton <jackson@jacksonrayhamilton.com>
Date: Thu, 12 Nov 2015 22:21:38 -0800
Subject: [PATCH] Use `font-lock-constant-face' for scss variables
* css-mode.el (scss-font-lock-keywords): Change face.
---
lisp/textmodes/css-mode.el | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lisp/textmodes/css-mode.el b/lisp/textmodes/css-mode.el
index 3e84b43..e35e6cb 100644
--- a/lisp/textmodes/css-mode.el
+++ b/lisp/textmodes/css-mode.el
@@ -525,7 +525,7 @@ pseudo-classes, and at-rules."
st))
(defvar scss-font-lock-keywords
- (append `((,(concat "$" css-ident-re) (0 font-lock-variable-name-face)))
+ (append `((,(concat "$" css-ident-re) (0 font-lock-constant-face)))
(css--font-lock-keywords 'sassy)
`((,(concat "@mixin[ \t]+\\(" css-ident-re "\\)[ \t]*(")
(1 font-lock-function-name-face)))))
--
2.6.3
^ permalink raw reply related [flat|nested] 5+ messages in thread* bug#21898: scss-mode font-lock face for variables
2015-11-13 6:37 bug#21898: scss-mode font-lock face for variables Jackson Hamilton
@ 2015-11-13 20:29 ` Simen Heggestøyl
2017-07-29 16:46 ` Simen Heggestøyl
0 siblings, 1 reply; 5+ messages in thread
From: Simen Heggestøyl @ 2015-11-13 20:29 UTC (permalink / raw)
To: Jackson Hamilton; +Cc: 21898, Stefan Monnier
[-- Attachment #1: Type: text/plain, Size: 2165 bytes --]
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.
[-- Attachment #2: Type: text/html, Size: 2761 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-07-30 9:33 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-13 6:37 bug#21898: scss-mode font-lock face for variables Jackson Hamilton
2015-11-13 20:29 ` Simen Heggestøyl
2017-07-29 16:46 ` Simen Heggestøyl
2017-07-29 17:24 ` Jackson Ray Hamilton
2017-07-30 9:33 ` Simen Heggestøyl
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).