Strange problem with latest CVS

Strange problem with latest CVS

[Piet van Oostrum]

I just rebuilt emacs with a CVS chackout of this week (src/Changelog has
30 Mar 15:38) and got a strange problem with VM that I use as mail reader.

When I hit a message that ends with an attachment thereafter every message
that I see has all text with the gui-button face. (This is the VM
Presentation buffer)

I think this is caused because the end of the buffer has this face, and VM
reuses the buffer, clears it and fills it with the new message. Apparently
the face is sticking with the empty buffer and is then inherited by the
new text. 
Maybe this change is causing it:

2004-03-26  Masatake YAMATO  <jet@gyve.org>

	* buffer.c (fix_start_end_in_overlays): Rename fix_overlays_in_range.

	* lisp.h (fix_start_end_in_overlays): Likewise.

	* insdel.c (adjust_markers_for_insert): Call fix_start_end_in_overlays.

	* editfns.c (Ftranspose_regions): Likewise.

Is this the intended behaviour? 
-- 
Piet van Oostrum <piet@cs.uu.nl>
URL: http://www.cs.uu.nl/~piet [PGP]
Private email: P.van.Oostrum@hccnet.nl

Re: Strange problem with latest CVS

[Matt Hodges]

>>>>> Piet van Oostrum writes:

 > I just rebuilt emacs with a CVS chackout of this week
 > (src/Changelog has 30 Mar 15:38) and got a strange problem with VM
 > that I use as mail reader.

 > When I hit a message that ends with an attachment thereafter every
 > message that I see has all text with the gui-button face. (This is
 > the VM Presentation buffer)

 > I think this is caused because the end of the buffer has this face,
 > and VM reuses the buffer, clears it and fills it with the new
 > message. Apparently the face is sticking with the empty buffer and
 > is then inherited by the new text. Maybe this change is causing it:

I think emacs-w3m hits the same problem. Overlays from 1 to 1 in
otherwise empty buffers span 1 to (point-max) after insertion of text;
previously (before the change you mention, I think) such overlays
still spanned 1 to 1 after insertion of text. See:

          <URL:http://emacs-w3m.namazu.org/ml/msg06543.html>

One such overlay has these properties:

 (help-echo nil mouse-face w3m-form-button-mouse-face face w3m-form-button-face keymap
            (keymap
             (13 . widget-button-press)
             (down-mouse-2 . widget-button-click)
             (backtab)
             (S-tab)
             (9))
            button
            (w3m-form-button :delete widget-leave-text :w3m-form-action
                             (w3m-form-submit
                              [w3m-form-object get "http://www.google.com/search" nil urlencoded
                                               (hl
                                                (:value "en")
                                                ie
                                                (:value "ISO-8859-1")
                                                q
                                                (:value "xmakemol")
                                                btnG
                                                (:value nil)
                                                btnI
                                                (:value nil))]
                              "btnG" "Google Search")
                             :from #<marker
                             (moves after insertion)
                             at 3668 in *w3m*> :to #<marker at 1 in *w3m*> :button-overlay #<overlay from 97 to 3668 in *w3m*>))

Matt

Re: Strange problem with latest CVS

[Matt Hodges]

>>>>> Matt Hodges writes:

 > > When I hit a message that ends with an attachment thereafter
 > > every message that I see has all text with the gui-button face.
 > > (This is the VM Presentation buffer)

 > > I think this is caused because the end of the buffer has this
 > > face, and VM reuses the buffer, clears it and fills it with the
 > > new message. Apparently the face is sticking with the empty
 > > buffer and is then inherited by the new text. Maybe this change
 > > is causing it:

 > I think emacs-w3m hits the same problem. Overlays from 1 to 1 in
 > otherwise empty buffers span 1 to (point-max) after insertion of
 > text; previously (before the change you mention, I think) such
 > overlays still spanned 1 to 1 after insertion of text.

I think the same problem can be seen in Gnus (v5.10.6). If I do
gnus-summary-toggle-header, then an overlay including mouse-face
highlight spans all the headers.

Does anyone else see this?

Matt

Re: Strange problem with latest CVS

[Romain Francoise]

Matt Hodges <matt@tc.bham.ac.uk> writes:

> I think the same problem can be seen in Gnus (v5.10.6). If I do
> gnus-summary-toggle-header, then an overlay including mouse-face
> highlight spans all the headers.

> Does anyone else see this?

Yes; I have the same problems with both Gnus and emacs-w3m.

(With the latest update from the multi-tty branch.)

-- 
Romain Francoise <romain@orebokech.com> | It was fourteen degrees below
it's a miracle -- http://orebokech.com/ | on a screeching march 23.

Re: Strange problem with latest CVS

[Masatake YAMATO]

Sorry to be late.

> >>>>> Matt Hodges writes:
> 
>  > > When I hit a message that ends with an attachment thereafter
>  > > every message that I see has all text with the gui-button face.
>  > > (This is the VM Presentation buffer)
> 
>  > > I think this is caused because the end of the buffer has this
>  > > face, and VM reuses the buffer, clears it and fills it with the
>  > > new message. Apparently the face is sticking with the empty
>  > > buffer and is then inherited by the new text. Maybe this change
>  > > is causing it:

Yes.

This is something to do with my change:

2004-03-26  Masatake YAMATO  <jet@gyve.org>

	* buffer.c (fix_start_end_in_overlays): Rename fix_overlays_in_range.

	* lisp.h (fix_start_end_in_overlays): Likewise.

	* insdel.c (adjust_markers_for_insert): Call fix_start_end_in_overlays.

	* editfns.c (Ftranspose_regions): Likewise.

See also http://mail.gnu.org/archive/html/emacs-devel/2004-03/msg00476.html

>  > I think emacs-w3m hits the same problem. Overlays from 1 to 1 in
>  > otherwise empty buffers span 1 to (point-max) after insertion of
>  > text; previously (before the change you mention, I think) such
>  > overlays still spanned 1 to 1 after insertion of text.
> 
> I think the same problem can be seen in Gnus (v5.10.6). If I do
> gnus-summary-toggle-header, then an overlay including mouse-face
> highlight spans all the headers.

How do you think make evaporate overlay's property t by default(when make-overlay)?

    `evaporate'
	 If this property is non-`nil', the overlay is deleted automatically
	 if it becomes empty (i.e., if its length becomes zero).  However,
	 if the overlay is _already_ empty, `evaporate' does not delete it.

Re: Strange problem with latest CVS

[peta]

Masatake YAMATO <jet@gyve.org> wrote:
> Sorry to be late.
> 
> > >>>>> Matt Hodges writes:
> > 
> >  > > I think this is caused because the end of the buffer has this
> >  > > face, and VM reuses the buffer, clears it and fills it with the
> >  > > new message. Apparently the face is sticking with the empty
> >  > > buffer and is then inherited by the new text. Maybe this change
> >  > > is causing it:
> 
> >  > I think emacs-w3m hits the same problem. Overlays from 1 to 1 in
> >  > otherwise empty buffers span 1 to (point-max) after insertion of
> >  > text; previously (before the change you mention, I think) such
> >  > overlays still spanned 1 to 1 after insertion of text.
> > 
> > I think the same problem can be seen in Gnus (v5.10.6). If I do
> > gnus-summary-toggle-header, then an overlay including mouse-face
> > highlight spans all the headers.

I'd just like to add that mh-e also has this problem when displaying
mime buttons and parts.  It was fixed by setting the overlay 'evaporate
property to t.

Inserting at a zero length overlay is inherently ambiguous as to whether
the insertion is at the front or end the overlay.  The default behaviour
of (make-overlay) is different in these two cases -- at the front the
insertion will get the overlay properties while at the end it will not.

It looks like the modifications have changed the notion of whether
insertion is at the front or end of the overlay.  I dont know enough
about the original design specs to say if this new behaviour is right or
wrong.

In any case it seems like a change for the better.  Previously all the
gui-button faces would shrink to zero-length overlays and remain there,
invisible, when the buffer contents were deleted and new contents
inserted.  I now see that a lot of overlays accumulated after may reuses
of the same buffer.

> How do you think make evaporate overlay's property t by default(when
> make-overlay)?

On the face of it I think that overlays should evaporate by default.  I
can see that not evaporating is useful when an overlay in a region being
deleted completely spans that region (re-insertions take on the overlay
properties of the deletion), but it looks like the less frequent case.

What do the original designers of the overlay system say, and what will
it break if changed?

-- 
Peter Whaite (http://whaite.ca)

Re: Strange problem with latest CVS

[Richard Stallman]

    How do you think make evaporate overlay's property t by default(when make-overlay)?

That would break many programs.  It is the wrong solution.

I think the right fix is in fix_start_end_in_overlays.  When an
insertion occurs next to an overlay whose beginning-marker is the
advancing kind and whose end-marker is not, the overlay should stay
empty.

Re: Strange problem with latest CVS

[Richard Stallman]

    I'd just like to add that mh-e also has this problem when displaying
    mime buttons and parts.  It was fixed by setting the overlay 'evaporate
    property to t.

    In any case it seems like a change for the better.  Previously all the
    gui-button faces would shrink to zero-length overlays and remain there,
    invisible, when the buffer contents were deleted and new contents
    inserted.  I now see that a lot of overlays accumulated after may reuses
    of the same buffer.

It sounds like THESE overlays should be set to evaporate.
But I think MH-E should do that.  I cc'd the MH-E maintainer.

Re: Strange problem with latest CVS

[Masatake YAMATO]

>     I'd just like to add that mh-e also has this problem when displaying
>     mime buttons and parts.  It was fixed by setting the overlay 'evaporate
>     property to t.
> 
>     In any case it seems like a change for the better.  Previously all the
>     gui-button faces would shrink to zero-length overlays and remain there,
>     invisible, when the buffer contents were deleted and new contents
>     inserted.  I now see that a lot of overlays accumulated after may reuses
>     of the same buffer.
> 
> It sounds like THESE overlays should be set to evaporate.
> But I think MH-E should do that.  I cc'd the MH-E maintainer.

How do you think about widgets?
I think widgets should set evaporate to their overlays by themselves.

Re: Strange problem with latest CVS

[Masatake YAMATO]

>     How do you think make evaporate overlay's property t by default(when make-overlay)?
> 
> That would break many programs.  It is the wrong solution.
> 
> I think the right fix is in fix_start_end_in_overlays.  When an
> insertion occurs next to an overlay whose beginning-marker is the
> advancing kind and whose end-marker is not, the overlay should stay
> empty.

I will inspect this. Give me time. I'm recovering my hard disk now.

Re: Strange problem with latest CVS

[Masatake YAMATO]

>     How do you think make evaporate overlay's property t by default(when make-overlay)?
> 
> That would break many programs.  It is the wrong solution.
> 
> I think the right fix is in fix_start_end_in_overlays.  When an
> insertion occurs next to an overlay whose beginning-marker is the
> advancing kind and whose end-marker is not, the overlay should stay
> empty.


I've tried. I have done in two things in the attached patch.
1) I could still find the situation that Emacs returned an overlay 
   whose start is greater than its end. I have fixed in the patch.
2) As you wrote, I made overlays empty in fix_start_end_in_overlays.

The biggest question is that whether I should use overlay-start or 
overlay-end to make an overlay empty. I'm using following code
in the patch. 

	    // for overlay_before
	    int pivot = (startpos >= start)? endpos: startpos; 
	    // for overlay_after
	    int pivot = (endpos < end)? startpos: endpos;	     

	    Fset_marker (OVERLAY_START (overlay), make_number (pivot),
  			 Qnil);
            Fset_marker (OVERLAY_END (overlay), make_number (pivot),
  			 Qnil);

Masatake YAMATO

2004-04-08  Masatake YAMATO  <jet@gyve.org>

	* buffer.c (fix_start_end_in_overlays): Normalize
	the order of start and end in overlays before comparing
	the edited range and overlay.
	Introduce `make_empty' local variable. Make it true if
	an overlay is upside-down.
	Make overlay empty if it is upside-down.

cvs diff: warning: unrecognized response `access control disabled, clients can connect from any host' from cvs server
Index: src/buffer.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/buffer.c,v
retrieving revision 1.446
diff -u -r1.446 buffer.c
*** src/buffer.c	25 Mar 2004 18:05:29 -0000	1.446
--- src/buffer.c	8 Apr 2004 10:53:03 -0000
***************
*** 3290,3297 ****
     endpoint in this range will need to be unlinked from the overlay
     list and reinserted in its proper place.
     Such an overlay might even have negative size at this point.
!    If so, we'll reverse the endpoints.  Can you think of anything
!    better to do in this situation?  */
  void
  fix_start_end_in_overlays (start, end)
       register int start, end;
--- 3290,3296 ----
     endpoint in this range will need to be unlinked from the overlay
     list and reinserted in its proper place.
     Such an overlay might even have negative size at this point.
!    If so, we'll make the overlay empty. */
  void
  fix_start_end_in_overlays (start, end)
       register int start, end;
***************
*** 3308,3313 ****
--- 3307,3313 ----
    struct Lisp_Overlay *tail, *parent;
    int startpos, endpos;
  
+   int make_empty;
    /* This algorithm shifts links around instead of consing and GCing.
       The loop invariant is that before_list (resp. after_list) is a
       well-formed list except that its last element, the CDR of beforep
***************
*** 3318,3339 ****
    for (parent = NULL, tail = current_buffer->overlays_before; tail;)
      {
        XSETMISC (overlay, tail);
        endpos = OVERLAY_POSITION (OVERLAY_END (overlay));
        if (endpos < start)
  	break;
!       startpos = OVERLAY_POSITION (OVERLAY_START (overlay));
        if (endpos < end
  	  || (startpos >= start && startpos < end))
  	{
! 	  /* If the overlay is backwards, fix that now.  */
! 	  if (startpos > endpos)
  	    {
! 	      int tem;
! 	      Fset_marker (OVERLAY_START (overlay), make_number (endpos),
  			   Qnil);
! 	      Fset_marker (OVERLAY_END (overlay), make_number (startpos),
  			   Qnil);
- 	      tem = startpos; startpos = endpos; endpos = tem;
  	    }
  	  /* Add it to the end of the wrong list.  Later on,
  	     recenter_overlay_lists will move it to the right place.  */
--- 3318,3354 ----
    for (parent = NULL, tail = current_buffer->overlays_before; tail;)
      {
        XSETMISC (overlay, tail);
+ 
+       /* Normalize the order of startpos and endpos. */
        endpos = OVERLAY_POSITION (OVERLAY_END (overlay));
+       startpos = OVERLAY_POSITION (OVERLAY_START (overlay));
+       if (endpos < startpos)
+ 	{
+ 	  int tem;
+ 	  make_empty = 1;
+ 	  tem 	   = startpos;
+ 	  startpos = endpos;
+ 	  endpos   = tem;
+ 	}
+       else
+ 	make_empty = 0;
+ 
+       /* The order of startpos and endpos is normalized.
+ 	 So we can use endpos to be comapred with start. */
        if (endpos < start)
  	break;
!       
        if (endpos < end
  	  || (startpos >= start && startpos < end))
  	{
! 	  /* If the overlay is backwards, make it empty.  */
! 	  if (make_empty)
  	    {
! 	      int pivot = (startpos >= start)? endpos: startpos;
! 	      Fset_marker (OVERLAY_START (overlay), make_number (pivot),
  			   Qnil);
! 	      Fset_marker (OVERLAY_END (overlay), make_number (pivot),
  			   Qnil);
  	    }
  	  /* Add it to the end of the wrong list.  Later on,
  	     recenter_overlay_lists will move it to the right place.  */
***************
*** 3362,3385 ****
        else
  	parent = tail, tail = parent->next;
      }
    for (parent = NULL, tail = current_buffer->overlays_after; tail;)
      {
        XSETMISC (overlay, tail);
        startpos = OVERLAY_POSITION (OVERLAY_START (overlay));
        if (startpos >= end)
  	break;
!       endpos = OVERLAY_POSITION (OVERLAY_END (overlay));
        if (startpos >= start
  	  || (endpos >= start && endpos < end))
  	{
! 	  if (startpos > endpos)
  	    {
! 	      int tem;
! 	      Fset_marker (OVERLAY_START (overlay), make_number (endpos),
  			   Qnil);
! 	      Fset_marker (OVERLAY_END (overlay), make_number (startpos),
  			   Qnil);
- 	      tem = startpos; startpos = endpos; endpos = tem;
  	    }
  	  if (endpos < current_buffer->overlay_center)
  	    {
--- 3377,3417 ----
        else
  	parent = tail, tail = parent->next;
      }
+   
+   make_empty = 0;
    for (parent = NULL, tail = current_buffer->overlays_after; tail;)
      {
        XSETMISC (overlay, tail);
        startpos = OVERLAY_POSITION (OVERLAY_START (overlay));
+       endpos = OVERLAY_POSITION (OVERLAY_END (overlay));
+ 
+       /* Normalize the order of startpos and endpos. */
+       if (endpos < startpos)
+ 	{
+ 	  int tem;
+ 	  make_empty = 1;
+ 	  tem 	   = startpos;
+ 	  startpos = endpos;
+ 	  endpos   = tem;
+ 	}
+       else
+ 	make_empty = 0;
+ 
+       /* The order of startpos and endpos is normalized.
+ 	 So we can use endpos to be comapred with start. */      
        if (startpos >= end)
  	break;
! 
        if (startpos >= start
  	  || (endpos >= start && endpos < end))
  	{
! 	  if (make_empty)
  	    {
! 	      int pivot = (endpos < end)? startpos: endpos;
! 	      Fset_marker (OVERLAY_START (overlay), make_number (pivot),
  			   Qnil);
! 	      Fset_marker (OVERLAY_END (overlay), make_number (pivot),
  			   Qnil);
  	    }
  	  if (endpos < current_buffer->overlay_center)
  	    {

Re: Strange problem with latest CVS

[Richard Stallman]

    + 	  make_empty = 1;
    + 	  tem 	   = startpos;
    + 	  startpos = endpos;
    + 	  endpos   = tem;

It is a bit convoluted, and I suspect it could be done more simply.
However, if it fixes the bug, please install it.

    The biggest question is that whether I should use overlay-start or 
    overlay-end to make an overlay empty.

It may not make much difference, but I think it is better to use
overlay-end.  That is the earlier address in this case, and the result
will be that the overlay stays before the inserted text.  However, I
wouldn't say that the other choice is wrong.  In this bizarre
situation, the best we can hope for is to keep things consistent, one
way or another.

Re: Strange problem with latest CVS

[Richard Stallman]

    How do you think about widgets?
    I think widgets should set evaporate to their overlays by themselves.

I don't understand the question, sorry.

Re: Strange problem with latest CVS

[Masatake YAMATO]

>     + 	  make_empty = 1;
>     + 	  tem 	   = startpos;
>     + 	  startpos = endpos;
>     + 	  endpos   = tem;
> 
> It is a bit convoluted, and I suspect it could be done more simply.
> However, if it fixes the bug, please install it.
> 
>     The biggest question is that whether I should use overlay-start or 
>     overlay-end to make an overlay empty.
> 
> It may not make much difference, but I think it is better to use
> overlay-end.  That is the earlier address in this case, and the result
> will be that the overlay stays before the inserted text.  However, I
> wouldn't say that the other choice is wrong.  In this bizarre
> situation, the best we can hope for is to keep things consistent, one
> way or another.

I have made the patch simpler. I will install this one.
Should I install it to HEAD?

2004-04-11  Masatake YAMATO  <jet@gyve.org>

	* buffer.c (fix_start_end_in_overlays): make overlays 
	empty if they are backwards.

cvs diff: warning: unrecognized response `access control disabled, clients can connect from any host' from cvs server
Index: src/buffer.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/buffer.c,v
retrieving revision 1.446
diff -u -r1.446 buffer.c
--- src/buffer.c	25 Mar 2004 18:05:29 -0000	1.446
+++ src/buffer.c	10 Apr 2004 17:36:02 -0000
@@ -3290,8 +3290,7 @@
    endpoint in this range will need to be unlinked from the overlay
    list and reinserted in its proper place.
    Such an overlay might even have negative size at this point.
-   If so, we'll reverse the endpoints.  Can you think of anything
-   better to do in this situation?  */
+   If so, we'll make the overlay empty. */
 void
 fix_start_end_in_overlays (start, end)
      register int start, end;
@@ -3318,23 +3317,24 @@
   for (parent = NULL, tail = current_buffer->overlays_before; tail;)
     {
       XSETMISC (overlay, tail);
+
       endpos = OVERLAY_POSITION (OVERLAY_END (overlay));
+      startpos = OVERLAY_POSITION (OVERLAY_START (overlay));
+
+      /* If the overlay is backwards, make it empty.  */
+      if (endpos < startpos)
+	{
+	  startpos = endpos;
+	  Fset_marker (OVERLAY_START (overlay), make_number (startpos),
+		       Qnil);
+	}
+
       if (endpos < start)
 	break;
-      startpos = OVERLAY_POSITION (OVERLAY_START (overlay));
+      
       if (endpos < end
 	  || (startpos >= start && startpos < end))
 	{
-	  /* If the overlay is backwards, fix that now.  */
-	  if (startpos > endpos)
-	    {
-	      int tem;
-	      Fset_marker (OVERLAY_START (overlay), make_number (endpos),
-			   Qnil);
-	      Fset_marker (OVERLAY_END (overlay), make_number (startpos),
-			   Qnil);
-	      tem = startpos; startpos = endpos; endpos = tem;
-	    }
 	  /* Add it to the end of the wrong list.  Later on,
 	     recenter_overlay_lists will move it to the right place.  */
 	  if (endpos < current_buffer->overlay_center)
@@ -3365,22 +3365,24 @@
   for (parent = NULL, tail = current_buffer->overlays_after; tail;)
     {
       XSETMISC (overlay, tail);
+
       startpos = OVERLAY_POSITION (OVERLAY_START (overlay));
+      endpos = OVERLAY_POSITION (OVERLAY_END (overlay));
+
+      /* If the overlay is backwards, make it empty.  */
+      if (endpos < startpos)
+	{
+	  startpos = endpos;
+	  Fset_marker (OVERLAY_START (overlay), make_number (startpos),
+		       Qnil);	  
+	}
+
       if (startpos >= end)
 	break;
-      endpos = OVERLAY_POSITION (OVERLAY_END (overlay));
+
       if (startpos >= start
 	  || (endpos >= start && endpos < end))
 	{
-	  if (startpos > endpos)
-	    {
-	      int tem;
-	      Fset_marker (OVERLAY_START (overlay), make_number (endpos),
-			   Qnil);
-	      Fset_marker (OVERLAY_END (overlay), make_number (startpos),
-			   Qnil);
-	      tem = startpos; startpos = endpos; endpos = tem;
-	    }
 	  if (endpos < current_buffer->overlay_center)
 	    {
 	      if (!afterp)

Re: Strange problem with latest CVS

[Masatake YAMATO]

> > I have made the patch simpler. I will install this one.
> > Should I install it to HEAD?
> 
> On the trunk, yes.

Thanks. I've just installed.

Masatake YAMATO

Re: Strange problem with latest CVS

[Kim F. Storm]

Masatake YAMATO <jet@gyve.org> writes:

> I have made the patch simpler. I will install this one.
> Should I install it to HEAD?

On the trunk, yes.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Strange problem with latest CVS

[Masatake YAMATO]

>     How do you think about widgets?
>     I think widgets should set evaporate to their overlays by themselves.
> 
> I don't understand the question, sorry.

It seems that (at least) widgets related code doesn't set evaporate to
their overlays(*). My question was whether I should fix it.
Now, your answer is obvious. I will fix it.

Masatake YAMATO
-------------------------------------------------------------------------
(*) Produce typical problem:

1. eval the attached code.
2. Do M-x widget-example and you will get *Widget Example Buffer*.
3. Press return key at [Reset Form]
4. Overlay fills the buffer. and it should not do.

;; "Programming Example" in widget.info.

     (require 'widget)
     
     (eval-when-compile
       (require 'wid-edit))
     
     (defvar widget-example-repeat)
     
     (defun widget-example ()
       "Create the widgets from the Widget manual."
       (interactive)
       (switch-to-buffer "*Widget Example*")
       (kill-all-local-variables)
       (make-local-variable 'widget-example-repeat)
       (let ((inhibit-read-only t))
         (erase-buffer))
       (widget-insert "Here is some documentation.\n\nName: ")
       (widget-create 'editable-field
     		 :size 13
     		 "My Name")
       (widget-create 'menu-choice
     		 :tag "Choose"
     		 :value "This"
     		 :help-echo "Choose me, please!"
     		 :notify (lambda (widget &rest ignore)
     			   (message "%s is a good choice!"
     				    (widget-value widget)))
     		 '(item :tag "This option" :value "This")
     		 '(choice-item "That option")
     		 '(editable-field :menu-tag "No option" "Thus option"))
       (widget-insert "Address: ")
       (widget-create 'editable-field
     		 "Some Place\nIn some City\nSome country.")
       (widget-insert "\nSee also ")
       (widget-create 'link
     		 :notify (lambda (&rest ignore)
     			   (widget-value-set widget-example-repeat
     					     '("En" "To" "Tre"))
     			   (widget-setup))
     		 "other work")
       (widget-insert
         " for more information.\n\nNumbers: count to three below\n")
       (setq widget-example-repeat
     	(widget-create 'editable-list
     		       :entry-format "%i %d %v"
     		       :notify (lambda (widget &rest ignore)
     				 (let ((old (widget-get widget
     							':example-length))
     				       (new (length (widget-value widget))))
     				   (unless (eq old new)
     				     (widget-put widget ':example-length new)
     				     (message "You can count to %d." new))))
     		       :value '("One" "Eh, two?" "Five!")
     		       '(editable-field :value "three")))
       (widget-insert "\n\nSelect multiple:\n\n")
       (widget-create 'checkbox t)
       (widget-insert " This\n")
       (widget-create 'checkbox nil)
       (widget-insert " That\n")
       (widget-create 'checkbox
     		 :notify (lambda (&rest ignore) (message "Tickle"))
     		 t)
       (widget-insert " Thus\n\nSelect one:\n\n")
       (widget-create 'radio-button-choice
     		 :value "One"
     		 :notify (lambda (widget &rest ignore)
     			   (message "You selected %s"
     				    (widget-value widget)))
     		 '(item "One") '(item "Another One.") '(item "A Final One."))
       (widget-insert "\n")
       (widget-create 'push-button
     		 :notify (lambda (&rest ignore)
     			   (if (= (length (widget-value widget-example-repeat))
     				  3)
     			       (message "Congratulation!")
     			     (error "Three was the count!")))
     		 "Apply Form")
       (widget-insert " ")
       (widget-create 'push-button
     		 :notify (lambda (&rest ignore)
     			   (widget-example))
     		 "Reset Form")
       (widget-insert "\n")
       (use-local-map widget-keymap)
       (widget-setup))

Re: Strange problem with latest CVS

[Richard Stallman]

    It seems that (at least) widgets related code doesn't set evaporate to
    their overlays(*). My question was whether I should fix it.
    Now, your answer is obvious. I will fix it.

No!  These overlays should not evaporate.
If they evaporate, the form won't work any more.
It needs to have those overlays.
The fix must be something else.

Perhaps the change in fix_start_end_in_overlays will fix this.

Re: Strange problem with latest CVS

[David Kastrup]

Richard Stallman <rms@gnu.org> writes:

>     It seems that (at least) widgets related code doesn't set evaporate to
>     their overlays(*). My question was whether I should fix it.
>     Now, your answer is obvious. I will fix it.
> 
> No!  These overlays should not evaporate.
> If they evaporate, the form won't work any more.

Unless I misunderstand, they will evaporate only when the underlying
buffer content gets deleted to prepare the buffer for insertion of new
contents.  The old overlays/buttons are then completely useless to
keep around.

When they evaporate, they do it at a time when there is no form left
that could be working.

So I think the solution proposed by Masatake Yamato is the correct
one, unless I misunderstand the problem.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Strange problem with latest CVS

[Masatake YAMATO]

> Richard Stallman <rms@gnu.org> writes:
> 
> >     It seems that (at least) widgets related code doesn't set evaporate to
> >     their overlays(*). My question was whether I should fix it.
> >     Now, your answer is obvious. I will fix it.
> > 
> > No!  These overlays should not evaporate.
> > If they evaporate, the form won't work any more.
> 
> Unless I misunderstand, they will evaporate only when the underlying
> buffer content gets deleted to prepare the buffer for insertion of new
> contents.  The old overlays/buttons are then completely useless to
> keep around.
> 
> When they evaporate, they do it at a time when there is no form left
> that could be working.

Thank you for your explanation. You write what I want to say.
However, as RMS wrote, I wondner what I should do when a editable-field
becomes empty during the user editing. Try the next code with the attached
patch. Delete "Delete me" in *Widget Example* buffer. You will get a signal.

(progn
  (interactive)
  (switch-to-buffer "*Widget Example*")
  (kill-all-local-variables)
  (let ((inhibit-read-only t))
    (erase-buffer))
  (widget-create 'editable-field
     		 "Delete me")
  (use-local-map widget-keymap)
  (widget-setup))


cvs diff: warning: unrecognized response `access control disabled, clients can connect from any host' from cvs server
Index: lisp/wid-edit.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/wid-edit.el,v
retrieving revision 1.124
diff -u -r1.124 wid-edit.el
--- lisp/wid-edit.el	4 Jan 2004 15:11:59 -0000	1.124
+++ lisp/wid-edit.el	14 Apr 2004 03:11:19 -0000
@@ -343,7 +343,9 @@
 	;; works in the field when, say, Custom uses `suppress-keymap'.
 	(overlay-put overlay 'local-map keymap)
 	(overlay-put overlay 'face face)
-	(overlay-put overlay 'help-echo help-echo))
+	(overlay-put overlay 'help-echo help-echo)
+	(overlay-put overlay 'evaporate t)
+	)
       (setq to (1- to))
       (setq rear-sticky t))
     (let ((overlay (make-overlay from to nil nil rear-sticky)))
@@ -352,7 +354,9 @@
       (overlay-put overlay 'field widget)
       (overlay-put overlay 'local-map keymap)
       (overlay-put overlay 'face face)
-      (overlay-put overlay 'help-echo help-echo)))
+      (overlay-put overlay 'help-echo help-echo)
+      (overlay-put overlay 'evaporate t)
+      ))
   (widget-specify-secret widget))
 
 (defun widget-specify-secret (field)
@@ -382,6 +386,7 @@
       (setq help-echo 'widget-mouse-help))
     (overlay-put overlay 'button widget)
     (overlay-put overlay 'keymap (widget-get widget :keymap))
+    (overlay-put overlay 'evaporate t)
     ;; We want to avoid the face with image buttons.
     (unless (widget-get widget :suppress-face)
       (overlay-put overlay 'face (widget-apply widget :button-face-get))
@@ -401,6 +406,7 @@
   "Specify sample for WIDGET between FROM and TO."
   (let ((overlay (make-overlay from to nil t nil)))
     (overlay-put overlay 'face (widget-apply widget :sample-face-get))
+    (overlay-put overlay 'evaporate t)
     (widget-put widget :sample-overlay overlay)))
 
 (defun widget-specify-doc (widget from to)
@@ -408,6 +414,7 @@
   (let ((overlay (make-overlay from to nil t nil)))
     (overlay-put overlay 'widget-doc widget)
     (overlay-put overlay 'face widget-documentation-face)
+    (overlay-put overlay 'evaporate t)
     (widget-put widget :doc-overlay overlay)))
 
 (defmacro widget-specify-insert (&rest form)

Re: Strange problem with latest CVS

[Richard Stallman]

    However, as RMS wrote, I wondner what I should do when a editable-field
    becomes empty during the user editing.

I think the overlay should remain in existence, empty.
Then if the user inserts more text, the overlay should include it.

So the overlay's end-marker should advance over insertion, and the
start-marker should not advance.  That way everything should work right
whether the field is empty or not.

Re: Strange problem with latest CVS

[Richard Stallman]

    >     It seems that (at least) widgets related code doesn't set evaporate to
    >     their overlays(*). My question was whether I should fix it.
    >     Now, your answer is obvious. I will fix it.
    > 
    > No!  These overlays should not evaporate.
    > If they evaporate, the form won't work any more.

    Unless I misunderstand, they will evaporate only when the underlying
    buffer content gets deleted to prepare the buffer for insertion of new
    contents.

As I understand it, each overlay covers one field,
and if the field becomes empty, the overlay will evaporate.

	       The old overlays/buttons are then completely useless to
    keep around.

Is that really true?  If the user adds more text in the field, isn't
the overlay supposed to cover that new text?

Re: Strange problem with latest CVS

[Masatake YAMATO]

>     >     It seems that (at least) widgets related code doesn't set evaporate to
>     >     their overlays(*). My question was whether I should fix it.
>     >     Now, your answer is obvious. I will fix it.
>     > 
>     > No!  These overlays should not evaporate.
>     > If they evaporate, the form won't work any more.
> 
>     Unless I misunderstand, they will evaporate only when the underlying
>     buffer content gets deleted to prepare the buffer for insertion of new
>     contents.
> 
> As I understand it, each overlay covers one field,
> and if the field becomes empty, the overlay will evaporate.
> 
> 	       The old overlays/buttons are then completely useless to
>     keep around.
> 
> Is that really true?  If the user adds more text in the field, isn't
> the overlay supposed to cover that new text?

Difficult to answer.  At least "Programming Example" in widget's info
(attached to this mail) doesn't work well.  Because `erase-buffer'
makes all overlays empty. Should remove-overlays be used in the
example?  Another idea is that adding an optional argument to
`erase-buffer' like

(defun erase-buffer (&optional delete-overlay-too)
       ...)

If the user gives non-nil value to erase-buffer as the argument, all
overlays are also removed.

Anyway, overlays for button, sample and doc should be put evaporate.
See the attachd patch.

Masatake YAMATO

\f
File: widget,  Node: Programming Example,  Next: Setting Up the Buffer,  Prev: User Interface,  Up: Top

Programming Example
===================

   Here is the code to implement the user interface example (see *note User
Interface::).

     (require 'widget)
     
     (eval-when-compile
       (require 'wid-edit))
     
     (defvar widget-example-repeat)
     
     (defun widget-example ()
       "Create the widgets from the Widget manual."
       (interactive)
       (switch-to-buffer "*Widget Example*")
       (kill-all-local-variables)
       (make-local-variable 'widget-example-repeat)
       (let ((inhibit-read-only t))
         (erase-buffer))
       (widget-insert "Here is some documentation.\n\nName: ")
       (widget-create 'editable-field
     		 :size 13
     		 "My Name")
       (widget-create 'menu-choice
     		 :tag "Choose"
     		 :value "This"
     		 :help-echo "Choose me, please!"
     		 :notify (lambda (widget &rest ignore)
     			   (message "%s is a good choice!"
     				    (widget-value widget)))
     		 '(item :tag "This option" :value "This")
     		 '(choice-item "That option")
     		 '(editable-field :menu-tag "No option" "Thus option"))
       (widget-insert "Address: ")
       (widget-create 'editable-field
     		 "Some Place\nIn some City\nSome country.")
       (widget-insert "\nSee also ")
       (widget-create 'link
     		 :notify (lambda (&rest ignore)
     			   (widget-value-set widget-example-repeat
     					     '("En" "To" "Tre"))
     			   (widget-setup))
     		 "other work")
       (widget-insert
         " for more information.\n\nNumbers: count to three below\n")
       (setq widget-example-repeat
     	(widget-create 'editable-list
     		       :entry-format "%i %d %v"
     		       :notify (lambda (widget &rest ignore)
     				 (let ((old (widget-get widget
     							':example-length))
     				       (new (length (widget-value widget))))
     				   (unless (eq old new)
     				     (widget-put widget ':example-length new)
     				     (message "You can count to %d." new))))
     		       :value '("One" "Eh, two?" "Five!")
     		       '(editable-field :value "three")))
       (widget-insert "\n\nSelect multiple:\n\n")
       (widget-create 'checkbox t)
       (widget-insert " This\n")
       (widget-create 'checkbox nil)
       (widget-insert " That\n")
       (widget-create 'checkbox
     		 :notify (lambda (&rest ignore) (message "Tickle"))
     		 t)
       (widget-insert " Thus\n\nSelect one:\n\n")
       (widget-create 'radio-button-choice
     		 :value "One"
     		 :notify (lambda (widget &rest ignore)
     			   (message "You selected %s"
     				    (widget-value widget)))
     		 '(item "One") '(item "Another One.") '(item "A Final One."))
       (widget-insert "\n")
       (widget-create 'push-button
     		 :notify (lambda (&rest ignore)
     			   (if (= (length (widget-value widget-example-repeat))
     				  3)
     			       (message "Congratulation!")
     			     (error "Three was the count!")))
     		 "Apply Form")
       (widget-insert " ")
       (widget-create 'push-button
     		 :notify (lambda (&rest ignore)
     			   (widget-example))
     		 "Reset Form")
       (widget-insert "\n")
       (use-local-map widget-keymap)
       (widget-setup))

\f
2004-04-19  Masatake YAMATO  <jet@gyve.org>

	* wid-edit.el (widget-specify-button): Put evaporate to the 
	overlay for sample.
	(widget-specify-sample): Put evaporate to the overlay for sample.
	(widget-specify-doc): Put evaporate to the overlay for documentation.

Index: lisp/wid-edit.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/wid-edit.el,v
retrieving revision 1.124
diff -u -r1.124 wid-edit.el
--- lisp/wid-edit.el	4 Jan 2004 15:11:59 -0000	1.124
+++ lisp/wid-edit.el	19 Apr 2004 08:24:37 -0000
@@ -382,6 +382,7 @@
       (setq help-echo 'widget-mouse-help))
     (overlay-put overlay 'button widget)
     (overlay-put overlay 'keymap (widget-get widget :keymap))
+    (overlay-put overlay 'evaporate t)
     ;; We want to avoid the face with image buttons.
     (unless (widget-get widget :suppress-face)
       (overlay-put overlay 'face (widget-apply widget :button-face-get))
@@ -401,6 +402,7 @@
   "Specify sample for WIDGET between FROM and TO."
   (let ((overlay (make-overlay from to nil t nil)))
     (overlay-put overlay 'face (widget-apply widget :sample-face-get))
+    (overlay-put overlay 'evaporate t)
     (widget-put widget :sample-overlay overlay)))
 
 (defun widget-specify-doc (widget from to)
@@ -408,6 +410,7 @@
   (let ((overlay (make-overlay from to nil t nil)))
     (overlay-put overlay 'widget-doc widget)
     (overlay-put overlay 'face widget-documentation-face)
+    (overlay-put overlay 'evaporate t)
     (widget-put widget :doc-overlay overlay)))
 
 (defmacro widget-specify-insert (&rest form)

Re: Strange problem with latest CVS

[Richard Stallman]

    Difficult to answer.  At least "Programming Example" in widget's info
    (attached to this mail) doesn't work well.  Because `erase-buffer'
    makes all overlays empty.

I don't follow you.  The example code, widget-example,
seems to be intended to set up the widgets in the buffer.
Why would you use it again once the widgets exist
in the buffer?

Re: Strange problem with latest CVS

[Masatake YAMATO]

>     Difficult to answer.  At least "Programming Example" in widget's info
>     (attached to this mail) doesn't work well.  Because `erase-buffer'
>     makes all overlays empty.
> 
> I don't follow you.  The example code, widget-example,
> seems to be intended to set up the widgets in the buffer.
> Why would you use it again once the widgets exist
> in the buffer?

In order to reset the widgets.
Try to type return at [Reset Form] in *Widget Example*.

Masatake YAMATO

Re: Strange problem with latest CVS

[Richard Stallman]

    > Why would you use it again once the widgets exist
    > in the buffer?

    In order to reset the widgets.

In that case, the example code should start by explicitly deleting the
overlays it previously made, if it was called before.

Would you like to fix the example code in the manual?

Re: Strange problem with latest CVS

[Masatake YAMATO]

>     > Why would you use it again once the widgets exist
>     > in the buffer?
> 
>     In order to reset the widgets.
> 
> In that case, the example code should start by explicitly deleting the
> overlays it previously made, if it was called before.
> 
> Would you like to fix the example code in the manual?

The patch for the manual is obvious.

How do you think arugments for `remove-overlays' optional?
So if one wants to remove all overlays in the buffer, like
the example of widget.texi, one has to just call
(remove-overlays).

Masatake YAMATO

Index: man/widget.texi
===================================================================
RCS file: /cvsroot/emacs/emacs/man/widget.texi,v
retrieving revision 1.24
diff -u -r1.24 widget.texi
--- man/widget.texi	2 Nov 2003 07:01:18 -0000	1.24
+++ man/widget.texi	21 Apr 2004 13:17:00 -0000
@@ -341,6 +341,7 @@
   (make-local-variable 'widget-example-repeat)
   (let ((inhibit-read-only t))
     (erase-buffer))
+  (remove-overlays)
   (widget-insert "Here is some documentation.\n\nName: ")
   (widget-create 'editable-field
 		 :size 13
Index: lisp/subr.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/subr.el,v
retrieving revision 1.385
diff -u -r1.385 subr.el
--- lisp/subr.el	20 Apr 2004 20:56:32 -0000	1.385
+++ lisp/subr.el	21 Apr 2004 13:17:00 -0000
@@ -1512,9 +1512,13 @@
       (overlay-put o1 (pop props) (pop props)))
     o1))
 
-(defun remove-overlays (beg end name val)
+(defun remove-overlays (&optional beg end name val)
   "Clear BEG and END of overlays whose property NAME has value VAL.
-Overlays might be moved and or split."
+Overlays might be moved and or split.
+If BEG is nil, `(point-min)' is used. If END is nil, `(point-max)' 
+is used."
+  (unless beg (setq beg (point-min)))
+  (unless end (setq end (point-max)))
   (if (< end beg)
       (setq beg (prog1 end (setq end beg))))
   (save-excursion