unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Juri Linkov <juri@jurta.org>
To: Stefan Monnier <monnier@iro.umontreal.ca>
Cc: 10181@debbugs.gnu.org
Subject: bug#10181: 24.0.92; [wishlist] split `diff-refine-change' in several faces
Date: Mon, 21 May 2012 03:28:17 +0300	[thread overview]
Message-ID: <87k406uz7n.fsf@mail.jurta.org> (raw)
In-Reply-To: <jwvsjeu6gop.fsf-monnier+emacs@gnu.org> (Stefan Monnier's message of "Sun, 20 May 2012 11:06:38 -0400")

>>  (defface diff-removed
>> -  '((t :inherit diff-changed))
>> +  '((((class color) (min-colors 88))
>> +     :background "#ffdddd")
>> +    (((class color))
>> +     :foreground "red"))
>
> Please keep the inheritance, even if the default overrides all fields.

Point taken, in the new version of the patch it keeps the inheritance.

> Tho, I don't see what's the point of this `changed' color-scheme: you
> get the same visual result if you use the `removed-added' color scheme
> and pretty much the same resource usage.  The only point of the
> distinction between removed-added and changed-removed-added is to avoid
> the additional resource usage when it would have no visual effect.

Agreed, and the patch adjusted accordingly.

>> +(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")
>
> This is just an accidental left-over, right?

This is a left-over from the second part of the patch that deals with
the diff-refine-hook case.  The whole patch is:

=== 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-21 00:27:43 +0000
@@ -277,14 +275,32 @@ (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))
+     :inherit diff-changed
+     :background "#ffdddd")
+    (((class color))
+     :inherit diff-changed
+     :foreground "red"
+     :weight normal
+     :slant normal)
+    (t
+     :inherit diff-changed))
   "`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))
+     :inherit diff-changed
+     :background "#ddffdd")
+    (((class color))
+     :inherit diff-changed
+     :foreground "green"
+     :weight normal
+     :slant normal)
+    (t
+     :inherit diff-changed))
   "`diff-mode' face used to highlight added lines."
   :group 'diff-mode)
 (define-obsolete-face-alias 'diff-added-face 'diff-added "22.1")
@@ -393,7 +409,18 @@ (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
+      (if (not (or (face-equal diff-changed-face diff-added-face)
+		   (face-equal diff-changed-face diff-removed-face)))
+	  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 +1893,32 @@ (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))
+     :inherit diff-refine-change
+     :background "#ffaaaa")
+    (((class color))
+     :inherit diff-refine-change
+     :background "red")
+    (t
+     :inherit diff-refine-change))
+  "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))
+     :inherit diff-refine-change
+     :background "#aaffaa")
+    (((class color))
+     :inherit diff-refine-change
+     :background "green")
+    (t
+     :inherit diff-refine-change))
+  "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
@@ -1879,7 +1932,7 @@ (defun diff-refine-preproc ()
   )
 
 (declare-function smerge-refine-subst "smerge-mode"
-                  (beg1 end1 beg2 end2 props &optional preproc))
+                  (beg1 end1 beg2 end2 props &optional preproc props2))
 
 (defun diff-refine-hunk ()
   "Highlight changes of hunk at point at a finer granularity."
@@ -1890,7 +1943,11 @@ (defun diff-refine-hunk ()
     (let* ((start (point))
            (style (diff-hunk-style))    ;Skips the hunk header as well.
            (beg (point))
-           (props '((diff-mode . fine) (face diff-refine-change)))
+           (props-c '((diff-mode . fine) (face diff-refine-change)))
+           (props-r '((diff-mode . fine) (face diff-refine-removed)))
+           (props-a '((diff-mode . fine) (face diff-refine-added)))
+           (diff-use-changed (not (or (face-equal diff-changed-face diff-added-face)
+				      (face-equal diff-changed-face diff-removed-face))))
            ;; Be careful to go back to `start' so diff-end-of-hunk gets
            ;; to read the hunk header's line info.
            (end (progn (goto-char start) (diff-end-of-hunk) (point))))
@@ -1904,7 +1961,7 @@ (defun diff-refine-hunk ()
                                    end t)
            (smerge-refine-subst (match-beginning 0) (match-end 1)
                                 (match-end 1) (match-end 0)
-                                props 'diff-refine-preproc)))
+                                props-r 'diff-refine-preproc props-a)))
         (context
          (let* ((middle (save-excursion (re-search-forward "^---")))
                 (other middle))
@@ -1916,14 +1973,16 @@ (defun diff-refine-hunk ()
                                     (setq other (match-end 0))
                                     (match-beginning 0))
                                   other
-                                  props 'diff-refine-preproc))))
+                                  (if diff-use-changed props-c props-r)
+				  'diff-refine-preproc
+				  (if diff-use-changed props-c props-a)))))
         (t ;; Normal diffs.
          (let ((beg1 (1+ (point))))
            (when (re-search-forward "^---.*\n" end t)
              ;; It's a combined add&remove, so there's something to do.
              (smerge-refine-subst beg1 (match-beginning 0)
                                   (match-end 0) end
-                                  props 'diff-refine-preproc))))))))
+                                  props-r 'diff-refine-preproc props-a))))))))
 
 (defun diff-undo (&optional arg)
   "Perform `undo', ignoring the buffer's read-only status."

=== modified file 'lisp/vc/smerge-mode.el'
--- lisp/vc/smerge-mode.el	2012-05-04 23:16:47 +0000
+++ lisp/vc/smerge-mode.el	2012-05-21 00:27:50 +0000
@@ -128,6 +128,32 @@ (defface smerge-refined-change
   "Face used for char-based changes shown by `smerge-refine'."
   :group 'smerge)
 
+(defface smerge-refined-removed
+  '((((class color) (min-colors 88))
+     :inherit smerge-refined-change
+     :background "#ffaaaa")
+    (((class color))
+     :inherit smerge-refined-change
+     :background "red")
+    (t
+     :inherit smerge-refined-change))
+  "Face used for removed characters shown by `smerge-refine'."
+  :group 'smerge
+  :version "24.2")
+
+(defface smerge-refined-added
+  '((((class color) (min-colors 88))
+     :inherit smerge-refined-change
+     :background "#aaffaa")
+    (((class color))
+     :inherit smerge-refined-change
+     :background "green")
+    (t
+     :inherit smerge-refined-change))
+  "Face used for added characters shown by `smerge-refine'."
+  :group 'smerge
+  :version "24.2")
+
 (easy-mmode-defmap smerge-basic-map
   `(("n" . smerge-next)
     ("p" . smerge-prev)
@@ -980,9 +1006,11 @@ (defun smerge-refine-highlight-change (b
           (dolist (x props) (overlay-put ol (car x) (cdr x)))
           ol)))))
 
-(defun smerge-refine-subst (beg1 end1 beg2 end2 props &optional preproc)
+(defun smerge-refine-subst (beg1 end1 beg2 end2 props &optional preproc props2)
   "Show fine differences in the two regions BEG1..END1 and BEG2..END2.
-PROPS is an alist of properties to put (via overlays) on the changes.
+PROPS is an alist of properties to put (via overlays) on the changes,
+or only on removed characters when PROPS2 is non-nil.
+PROPS2 is an alist of properties to put on added characters.
 If non-nil, PREPROC is called with no argument in a buffer that contains
 a copy of a region, just before preparing it to for `diff'.  It can be
 used to replace chars to try and eliminate some spurious differences."
@@ -1029,7 +1057,7 @@ (defun smerge-refine-subst (beg1 end1 be
                         (smerge-refine-highlight-change buf beg1 m1 m2 props)))
                 (when (memq op '(?a ?c))
                   (setq last2
-                        (smerge-refine-highlight-change buf beg2 m4 m5 props))))
+                        (smerge-refine-highlight-change buf beg2 m4 m5 (or props2 props)))))
               (forward-line 1)                            ;Skip hunk header.
               (and (re-search-forward "^[0-9]" nil 'move) ;Skip hunk body.
                    (goto-char (match-beginning 0))))
@@ -1081,7 +1109,10 @@ (defun smerge-refine (&optional part)
                    ((eq (match-end 3) (match-beginning 3)) 3)
                    (t 2)))
   (let ((n1 (if (eq part 1) 2 1))
-        (n2 (if (eq part 3) 2 3)))
+        (n2 (if (eq part 3) 2 3))
+	(smerge-use-changed
+	 (not (or (face-equal 'smerge-refined-change 'smerge-refined-added)
+		  (face-equal 'smerge-refined-change 'smerge-refined-removed)))))
     (smerge-ensure-match n1)
     (smerge-ensure-match n2)
     (with-silent-modifications
@@ -1090,8 +1121,13 @@ (defun smerge-refine (&optional part)
                          (cons (buffer-chars-modified-tick) part)))
     (smerge-refine-subst (match-beginning n1) (match-end n1)
                          (match-beginning n2)  (match-end n2)
-                         '((smerge . refine)
-                           (face . smerge-refined-change)))))
+                         (if smerge-use-changed
+			     '((smerge . refine) (face . smerge-refined-change))
+			   '((smerge . refine) (face . smerge-refined-removed)))
+			 nil
+			 (if smerge-use-changed
+			     '((smerge . refine) (face . smerge-refined-change))
+			   '((smerge . refine) (face . smerge-refined-added))))))
 
 (defun smerge-diff (n1 n2)
   (smerge-match-conflict)






  reply	other threads:[~2012-05-21  0:28 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-12-01 15:56 bug#10181: 24.0.92; [wishlist] split `diff-refine-change' in several faces Dani Moncayo
2011-12-02 10:47 ` Juri Linkov
2012-05-17  0:33 ` Juri Linkov
2012-05-17  1:37   ` Stefan Monnier
2012-05-18  0:32     ` Juri Linkov
2012-05-18 18:12       ` Stefan Monnier
2012-05-18 23:07         ` Juri Linkov
2012-05-19 18:24           ` Stefan Monnier
2012-05-19 23:55             ` Juri Linkov
2012-05-20 15:06               ` Stefan Monnier
2012-05-21  0:28                 ` Juri Linkov [this message]
2012-05-21  1:45                   ` Stefan Monnier
2012-05-23  0:36                     ` Juri Linkov
2012-05-23 13:53                       ` Stefan Monnier
2012-05-25  0:57                         ` Juri Linkov
2012-05-25 13:21                           ` Stefan Monnier
2012-05-25 20:35                             ` Juri Linkov
2012-05-26 13:13                               ` Stefan Monnier
2012-05-27  9:46                                 ` Juri Linkov
     [not found] ` <handler.10181.D10181.133811261729958.notifdone@debbugs.gnu.org>
2012-09-30 16:38   ` Juri Linkov
2014-06-14 11:58 ` bug#10181: " Dani Moncayo
2014-06-14 12:54   ` Juri Linkov
2014-06-14 13:12     ` Juri Linkov
2014-06-16  6:55       ` Juri Linkov
2014-06-18  8:57         ` Juri Linkov
2014-06-18  9:14           ` Dani Moncayo
2014-06-19  6:54             ` Juri Linkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87k406uz7n.fsf@mail.jurta.org \
    --to=juri@jurta.org \
    --cc=10181@debbugs.gnu.org \
    --cc=monnier@iro.umontreal.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).