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

From: Protesilaos Stavrou <[email-masked]>
Date: Mon, 08 Feb 2021 18:35:31 +0200

On 2021-02-08, 17:54 +0200, Dmitry Gutov <[email-masked]> wrote:

> 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?

Yes, that would be better.

>>>> 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.

Thanks!  I still have not had the time to check those, though I plan to
do so.  It would be nice to ensure consistency between all backends.

>>> 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?

Can you please specify which are those?

I only applied the 'vc-dir-ignored' face to the empty Git stash and only
did so because there was a "FIXME" for it.  Otherwise, yes, the new face
should be used wherever it makes sense.

> - 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?
>

There is an omission from my part that I will now prepare a patch for
with regard to the "edited" check of 'vc-default-dir-printer'.  It needs
to specify 'vc-dir-status-edited' instead of 'font-lock-constant-face'.

As for its default value, I was not sure what those other states were,
so I just used 'vc-dir-header-value', thinking that this is a neutral
value.

Should the default look like "edited" as well?  Or does it have some
other meaning?  If the latter, then maybe we must have extra face?

-- 
Protesilaos Stavrou
protesilaos.com