* bug#17388: 24.4.50; REGRESSION: Ediff - 1) wrong face, 2) incorrect diffing
@ 2014-05-02 15:15 Drew Adams
2014-05-02 18:57 ` Stephen Berman
0 siblings, 1 reply; 12+ messages in thread
From: Drew Adams @ 2014-05-02 15:15 UTC (permalink / raw)
To: 17388
[-- Attachment #1: Type: text/plain, Size: 1889 bytes --]
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@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''
[-- Attachment #2: throw-emacs-bug-ediff.png --]
[-- Type: image/png, Size: 19965 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#17388: 24.4.50; REGRESSION: Ediff - 1) wrong face, 2) incorrect diffing
2014-05-02 15:15 bug#17388: 24.4.50; REGRESSION: Ediff - 1) wrong face, 2) incorrect diffing Drew Adams
@ 2014-05-02 18:57 ` Stephen Berman
2014-05-03 2:27 ` Stefan Monnier
0 siblings, 1 reply; 12+ messages in thread
From: Stephen Berman @ 2014-05-02 18:57 UTC (permalink / raw)
To: Drew Adams; +Cc: 17388
On Fri, 2 May 2014 08:15:09 -0700 (PDT) Drew Adams <drew.adams@oracle.com> wrote:
> 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@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''
I see both of these problematic highlightings on GNU/Linux builds from
both the trunk (bzr 117042) and the emacs-24 branch (bzr 117049).
Steve Berman
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#17388: 24.4.50; REGRESSION: Ediff - 1) wrong face, 2) incorrect diffing
2014-05-02 18:57 ` Stephen Berman
@ 2014-05-03 2:27 ` Stefan Monnier
2014-05-03 3:11 ` Drew Adams
0 siblings, 1 reply; 12+ messages in thread
From: Stefan Monnier @ 2014-05-03 2:27 UTC (permalink / raw)
To: Drew Adams; +Cc: 17388-done, Michael Kifer
>> 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@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@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)
^ permalink raw reply [flat|nested] 12+ messages in thread* bug#17388: 24.4.50; REGRESSION: Ediff - 1) wrong face, 2) incorrect diffing
2014-05-03 2:27 ` Stefan Monnier
@ 2014-05-03 3:11 ` Drew Adams
2014-05-03 6:48 ` Eli Zaretskii
0 siblings, 1 reply; 12+ messages in thread
From: Drew Adams @ 2014-05-03 3:11 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 17388-done, Michael Kifer
> >> 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.
Not at all. How do you get that from what I said?
There should be EITHER, (a) as previously, NO fine diffs shown for
other than the current diff OR (b) CORRECT (helpful) fine diffs
shown for the non-current diffs.
In the text you quoted I spoke only of (a): return to the previous
behavior of NOT showing fine diffs except for the current diff.
> >> 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.
It's not very clear from the above what you see and do not see,
especially since you apparently did not understand the description.
Do you see a bug for Emacs 24.3 or 23.4? There is none that I see -
the behavior is as I described it. No fine diffs are shown for the
non-current diff highlighting with Emacs 24.3 or 23.4.
Are you sure you are looking at the non-current diffs (the gray ones)?
That's where both bugs are: (1) wrong face (`default'), (2) incorrect
fine diffing.
I just repeated everything, again starting from emacs -Q with the
above recipe, using 24.3. That's 5 times now I've done it (2x for
24.4, 24.3, 1x for 23.4). Same thing - just what I described originally.
> 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.
I can no longer reopen bugs - Glenn apparently banned me some time ago.
This has nothing to do with `ediff-word-mode' or pilot error.
What I showed and described was already from emacs -Q. And S. Berman
had no trouble following the recipe and confirming the behavior.
But let me try again.
emacs -Q
Load file `cygwin-mount.el', then `setup-cygwin.el' (from Emacs Wiki).
Cygwin `diff' is apparently irrelevant, but that's what I used anyway.
In foo.el type some text. E.g.:
(defvar advertised-signature-table)
(defvar dir-local-variables-alist)
(defvar dir-locals-file)
(defvar file-local-variables-alist)
(defvar Info-indexed-nodes)
Put the same text in bar.el. Then modify it a bit in bar.el:
(defvar advertiseyyyyyyyyyyyd-signature-table)
(defvar dir-local-variables-alist)
(defvar dir-locals-file)
(defvar filxxxxxxxxxxxxxxs-alist)
(defvar Info-indexed-nodes)
M-x ediff-buffers ; for foo.el and bar.el
Cycle among the two diffs. You will see the screenshot I sent.
Now read the bug report as to what is wrong with what you see,
if it is still not clear why it is a bug (2 bugs).
Stephen Berman's confirmation indicates that Cygwin `diff' is
irrelevant:
> I see both of these problematic highlightings on GNU/Linux builds from
> both the trunk (bzr 117042) and the emacs-24 branch (bzr 117049).
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#17388: 24.4.50; REGRESSION: Ediff - 1) wrong face, 2) incorrect diffing
2014-05-03 3:11 ` Drew Adams
@ 2014-05-03 6:48 ` Eli Zaretskii
0 siblings, 0 replies; 12+ messages in thread
From: Eli Zaretskii @ 2014-05-03 6:48 UTC (permalink / raw)
To: Drew Adams; +Cc: 17388, kifer
> Date: Fri, 2 May 2014 20:11:54 -0700 (PDT)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: 17388-done@debbugs.gnu.org, Michael Kifer <kifer@cs.stonybrook.edu>
>
> There should be EITHER, (a) as previously, NO fine diffs shown for
> other than the current diff OR (b) CORRECT (helpful) fine diffs
> shown for the non-current diffs.
Ediff's "fine diffs" are word-granular. That is, Ediff breaks each
line into "words", then passes the result to the Diff program for
comparisoon, and reflects the results with different faces. AFAIR,
this has always been that way.
The Ediff manual says:
`ediff-forward-word-function'
This variable controls how fine differences are computed. The
value must be a Lisp function that determines how the current
difference region should be split into words.
Fine differences are computed by first splitting the current
difference region into words and then passing the result to
`ediff-diff-program'. For the default forward word function
(which is `ediff-forward-word'), a word is a string consisting of
letters, `-', or `_'; a string of punctuation symbols; a string of
digits, or a string consisting of symbols that are neither space,
nor a letter.
This default behavior is controlled by four variables:
`ediff-word-1', ..., `ediff-word-4'. See the on-line
documentation for these variables and for the function
`ediff-forward-word' for an explanation of how to modify these
variables.
I think what you describe in your item #2 is exactly the above
behavior. (The problem described under #1 is now fixed by Stefan.)
> Stephen Berman's confirmation indicates that Cygwin `diff' is
> irrelevant:
>
> > I see both of these problematic highlightings on GNU/Linux builds from
> > both the trunk (bzr 117042) and the emacs-24 branch (bzr 117049).
I can confirm that too, but (a) I don't think the 2nd issue
constitutes a "problem" (see above), and (b) it is definitely not a
"REGRESSION", because older Emacsen behaved the same wrt fine diffs
inside a line.
^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <<eb90ee40-d0cb-46f0-8ce1-1759d110cbbd@default>]
end of thread, other threads:[~2014-05-04 7:16 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-02 15:15 bug#17388: 24.4.50; REGRESSION: Ediff - 1) wrong face, 2) incorrect diffing Drew Adams
2014-05-02 18:57 ` Stephen Berman
2014-05-03 2:27 ` Stefan Monnier
2014-05-03 3:11 ` Drew Adams
2014-05-03 6:48 ` Eli Zaretskii
[not found] <<eb90ee40-d0cb-46f0-8ce1-1759d110cbbd@default>
[not found] ` <<87wqe43t55.fsf@rosalinde.fritz.box>
[not found] ` <<jwvwqe3mx1m.fsf-monnier+emacsbugs@gnu.org>
[not found] ` <<111c9271-6a23-426e-adb2-ff5520c02806@default>
[not found] ` <<83a9az1hok.fsf@gnu.org>
2014-05-03 14:01 ` Drew Adams
2014-05-03 16:42 ` Eli Zaretskii
[not found] ` <8613042fc3be4f9995e33e38d4f079f0@HUBCAS1.cs.stonybrook.edu>
2014-05-03 15:57 ` Michael Kifer
2014-05-03 20:41 ` Drew Adams
2014-05-04 3:54 ` Stefan Monnier
2014-05-04 7:16 ` Drew Adams
[not found] ` <<bbf71aaa-f669-4542-a8d1-b7ff9d40d66e@default>
[not found] ` <<83y4yizue3.fsf@gnu.org>
2014-05-03 20:56 ` Drew Adams
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).