bug#13648: 24.3.50; remove-overlays bugs

[Stephen Berman <stephen.berman@gmx.net>]

[-- Attachment #1: Type: text/plain, Size: 2978 bytes --]

0. emacs -Q
1. Evaluate the following sexp:

   (progn
     (switch-to-buffer (get-buffer-create "*test*"))
     (insert "one two three")
     (setq ov1 (make-overlay 1 4))
     (setq ov2 (make-overlay 5 9))
     (setq ov3 (make-overlay 9 14))
     (sit-for 1)
     (overlay-put ov1 'display "two")
     (overlay-put ov2 'display "")
     (overlay-put ov3 'after-string " four")
     (sit-for 1)
     (remove-overlays nil nil 'display "two")
     (remove-overlays nil nil 'display "")
     (sit-for 1)
     (remove-overlays nil nil 'after-string))

In buffer *test* you see the following:

1. First:

one two three

2. Then after one second:

two three four

3. Then after another second:

two two three four

4. Finally, after one more second:

one two three four


I think the last two displays demonstrate buggy behavior.  In the third,
remove-overlays has failed to clear the display string "two" but has
cleared the empty string.  In the fourth, the remove-overlays call
specified the after-string property, yet what has been cleared is the
display overlay "two" but not the after-string overlay "four".  

The reason for the first problem is that remove-overlays tests the
overlay value with eq, which fails for all strings except the empty
string.  Hence, both the display overlay "two" and the after-string
overlay "four" are not cleared by the second and third calls of
remove-overlays, respectively.

But the third call does remove "two" because overlay-get tries to get
the value of the overlay's after-string property, but it only has a
display property, so overlay-get returns nil, and since the fourth
argument of remove-overlays is also nil here, they are eq, so the
overlay is cleared.  The general problem here, I believe, is that,
although all the arguments of remove-overlays are optional (so the last
invocation of remove-overlays is legitimate), the logic of the code is
that the NAME and VAL arguments are either both nil or both non-nil,
which conflicts with the semantics of the &optional keyword.

I think the easiest and best fix is to make the NAME and VAL arguments
conform to the &optional semantics by allowing independent values.  This
means that the last call of remove-overlays in the above sexp would
clear any after-string overlays, regardless of their value.  I think
this would be useful, and it is backward compatible, because all uses of
remove-overlays in Emacs have either both or neither of the NAME and VAL
arguments (and any third-party code that only has the NAME argument is
either buggy, like the above sexp, or works by chance).  The patch below
implements this, and also fixes the first problem by testing with equal
if the eq test fails.


2013-02-07  Stephen Berman  <stephen.berman@gmx.net>

	* subr.el (remove-overlays): Handle string and list values of
	overlay properties.  If the property argument is non-nil and the
	value argument is nil, clear all overlays with that property
	regardless of their values.  (Bug#XXXXX)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: remove-overlays patch --]
[-- Type: text/x-patch, Size: 2629 bytes --]

=== modified file 'lisp/subr.el'
*** lisp/subr.el	2013-02-03 16:13:36 +0000
--- lisp/subr.el	2013-02-07 14:45:09 +0000
***************
*** 2579,2585 ****
  (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.
! BEG and END default respectively to the beginning and end of buffer."
    ;; This speeds up the loops over overlays.
    (unless beg (setq beg (point-min)))
    (unless end (setq end (point-max)))
--- 2579,2589 ----
  (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.
! 
! BEG and END default respectively to the beginning and end of
! buffer.  If VAL is nil and NAME is non-nil, clear all NAME
! overlays regardless of their values.  If both NAME and VAL are
! nil, clear all overlays from BEG to END."
    ;; This speeds up the loops over overlays.
    (unless beg (setq beg (point-min)))
    (unless end (setq end (point-max)))
***************
*** 2588,2607 ****
        (setq beg (prog1 end (setq end beg))))
    (save-excursion
      (dolist (o (overlays-in beg end))
!       (when (eq (overlay-get o name) val)
! 	;; Either push this overlay outside beg...end
! 	;; or split it to exclude beg...end
! 	;; or delete it entirely (if it is contained in beg...end).
! 	(if (< (overlay-start o) beg)
  	    (if (> (overlay-end o) end)
! 		(progn
! 		  (move-overlay (copy-overlay o)
! 				(overlay-start o) beg)
! 		  (move-overlay o end (overlay-end o)))
! 	      (move-overlay o (overlay-start o) beg))
! 	  (if (> (overlay-end o) end)
! 	      (move-overlay o end (overlay-end o))
! 	    (delete-overlay o)))))))
  \f
  ;;;; Miscellanea.
  
--- 2592,2614 ----
        (setq beg (prog1 end (setq end beg))))
    (save-excursion
      (dolist (o (overlays-in beg end))
!       (let ((v (overlay-get o name)))
! 	;; An overlay property value can be not just a symbol,
! 	;; but also a string or a list.
! 	(when (if val (or (eq v val) (equal v val)) name)
! 	  ;; Either push this overlay outside beg...end
! 	  ;; or split it to exclude beg...end
! 	  ;; or delete it entirely (if it is contained in beg...end).
! 	  (if (< (overlay-start o) beg)
! 	      (if (> (overlay-end o) end)
! 		  (progn
! 		    (move-overlay (copy-overlay o)
! 				  (overlay-start o) beg)
! 		    (move-overlay o end (overlay-end o)))
! 		(move-overlay o (overlay-start o) beg))
  	    (if (> (overlay-end o) end)
! 		(move-overlay o end (overlay-end o))
! 	      (delete-overlay o))))))))
  \f
  ;;;; Miscellanea.
  


[-- Attachment #3: Type: text/plain, Size: 557 bytes --]



In GNU Emacs 24.3.50.6 (x86_64-suse-linux-gnu, GTK+ Version 3.4.4)
 of 2013-02-07 on rosalinde
Bzr revision: 111689 michael.albinus@gmx.de-20130207085004-ztc6mdtkh756peam
Windowing system distributor `The X.Org Foundation', version 11.0.11203000
System Description:	openSUSE 12.2 (x86_64)

Configured using:
 `configure --without-toolkit-scroll-bars CFLAGS=-g3 -O0 --no-create
 --no-recursion'

Important settings:
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=local
  locale-coding-system: utf-8-unix
  default enable-multibyte-characters: t