Subject: Re: bug#46358: 28.0.50; [PATCH] Add vc-dir faces; also apply them to vc-git

From: Dmitry Gutov <[email-masked]>
Date: Mon, 8 Feb 2021 17:54:17 +0200

On 07.02.2021 18:15, Protesilaos Stavrou wrote:
> On 2021-02-07, 17:15 +0200, Eli Zaretskii <[email-masked]> wrote:
> 
>>> From: Protesilaos Stavrou <[email-masked]>
>>> Date: Sun, 07 Feb 2021 13:42:09 +0200
>>>
>>> In the attached patch, I do the following:
>>>
>>> 1. Define new faces.  Each has semantic value in that it applies to
>>>     constructs implied by its name.
>>
>> Thanks.  Would it be possible to use color names rather than #RRGGBB
>> values?  The latter makes it very hard to figure out the color that
>> will be used by the face.
> 
> I will keep this in mind for the next time.  For this case I removed all
> color specifications (please find the revised patch attached to this
> message).

Thanks, this is better.

I'm not opposed to changing colors, but this probably should be done 
systematically across many faces in the default theme, rather than in 
one specific UI element. Shouldn't it?

>>> 4. Use new color combinations which conform with the WCAG AAA standard
>>>     for color contrast against pure white/black (this standard pertains
>>>     to legibility and is the highest of its kind).
>>
>> Not sure what that means in practical terms: most Emacs users I've
>> watched working (myself included) use some background color other than
>> pure black or white.  Doesn't that change the contrast and the optimal
>> colors?
> 
> You are right: I should have clarified that I meant the default white
> background and its inverse.  Other themes would indeed have to adapt
> things to their needs.

True.

>>> With regard to point 2, I only use Git and thus cannot test the other
>>> backends with the requisite degree of confidence.  Do you think I should
>>> try regardless?  Or should we just support the Git backend and hope that
>>> someone else will work on [some of] the other backends?
>>
>> If you can easily try other backends, it will be appreciated.  But it
>> is not mandatory, IMO.
> 
> I will inspect their code and try to identify whatever looks the same as
> vc-git.  Then I will prepare a separate patch.

FWIW, Git is the only backend that has a complex dir-printer method.

The rest look pretty much like vc-hg-dir-printer, but 
'font-lock-comment-face' in there should be changed to some new face too.

>> Personally, I think inheriting from the existing faces will be less
>> drastic, so it's probably better.
> 
> Very well!  I am doing just that in the revised patch.  So there should
> be no visual difference between this and the prior state, except for one
> case: the empty Git stash header, which will ultimately inherit from
> 'shadow' (before there was a "FIXME" to disambiguate it from other
> header values).

Some questions:

- vc-dir-ignored face doesn't seem to be used the 'ignored' entries in 
the list. Wasn't that its main point?

- vc-git-dir-printer defaults entries to the 'vc-dir-status-edited' 
face, whereas vc-default-dir-printer defaults to vc-dir-header-value' 
(statuses that are not 'up-to-date', 'missing', 'conflict' or 'edited'). 
Which is the intended behavior? Which one do we want?