GNU bug report logs - #15745
[PATCH] Define semantic-idle-symbol-highlight-face with defface

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 15745 in the body.
You can then email your comments to 15745 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Barry OReilly <gundaetiapo <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Define semantic-idle-symbol-highlight-face with defface
Date: Mon, 28 Oct 2013 17:04:00 -0400

[Message part 1 (text/plain, inline)]

It would be more convenient to customize
semantic-idle-symbol-highlight-face in the manner defface usually
allows. However, semantic-idle-symbol-highlight-face is currently
defined with defvar.

The attached patch implements the fix.

[Message part 2 (text/html, inline)]

[semantic-face.diff (application/octet-stream, attachment)]

Message #8 received at 15745 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Barry OReilly <gundaetiapo <at> gmail.com>
Cc: 15745 <at> debbugs.gnu.org
Subject: Re: bug#15745: [PATCH] Define semantic-idle-symbol-highlight-face
 with defface
Date: Mon, 28 Oct 2013 17:38:47 -0400

New face names should not end in -face.

The standard backwards-compatible way to make a change like this is:

1) Add:
(defface semantic-idle-symbol-highlight ...)

2) Change:
(defvar semantic-idle-symbol-highlight-face 'semantic-idle-symbol-highlight)

3) Mark semantic-idle-symbol-highlight-face the variable obsolete,
in favour of semantic-idle-symbol-highlight the face.

See eg diary-face.

Message #11 received at 15745 <at> debbugs.gnu.org (full text, mbox):

From: Barry OReilly <gundaetiapo <at> gmail.com>
To: Glenn Morris <rgm <at> gnu.org>
Cc: 15745 <at> debbugs.gnu.org
Subject: Re: bug#15745: [PATCH] Define semantic-idle-symbol-highlight-face
 with defface
Date: Mon, 28 Oct 2013 18:36:51 -0400

[Message part 1 (text/plain, inline)]

Thanks, how is the attached revised patch?

[Message part 2 (text/html, inline)]

[semantic-face.diff (application/octet-stream, attachment)]

Message #14 received at 15745 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Barry OReilly <gundaetiapo <at> gmail.com>
Cc: 15745 <at> debbugs.gnu.org
Subject: Re: bug#15745: [PATCH] Define semantic-idle-symbol-highlight-face
 with defface
Date: Mon, 28 Oct 2013 21:38:41 -0400

> -(defvar semantic-idle-symbol-highlight-face 'region
> -  "Face used for highlighting local symbols.")
> +(defface semantic-idle-symbol-highlight-face
> +  '((((class color) (background dark))
> +     ;; Put this back to something closer to black later.
> +     (:background "gray20"))
> +    (((class color) (background light))
> +     (:background "gray90")))
> +  "Face used for highlighting local symbols."
> +  :group 'semantic-faces)

Just define the new face to inherit from `region', so as to preserve the
old behavior:

(defface semantic-idle-symbol-highlight
  '((t :inherit region))
  "Face used for highlighting local symbols.")


        Stefan

Message #17 received at 15745 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Barry OReilly <gundaetiapo <at> gmail.com>
Cc: 15745 <at> debbugs.gnu.org
Subject: Re: bug#15745: [PATCH] Define semantic-idle-symbol-highlight-face
 with defface
Date: Mon, 28 Oct 2013 22:29:57 -0400

Barry OReilly wrote:

> Thanks, how is the attached revised patch?

Not quite what I had in mind (taking something that was a variable and
turning it into a face alias won't work right, will it?).

-(defvar semantic-idle-symbol-highlight-face 'region
-  "Face used for highlighting local symbols.")
+(defface semantic-idle-symbol-highlight
+  '((((class color) (background dark))
+     ;; Put this back to something closer to black later.
+     (:background "gray20"))
+    (((class color) (background light))
+     (:background "gray90")))
+  "Face used for highlighting local symbols."
+  :group 'semantic-faces)

As has been said:

  (defface semantic-idle-symbol-highlight '((t (:inherit region))) ...)

+(define-obsolete-face-alias
+  'semantic-idle-symbol-highlight-face
+  'semantic-idle-symbol-highlight
+  "24.4")

Rather than the above, I meant:

  (make-obsolete-variable 'semantic-idle-symbol-highlight-face
    "customize the face `semantic-idle-symbol-highlight' instead" "24.4" 'set)

Then these bits:

-                     region semantic-idle-symbol-highlight-face)
+                     region 'semantic-idle-symbol-highlight)

are not necessary (or appropriate).

Message #20 received at 15745 <at> debbugs.gnu.org (full text, mbox):

From: Barry OReilly <gundaetiapo <at> gmail.com>
To: Glenn Morris <rgm <at> gnu.org>
Cc: 15745 <at> debbugs.gnu.org
Subject: Re: bug#15745: [PATCH] Define semantic-idle-symbol-highlight-face
 with defface
Date: Tue, 29 Oct 2013 11:56:51 -0400

[Message part 1 (text/plain, inline)]

Thank you for your patience and guidance.

> Just define the new face to inherit from `region', so as to preserve
> the old behavior:
>
> (defface semantic-idle-symbol-highlight
>   '((t :inherit region))
>   "Face used for highlighting local symbols.")

It's weird that the region face would be used for this, because it has
nothing to do with the region. The fact that it can make me think I
have a region marked when I don't is a motivator for this bug report.
My primary concern is that I can customize it personally, so I revised
the patch to have the face inherit region.

> Not quite what I had in mind (taking something that was a variable
> and turning it into a face alias won't work right, will it?).

I probably misinterpreted your earlier tip. I looked off of
diary-face:

  (define-obsolete-face-alias 'diary-face 'diary "22.1")

I tested the last patch before sending it and it worked. Although that
patch used the defface variable and not the obsoleted one.

> Then these bits:
>
> -                     region semantic-idle-symbol-highlight-face)
> +                     region 'semantic-idle-symbol-highlight)
>
> are not necessary (or appropriate).

I don't know why using obsolete variables is most appropriate, but
I've done as requested in the revised attached patch.

[Message part 2 (text/html, inline)]

[semantic-face.diff (application/octet-stream, attachment)]

Message #23 received at 15745 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Barry OReilly <gundaetiapo <at> gmail.com>
Cc: 15745 <at> debbugs.gnu.org
Subject: Re: bug#15745: [PATCH] Define semantic-idle-symbol-highlight-face
 with defface
Date: Tue, 29 Oct 2013 18:37:45 -0400

Barry OReilly wrote:

> It's weird that the region face would be used for this, because it has
> nothing to do with the region. The fact that it can make me think I
> have a region marked when I don't is a motivator for this bug report.

Wanting to change the default appearance is a separate issue! :)
You could ask cedet-devel.

> I probably misinterpreted your earlier tip. I looked off of
> diary-face:
>
>   (define-obsolete-face-alias 'diary-face 'diary "22.1")

Sorry, diary-face was a bad, confusing example for me to pick.
I was looking in diary-lib.el, not calendar.el.

> I've done as requested in the revised attached patch.

Looks fine to me.

Message #28 received at 15745-done <at> debbugs.gnu.org (full text, mbox):

From: Barry OReilly <gundaetiapo <at> gmail.com>
Cc: 15745-done <at> debbugs.gnu.org
Subject: Re: bug#15745: [PATCH] Define semantic-idle-symbol-highlight-face
 with defface
Date: Wed, 30 Oct 2013 21:55:31 -0400

[Message part 1 (text/plain, inline)]

Rev 114879

[Message part 2 (text/html, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.