list-colors-display: filter same adjecent colors

list-colors-display: filter same adjecent colors

[Michael Mauger]

This was another conversation from November.  

See http://mail.gnu.org/archive/html/emacs-devel/2003-11/msg00189.html

Currently a color in the color list is not shown if the previous color
was the same numeric color value.  This patch filters out the color if
the *names* are the same (i.e., "gray" and "grey" are the same,
whitespace and letter case are ignored).  

The benefit of this change is that under w32 we have added colors based
on the current desktop theme.  Some of these colors may be the same but
logically be different colors.

As an added benefit, on my slow machine, the patch runs faster than the
original code.


Index: emacs/lisp/facemenu.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/facemenu.el,v
retrieving revision 1.71
diff -u -b -r1.71 facemenu.el
--- emacs/lisp/facemenu.el      1 Sep 2003 15:45:11 -0000       1.71
+++ emacs/lisp/facemenu.el      18 Nov 2003 18:04:08 -0000
@@ -508,12 +510,19 @@

 (defun facemenu-color-equal (a b)
   "Return t if colors A and B are the same color.
-A and B should be strings naming colors.
-This function queries the display system to find out what the color
-names mean.  It returns nil if the colors differ or if it can't
-determine the correct answer."
-  (cond ((equal a b) t)
-       ((equal (color-values a) (color-values b)))))
+A and B should be strings naming colors.  These names are
+downcased, stripped of spaces and the string `grey' is turned
+into `gray'.  This accomidates alternative spellings of colors
+found commonly in the list.  It returns nil if the colors differ."
+  (progn
+    (setq a (replace-regexp-in-string "grey" "gray"
+            (replace-regexp-in-string " " ""
+             (downcase a)))
+         b (replace-regexp-in-string "grey" "gray"
+            (replace-regexp-in-string " " ""
+             (downcase b))))
+
+    (equal a b)))

 (defun facemenu-add-face (face &optional start end)
   "Add FACE to text between START and END.


__________________________________
Do you Yahoo!?
Yahoo! Mail SpamGuard - Read only the mail you want.
http://antispam.yahoo.com/tools

Re: list-colors-display: filter same adjecent colors

[Eli Zaretskii]

> Date: Mon, 23 Feb 2004 19:43:58 -0800 (PST)
> From: Michael Mauger <mmaug@yahoo.com>
> 
> This was another conversation from November.  
> 
> See http://mail.gnu.org/archive/html/emacs-devel/2003-11/msg00189.html

Thanks for following up.

>  (defun facemenu-color-equal (a b)
>    "Return t if colors A and B are the same color.
> -A and B should be strings naming colors.
> -This function queries the display system to find out what the color
> -names mean.  It returns nil if the colors differ or if it can't
> -determine the correct answer."
> -  (cond ((equal a b) t)
> -       ((equal (color-values a) (color-values b)))))
> +A and B should be strings naming colors.  These names are
> +downcased, stripped of spaces and the string `grey' is turned
> +into `gray'.  This accomidates alternative spellings of colors
> +found commonly in the list.  It returns nil if the colors differ."
> +  (progn
> +    (setq a (replace-regexp-in-string "grey" "gray"
> +            (replace-regexp-in-string " " ""
> +             (downcase a)))
> +         b (replace-regexp-in-string "grey" "gray"
> +            (replace-regexp-in-string " " ""
> +             (downcase b))))
> +
> +    (equal a b)))

Hmm, how about making this a new function, and leaving the original
facemenu-color-equal alone?  None of the Lisp files bundled with
Emacs use facemenu-color-equal, but perhaps some add-on packages do,
as it sounds like useful functionality and is there since 1994.

Also, I think we should mention the MS-Windows case and the special
color names used there explicitly, at least in a comment to the
function's code, if not in the doc string.  A year from now, no one
will remember why we modified the way colors are compared.

Re: list-colors-display: filter same adjecent colors

[Michael Mauger]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=us-ascii, Size: 3942 bytes --]


--- Eli Zaretskii <eliz@elta.co.il> wrote:
> > Date: Mon, 23 Feb 2004 19:43:58 -0800 (PST)
> > From: Michael Mauger <mmaug@yahoo.com>
> > 
> > This was another conversation from November.  
> > 
> > See
> http://mail.gnu.org/archive/html/emacs-devel/2003-11/msg00189.html
> 
> Thanks for following up.
> 
> >  (defun facemenu-color-equal (a b)
> >    "Return t if colors A and B are the same color.
> > -A and B should be strings naming colors.
> > -This function queries the display system to find out what the color
> > -names mean.  It returns nil if the colors differ or if it can't
> > -determine the correct answer."
> > -  (cond ((equal a b) t)
> > -       ((equal (color-values a) (color-values b)))))
> > +A and B should be strings naming colors.  These names are
> > +downcased, stripped of spaces and the string `grey' is turned
> > +into `gray'.  This accomidates alternative spellings of colors
> > +found commonly in the list.  It returns nil if the colors differ."
> > +  (progn
> > +    (setq a (replace-regexp-in-string "grey" "gray"
> > +            (replace-regexp-in-string " " ""
> > +             (downcase a)))
> > +         b (replace-regexp-in-string "grey" "gray"
> > +            (replace-regexp-in-string " " ""
> > +             (downcase b))))
> > +
> > +    (equal a b)))
> 
> Hmm, how about making this a new function, and leaving the original
> facemenu-color-equal alone?  None of the Lisp files bundled with
> Emacs use facemenu-color-equal, but perhaps some add-on packages do,
> as it sounds like useful functionality and is there since 1994.
> 
> Also, I think we should mention the MS-Windows case and the special
> color names used there explicitly, at least in a comment to the
> function's code, if not in the doc string.  A year from now, no one
> will remember why we modified the way colors are compared.
> 

How does this patch look:

Index: emacs/lisp/facemenu.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/facemenu.el,v
retrieving revision 1.71
diff -u -r1.71 facemenu.el
--- emacs/lisp/facemenu.el	1 Sep 2003 15:45:11 -0000	1.71
+++ emacs/lisp/facemenu.el	27 Feb 2004 04:57:36 -0000
@@ -480,9 +480,15 @@
   (when (and (null list) (> (display-color-cells) 0))
     (setq list (defined-colors))
     ;; Delete duplicate colors.
+
+    ;; Identify duplicate colors by the name rather than the color
+    ;; value.  On w32, logical colors are added to the list that might
+    ;; have the same value but have different names and meanings.
+    ;; Detecting duplicates by name insures that all of these logical
+    ;; colors remain despite similar color values.
     (let ((l list))
       (while (cdr l)
-	(if (facemenu-color-equal (car l) (car (cdr l)))
+	(if (facemenu-color-name-equal (car l) (car (cdr l)))
 	    (setcdr l (cdr (cdr l)))
 	  (setq l (cdr l)))))
     (when (memq (display-visual-class) '(gray-scale pseudo-color
direct-color))
@@ -514,6 +520,22 @@
 determine the correct answer."
   (cond ((equal a b) t)
 	((equal (color-values a) (color-values b)))))
+
+(defun facemenu-color-name-equal (a b)
+  "Return t if colors A and B are the same color.
+A and B should be strings naming colors.  These names are
+downcased, stripped of spaces and the string `grey' is turned
+into `gray'.  This accomidates alternative spellings of colors
+found commonly in the list.  It returns nil if the colors differ."
+  (progn
+    (setq a (replace-regexp-in-string "grey" "gray"
+            (replace-regexp-in-string " " ""
+             (downcase a)))
+         b (replace-regexp-in-string "grey" "gray"
+            (replace-regexp-in-string " " ""
+             (downcase b))))
+
+    (equal a b)))
 
 (defun facemenu-add-face (face &optional start end)
   "Add FACE to text between START and END.


__________________________________
Do you Yahoo!?
Yahoo! Search - Find what you’re looking for faster
http://search.yahoo.com

Re: list-colors-display: filter same adjecent colors

[Eli Zaretskii]

> Date: Fri, 5 Mar 2004 15:49:19 -0800 (PST)
> From: Michael Mauger <mmaug@yahoo.com>
> 
> How does this patch look:

It's okay with me, but I'd suggest to give an explicit example in
this comment:

> +    ;; Identify duplicate colors by the name rather than the color
> +    ;; value.  On w32, logical colors are added to the list that might
> +    ;; have the same value but have different names and meanings.
> +    ;; Detecting duplicates by name insures that all of these logical
> +    ;; colors remain despite similar color values.

I'd mention the special ``system'' colors and why they are important,
just so people could see a specific example.

Thanks.

Re: list-colors-display: filter same adjecent colors

[Michael Mauger]

Sorry for the delay on this -- I've been offline for a bit.

How does this version (with an expanded comment block) look?

Index: emacs/lisp/facemenu.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/facemenu.el,v
retrieving revision 1.71
diff -u -r1.71 facemenu.el
--- emacs/lisp/facemenu.el	1 Sep 2003 15:45:11 -0000	1.71
+++ emacs/lisp/facemenu.el	13 Mar 2004 21:59:20 -0000
@@ -480,9 +480,19 @@
   (when (and (null list) (> (display-color-cells) 0))
     (setq list (defined-colors))
     ;; Delete duplicate colors.
+
+    ;; Identify duplicate colors by the name rather than the color
+    ;; value.  On w32, logical colors are added to the list that might
+    ;; have the same value but have different names and meanings.  For
+    ;; eaxmple, `SystemMenuText' (the color w32 uses for the text in
+    ;; menu entries) and `SystemWindowText' (the default color w32
+    ;; uses for the text in windows and dialogs) may be the same
+    ;; display color and be adjacent in the list.  Detecting
+    ;; duplicates by name insures that both of these colors remain
+    ;; despite similar color values.
     (let ((l list))
       (while (cdr l)
-	(if (facemenu-color-equal (car l) (car (cdr l)))
+	(if (facemenu-color-name-equal (car l) (car (cdr l)))
 	    (setcdr l (cdr (cdr l)))
 	  (setq l (cdr l)))))
     (when (memq (display-visual-class) '(gray-scale pseudo-color
direct-color))
@@ -514,6 +524,22 @@
 determine the correct answer."
   (cond ((equal a b) t)
 	((equal (color-values a) (color-values b)))))
+
+(defun facemenu-color-name-equal (a b)
+  "Return t if colors A and B are the same color.
+A and B should be strings naming colors.  These names are
+downcased, stripped of spaces and the string `grey' is turned
+into `gray'.  This accomidates alternative spellings of colors
+found commonly in the list.  It returns nil if the colors differ."
+  (progn
+    (setq a (replace-regexp-in-string "grey" "gray"
+            (replace-regexp-in-string " " ""
+             (downcase a)))
+         b (replace-regexp-in-string "grey" "gray"
+            (replace-regexp-in-string " " ""
+             (downcase b))))
+
+    (equal a b)))
 
 (defun facemenu-add-face (face &optional start end)
   "Add FACE to text between START and END.


--- Eli Zaretskii <eliz@elta.co.il> wrote:
> > Date: Fri, 5 Mar 2004 15:49:19 -0800 (PST)
> > From: Michael Mauger <mmaug@yahoo.com>
> > 
> > How does this patch look:
> 
> It's okay with me, but I'd suggest to give an explicit example in
> this comment:
> 
> > +    ;; Identify duplicate colors by the name rather than the color
> > +    ;; value.  On w32, logical colors are added to the list that
> might
> > +    ;; have the same value but have different names and meanings.
> > +    ;; Detecting duplicates by name insures that all of these
> logical
> > +    ;; colors remain despite similar color values.
> 
> I'd mention the special ``system'' colors and why they are important,
> just so people could see a specific example.
> 
> Thanks.


__________________________________
Do you Yahoo!?
Yahoo! Small Business $15K Web Design Giveaway 
http://promotions.yahoo.com/design_giveaway/

Re: list-colors-display: filter same adjecent colors

[Eli Zaretskii]

> Date: Mon, 5 Apr 2004 20:08:29 -0700 (PDT)
> From: Michael Mauger <mmaug@yahoo.com>
> 
> Sorry for the delay on this -- I've been offline for a bit.
> 
> How does this version (with an expanded comment block) look?

Looks very good to me.  Thanks!

Re: list-colors-display: filter same adjecent colors

[Michael Mauger]

I think we had agreed on this one.  Can it be committed?

Index: emacs/lisp/facemenu.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/facemenu.el,v
retrieving revision 1.71
diff -u -r1.71 facemenu.el
--- emacs/lisp/facemenu.el	1 Sep 2003 15:45:11 -0000	1.71
+++ emacs/lisp/facemenu.el	13 Mar 2004 21:59:20 -0000
@@ -480,9 +480,19 @@
   (when (and (null list) (> (display-color-cells) 0))
     (setq list (defined-colors))
     ;; Delete duplicate colors.
+
+    ;; Identify duplicate colors by the name rather than the color
+    ;; value.  On w32, logical colors are added to the list that might
+    ;; have the same value but have different names and meanings.  For
+    ;; example, `SystemMenuText' (the color w32 uses for the text in
+    ;; menu entries) and `SystemWindowText' (the default color w32
+    ;; uses for the text in windows and dialogs) may be the same
+    ;; display color and be adjacent in the list.  Detecting
+    ;; duplicates by name insures that both of these colors remain
+    ;; despite similar color values.
     (let ((l list))
       (while (cdr l)
-	(if (facemenu-color-equal (car l) (car (cdr l)))
+	(if (facemenu-color-name-equal (car l) (car (cdr l)))
 	    (setcdr l (cdr (cdr l)))
 	  (setq l (cdr l)))))
     (when (memq (display-visual-class) '(gray-scale pseudo-color
direct-color))
@@ -514,6 +524,22 @@
 determine the correct answer."
   (cond ((equal a b) t)
 	((equal (color-values a) (color-values b)))))
+
+(defun facemenu-color-name-equal (a b)
+  "Return t if colors A and B are the same color.
+A and B should be strings naming colors.  These names are
+downcased, stripped of spaces and the string `grey' is turned
+into `gray'.  This accommodates alternative spellings of colors
+found commonly in the list.  It returns nil if the colors differ."
+  (progn
+    (setq a (replace-regexp-in-string "grey" "gray"
+            (replace-regexp-in-string " " ""
+             (downcase a)))
+         b (replace-regexp-in-string "grey" "gray"
+            (replace-regexp-in-string " " ""
+             (downcase b))))
+
+    (equal a b)))
  
 (defun facemenu-add-face (face &optional start end)
   "Add FACE to text between START and END.

Re: list-colors-display: filter same adjecent colors

[Eli Zaretskii]

> Date: Tue, 18 May 2004 14:13:15 -0700 (PDT)
> From: Michael Mauger <mmaug@yahoo.com>
> 
> I think we had agreed on this one.  Can it be committed?

Oops!  I thought it was committed eons ago.  Done.

Thanks.