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

[Protesilaos Stavrou <info@protesilaos.com>]

On 2021-02-09, 11:19 +0200, Juri Linkov <juri@linkov.net> wrote:

>>>> 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.
>
> It could continue looking the same as all other headers' values.

Thinking again about this, I now agree with this view.  An empty stash
is not an "inactive" status but rather a valid header value.

>> This meant that there was no distinction between empty and non-empty
>> stashes, something that was noted as a "FIXME" in the source code.
>
> Indeed, it could be better to have a distinction between empty and
> non-empty stashes.  The FIXME proposed to use a different face
> when nothing is stashed.
>
> But it seems better to use a different face when something is stashed.
> It's important to attract user attention to the fact that there are
> stashes laying around.

I actually feel that this level of distinction already exists.  When the
stash is non-empty, there is a button/link which by default is blue, so
it contrasts well with the orange header values.  This button can be
used to toggle the visibility of the stash list.  Stash entries are the
same color as all other header values, yet their presence is already
quite obvious in context.

>> 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'.
>
> Could some of new vc-dir faces that you added recently
> be used for non-empty stashes?

In principle yes, though I now believe that we do not need to introduce
such a qualification.

>> If you feel this is a problem 'vc-dir-ignored' could also inherit from
>> 'font-lock-variable-name-face'.
>
> It's good that 'vc-dir-ignored' inherits from the 'shadow' face
> when it's used on files with the "ignored" status.  Then it
> could be renamed to 'vc-dir-status-ignored' as you already proposed.

Yes, it should be renamed.

I will now use this information, as well as what Dmitry shared in the
other comment to prepare a new patch that covers everything.  You can
all test it before it gets applied.  I will share it in this thread
either later in the day or tomorrow.

Thank you!

-- 
Protesilaos Stavrou
protesilaos.com