bug#10181: 24.0.92; [wishlist] split `diff-refine-change' in several faces

[Juri Linkov]

>> One solution would be to define `diff-changed-face' (currently defvar)
>> with defcustom.  Then users will be able to customize it to nil to use
>> just two faces `diff-removed' and `diff-added'.
>
> I think
>
>   (defvar diff-use-changed (not (or (face-equal diff-changed diff-added)
>                                     (face-equal diff-changed diff-removed))))
>
> would be good enough (together with the rest of your patch, which
> decides whether to use added-vs-changed or removed-vs-changed.

Another variant to try is to check the customization state of these faces.
This gives more flexibility.  What I'm trying to achieve is to prepare
a color scheme used on most modern version control systems that would be
easy for users to enable in diff mode.  At the same time I'd rather be careful
not to ignite a flamewar about the default highlighting.

So a new version of the patch below keeps the current default highlighting
that highlights all changes with one face `diff-changed' for users who
not customized `diff-removed' and `diff-added' to non-default values.

Users who customized `diff-removed', `diff-added' as well as
`diff-changed' to non-default values will see the same highlighting
as before this patch.

Users who want to use the modern color scheme with red for removed and
green for added lines could customize the new variable `diff-color-scheme'
to the `removed-added' value.

If this heuristic for the default value of `diff-color-scheme'
is acceptable then I'll adapt the diff-refine-hook case to these
three color schemes.

=== modified file 'lisp/vc/diff-mode.el'
--- lisp/vc/diff-mode.el        2012-05-01 02:48:41 +0000
+++ lisp/vc/diff-mode.el        2012-05-19 23:54:38 +0000
@@ -277,14 +275,20 @@ (define-obsolete-face-alias 'diff-hunk-h
 (defvar diff-hunk-header-face 'diff-hunk-header)
 
 (defface diff-removed
-  '((t :inherit diff-changed))
+  '((((class color) (min-colors 88))
+     :background "#ffdddd")
+    (((class color))
+     :foreground "red"))
   "`diff-mode' face used to highlight removed lines."
   :group 'diff-mode)
 (define-obsolete-face-alias 'diff-removed-face 'diff-removed "22.1")
 (defvar diff-removed-face 'diff-removed)
 
 (defface diff-added
-  '((t :inherit diff-changed))
+  '((((class color) (min-colors 88))
+     :background "#ddffdd")
+    (((class color))
+     :foreground "green"))
   "`diff-mode' face used to highlight added lines."
   :group 'diff-mode)
 (define-obsolete-face-alias 'diff-added-face 'diff-added "22.1")
@@ -369,6 +373,27 @@ (defun diff-yank-function (text)
          (while (re-search-backward re start t)
            (replace-match "" t t)))))))
 
+(defvar diff-color-scheme (cond
+                          ((not (or (get diff-removed-face 'customized-face)
+                                    (get diff-removed-face 'saved-face)
+                                    (get diff-added-face 'customized-face)
+                                    (get diff-added-face 'saved-face)))
+                           'changed)
+                          ((or (face-equal diff-changed-face diff-added-face)
+                               (face-equal diff-changed-face 
diff-removed-face))
+                           'removed-added)
+                          ((or (get diff-changed-face 'customized-face)
+                               (get diff-changed-face 'saved-face))
+                           'changed-removed-added)
+                          (t
+                           'removed-added))
+  "Color scheme used to highlight changes.
+When the default definitions of faces `diff-removed' and `diff-added',
+are not customized then highlight all changes with the same
+face `diff-changed-face'.  This is the default scheme.
+When all three faces were customized then use them all.
+Otherwise, use just `diff-removed' and `diff-added'.")
+
 (defconst diff-hunk-header-re-unified
   "^@@ -\\([0-9]+\\)\\(?:,\\([0-9]+\\)\\)? 
\\+\\([0-9]+\\)\\(?:,\\([0-9]+\\)\\)? @@")
 (defconst diff-context-mid-hunk-header-re
@@ -389,11 +414,25 @@ (defvar diff-font-lock-keywords
      (0 diff-header-face)
      (2 (if (not (match-end 3)) diff-file-header-face) prepend))
     ("^\\([-<]\\)\\(.*\n\\)"
-     (1 diff-indicator-removed-face) (2 diff-removed-face))
+     (1 (if (eq diff-color-scheme 'changed) diff-indicator-changed-face 
diff-indicator-removed-face))
+     (2 (if (eq diff-color-scheme 'changed) diff-changed-face 
diff-removed-face)))
     ("^\\([+>]\\)\\(.*\n\\)"
-     (1 diff-indicator-added-face) (2 diff-added-face))
+     (1 (if (eq diff-color-scheme 'changed) diff-indicator-changed-face 
diff-indicator-added-face))
+     (2 (if (eq diff-color-scheme 'changed) diff-changed-face 
diff-added-face)))
     ("^\\(!\\)\\(.*\n\\)"
-     (1 diff-indicator-changed-face) (2 diff-changed-face))
+     (1 (if (memq diff-color-scheme '(changed changed-removed-added))
+           diff-indicator-changed-face
+         diff-indicator-changed-face))
+     (2
+      (if (memq diff-color-scheme '(changed changed-removed-added))
+         diff-changed-face
+       ;; Otherwise, search for `diff-context-mid-hunk-header-re' and
+       ;; if the line of context diff is above, use `diff-removed-face';
+       ;; if below, use `diff-added-face'.
+       (let ((limit (save-excursion (diff-beginning-of-hunk))))
+         (if (save-excursion (re-search-backward 
diff-context-mid-hunk-header-re limit t))
+             diff-added-face
+           diff-removed-face)))))
     ("^\\(?:Index\\|revno\\): \\(.+\\).*\n"
      (0 diff-header-face) (1 diff-index-face prepend))
     ("^Only in .*\n" . diff-nonexistent-face)
@@ -1866,6 +1905,22 @@ (defface diff-refine-change
   "Face used for char-based changes shown by `diff-refine-hunk'."
   :group 'diff-mode)
 
+(defface diff-refine-removed
+  '((((class color) (min-colors 88))
+     :background "#ffaaaa")
+    (t :inherit diff-removed :inverse-video t))
+  "Face used for removed characters shown by `diff-refine-hunk'."
+  :group 'diff-mode
+  :version "24.2")
+
+(defface diff-refine-added
+  '((((class color) (min-colors 88))
+     :background "#aaffaa")
+    (t :inherit diff-added :inverse-video t))
+  "Face used for added characters shown by `diff-refine-hunk'."
+  :group 'diff-mode
+  :version "24.2")
+
 (defun diff-refine-preproc ()
   (while (re-search-forward "^[+>]" nil t)
     ;; Remove spurious changes due to the fact that one side of the hunk is