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

From: Juri Linkov <[email-masked]>
Date: Fri, 18 May 2012 03:32:45 +0300

>> This could be implemented exactly the same way as currently other
>> diff faces are implemented where `diff-removed' and `diff-added'
>> both inherit from `diff-changed' by default.  Anyone wishing to use
>
> I think this only covers part of what is requested, because it doesn't
> distinguish "changed" (meaning that the modification replaces something
> with something else) from "added"/"removed".

A modification can be represented as deletion of old lines with subsequent
insertion of new lines.

This is what the unified diff format cleverly does with just
`+' and `-' indicators.

Unfortunately, the context diff format uses an additional indicator `!'
that is ambiguous whether it indicates deletions or insertions.

I see no other way to highlight deletions and insertions of the context
diff format in distinct faces than to search for the --- line that
separates them.  This is possible thanks to conditionalized font-locking.

To not slow down font-locking of context diffs by default
it could check if `diff-added' and `diff-removed' inherit
from `diff-changed', and not search for the --- line in this case
when deletions and insertions will be highlighted in the same face.
If this is too ugly then we could add a new defcustom to define
user preference.

But at least this patch keeps the same highlighting of changes
after executing `diff-unified->context' that adds the `!' indicator
when the faces `diff-added' and `diff-removed' are customized
not to inherit from `diff-changed':

=== 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-18 00:32:18 +0000
@@ -393,7 +393,21 @@ (defvar diff-font-lock-keywords
     ("^\\([+>]\\)\\(.*\n\\)"
      (1 diff-indicator-added-face) (2 diff-added-face))
     ("^\\(!\\)\\(.*\n\\)"
-     (1 diff-indicator-changed-face) (2 diff-changed-face))
+     (1 diff-indicator-changed-face)
+     (2
+      ;; When the default face definitions are not customized and
+      ;; `diff-added' and `diff-removed' inherit from `diff-changed'
+      ;; then just use `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'.
+      (if (and (eq (face-attribute 'diff-added   :inherit) 'diff-changed)
+               (eq (face-attribute 'diff-removed :inherit) 'diff-changed))
+         diff-changed-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)