From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Juri Linkov Newsgroups: gmane.emacs.bugs Subject: bug#10181: 24.0.92; [wishlist] split `diff-refine-change' in several faces Date: Sun, 20 May 2012 02:55:58 +0300 Organization: JURTA Message-ID: <87aa13k9o1.fsf@mail.jurta.org> References: <87txzftzn0.fsf@mail.jurta.org> <87396yxr9u.fsf@mail.jurta.org> <87d361w2ea.fsf@mail.jurta.org> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: dough.gmane.org 1337473961 6060 80.91.229.3 (20 May 2012 00:32:41 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Sun, 20 May 2012 00:32:41 +0000 (UTC) Cc: 10181@debbugs.gnu.org To: Stefan Monnier Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Sun May 20 02:32:40 2012 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1SVu4c-0007sD-Tx for geb-bug-gnu-emacs@m.gmane.org; Sun, 20 May 2012 02:32:39 +0200 Original-Received: from localhost ([::1]:57474 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SVu4c-0004gi-Cz for geb-bug-gnu-emacs@m.gmane.org; Sat, 19 May 2012 20:32:38 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:38829) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SVu4Z-0004ga-1w for bug-gnu-emacs@gnu.org; Sat, 19 May 2012 20:32:36 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SVu4W-0007dn-UW for bug-gnu-emacs@gnu.org; Sat, 19 May 2012 20:32:34 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:53536) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SVu4W-0007dj-RO for bug-gnu-emacs@gnu.org; Sat, 19 May 2012 20:32:32 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.72) (envelope-from ) id 1SVu4z-0001T0-Ht for bug-gnu-emacs@gnu.org; Sat, 19 May 2012 20:33:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Juri Linkov Original-Sender: debbugs-submit-bounces@debbugs.gnu.org Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 20 May 2012 00:33:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 10181 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 10181-submit@debbugs.gnu.org id=B10181.13374739555595 (code B ref 10181); Sun, 20 May 2012 00:33:01 +0000 Original-Received: (at 10181) by debbugs.gnu.org; 20 May 2012 00:32:35 +0000 Original-Received: from localhost ([127.0.0.1]:34847 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1SVu4Y-0001SB-Jx for submit@debbugs.gnu.org; Sat, 19 May 2012 20:32:35 -0400 Original-Received: from ps18281.dreamhost.com ([69.163.218.105]:48574 helo=ps18281.dreamhostps.com) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1SVu4C-0001Rc-5G for 10181@debbugs.gnu.org; Sat, 19 May 2012 20:32:32 -0400 Original-Received: from localhost (ps18281.dreamhostps.com [69.163.218.105]) by ps18281.dreamhostps.com (Postfix) with ESMTP id 8B19D451CA16; Sat, 19 May 2012 17:31:40 -0700 (PDT) In-Reply-To: (Stefan Monnier's message of "Sat, 19 May 2012 14:24:21 -0400") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.1.50 (x86_64-pc-linux-gnu) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.13 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) X-Received-From: 140.186.70.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:60230 Archived-At: >> 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