From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Peter Whaite Newsgroups: gmane.emacs.devel Subject: Re: Should overlays evaporate by default? Conclusion: No! Date: Fri, 27 May 2005 15:23:07 -0400 Message-ID: <200505271923.j4RJN7o5030391@brains.moreideas.ca> References: NNTP-Posting-Host: main.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Trace: sea.gmane.org 1117222874 7165 80.91.229.2 (27 May 2005 19:41:14 GMT) X-Complaints-To: usenet@sea.gmane.org NNTP-Posting-Date: Fri, 27 May 2005 19:41:14 +0000 (UTC) Cc: rms@gnu.org, emacs-devel@gnu.org Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Fri May 27 21:41:08 2005 Return-path: Original-Received: from lists.gnu.org ([199.232.76.165]) by ciao.gmane.org with esmtp (Exim 4.43) id 1DbkgK-00068x-GI for ged-emacs-devel@m.gmane.org; Fri, 27 May 2005 21:39:45 +0200 Original-Received: from localhost ([127.0.0.1] helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Dbkke-00087I-Kc for ged-emacs-devel@m.gmane.org; Fri, 27 May 2005 15:44:12 -0400 Original-Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1DbkhK-0005Qp-FB for emacs-devel@gnu.org; Fri, 27 May 2005 15:40:47 -0400 Original-Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1DbkhB-0005MT-0i for emacs-devel@gnu.org; Fri, 27 May 2005 15:40:39 -0400 Original-Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Dbkh7-0005DQ-GY for emacs-devel@gnu.org; Fri, 27 May 2005 15:40:33 -0400 Original-Received: from [209.5.194.98] (helo=orval.sprint.ca) by monty-python.gnu.org with esmtp (Exim 4.34) id 1DbkRJ-0005Su-Qa; Fri, 27 May 2005 15:24:14 -0400 Original-Received: from brains.moreideas.ca ([149.99.129.24]) by orval.sprint.ca (InterMail vM.5.01.02.00 201-253-122-103-101-20001108) with ESMTP id <20050527192310.SEVP16497.orval.sprint.ca@brains.moreideas.ca>; Fri, 27 May 2005 15:23:10 -0400 Original-Received: from brains.moreideas.ca (localhost.localdomain [127.0.0.1]) by brains.moreideas.ca (8.12.8/8.12.8) with ESMTP id j4RJN79m030395; Fri, 27 May 2005 15:23:07 -0400 Original-Received: from brains.moreideas.ca (peta@localhost) by brains.moreideas.ca (8.12.8/8.12.8/Submit) with ESMTP id j4RJN7o5030391; Fri, 27 May 2005 15:23:07 -0400 Original-To: Thien-Thi Nguyen In-Reply-To: Message from Thien-Thi Nguyen of "19 May 2005 04:24:39 EDT." X-Mailer: MH-E 7.83+cvs; nmh 1.0.4; GNU Emacs 22.0.50.83 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:37800 X-Report-Spam: http://spam.gmane.org/gmane.emacs.devel:37800 --=-=-= Thien-Thi Nguyen wrote: > Richard Stallman writes: > > > The goal now is to *find* possible bugs. The patch should set the > > property to something unusual (maybe `never-set') by default. When > > such an overlay becomes empty, there should be some sort of warning. > > That way, people will find code that perhaps ought to set the > > evaporate property to something explicitly. > > ok, below is a patch to buffer.c that does this, more or less. the > (%d,%d) is perhaps redundant, but since we're in the checking mood... Thanks Thi. I had to make a couple of changes (see below), but I have been running with it for a few days now. >>From what I've seen I do not think it would be a good idea to change the default. The main reason is that there is a third, and common, use case which I had not thought of. This is where an overlay is used dynamically to respond to user input, e.g. mouse.el, hl-line.el, comint, ediff, etc. Typically a global or buffer local variable is used to hold the overlay, which is positioned dynamically by applying move-overlay in response to user input. The positioning can result in the overlay becoming of zero length. Obviously it should not evaporate. I did start to patch files exposed by my limited testing (see below), but there were many, and I found I was setting 'evaporate to nil, not t. Given that, and given that magically disappearing is in general surprising default behaviour, then it would be better to retain the current overlay defaults. There remains the question of how to prevent overlay leaks. It would help if the elisp overlay documentation outlined the problem and remedies ((overlay-put ovl 'evaporate t) or (remove-overlays)). Perhaps my original suggestion that erase-buffer also remove-overlays should be considered. For the record I'm attaching the patches I made to the elisp files. A quick grep through the lisp directory reveals many more that would need patching. --=-=-= Content-Disposition: attachment; filename=evaporate-el.patch Index: comint.el =================================================================== RCS file: /cvsroot/emacs/emacs/lisp/comint.el,v retrieving revision 1.319 diff -u -r1.319 comint.el --- comint.el 26 May 2005 12:39:32 -0000 1.319 +++ comint.el 27 May 2005 17:05:46 -0000 @@ -1748,6 +1748,7 @@ ;; Need to create the overlay (setq comint-last-prompt-overlay (make-overlay prompt-start (point))) + (overlay-put comint-last-prompt-overlay 'evaporate nil) (overlay-put comint-last-prompt-overlay 'font-lock-face 'comint-highlight-prompt)))) Index: compare-w.el =================================================================== RCS file: /cvsroot/emacs/emacs/lisp/compare-w.el,v retrieving revision 1.26 diff -u -r1.26 compare-w.el --- compare-w.el 28 Nov 2004 07:56:01 -0000 1.26 +++ compare-w.el 27 May 2005 17:05:46 -0000 @@ -341,12 +341,14 @@ (if compare-windows-overlay1 (move-overlay compare-windows-overlay1 beg1 end1 b1) (setq compare-windows-overlay1 (make-overlay beg1 end1 b1)) + (overlay-put compare-windows-overlay1 'evaporate 'nil) (overlay-put compare-windows-overlay1 'face 'compare-windows-face) (overlay-put compare-windows-overlay1 'priority 1)) (overlay-put compare-windows-overlay1 'window w1) (if compare-windows-overlay2 (move-overlay compare-windows-overlay2 beg2 end2 b2) (setq compare-windows-overlay2 (make-overlay beg2 end2 b2)) + (overlay-put compare-windows-overlay2 'evaporate 'nil) (overlay-put compare-windows-overlay2 'face 'compare-windows-face) (overlay-put compare-windows-overlay2 'priority 1)) (overlay-put compare-windows-overlay2 'window w2) Index: hl-line.el =================================================================== RCS file: /cvsroot/emacs/emacs/lisp/hl-line.el,v retrieving revision 1.28 diff -u -r1.28 hl-line.el --- hl-line.el 4 Apr 2005 08:57:54 -0000 1.28 +++ hl-line.el 27 May 2005 17:05:46 -0000 @@ -134,6 +134,7 @@ (progn (unless hl-line-overlay (setq hl-line-overlay (make-overlay 1 1)) ; to be moved + (overlay-put hl-line-overlay 'evaporate nil) (overlay-put hl-line-overlay 'face hl-line-face)) (overlay-put hl-line-overlay 'window (unless hl-line-sticky-flag (selected-window))) @@ -168,6 +169,7 @@ (unless (window-minibuffer-p (selected-window)) (unless global-hl-line-overlay (setq global-hl-line-overlay (make-overlay 1 1)) ; to be moved + (overlay-put global-hl-line-overlay 'evaporate nil) (overlay-put global-hl-line-overlay 'face hl-line-face)) (overlay-put global-hl-line-overlay 'window (selected-window)) (hl-line-move global-hl-line-overlay)))) Index: image.el =================================================================== RCS file: /cvsroot/emacs/emacs/lisp/image.el,v retrieving revision 1.46 diff -u -r1.46 image.el --- image.el 27 May 2005 13:17:50 -0000 1.46 +++ image.el 27 May 2005 17:05:46 -0000 @@ -170,6 +170,7 @@ (let ((overlay (make-overlay pos pos buffer)) (prop (if (null area) image (list (list 'margin area) image)))) (put-text-property 0 (length string) 'display prop string) + (overlay-put overlay 'evaporate nil) (overlay-put overlay 'put-image t) (overlay-put overlay 'before-string string)))) Index: minibuf-eldef.el =================================================================== RCS file: /cvsroot/emacs/emacs/lisp/minibuf-eldef.el,v retrieving revision 1.6 diff -u -r1.6 minibuf-eldef.el --- minibuf-eldef.el 2 Jun 2004 22:43:42 -0000 1.6 +++ minibuf-eldef.el 27 May 2005 17:05:46 -0000 @@ -98,6 +98,7 @@ (setq match (if (consp match) (cdr match) 0)) (setq minibuf-eldef-overlay (make-overlay (match-beginning match) (match-end match))) + (overlay-put minibuf-eldef-overlay 'evaporate nil) (setq minibuf-eldef-showing-default-in-prompt t) (setq minibuf-eldef-initial-input (minibuffer-contents-no-properties)) Index: mouse.el =================================================================== RCS file: /cvsroot/emacs/emacs/lisp/mouse.el,v retrieving revision 1.271 diff -u -r1.271 mouse.el --- mouse.el 23 Apr 2005 16:40:55 -0000 1.271 +++ mouse.el 27 May 2005 17:06:04 -0000 @@ -748,6 +748,7 @@ ;; Create an overlay and immediately delete it, to get "overlay in no buffer". (defvar mouse-drag-overlay (make-overlay 1 1)) +(overlay-put mouse-drag-overlay 'evaporate nil) (delete-overlay mouse-drag-overlay) (overlay-put mouse-drag-overlay 'face 'region) Index: gnus/gnus-cite.el =================================================================== RCS file: /cvsroot/emacs/emacs/lisp/gnus/gnus-cite.el,v retrieving revision 1.16 diff -u -r1.16 gnus-cite.el --- gnus/gnus-cite.el 26 May 2005 15:03:29 -0000 1.16 +++ gnus/gnus-cite.el 27 May 2005 17:06:04 -0000 @@ -1017,6 +1017,7 @@ (when (< from to) (push (setq overlay (gnus-make-overlay from to)) gnus-cite-overlay-list) + (gnus-overlay-put overlay 'evaporate t) (gnus-overlay-put overlay 'face face)))))) (defun gnus-cite-toggle (prefix) --=-=-= Also here is my version of Thi's patch to buffer.c. I had to avoid calling Foverlay_put from Fmake_overlay as it caused (make-overlay 1 1) to immediately evaporate. The only other change was to print more information about the overlay so it was easier to track down. --=-=-= Content-Disposition: attachment; filename=evaporate.patch Index: buffer.c =================================================================== RCS file: /cvsroot/emacs/emacs/src/buffer.c,v retrieving revision 1.480 diff -u -r1.480 buffer.c --- buffer.c 27 May 2005 11:01:38 -0000 1.480 +++ buffer.c 27 May 2005 17:06:47 -0000 @@ -178,6 +178,12 @@ Lisp_Object Qinsert_in_front_hooks; Lisp_Object Qinsert_behind_hooks; +#define OVERLAY_EVAPORATE_TESTING +#ifdef OVERLAY_EVAPORATE_TESTING +Lisp_Object Qnever_set; +static Lisp_Object evaporate_overlay P_ ((Lisp_Object)); +#endif + static void alloc_buffer_text P_ ((struct buffer *, size_t)); static void free_buffer_text P_ ((struct buffer *b)); static struct Lisp_Overlay * copy_overlays P_ ((struct buffer *, struct Lisp_Overlay *)); @@ -3602,6 +3608,11 @@ XOVERLAY (overlay)->plist = Qnil; XOVERLAY (overlay)->next = NULL; +#ifdef OVERLAY_EVAPORATE_TESTING + XOVERLAY (overlay)->plist + = Fcons (Qevaporate, Fcons (Qnever_set, XOVERLAY (overlay)->plist)); +#endif + /* Put the new overlay on the wrong list. */ end = OVERLAY_END (overlay); if (OVERLAY_POSITION (end) < b->overlay_center) @@ -3656,6 +3667,24 @@ Lisp_Object Fdelete_overlay (); +#ifdef OVERLAY_EVAPORATE_TESTING +static Lisp_Object +evaporate_overlay (overlay) + Lisp_Object overlay; +{ + if (EQ (Qnever_set, Foverlay_get (overlay, Qevaporate))) + { + Lisp_Object args[3]; + args[0] = build_string("WARNING: Evaporating %S %S"); + args[1] = overlay; + args[2] = Foverlay_properties(overlay); + Fmessage(3,args); + } + + return Fdelete_overlay (overlay); +} +#endif + static struct Lisp_Overlay * unchain_overlay (list, overlay) struct Lisp_Overlay *list, *overlay; @@ -3704,7 +3733,13 @@ CHECK_NUMBER_COERCE_MARKER (end); if (XINT (beg) == XINT (end) && ! NILP (Foverlay_get (overlay, Qevaporate))) - return Fdelete_overlay (overlay); + return +#ifndef OVERLAY_EVAPORATE_TESTING + Fdelete_overlay +#else + evaporate_overlay +#endif + (overlay); if (XINT (beg) > XINT (end)) { @@ -4088,7 +4123,13 @@ if (EQ (prop, Qevaporate) && ! NILP (value) && (OVERLAY_POSITION (OVERLAY_START (overlay)) == OVERLAY_POSITION (OVERLAY_END (overlay)))) - Fdelete_overlay (overlay); +#ifndef OVERLAY_EVAPORATE_TESTING + Fdelete_overlay +#else + evaporate_overlay +#endif + (overlay); + } return value; } @@ -4330,7 +4371,12 @@ hit_list = Fcons (overlay, hit_list); } for (; CONSP (hit_list); hit_list = XCDR (hit_list)) - Fdelete_overlay (XCAR (hit_list)); +#ifndef OVERLAY_EVAPORATE_TESTING + Fdelete_overlay +#else + evaporate_overlay +#endif + (XCAR (hit_list)); } /* Somebody has tried to store a value with an unacceptable type @@ -5207,6 +5253,10 @@ staticpro (&Qoverlayp); Qevaporate = intern ("evaporate"); staticpro (&Qevaporate); +#ifdef OVERLAY_EVAPORATE_TESTING + Qnever_set = intern ("never-set"); + staticpro (&Qnever_set); +#endif Qmodification_hooks = intern ("modification-hooks"); staticpro (&Qmodification_hooks); Qinsert_in_front_hooks = intern ("insert-in-front-hooks"); --=-=-= -- Peter Whaite --=-=-= Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Emacs-devel mailing list Emacs-devel@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-devel --=-=-=--