bug#19043: 25.0.50; [PATCH]: faces for VC modeline state indicator

bug#19043: 25.0.50; [PATCH]: faces for VC modeline state indicator

[Óscar Fuentes]

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


2014-11-13 Oscar Fuentes <ofv@wanadoo.es>

	Add faces for the VC modeline state indicator.
	* lisp/vc/vc-hooks.el:
	(vc-state-faces, vc-state-base-face)
	(vc-up-to-date-state, vc-needs-update-state)
	(vc-locked-state, vc-locally-added-state)
	(vc-conflict-state, vc-removed-state)
	(vc-missing-state, vc-edited-state):
	New faces.
	(vc-default-mode-line-string): Use them


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vc-faces-upstream.patch --]
[-- Type: text/x-diff, Size: 3830 bytes --]

diff --git a/lisp/vc/vc-hooks.el b/lisp/vc/vc-hooks.el
index df660d1..6359e19 100644
--- a/lisp/vc/vc-hooks.el
+++ b/lisp/vc/vc-hooks.el
@@ -32,6 +32,69 @@
 
 (eval-when-compile (require 'cl-lib))
 
+;; Faces
+
+(defgroup vc-state-faces nil
+  "Faces used in the mode line by the VC state indicator."
+  :group 'vc-faces
+  :group 'mode-line
+  :version "25.1")
+
+(defface vc-state-base-face
+  '((default :inherit mode-line))
+  "Base face for VC state indicator."
+  :group 'vc-faces
+  :group 'mode-line
+  :version "25.1")
+
+(defface vc-up-to-date-state
+  '((default :inherit vc-state-base-face))
+  "Face for VC modeline state when the file is up to date."
+  :version "25.1"
+  :group 'vc-faces)
+
+(defface vc-needs-update-state
+  '((default :inherit vc-state-base-face))
+  "Face for VC modeline state when the file needs update."
+  :version "25.1"
+  :group 'vc-faces)
+
+(defface vc-locked-state
+  '((default :inherit vc-state-base-face))
+  "Face for VC modeline state when the file locked."
+  :version "25.1"
+  :group 'vc-faces)
+
+(defface vc-locally-added-state
+  '((default :inherit vc-state-base-face))
+  "Face for VC modeline state when the file is locally added."
+  :version "25.1"
+  :group 'vc-faces)
+
+(defface vc-conflict-state
+  '((default :inherit vc-state-base-face))
+  "Face for VC modeline state when the file contains merge conflicts."
+  :version "25.1"
+  :group 'vc-faces)
+
+(defface vc-removed-state
+  '((default :inherit vc-state-base-face))
+  "Face for VC modeline state when the file was removed from the VC system."
+  :version "25.1"
+  :group 'vc-faces)
+
+(defface vc-missing-state
+  '((default :inherit vc-state-base-face))
+  "Face for VC modeline state when the file is missing from the file system."
+  :version "25.1"
+  :group 'vc-faces)
+
+(defface vc-edited-state
+  '((default :inherit vc-state-base-face))
+  "Face for VC modeline state when the file is up to date."
+  :version "25.1"
+  :group 'vc-faces)
+
 ;; Customization Variables (the rest is in vc.el)
 
 (defcustom vc-ignore-dir-regexp
@@ -800,33 +863,42 @@ This function assumes that the file is registered."
   (let* ((backend-name (symbol-name backend))
 	 (state   (vc-state file backend))
 	 (state-echo nil)
+	 (face nil)
 	 (rev     (vc-working-revision file backend)))
     (propertize
      (cond ((or (eq state 'up-to-date)
 		(eq state 'needs-update))
 	    (setq state-echo "Up to date file")
+	    (setq face 'vc-up-to-date-state)
 	    (concat backend-name "-" rev))
 	   ((stringp state)
 	    (setq state-echo (concat "File locked by" state))
+	    (setq face 'vc-locked-state)
 	    (concat backend-name ":" state ":" rev))
            ((eq state 'added)
             (setq state-echo "Locally added file")
+	    (setq face 'vc-locally-added-state)
             (concat backend-name "@" rev))
            ((eq state 'conflict)
             (setq state-echo "File contains conflicts after the last merge")
+	    (setq face 'vc-conflict-state)
             (concat backend-name "!" rev))
            ((eq state 'removed)
             (setq state-echo "File removed from the VC system")
+	    (setq face 'vc-removed-state)
             (concat backend-name "!" rev))
            ((eq state 'missing)
             (setq state-echo "File tracked by the VC system, but missing from the file system")
+	    (setq face 'vc-missing-state)
             (concat backend-name "?" rev))
 	   (t
 	    ;; Not just for the 'edited state, but also a fallback
 	    ;; for all other states.  Think about different symbols
 	    ;; for 'needs-update and 'needs-merge.
 	    (setq state-echo "Locally modified file")
+	    (setq face 'vc-edited-state)
 	    (concat backend-name ":" rev)))
+     'face face
      'help-echo (concat state-echo " under the " backend-name
 			" version control system"))))
 

bug#19043: 25.0.50; [PATCH]: faces for VC modeline state indicator

[Lars Magne Ingebrigtsen]

Óscar Fuentes <ofv@wanadoo.es> writes:

> 2014-11-13 Oscar Fuentes <ofv@wanadoo.es>
>
> 	Add faces for the VC modeline state indicator.
> 	* lisp/vc/vc-hooks.el:
> 	(vc-state-faces, vc-state-base-face)
> 	(vc-up-to-date-state, vc-needs-update-state)
> 	(vc-locked-state, vc-locally-added-state)
> 	(vc-conflict-state, vc-removed-state)
> 	(vc-missing-state, vc-edited-state):
> 	New faces.
> 	(vc-default-mode-line-string): Use them

I tried applying this, but I didn't see anything different in the mode
lines of VC-covered files.  

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#19043: 25.0.50; [PATCH]: faces for VC modeline state indicator

[Lars Magne Ingebrigtsen]

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

> Óscar Fuentes <ofv@wanadoo.es> writes:
>
>> 2014-11-13 Oscar Fuentes <ofv@wanadoo.es>
>>
>> 	Add faces for the VC modeline state indicator.
>> 	* lisp/vc/vc-hooks.el:
>> 	(vc-state-faces, vc-state-base-face)
>> 	(vc-up-to-date-state, vc-needs-update-state)
>> 	(vc-locked-state, vc-locally-added-state)
>> 	(vc-conflict-state, vc-removed-state)
>> 	(vc-missing-state, vc-edited-state):
>> 	New faces.
>> 	(vc-default-mode-line-string): Use them
>
> I tried applying this, but I didn't see anything different in the mode
> lines of VC-covered files.  

Oh, right.  All the faces remain as the default face, but the user can
customize them.  Perhaps they should be something else by default, too?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#19043: 25.0.50; [PATCH]: faces for VC modeline state indicator

[Óscar Fuentes]

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

>> I tried applying this, but I didn't see anything different in the mode
>> lines of VC-covered files.  
>
> Oh, right.  All the faces remain as the default face, but the user can
> customize them.  Perhaps they should be something else by default, too?

I hope that just providing the possibility of customizing the VC
modeline face is seen as uncontroversial. Changing the default
appearance is something else altogether and it is not the goal of this
patch. You can assing different defaults on a subsequent patch, but
that's your battle, not mine ;-)

bug#19043: 25.0.50; [PATCH]: faces for VC modeline state indicator

[Lars Magne Ingebrigtsen]

Óscar Fuentes <ofv@wanadoo.es> writes:

> I hope that just providing the possibility of customizing the VC
> modeline face is seen as uncontroversial. Changing the default
> appearance is something else altogether and it is not the goal of this
> patch. You can assing different defaults on a subsequent patch, but
> that's your battle, not mine ;-)

I think we should push defaults (since they are what will be used by
almost everyone) that are good.  (Or "best".)  Since you did this patch,
you probably think that the defaults could be better.  So I think we
should push a change that includes these better defaults...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#19043: 25.0.50; [PATCH]: faces for VC modeline state indicator

[Óscar Fuentes]

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

> I think we should push defaults (since they are what will be used by
> almost everyone) that are good.  (Or "best".)  Since you did this patch,
> you probably think that the defaults could be better.

That's true, I use values that are better *IMO*. Preparing the patch
consisted on removing those values for achieving a neutral impact on
appearance.

> So I think we should push a change that includes these better
> defaults...

In my experience, contributing a patch to Emacs is difficult enough when
only technical issues are considered. It is a nightmare when personal
opinion is a key part.

Please let's not block this feature just because it does not address an
orthogonal aspect that we can take care of later.

Just to convince you to stop asking me for default values, I'll mention
that on my setup edited files are shown with bright red text ;-)

bug#19043: 25.0.50; [PATCH]: faces for VC modeline state indicator

[Stefan Monnier]

> I hope that just providing the possibility of customizing the VC
> modeline face is seen as uncontroversial.

Very wise, indeed.
I think I'm OK with your proposed patch.


        Stefan

bug#19043: 25.0.50; [PATCH]: faces for VC modeline state indicator

[Óscar Fuentes]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> I think I'm OK with your proposed patch.

Thanks. Now I need someone with a kind soul and commit rights for
applying the change.

bug#19043: 25.0.50; [PATCH]: faces for VC modeline state indicator

[Lars Magne Ingebrigtsen]

Óscar Fuentes <ofv@wanadoo.es> writes:

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>> I think I'm OK with your proposed patch.
>
> Thanks. Now I need someone with a kind soul and commit rights for
> applying the change.

I've applied it to trunk.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#19043: 25.0.50; [PATCH]: faces for VC modeline state indicator

[Óscar Fuentes]

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

> Óscar Fuentes <ofv@wanadoo.es> writes:
>
>> Thanks. Now I need someone with a kind soul and commit rights for
>> applying the change.
>
> I've applied it to trunk.

Thank you Lars, but it was already applied by me. Now there is an extra
Changelog entry that you commit created. I'll delete it.

It is my fault for not closing the PR after the commit, sorry.

bug#19043: 25.0.50; [PATCH]: faces for VC modeline state indicator

[Óscar Fuentes]