Using overlay in register

Using overlay in register

[Masatake YAMATO]

How do you think using overlay instead of marker to represent a point in register.el?

The advantage is strong visual feed back. During editing emacs,
sometime I forget the position of a register. With overlay, we can put
a face on the position. The faces let me keep the memory about the
position of registers. We can also put help-echo on the overlay.

BTW, (overlay-start overlay) < (overlay-end overlay) is guaranteed?

Masatake YAMATO

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

	* register.el (register): New customize group.
	(register-point): New face.
	(register-alist): Doc fix.
	(set-register): Delete overlay before
	setting new value to the register.
	(register-put-overlay-at-point): New function.
	(register-get-overlay-position): New function.
	(point-to-register): Use overlay instead of marker.
	(jump-to-register): Ditto.
	(describe-register-1): Ditto.
	(insert-register): Ditto.
	(register-swap-out): Ditto. Doc fix.

cvs diff: warning: unrecognized response `access control disabled, clients can connect from any host' from cvs server
Index: lisp/register.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/register.el,v
retrieving revision 1.48
diff -u -r1.48 register.el
--- lisp/register.el	16 Mar 2004 09:52:36 -0000	1.48
+++ lisp/register.el	18 Mar 2004 10:20:36 -0000
@@ -31,9 +31,20 @@
 
 ;;; Code:
 
+(defgroup register ()
+  "register commands for Emacs."
+  :group 'editing
+  :prefix "register-")
+
+(defface register-point
+  '((((class color)) (:box "blue"))
+    (t (:underline t)))
+  "Face for the overlay associated with register."
+  :group 'register)
+
 (defvar register-alist nil
   "Alist of elements (NAME . CONTENTS), one for each Emacs register.
-NAME is a character (a number).  CONTENTS is a string, number, marker or list.
+NAME is a character (a number).  CONTENTS is a string, number, overlay or list.
 A list of strings represents a rectangle.
 A list of the form (file . NAME) represents the file named NAME.
 A list of the form (file-query NAME POSITION) represents position POSITION
@@ -50,23 +61,46 @@
 (defun set-register (register value)
   "Set contents of Emacs register named REGISTER to VALUE.  Returns VALUE.
 See the documentation of the variable `register-alist' for possible VALUE."
+  ;; If the old value is overlay, delete it first.
+  (let ((overlay (get-register register)))
+    (if (overlayp overlay)
+	(delete-overlay overlay)))
   (let ((aelt (assq register register-alist)))
     (if aelt
 	(setcdr aelt value)
       (push (cons register value) register-alist))
     value))
 
+(defun register-put-overlay-at-point (register)
+  "Put a overlay associated with REGISTER at the point."
+  (let ((overlay (make-overlay (point) 
+			       (1+ (point))
+			       (current-buffer)
+			       t))
+	(help (format "Register: %s\nType \"%s %s\" to jump here." 
+		      (single-key-description register)
+		      (substitute-command-keys "\\[jump-to-register]")
+		      (single-key-description register))))
+    (overlay-put overlay 'face 'register-point)
+    (overlay-put overlay 'help-echo help)
+    (overlay-put overlay 'register register)
+    overlay))
+
+(defun register-get-overlay-position (overlay)
+  "Get the position for OVERLAY."
+  (min (overlay-start overlay) (overlay-end overlay)))
+ 
 (defun point-to-register (register &optional arg)
   "Store current location of point in register REGISTER.
 With prefix argument, store current frame configuration.
 Use \\[jump-to-register] to go to that location or restore that configuration.
 Argument is a character, naming the register."
   (interactive "cPoint to register: \nP")
-  ;; Turn the marker into a file-ref if the buffer is killed.
+  ;; Turn the overlay into a file-ref if the buffer is killed.
   (add-hook 'kill-buffer-hook 'register-swap-out nil t)
   (set-register register
 		(if arg (list (current-frame-configuration) (point-marker))
-		  (point-marker))))
+		  (register-put-overlay-at-point register))))
 
 (defun window-configuration-to-register (register &optional arg)
   "Store the window configuration of the selected frame in register REGISTER.
@@ -106,11 +140,11 @@
      ((and (consp val) (window-configuration-p (car val)))
       (set-window-configuration (car val))
       (goto-char (cadr val)))
-     ((markerp val)
-      (or (marker-buffer val)
+     ((overlayp val)
+      (or (overlay-buffer val)
 	  (error "That register's buffer no longer exists"))
-      (switch-to-buffer (marker-buffer val))
-      (goto-char val))
+      (switch-to-buffer (overlay-buffer val))
+      (goto-char (register-get-overlay-position val)))
      ((and (consp val) (eq (car val) 'file))
       (find-file (cdr val)))
      ((and (consp val) (eq (car val) 'file-query))
@@ -123,15 +157,15 @@
       (error "Register doesn't contain a buffer position or configuration")))))
 
 (defun register-swap-out ()
-  "Turn markers into file-query references when a buffer is killed."
+  "Turn overlays into file-query references when a buffer is killed."
   (and buffer-file-name
        (dolist (elem register-alist)
-	 (and (markerp (cdr elem))
-	      (eq (marker-buffer (cdr elem)) (current-buffer))
+	 (and (overlayp (cdr elem))
+	      (eq (overlay-buffer (cdr elem)) (current-buffer))
 	      (setcdr elem
 		      (list 'file-query
 			    buffer-file-name
-			    (marker-position (cdr elem))))))))
+			    (overlay-start (cdr elem))))))))
 
 (defun number-to-register (number register)
   "Store a number in a register.
@@ -187,14 +221,14 @@
      ((numberp val)
       (princ val))
 
-     ((markerp val)
-      (let ((buf (marker-buffer val)))
+     ((overlayp val)
+      (let ((buf (overlay-buffer val)))
 	(if (null buf)
-	    (princ "a marker in no buffer")
+	    (princ "a overlay in no buffer")
 	  (princ "a buffer position:\n    buffer ")
 	  (princ (buffer-name buf))
 	  (princ ", position ")
-	  (princ (marker-position val)))))
+	  (princ (register-get-overlay-position val)))))
 
      ((and (consp val) (window-configuration-p (car val)))
       (princ "a window configuration."))
@@ -264,8 +298,8 @@
       (insert-for-yank val))
      ((numberp val)
       (princ val (current-buffer)))
-     ((and (markerp val) (marker-position val))
-      (princ (marker-position val) (current-buffer)))
+     ((and (overlayp val) (overlay-start val))
+      (princ (register-get-overlay-position val) (current-buffer)))
      (t
       (error "Register does not contain text"))))
   (if (not arg) (exchange-point-and-mark)))

Re: Using overlay in register

[Richard Stallman]

This is a very good idea.  Thanks for working on it.

I see a possible bug here:

    +(defun register-put-overlay-at-point (register)
    +  "Put a overlay associated with REGISTER at the point."
    +  (let ((overlay (make-overlay (point) 
    +			       (1+ (point))
    +			       (current-buffer)

This will get an error at the end of the buffer.

What should it do at the end?  Make an empty overlay?
It could have a before-string, perhaps.

    +(defun register-get-overlay-position (overlay)
    +  "Get the position for OVERLAY."
    +  (min (overlay-start overlay) (overlay-end overlay)))

What's the purpose of using min here?
Why not just use overlay-start?
(In some places you did so.)

Re: Using overlay in register

[Masatake YAMATO]

> This is a very good idea.  Thanks for working on it.
> 
> I see a possible bug here:
> 
>     +(defun register-put-overlay-at-point (register)
>     +  "Put a overlay associated with REGISTER at the point."
>     +  (let ((overlay (make-overlay (point) 
>     +			       (1+ (point))
>     +			       (current-buffer)
> 
> This will get an error at the end of the buffer.
> 
> What should it do at the end?  Make an empty overlay?
> It could have a before-string, perhaps.

Thank you for reviwing and suggestion. I have used after-string.
If the point is at the end of line, set a space to an overlay's
'after-string property. 

I have added an function: With mouse-3 on an overlay(or 
after-string " "), you can clear the overlay.

> What should it do at the end?  Make an empty overlay?
> It could have a before-string, perhaps.
> 
>     +(defun register-get-overlay-position (overlay)
>     +  "Get the position for OVERLAY."
>     +  (min (overlay-start overlay) (overlay-end overlay)))
> 
> What's the purpose of using min here?
> Why not just use overlay-start?
> (In some places you did so.)

I have seen emacs's strange behaviors twice; Emacs returned an overlay whose start is
greater than its end. I cannot find the way to reproduce it.
(overlay-start overlay) < (overlay-end overlay) is guaranteed?

Masatake YAMATO

Index: lisp/register.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/register.el,v
retrieving revision 1.48
diff -u -r1.48 register.el
--- lisp/register.el	16 Mar 2004 09:52:36 -0000	1.48
+++ lisp/register.el	21 Mar 2004 13:44:58 -0000
@@ -31,9 +31,20 @@
 
 ;;; Code:
 
+(defgroup register ()
+  "register commands for Emacs."
+  :group 'editing
+  :prefix "register-")
+
+(defface register-point
+  '((((class color)) (:box "blue"))
+    (t (:underline t)))
+  "Face for the overlay associated with register."
+  :group 'register)
+
 (defvar register-alist nil
   "Alist of elements (NAME . CONTENTS), one for each Emacs register.
-NAME is a character (a number).  CONTENTS is a string, number, marker or list.
+NAME is a character (a number).  CONTENTS is a string, number, overlay or list.
 A list of strings represents a rectangle.
 A list of the form (file . NAME) represents the file named NAME.
 A list of the form (file-query NAME POSITION) represents position POSITION
@@ -50,23 +61,80 @@
 (defun set-register (register value)
   "Set contents of Emacs register named REGISTER to VALUE.  Returns VALUE.
 See the documentation of the variable `register-alist' for possible VALUE."
+  ;; If the old value is overlay, delete it first.
+  (let ((overlay (get-register register)))
+    (if (overlayp overlay)
+	(delete-overlay overlay)))
   (let ((aelt (assq register register-alist)))
     (if aelt
 	(setcdr aelt value)
       (push (cons register value) register-alist))
     value))
 
+(defun register-clear-register (register)
+  (interactive "cClear register: ")
+  (set-register register nil))
+
+(defun register-clear-overlay-at-point (event)
+  "Clear the register associated with an overlay pointed by EVENT."
+  (interactive "e")
+  (save-excursion
+    (mouse-set-point event)
+    (let ((overlays (overlays-in (point) 
+				 (+ (point) (if (eolp) 0 1))))
+	  o
+	  r)
+      ;; Search 'register property from the all overlays.
+      (while (and overlays (null r))
+	(setq o (car overlays)
+	      r (overlay-get o 'register)
+	      overlays (cdr overlays))
+	(when r
+	  (register-clear-register r)))
+      (unless r
+	(error "No overlay for register at this point.")))))
+
+(defun register-put-overlay-at-point (register)
+  "Put a overlay associated with REGISTER at the point."
+  (let* ((help (format "Register: %s\nType \"%s %s\" to jump here.\nMouse-3 to clear this register."
+		       (single-key-description register)
+		       (substitute-command-keys "\\[jump-to-register]")
+		       (single-key-description register)))
+	 (overlay (make-overlay (point)
+				(+ (point) (if (eolp) 0 1))
+				(current-buffer)
+				t))
+	 (keymap '(keymap (mouse-3 . register-clear-overlay-at-point))))
+    (overlay-put overlay 'register register)
+    (if (eolp)
+	;; Zero length overlay with " ".
+	(overlay-put overlay 'after-string
+		     (propertize " "
+				 'face 'register-point
+				 'help-echo help
+				 'mouse-face 'highlight
+				 'keymap keymap))
+      (overlay-put overlay 'face 'register-point)
+      (overlay-put overlay 'help-echo help)
+      (overlay-put overlay 'mouse-face 'highlight)
+      (overlay-put overlay 'keymap keymap))
+    overlay))
+
+(defun register-get-overlay-position (overlay)
+  "Get the position for OVERLAY."
+  (min (overlay-start overlay) (overlay-end overlay)))
+ 
 (defun point-to-register (register &optional arg)
   "Store current location of point in register REGISTER.
 With prefix argument, store current frame configuration.
 Use \\[jump-to-register] to go to that location or restore that configuration.
 Argument is a character, naming the register."
   (interactive "cPoint to register: \nP")
-  ;; Turn the marker into a file-ref if the buffer is killed.
+  ;; Turn the overlay into a file-ref if the buffer is killed.
   (add-hook 'kill-buffer-hook 'register-swap-out nil t)
   (set-register register
 		(if arg (list (current-frame-configuration) (point-marker))
-		  (point-marker))))
+		  (register-put-overlay-at-point register))))
 
 (defun window-configuration-to-register (register &optional arg)
   "Store the window configuration of the selected frame in register REGISTER.
@@ -106,11 +174,11 @@
      ((and (consp val) (window-configuration-p (car val)))
       (set-window-configuration (car val))
       (goto-char (cadr val)))
-     ((markerp val)
-      (or (marker-buffer val)
+     ((overlayp val)
+      (or (overlay-buffer val)
 	  (error "That register's buffer no longer exists"))
-      (switch-to-buffer (marker-buffer val))
-      (goto-char val))
+      (switch-to-buffer (overlay-buffer val))
+      (goto-char (register-get-overlay-position val)))
      ((and (consp val) (eq (car val) 'file))
       (find-file (cdr val)))
      ((and (consp val) (eq (car val) 'file-query))
@@ -123,15 +191,15 @@
       (error "Register doesn't contain a buffer position or configuration")))))
 
 (defun register-swap-out ()
-  "Turn markers into file-query references when a buffer is killed."
+  "Turn overlays into file-query references when a buffer is killed."
   (and buffer-file-name
        (dolist (elem register-alist)
-	 (and (markerp (cdr elem))
-	      (eq (marker-buffer (cdr elem)) (current-buffer))
+	 (and (overlayp (cdr elem))
+	      (eq (overlay-buffer (cdr elem)) (current-buffer))
 	      (setcdr elem
 		      (list 'file-query
 			    buffer-file-name
-			    (marker-position (cdr elem))))))))
+			    (overlay-start (cdr elem))))))))
 
 (defun number-to-register (number register)
   "Store a number in a register.
@@ -187,14 +255,14 @@
      ((numberp val)
       (princ val))
 
-     ((markerp val)
-      (let ((buf (marker-buffer val)))
+     ((overlayp val)
+      (let ((buf (overlay-buffer val)))
 	(if (null buf)
-	    (princ "a marker in no buffer")
+	    (princ "a overlay in no buffer")
 	  (princ "a buffer position:\n    buffer ")
 	  (princ (buffer-name buf))
 	  (princ ", position ")
-	  (princ (marker-position val)))))
+	  (princ (register-get-overlay-position val)))))
 
      ((and (consp val) (window-configuration-p (car val)))
       (princ "a window configuration."))
@@ -264,8 +332,8 @@
       (insert-for-yank val))
      ((numberp val)
       (princ val (current-buffer)))
-     ((and (markerp val) (marker-position val))
-      (princ (marker-position val) (current-buffer)))
+     ((and (overlayp val) (overlay-start val))
+      (princ (register-get-overlay-position val) (current-buffer)))
      (t
       (error "Register does not contain text"))))
   (if (not arg) (exchange-point-and-mark)))

Re: Using overlay in register

[Richard Stallman]

    Thank you for reviwing and suggestion. I have used after-string.
    If the point is at the end of line, set a space to an overlay's
    `after-string' property. 

That may give inconsistent results.  What if I delete the following
newline?  Then it will no longer be at the end of the line, but the
overlay will still have a space in the `after-string' property.

    +(defun register-clear-overlay-at-point (event)
    +  "Clear the register associated with an overlay pointed by EVENT."
    +  (interactive "e")
    +  (save-excursion

Why is this feature useful?

Re: Using overlay in register

[Richard Stallman]

    I have seen emacs's strange behaviors twice; Emacs returned an overlay whose start is
    greater than its end. I cannot find the way to reproduce it.

That is a bug.  It should never happen.

    (defun register-get-overlay-position (overlay)
      "Get the position for OVERLAY."
      (min (overlay-start overlay) (overlay-end overlay)))

Please eliminate that function and use overlay-start.

overlay-start > overlay-end

[Masatake YAMATO]

>     I have seen emacs's strange behaviors twice; Emacs returned an overlay whose start is
>     greater than its end. I cannot find the way to reproduce it.
> 
> That is a bug.  It should never happen.
> 

I've just found the way to reproduce this:

(save-excursion
  (let (o)
    (goto-char (point-min))
    (delete-region (line-beginning-position) (line-end-position))
    (make-overlay (point) (point) (current-buffer) t)
    (insert ?x)
    (overlay-lists)))
=> ((#<overlay from 2 to 1 in *scratch*>))

I've tried to fix the problem. See the patch attached to this mail.
However, I have no confidence. 

Regards,
Masatake YAMATO

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.445
diff -u -r1.445 buffer.c
--- src/buffer.c	4 Mar 2004 17:15:13 -0000	1.445
+++ src/buffer.c	22 Mar 2004 07:59:04 -0000
@@ -3465,6 +3465,19 @@
       tail = tail->next;
     }
 
+  /* Canonicalize the order of start and end. */
+  if (tail)
+    {
+      EMACS_INT   start = OVERLAY_POSITION (OVERLAY_START (tem));
+      Lisp_Object start_lobj;
+      if (start > end)
+	{
+	  start_lobj  = tail->start;
+	  tail->start = tail->end;
+	  tail->end   = start_lobj;
+	}
+    }
+	  
   /* If we don't find such an overlay,
      or the found one ends before PREV,
      or the found one is the last one in the list,

Re: overlay-start > overlay-end

[Richard Stallman]

	(make-overlay (point) (point) (current-buffer) t)
	(insert ?x)
	(overlay-lists)))
    => ((#<overlay from 2 to 1 in *scratch*>))

Cute.

    +  /* Canonicalize the order of start and end. */
    +  if (tail)
    +    {

This fixes just one overlay.  What if there are several
such overlays?  I think it is necessary to have a loop
here, to fix as many broken overlays as there may be.

Re: overlay-start > overlay-end

[Masatake YAMATO]

>     +  /* Canonicalize the order of start and end. */
>     +  if (tail)
>     +    {
> 
> This fixes just one overlay.  What if there are several
> such overlays?  I think it is necessary to have a loop
> here, to fix as many broken overlays as there may be.

Thank you for suggestion. I found a function for the
purpose: fix_overlays_in_range. I put it inside
`fix_overlays_before'. Instead I put it to the upper
level function.

BTW, how do you think changing function names:

fix_overlays_in_range => fix_start_end_in_overlays
fix_overlays_before   => fix_order_of_overlays_in_buffer

Both functions fix orders. However the scope(target?) of order
is different; fix_overlays_in_range fixes the order inside
an overlay and fix_overlays_before fixed the order insdie
buffer's overlay list. The function names don't show the
difference of scope.

Regards,
Masatake YAMATO

cvs diff: warning: unrecognized response `access control disabled, clients can connect from any host' from cvs server
Index: src/insdel.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/insdel.c,v
retrieving revision 1.176
diff -u -r1.176 insdel.c
--- src/insdel.c	1 Sep 2003 15:45:52 -0000	1.176
+++ src/insdel.c	23 Mar 2004 08:37:12 -0000
@@ -441,9 +441,13 @@
     }
 
   /* Adjusting only markers whose insertion-type is t may result in
-     disordered overlays in the slot `overlays_before'.  */
+     - disordered start and end in overlays, and 
+     - disordered overlays in the slot `overlays_before' of current_buffer.  */
   if (adjusted)
-    fix_overlays_before (current_buffer, from, to);
+    {
+      fix_overlays_in_range(from, to);
+      fix_overlays_before (current_buffer, from, to);
+    }
 }
 
 /* Adjust point for an insertion of NBYTES bytes, which are NCHARS characters.

Re: overlay-start > overlay-end

[Stefan Monnier]

> BTW, how do you think changing function names:

> fix_overlays_in_range => fix_start_end_in_overlays
> fix_overlays_before   => fix_order_of_overlays_in_buffer

Sounds like a good idea.
I've played a bit with this code and didn't get to the point where
I understood this difference.


        Stefan

Re: overlay-start > overlay-end

[Richard Stallman]

    Thank you for suggestion. I found a function for the
    purpose: fix_overlays_in_range. I put it inside
    `fix_overlays_before'. Instead I put it to the upper
    level function.

Thanks.  That is correct, but maybe another change is needed too.
I saw this comment at fix_overlays_in_range:

       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?  */

That is the wrong thing to do!  These overlays should be empty.

Can you fix that too?

    fix_overlays_in_range => fix_start_end_in_overlays

That seems like a good idea.

    fix_overlays_before   => fix_order_of_overlays_in_buffer

That name is too clumsy, I'd say don't change this one.