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

[Protesilaos Stavrou <info@protesilaos.com>]

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

On 2021-02-09, 01:33 +0200, Dmitry Gutov <dgutov@yandex.ru> wrote:

> On 08.02.2021 18:35, Protesilaos Stavrou wrote:
>> On 2021-02-08, 17:54 +0200, Dmitry Gutov <dgutov@yandex.ru> wrote:
>>> 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.
>
> The 'ignored' files in the vc-dir tree.
>
> To see one, edit some file that has a matching entry in .gitignore (such
> as ChangeLog in a Emacs repo checkout). You should see it in VC-Dir
> buffer now, with status 'ignored'.

Please see the attached patch (unless you want me to open a new bug
report).  This should now account for the ignored state.  It also edits
a face that I had missed earlier, as was discussed herein.

>> 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.
>
> All possible states are listed in the docstring for 'vc-state'.
>
> About half of them (almost) are pretty rare, though.
>
>> 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?
>
> I don't have a strong opinion on this right now. But we should be
> consistent between the 'default' version and the backend-specific
> versions of the method.
>
> Having a face per status might be too much both for the user and the
> theme authors, though (who will have to pick appropriate colors).
>
> So I would keep the number of faces at 4: up-to-date, warning, ignored
> and edited.

I also think that 4 faces should suffice.  Having checked the doc string
of 'vc-state' this is how I feel they should be organised.

| status           | face ("?" means suggestion) |
|------------------+-----------------------------|
| up-to-date       | vc-dir-status-up-to-date    |
| edited           | vc-dir-status-edited        |
| USER             | vc-dir-status-warning?      |
| needs-update     | vc-dir-status-warning?      |
| unlocked-changes | vc-dir-status-warning?      |
| added            | vc-dir-status-edited        |
| removed          | vc-dir-status-edited        |
| conflict         | vc-dir-status-warning       |
| missing          | vc-dir-status-warning       |
| ignored          | vc-dir-ignored              |
| unregistered     | vc-dir-status-edited        |

With regard to 'vc-dir-ignored', do you think we should rename it to
'vc-dir-status-ignored' for the sake of consistency?  I wrote it that
way to denote that it is more generic than those that apply to files,
but I am okay either way.

-- 
Protesilaos Stavrou
protesilaos.com

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Refine-use-of-new-vc-dir-faces.patch --]
[-- Type: text/x-patch, Size: 1885 bytes --]

From 887b3333d09a93446dd7bd478e4e877df2884bd7 Mon Sep 17 00:00:00 2001
Message-Id: <887b3333d09a93446dd7bd478e4e877df2884bd7.1612846159.git.info@protesilaos.com>
From: Protesilaos Stavrou <info@protesilaos.com>
Date: Tue, 9 Feb 2021 06:49:05 +0200
Subject: [PATCH] Refine use of new vc-dir faces

* lisp/vc/vc-dir.el (vc-default-dir-printer): Apply check for the
"ignored" status and make 'vc-dir-status-edited' the default face.

* lisp/vc/vc-git.el (vc-git-dir-printer): Use the same conditions for
the Git backend.

This follows from the discussion in bug#46358.
---
 lisp/vc/vc-dir.el | 4 ++--
 lisp/vc/vc-git.el | 3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/lisp/vc/vc-dir.el b/lisp/vc/vc-dir.el
index 14c81578b7..6aebd1b2e0 100644
--- a/lisp/vc/vc-dir.el
+++ b/lisp/vc/vc-dir.el
@@ -1456,8 +1456,8 @@ vc-default-dir-printer
       (format "%-20s" state)
       'face (cond ((eq state 'up-to-date) 'vc-dir-status-up-to-date)
 		  ((memq state '(missing conflict)) 'vc-dir-status-warning)
-		  ((eq state 'edited) 'font-lock-constant-face)
-		  (t 'vc-dir-header-value))
+		  ((eq state 'ignored) 'vc-dir-ignored)
+		  (t 'vc-dir-status-edited))
       'mouse-face 'highlight
       'keymap vc-dir-status-mouse-map)
      " "
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index e7306386fe..18c5432ad9 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -479,7 +479,8 @@ vc-git-dir-printer
      (propertize
       (format "%-12s" state)
       'face (cond ((eq state 'up-to-date) 'vc-dir-status-up-to-date)
-                  ((eq state '(missing conflict)) 'vc-dir-status-warning)
+                  ((memq state '(missing conflict)) 'vc-dir-status-warning)
+                  ((eq state 'ignored) 'vc-dir-ignored)
                   (t 'vc-dir-status-edited))
       'mouse-face 'highlight
       'keymap vc-dir-status-mouse-map)
-- 
2.30.0