[PATCH] * etc/themes/wombat-theme.el: Don't set foreground on region

[PATCH] * etc/themes/wombat-theme.el: Don't set foreground on region

[Mitchel Humpherys]

Setting a foreground color on the `region' face is slightly annoying.
This is subjective, of course, but the consensus among other themes and
around the web is to *not* set a foreground color on the region face.
---
 etc/themes/wombat-theme.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/etc/themes/wombat-theme.el b/etc/themes/wombat-theme.el
index 08ae12f2b13b..fec910f7fb53 100644
--- a/etc/themes/wombat-theme.el
+++ b/etc/themes/wombat-theme.el
@@ -36,7 +36,7 @@ wombat
    `(fringe ((,class (:background "#303030"))))
    `(highlight ((,class (:background "#454545" :foreground "#ffffff"
 			 :underline t))))
-   `(region ((,class (:background "#444444" :foreground "#f6f3e8"))))
+   `(region ((,class (:background "#444444"))))
    `(secondary-selection ((,class (:background "#333366" :foreground "#f6f3e8"))))
    `(isearch ((,class (:background "#343434" :foreground "#857b6f"))))
    `(lazy-highlight ((,class (:background "#384048" :foreground "#a0a8b0"))))

Re: [PATCH] * etc/themes/wombat-theme.el: Don't set foreground on region

[Mitchel Humpherys]

On Mon, Feb 22 2016 at 05:04:31 PM, Mitchel Humpherys <mitch.special@gmail.com> wrote:
> Setting a foreground color on the `region' face is slightly annoying.
> This is subjective, of course, but the consensus among other themes and
> around the web is to *not* set a foreground color on the region face.

Any objections to this?  I should have mentioned that the reason it's
slightly annoying is because setting a foreground color hides existing
syntax highlighting within the region text.

-- 
Mitch

Re: [PATCH] * etc/themes/wombat-theme.el: Don't set foreground on region

[Lars Ingebrigtsen]

Mitchel Humpherys <mitch.special@gmail.com> writes:

> On Mon, Feb 22 2016 at 05:04:31 PM, Mitchel Humpherys
> <mitch.special@gmail.com> wrote:
>> Setting a foreground color on the `region' face is slightly annoying.
>> This is subjective, of course, but the consensus among other themes and
>> around the web is to *not* set a foreground color on the region face.
>
> Any objections to this?  I should have mentioned that the reason it's
> slightly annoying is because setting a foreground color hides existing
> syntax highlighting within the region text.

The patch sounds OK to me.

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

Re: [PATCH] * etc/themes/wombat-theme.el: Don't set foreground on region

[Kristoffer Grönlund]

[-- Attachment #1: Type: text/plain, Size: 816 bytes --]

Sounds good to me too.

On Sun, 28 Feb 2016 07:21 Lars Ingebrigtsen, <larsi@gnus.org> wrote:

> Mitchel Humpherys <mitch.special@gmail.com> writes:
>
> > On Mon, Feb 22 2016 at 05:04:31 PM, Mitchel Humpherys
> > <mitch.special@gmail.com> wrote:
> >> Setting a foreground color on the `region' face is slightly annoying.
> >> This is subjective, of course, but the consensus among other themes and
> >> around the web is to *not* set a foreground color on the region face.
> >
> > Any objections to this?  I should have mentioned that the reason it's
> > slightly annoying is because setting a foreground color hides existing
> > syntax highlighting within the region text.
>
> The patch sounds OK to me.
>
> --
> (domestic pets only, the antidote for overdose, milk.)
>    bloggy blog: http://lars.ingebrigtsen.no
>

[-- Attachment #2: Type: text/html, Size: 1358 bytes --]

RE: [PATCH] * etc/themes/wombat-theme.el: Don't set foreground on region

[Drew Adams]

> > Setting a foreground color on the `region' face is slightly
> > annoying.
> > This is subjective, of course, but the consensus among other
> > themes and around the web is to *not* set a foreground color
> > on the region face.
>
> Any objections to this?  I should have mentioned that the reason
> it's slightly annoying is because setting a foreground color hides
> existing syntax highlighting within the region text.

A particular theme can do anything its authors and users want,
of course.  But wrt the general question of whether region (aka
selection) highlighting should override other highlighting, see
Emacs bug #15899.

http://debbugs.gnu.org/cgi/bugreport.cgi?bug=15899

IMO, selection highlighting should, at least by default, visibly
cover the entire selection.  Otherwise, it is not always clear
what the region limits are.

When other highlighting takes precedence over region highlighting
a user can miss the fact that there is more than one (or more than
N) region pieces highlighted, and so mistake the region limits.

Whether region highlighting has higher precedence (priority)
should always be a user choice.  Users can at least customize face
`region' (including to specify its foreground), but that is not
enough to give region highlighting priority.

Re: [PATCH] * etc/themes/wombat-theme.el: Don't set foreground on region

[Mitchel Humpherys]

On Sun, Feb 28 2016 at 06:53:45 AM, Drew Adams <drew.adams@oracle.com> wrote:
>> > Setting a foreground color on the `region' face is slightly
>> > annoying.
>> > This is subjective, of course, but the consensus among other
>> > themes and around the web is to *not* set a foreground color
>> > on the region face.
>>
>> Any objections to this?  I should have mentioned that the reason
>> it's slightly annoying is because setting a foreground color hides
>> existing syntax highlighting within the region text.
>
> A particular theme can do anything its authors and users want,
> of course.  But wrt the general question of whether region (aka
> selection) highlighting should override other highlighting, see
> Emacs bug #15899.
>
> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=15899
>
> IMO, selection highlighting should, at least by default, visibly
> cover the entire selection.  Otherwise, it is not always clear
> what the region limits are.

Just to be clear, when I said "syntax highlighting" I meant it in the
general sense of colors applied to the *foreground* of code in order to
highlight the syntax of the language.  I believe you're referring to
background highlighting, which shouldn't be affected by this patch.

-- 
Mitch

RE: [PATCH] * etc/themes/wombat-theme.el: Don't set foreground on region

[Drew Adams]

> >> > Setting a foreground color on the `region' face is slightly
> >> > annoying.
> >> > This is subjective, of course, but the consensus among other
> >> > themes and around the web is to *not* set a foreground color
> >> > on the region face.
> >>
> >> Any objections to this?  I should have mentioned that the reason
> >> it's slightly annoying is because setting a foreground color hides
> >> existing syntax highlighting within the region text.
> >
> > A particular theme can do anything its authors and users want,
> > of course.  But wrt the general question of whether region (aka
> > selection) highlighting should override other highlighting, see
> > Emacs bug #15899.
> >
> > http://debbugs.gnu.org/cgi/bugreport.cgi?bug=15899
> >
> > IMO, selection highlighting should, at least by default, visibly
> > cover the entire selection.  Otherwise, it is not always clear
> > what the region limits are.
> 
> Just to be clear, when I said "syntax highlighting" I meant it in the
> general sense of colors applied to the *foreground* of code in order to
> highlight the syntax of the language.  I believe you're referring to
> background highlighting, which shouldn't be affected by this patch.

I too was speaking of foreground highlighting (as well as background
highlighting).  I was speaking directly to your point about "setting
a foreground color hides existing syntax highlighting within the
region text."

Please see the referenced bug thread, if you are interested in the
question.  It is about whether, when, and why region highlighting
(foreground, background, or both) should hide other highlighting.