bug#15427: 24.3.50; `customize-face' in Customize, show `Background' attribute using background

bug#15427: 24.3.50; `customize-face' in Customize, show `Background' attribute using background

[Drew Adams]

emacs -Q
M-x customize-face lazy-highlight

Attribute `Background' has its own `sample', which shows the color.
But it shows the color as a foreground (text).  Show it as the
background, instead.

This is independent of the overall `sample' for the face as a whole.
It's just more helpful to show the background color choice as a
background.



In GNU Emacs 24.3.50.1 (i686-pc-mingw32)
 of 2013-08-31 on ODIEONE
Bzr revision: 114088 rgm@gnu.org-20130901021232-sazo0qla2qgcko95
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --prefix=/c/Devel/emacs/binary --enable-checking=yes,glyphs
 'CFLAGS=-O0 -g3' LDFLAGS=-Lc:/Devel/emacs/lib
 CPPFLAGS=-Ic:/Devel/emacs/include'

bug#15427: 24.3.50; `customize-face' in Customize, show `Background' attribute using background

[Lars Ingebrigtsen]

Drew Adams <drew.adams@oracle.com> writes:

> emacs -Q
> M-x customize-face lazy-highlight
>
> Attribute `Background' has its own `sample', which shows the color.
> But it shows the color as a foreground (text).  Show it as the
> background, instead.
>
> This is independent of the overall `sample' for the face as a whole.
> It's just more helpful to show the background color choice as a
> background.

There's a whole bunch of things that can have a colour when customising
a face -- like stipple background/foreground, overline, etc.  I don't
know whether it makes sense to special-case just the (main) background
colour...  but perhaps that does make sense, because it's a pretty
common thing to customise.  And showing the colour as a background
colour would give a more correct impressions.

But might be confusing if that one thing behaved differently than the
rest.

Anybody have an opinion here?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#15427: 24.3.50; `customize-face' in Customize, show `Background' attribute using background

[Drew Adams]

> > emacs -Q
> > M-x customize-face lazy-highlight
> >
> > Attribute `Background' has its own `sample', which shows the color.
> > But it shows the color as a foreground (text).  Show it as the
> > background, instead.
> >
> > This is independent of the overall `sample' for the face as a whole.
> > It's just more helpful to show the background color choice as a
> > background.
> 
> There's a whole bunch of things that can have a colour when customising
> a face -- like stipple background/foreground, overline, etc.  I don't
> know whether it makes sense to special-case just the (main) background
> colour...  but perhaps that does make sense, because it's a pretty
> common thing to customise.  And showing the colour as a background
> colour would give a more correct impressions.
> 
> But might be confusing if that one thing behaved differently than the
> rest.
> 
> Anybody have an opinion here?

It's _not_ about one thing behaving differently.

We show an overline, underline, foreground, etc.
attribute using the foreground, because they are,
well, in the foreground.

Attribute background is not.  Attribute background
should, well, show the color sample as background,
not as foreground.  (I would think that this is
obviously TRT to do.)

bug#15427: 24.3.50; `customize-face' in Customize, show `Background' attribute using background

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Wed, 14 Aug 2019 20:36:13 -0700
> Cc: 15427@debbugs.gnu.org
> 
> Anybody have an opinion here?

My opinion is that there's nothing wrong with showing the color as
foreground.

bug#15427: 24.3.50; `customize-face' in Customize, show `Background' attribute using background

[Juri Linkov]

>> emacs -Q
>> M-x customize-face lazy-highlight
>>
>> Attribute `Background' has its own `sample', which shows the color.
>> But it shows the color as a foreground (text).  Show it as the
>> background, instead.
>>
>> This is independent of the overall `sample' for the face as a whole.
>> It's just more helpful to show the background color choice as a
>> background.
>
> There's a whole bunch of things that can have a colour when customising
> a face -- like stipple background/foreground, overline, etc.  I don't
> know whether it makes sense to special-case just the (main) background
> colour...  but perhaps that does make sense, because it's a pretty
> common thing to customise.  And showing the colour as a background
> colour would give a more correct impressions.

Using background colors is more helpful for users.  Please see also
bug#33799 (set-foreground-color completion shows background colors).

bug#15427: 24.3.50; `customize-face' in Customize, show `Background' attribute using background

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

> Using background colors is more helpful for users. 

I tried having a peek at the code to see whether this would be an easy
thing to fix, and it applies the colour here:

(defun widget-color-sample-face-get (widget)
  (let* ((value (condition-case nil
		    (widget-value widget)
		  (error (widget-get widget :value)))))
    (if (color-defined-p value)
	(list (cons 'foreground-color value))
      'default)))

But, as usual when dealing with widgets, I had to give up, because I can
never follow the calling logic (everything seems to happen somewhere
else), and I don't know how to inspect the widgets, either.

If somebody else wants to take a whack at it, be my guest.  It's
probably trivial if you know how widgets work.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#15427: 24.3.50; `customize-face' in Customize, show `Background' attribute using background

[Drew Adams]

FWIW, I agree about the widget and custom code.
And it's doc is not super helpful.

(defun widget-color-sample-face-get (widget)
  (let* ((value (condition-case nil
		    (widget-value widget)
		  (error (widget-get widget :value)))))
    (if (color-defined-p value)
	(list (cons 'background-color value))
      'default)))

But I think, as said in the enhancement
request, that it's good to show the
effect of the color BOTH as a foreground
and as a background.