Subject: bug#17388: closed (Re: bug#17388: 24.4.50; REGRESSION: Ediff - 1) wrong face, 2) incorrect diffing)

From: [email-masked] (GNU bug Tracking System)
Date: Sat, 03 May 2014 02:29:03 +0000

Your bug report

#17388: 24.4.50; REGRESSION: Ediff - 1) wrong face, 2) incorrect diffing

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
17388: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=17388
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#17388: 24.4.50; REGRESSION: Ediff - 1) wrong face, 2) incorrect diffing
From: Stefan Monnier <[email-masked]>
To: Drew Adams <[email-masked]>
Cc: [email-masked], Michael Kifer <[email-masked]>
Date: Fri, 02 May 2014 22:27:53 -0400
List-Id: <debbugs-submit.debbugs.gnu.org>

>> See the attached screenshot.  The first diff shown, with the gray
>> highlighting, shows the regression.  

I installed the patch below which should fix those problems, thanks.

>> 2. The fine diffs are also not correct.  See the same screenshot.  For
>> the first diff, "advertise" and "d-signature-table" should be
>> highlighted the same as "(defvar " and ")", and for the second diff,
>> "fil" and "s-alist" should be highlighted the same as "(defvar " and
>> ")".  These are not differences.

You mean you want finer granularity of fine diffs.

>> still a regression wrt prior Emacs versions.  With Emacs 24.3, for
>> instance (using the same Cygwin `diff'), there is no fine diff shown
>> here.  Instead, the diff, which is shown only as a main diff, is between
>> "file-local-variables-alist" and "filxxxxxxxxxxxxxxxxxxxxxs-alist".
>> Which is correct.

I don't see that here with Debian's Emacs-24.3, and neither with 23.4.
The behavior you describe seems to correspond to ediff-word-mode, IIRC,
so I assume this is not really a bug/regression but just a pilot error
on your part.  If not, feel free to re-open this bug report, providing
more details about the problem.

        Stefan

=== modified file 'lisp/ChangeLog'
--- lisp/ChangeLog	2014-05-02 11:11:35 +0000
+++ lisp/ChangeLog	2014-05-03 02:10:48 +0000
@@ -1,3 +1,13 @@
+2014-05-03  Stefan Monnier  <[email-masked]>
+
+	* vc/ediff-diff.el (ediff-set-fine-diff-properties-in-one-buffer):
+	Use nil rather than `default' for the "default" appearance (bug#17388).
+	* vc/ediff-util.el (ediff-inferior-compare-regions)
+	(ediff-toggle-autorefine, ediff-unselect-difference): Don't use
+	a misleading `default' value when it's really a boolean.
+	* vc/ediff-init.el (ediff-set-overlay-face): Don't set help-echo if the
+	overlay is not visible.
+
 2014-05-02  Leo Liu  <[email-masked]>
 
 	* emacs-lisp/cl-macs.el (cl-deftype): Fix indentation.

=== modified file 'lisp/vc/ediff-diff.el'
--- lisp/vc/ediff-diff.el	2014-04-14 02:21:12 +0000
+++ lisp/vc/ediff-diff.el	2014-05-03 01:49:01 +0000
@@ -818,10 +818,9 @@
 						     n &optional default)
   (let ((fine-diff-vector  (ediff-get-fine-diff-vector n buf-type))
 	(face (if default
-		  'default
+		  nil
 		(ediff-get-symbol-from-alist
-		 buf-type ediff-fine-diff-face-alist)
-		)))
+		 buf-type ediff-fine-diff-face-alist))))
     (mapc (lambda (overl)
 	    (ediff-set-overlay-face overl face))
 	  fine-diff-vector)))

=== modified file 'lisp/vc/ediff-init.el'
--- lisp/vc/ediff-init.el	2014-04-10 19:15:01 +0000
+++ lisp/vc/ediff-init.el	2014-05-03 02:05:06 +0000
@@ -807,7 +807,7 @@
 
 (defun ediff-set-overlay-face (extent face)
   (ediff-overlay-put extent 'face face)
-  (ediff-overlay-put extent 'help-echo 'ediff-region-help-echo))
+  (ediff-overlay-put extent 'help-echo (if face 'ediff-region-help-echo)))
 
 (defun ediff-region-help-echo (extent-or-window &optional overlay _point)
   (unless overlay

=== modified file 'lisp/vc/ediff-util.el'
--- lisp/vc/ediff-util.el	2014-04-10 19:15:01 +0000
+++ lisp/vc/ediff-util.el	2014-05-03 01:46:35 +0000
@@ -958,7 +958,7 @@
 	 (message "Auto-refining is OFF")
 	 (setq ediff-auto-refine 'off))
 	(t ;; nix 'em
-	 (ediff-set-fine-diff-properties ediff-current-difference 'default)
+	 (ediff-set-fine-diff-properties ediff-current-difference t)
 	 (message "Refinements are HIDDEN")
 	 (setq ediff-auto-refine 'nix))
 	))
@@ -2973,7 +2973,7 @@
 	       ))
 
 	;; unhighlight fine diffs
-	(ediff-set-fine-diff-properties ediff-current-difference 'default)
+	(ediff-set-fine-diff-properties ediff-current-difference t)
 	(run-hooks 'ediff-unselect-hook))))

@@ -3492,7 +3492,7 @@
 
     (if (ediff-valid-difference-p ediff-current-difference)
 	(progn
-	  (ediff-set-fine-diff-properties ediff-current-difference 'default)
+	  (ediff-set-fine-diff-properties ediff-current-difference t)
 	  (ediff-unhighlight-diff)))
     (ediff-paint-background-regions 'unhighlight)

Subject: 24.4.50; REGRESSION: Ediff - 1) wrong face, 2) incorrect diffing
From: Drew Adams <[email-masked]>
To: [email-masked]
Date: Fri, 2 May 2014 08:15:09 -0700 (PDT)
List-Id: <debbugs-submit.debbugs.gnu.org>

1. A diff that is not the current one should use face
`ediff-(odd|even)-diff-(A|B').  This is now broken.

It does use that face for the main diff, but it still tries to show fine
differences within that diff - which is new.  That would be OK, I guess,
but it should not use face `default' to show such fine diffs.  It should
use an Ediff face, which users can customize without affecting other
things (as happens with `default').

Using face `default' here is particular misguided, as it suggests that
there is no difference at those locations, whereas there likely is.

See the attached screenshot.  The first diff shown, with the gray
highlighting, shows the regression.  

2. The fine diffs are also not correct.  See the same screenshot.  For
the first diff, "advertise" and "d-signature-table" should be
highlighted the same as "(defvar " and ")", and for the second diff,
"fil" and "s-alist" should be highlighted the same as "(defvar " and
")".  These are not differences.

I am using Cygwin `diff', which could affect #2 presumably.  But #2 is
still a regression wrt prior Emacs versions.  With Emacs 24.3, for
instance (using the same Cygwin `diff'), there is no fine diff shown
here.  Instead, the diff, which is shown only as a main diff, is between
"file-local-variables-alist" and "filxxxxxxxxxxxxxxxxxxxxxs-alist".
Which is correct.

Emacs should not show fine diffs that are are patently wrong.
Better not to mislead.

In GNU Emacs 24.4.50.1 (i686-pc-mingw32)
 of 2014-04-29 on ODIEONE
Bzr revision: 117031 [email-masked]
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --prefix=/c/Devel/emacs/snapshot/trunk
 --enable-checking=yes,glyphs 'CFLAGS=-O0 -g3'
 LDFLAGS=-Lc:/Devel/emacs/lib 'CPPFLAGS=-DGC_MCHECK=1
 -Ic:/Devel/emacs/include''