GNU bug report logs -
#17234
24.3.50; overlay priority : cons cells make an error in ediff
Previous Next
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 17234 in the body.
You can then email your comments to 17234 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#17234
; Package
emacs
.
(Thu, 10 Apr 2014 09:38:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Nicolas Richard <theonewiththeevillook <at> yahoo.fr>
:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org
.
(Thu, 10 Apr 2014 09:38:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
Steps to reproduce from -Q :
M-x ediff-regions-wordwise RET
RET ; selects scratch buf
RET ; ditto
C-SPC C-n C-M-c ; select first line of scratch buffer
C-n C-SPC C-n C-M-c ; select second line of scratch buffer
n ; go to next difference
That makes an error
> Wrong type argument: number-or-marker-p, (nil . 100)
in function ediff-highest-priority, which comes from commit
> cdb3fff3f588caeed50cbb5b64c09bce0a0b31e3
> Author: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Date: Sun Mar 23 18:30:47 2014 -0400
> * lisp/simple.el (redisplay-highlight-region-function): Increase priority of
> overlay to make sure boundaries are visible.
> * src/buffer.c (struct sortvec): Add field `spriority'.
> (compare_overlays): Use it.
> (sort_overlays): Set it.
where the notion of the "priority" of an overlay was changed : it can
now be a cons cell (which holds a priority and a secondary priority).
The changelog mentions bug#15899 but I saw nothing in that (very long)
thread mentionning the fix.
At (info "(elisp) Overlay Properties") however it is said that the
priority "should be a non-negative integer".
Also, and that's where the error comes from, ediff-init.el relies on the
priority being either nil or an integer, in function
ediff-highest-priority. For this I can suggest a fix (see patch below)
but I don't know what lispref should say about the change.
In GNU Emacs 24.3.50.6 (i686-pc-linux-gnu, GTK+ Version 2.24.20)
of 2014-04-09 on LDLC-portable
Modified lisp/ChangeLog
diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index 54ac144..3ed0195 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,8 @@
+2014-04-10 Nicolas Richard <theonewiththeevillook <at> yahoo.fr>
+
+ * vc/ediff-init.el (ediff-highest-priority): Don't make an error
+ if overlay priority is a cons.
+
2014-04-09 Dmitry Gutov <dgutov <at> yandex.ru>
* progmodes/ruby-mode.el (ruby-font-lock-keywords): Highlight more
Modified lisp/vc/ediff-init.el
diff --git a/lisp/vc/ediff-init.el b/lisp/vc/ediff-init.el
index 000fdb9..110dc63 100644
--- a/lisp/vc/ediff-init.el
+++ b/lisp/vc/ediff-init.el
@@ -1352,7 +1352,12 @@ this variable represents.")
(null (ediff-overlay-get ovr 'ediff))
(null (ediff-overlay-get ovr 'ediff-diff-num)))
;; use the overlay priority or 0
- (or (ediff-overlay-get ovr 'priority) 0)
+ (let ((priority (ediff-overlay-get ovr 'priority)))
+ (cond ((integerp priority)
+ priority)
+ ((consp priority)
+ (or (car priority) (cdr priority)))
+ (t 0)))
0))
ovr-list))))))))
--
Nico.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#17234
; Package
emacs
.
(Thu, 10 Apr 2014 19:18:01 GMT)
Full text and
rfc822 format available.
Message #8 received at 17234 <at> debbugs.gnu.org (full text, mbox):
> That makes an error
>> Wrong type argument: number-or-marker-p, (nil . 100)
> in function ediff-highest-priority, which comes from commit
Indeed, thanks. Ediff's handling of overlay priorities is a good
example of what's wrong with overlay priorities, adding code to
make sure its overlays are "on top", then adding yet more code to ensure
its other overlays are "even more on top". And then yet more code to
try and defeat the other packages which might be taking part in this
race to the highest priority.
So, I just threw it out, which should solve those problem.
Stefan
bug closed, send any further explanations to
17234 <at> debbugs.gnu.org and Nicolas Richard <theonewiththeevillook <at> yahoo.fr>
Request was from
Stefan Monnier <monnier <at> iro.umontreal.ca>
to
control <at> debbugs.gnu.org
.
(Thu, 10 Apr 2014 20:29:02 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#17234
; Package
emacs
.
(Fri, 11 Apr 2014 00:57:02 GMT)
Full text and
rfc822 format available.
Message #13 received at 17234 <at> debbugs.gnu.org (full text, mbox):
Nicolas Richard wrote:
> [...] the notion of the "priority" of an overlay was changed : it can
> now be a cons cell (which holds a priority and a secondary priority).
[...]
> At (info "(elisp) Overlay Properties") however it is said that the
> priority "should be a non-negative integer".
What about the documentation/NEWS aspect?
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#17234
; Package
emacs
.
(Fri, 11 Apr 2014 12:31:01 GMT)
Full text and
rfc822 format available.
Message #16 received at 17234 <at> debbugs.gnu.org (full text, mbox):
>> [...] the notion of the "priority" of an overlay was changed : it can
>> now be a cons cell (which holds a priority and a secondary priority).
> [...]
>> At (info "(elisp) Overlay Properties") however it is said that the
>> priority "should be a non-negative integer".
> What about the documentation/NEWS aspect?
The new feature was introduced specifically for the region handling, and
I'm not yet convinced I'm satisfied with it, so I'd rather not
document it for now.
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#17234
; Package
emacs
.
(Fri, 11 Apr 2014 13:09:01 GMT)
Full text and
rfc822 format available.
Message #19 received at 17234 <at> debbugs.gnu.org (full text, mbox):
> From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
> Date: Fri, 11 Apr 2014 08:30:33 -0400
> Cc: 17234 <at> debbugs.gnu.org
>
> >> [...] the notion of the "priority" of an overlay was changed : it can
> >> now be a cons cell (which holds a priority and a secondary priority).
> > [...]
> >> At (info "(elisp) Overlay Properties") however it is said that the
> >> priority "should be a non-negative integer".
> > What about the documentation/NEWS aspect?
>
> The new feature was introduced specifically for the region handling, and
> I'm not yet convinced I'm satisfied with it, so I'd rather not
> document it for now.
Then how can we expect authors of other Lisp packages to be able to
fix their code so it works with Emacs 24.4 and later? We must say
_something_ in NEWS.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#17234
; Package
emacs
.
(Fri, 11 Apr 2014 15:57:02 GMT)
Full text and
rfc822 format available.
Message #22 received at 17234 <at> debbugs.gnu.org (full text, mbox):
> Then how can we expect authors of other Lisp packages to be able to
> fix their code so it works with Emacs 24.4 and later? We must say
> _something_ in NEWS.
It has always been the case that `priority' could be any value.
Any non-number value was treated as nil by the display engine, whereas
now some cons values are treated as something else.
But ediff's bug could already be triggered in Emacs<24.4 by a package
installing an overlay with a `priority' that's a cons cell.
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#17234
; Package
emacs
.
(Fri, 11 Apr 2014 16:32:01 GMT)
Full text and
rfc822 format available.
Message #25 received at 17234 <at> debbugs.gnu.org (full text, mbox):
Stefan Monnier wrote:
> It has always been the case that `priority' could be any value.
But the only documented values were and are nil and positive integers.
I really doubt anyone was intentionally using anything else to mean "nil".
> Any non-number value was treated as nil by the display engine, whereas
> now some cons values are treated as something else.
So even if your previous point was correct, it's still an incompatible
change. A cons cell can no longer be used to mean "no explicit priority".
> But ediff's bug could already be triggered in Emacs<24.4 by a package
> installing an overlay with a `priority' that's a cons cell.
Since we never (?) had any such reports, I conclude nobody did that.
But I imagine ediff was not the only package trying to get the priority
of an overlay and do something with the answer (however misguided you
think that might be).
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#17234
; Package
emacs
.
(Fri, 11 Apr 2014 17:13:02 GMT)
Full text and
rfc822 format available.
Message #28 received at 17234 <at> debbugs.gnu.org (full text, mbox):
> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: rgm <at> gnu.org, 17234 <at> debbugs.gnu.org
> Date: Fri, 11 Apr 2014 11:56:24 -0400
>
> > Then how can we expect authors of other Lisp packages to be able to
> > fix their code so it works with Emacs 24.4 and later? We must say
> > _something_ in NEWS.
>
> It has always been the case that `priority' could be any value.
>
> Any non-number value was treated as nil by the display engine, whereas
> now some cons values are treated as something else.
>
> But ediff's bug could already be triggered in Emacs<24.4 by a package
> installing an overlay with a `priority' that's a cons cell.
Sorry, I don't see how all this removes the need to inform Lisp
programmers of the change of a long-time traditional behavior.
I don't understand the reason(s) for hiding this information. If you
don't want to divulge some internal details, the information can be
conveyed in a purely functional manner ("don't do XXX; instead, do
YYY" or "don't assume that overlay priority is either an integer or
nil) that doesn't require to describe implementation details. But I
don't see how can we say nothing about this incompatible change.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#17234
; Package
emacs
.
(Fri, 11 Apr 2014 20:07:01 GMT)
Full text and
rfc822 format available.
Message #31 received at 17234 <at> debbugs.gnu.org (full text, mbox):
>> Any non-number value was treated as nil by the display engine, whereas
>> now some cons values are treated as something else.
> So even if your previous point was correct, it's still an incompatible
> change. A cons cell can no longer be used to mean "no explicit priority".
Indeed. Tho as you said:
I really doubt anyone was intentionally using anything else to mean "nil".
So I think we're safe on this side of the backward incompatibility.
> But I imagine ediff was not the only package trying to get the priority
> of an overlay and do something with the answer (however misguided you
> think that might be).
Hard to tell. Not sure what we can do about it, tho.
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#17234
; Package
emacs
.
(Fri, 11 Apr 2014 20:09:01 GMT)
Full text and
rfc822 format available.
Message #34 received at 17234 <at> debbugs.gnu.org (full text, mbox):
> "don't assume that overlay priority is either an integer or nil"
Feel free to put that in.
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#17234
; Package
emacs
.
(Fri, 11 Apr 2014 20:23:01 GMT)
Full text and
rfc822 format available.
Message #37 received at 17234 <at> debbugs.gnu.org (full text, mbox):
Stefan Monnier wrote:
>> But I imagine ediff was not the only package trying to get the priority
>> of an overlay and do something with the answer (however misguided you
>> think that might be).
>
> Hard to tell.
One minute of searching suggests that:
extent-at in lucid.el
ps-generate-postscript-with-faces1 in ps-def.el
will have problems now.
It would seem suprising if there are three files in Emacs that this
causes problems for, and nothing external.
> Not sure what we can do about it, tho.
The least you can do is make an "incompatible change" NEWS entry.
Ideally it would it explain what do if you still want to get the
priority of an overlay as a number.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#17234
; Package
emacs
.
(Tue, 15 Apr 2014 16:57:02 GMT)
Full text and
rfc822 format available.
Message #40 received at 17234 <at> debbugs.gnu.org (full text, mbox):
> extent-at in lucid.el
> ps-generate-postscript-with-faces1 in ps-def.el
Indeed. And htmlize also.
> The least you can do is make an "incompatible change" NEWS entry.
Not sure which part is incompatible.
> Ideally it would it explain what to do if you still want to get the
> priority of an overlay as a number.
You don't, really. But I've added an argument `sorted' to overlays-at
which seems to provide the information that was really needed in the
above cases.
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#17234
; Package
emacs
.
(Mon, 21 Apr 2014 13:48:01 GMT)
Full text and
rfc822 format available.
Message #43 received at 17234 <at> debbugs.gnu.org (full text, mbox):
> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: rgm <at> gnu.org, 17234 <at> debbugs.gnu.org
> Date: Fri, 11 Apr 2014 16:07:47 -0400
>
> > "don't assume that overlay priority is either an integer or nil"
>
> Feel free to put that in.
Done.
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org
.
(Tue, 20 May 2014 11:24:04 GMT)
Full text and
rfc822 format available.
This bug report was last modified 10 years ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.