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

Previous Next

Full log

View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: tracker <at> debbugs.gnu.org
Subject: bug#17388: closed (24.4.50; REGRESSION: Ediff - 1) wrong face, 2)
 incorrect diffing)
Date: Sat, 03 May 2014 02:29:02 +0000

[Message part 1 (text/plain, inline)]

Your message dated Fri, 02 May 2014 22:27:53 -0400
with message-id <jwvwqe3mx1m.fsf-monnier+emacsbugs <at> gnu.org>
and subject line Re: bug#17388: 24.4.50; REGRESSION: Ediff - 1) wrong face, 2) incorrect diffing
has caused the debbugs.gnu.org bug report #17388,
regarding 24.4.50; REGRESSION: Ediff - 1) wrong face, 2) incorrect diffing
to be marked as done.

(If you believe you have received this mail in error, please contact
help-debbugs <at> gnu.org.)


-- 
17388: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=17388
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems

[Message part 2 (message/rfc822, inline)]

From: Drew Adams <drew.adams <at> oracle.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 24.4.50; REGRESSION: Ediff - 1) wrong face, 2) incorrect diffing
Date: Fri, 2 May 2014 08:15:09 -0700 (PDT)

[Message part 3 (text/plain, inline)]

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 monnier <at> iro.umontreal.ca-20140429151607-qnkgbymwfaj5ut08
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''

[throw-emacs-bug-ediff.png (image/png, attachment)]

[Message part 5 (message/rfc822, inline)]

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: 17388-done <at> debbugs.gnu.org, Michael Kifer <kifer <at> cs.stonybrook.edu>
Subject: Re: bug#17388: 24.4.50;
 REGRESSION: Ediff - 1) wrong face, 2) incorrect diffing
Date: Fri, 02 May 2014 22:27:53 -0400

>> 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  <monnier <at> iro.umontreal.ca>
+
+	* 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  <sdl.web <at> gmail.com>
 
 	* 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)
 

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.