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

From: Protesilaos Stavrou <[email-masked]>
Date: Tue, 09 Feb 2021 08:42:26 +0200

On 2021-02-09, 01:24 +0200, Dmitry Gutov <[email-masked]> wrote:

> On 08.02.2021 20:17, Juri Linkov wrote:
>>>> 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).
>>>
>>> Looks good to me; pushed to Emacs 28 now.
>> I don't know if anyone else has such problem, but now highlighting
>> the empty Git stash header with a different color distinguishes it
>> from other header lines, and thus attracts more attention.
>> So now the most noticeable thing in the vc-dir is the Git stash header
>> (that I almost never use).
>
> It looks okay-ish for me, but that must depend on a particular theme.

For the default theme, the headers are green while their values are
orange.  The value of an empty stash is gray right now, while that of a
non-empty one is orange, just like the other headers' values.

>> Maybe better to display the empty Git stash header using the default
>> colors, and then highlight it differently only when it's non-empty?
>
> Not with vc-dir-ignored, though (it is based on the 'shadow' face).

Before this change, the empty stash looked the same as all other
headers' values.  This meant that there was no distinction between empty
and non-empty stashes, something that was noted as a "FIXME" in the
source code.

I think there is value in distinguishing between those two states,
though I am fine with some other choice of fallback color/face.
Currently the empty stash uses 'vc-dir-ignored' which inherits from
'shadow'.  Whereas others use 'vc-dir-header-value' which inherit from
'font-lock-variable-name-face'.

If you feel this is a problem 'vc-dir-ignored' could also inherit from
'font-lock-variable-name-face'.

-- 
Protesilaos Stavrou
protesilaos.com