popup menu support for smerge-mode

popup menu support for smerge-mode

[Masatake YAMATO]

Hi,

I've added popup menu support in smerge-mode.
Could you evaluate my patch? and if you prefer, please
add the patch to official source tree.

I use menu-bar often to invoke smerge because I cannot memorize
C-c ^ key binding. So each time I'd like to use smerge, I have to
do M-x menu-bar-mode:-P

Regards,
Masatake YAMATO

2003-01-10  Masatake YAMATO  <jet@gyve.org>

	* smerge-mode.el (smerge-match-conflict): put highlight
	as mouse-face to conflict text areas. show popup-menu if
	mouse-2 is down in conflict text areas.

Index: smerge-mode.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/smerge-mode.el,v
retrieving revision 1.20
diff -u -r1.20 smerge-mode.el
--- smerge-mode.el	10 Oct 2002 17:30:20 -0000	1.20
+++ smerge-mode.el	10 Jan 2003 12:35:28 -0000
@@ -370,7 +370,14 @@
 	   (setq base-end   mine-end)
 	   (setq mine-start other-start)
 	   (setq mine-end   other-end)))
-	       
+
+	  (put-text-property start end 
+			     'mouse-face 'highlight)
+	  (put-text-property start end 
+			     'keymap (let ((km (make-sparse-keymap)))
+				       (define-key km [down-mouse-2] #'smerge-mode-menu)
+				       km))
+
 	  (store-match-data (list start end
 				  mine-start mine-end
 				  base-start base-end

Re: popup menu support for smerge-mode

[Stefan Monnier]

> I've added popup menu support in smerge-mode.
> Could you evaluate my patch? and if you prefer, please
> add the patch to official source tree.

Looks like a good idea.  The patch has some problems (the menu and mouse-face
stays after the conflict is resolved and even after smerge-mode is turned off),
but I'll take care of it.

> I use menu-bar often to invoke smerge because I cannot memorize
> C-c ^ key binding. So each time I'd like to use smerge, I have to
> do M-x menu-bar-mode:-P

You can change the prefix to something else.  I personally use ESC so
I can use M-RET, ...


	Stefan

Re: popup menu support for smerge-mode

[Masatake YAMATO]

Several month ago, I wrote: 
> I've added popup menu support in smerge-mode.
> Could you evaluate my patch? and if you prefer, please
> add the patch to official source tree.

Stefan Monnier wrote:
> Looks like a good idea.  The patch has some problems (the menu and mouse-face
> stays after the conflict is resolved and even after smerge-mode is turned off),
> but I'll take care of it.

I've rewritten the patch using overlays. Overlays are removed after smerge-mode is 
turned off. Please, review.

2003-08-24  Masatake YAMATO  <jet@gyve.org>

	* smerge-mode.el (smerge-overlays): New variable.
	(smerge-mode-popup-menu): New menu.
	(smerge-auto-leave): Call `smerge-find-conflict' to put overlays.
	(smerge-keep-current-by-mouse): New function.
	(smerge-put-overlay): New function.
	(smerge-delete-overlays): New function.
	(smerge-match-conflict): Put overlay here.
	(smerge-find-conflict): Add new argument `put-overlay'.
	(smerge-mode): Add code to manage overlays.


Index: lisp/smerge-mode.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/smerge-mode.el,v
retrieving revision 1.21
diff -u -r1.21 smerge-mode.el
--- lisp/smerge-mode.el	4 Feb 2003 12:05:02 -0000	1.21
+++ lisp/smerge-mode.el	24 Aug 2003 05:50:28 -0000
@@ -137,6 +137,8 @@
   `((,smerge-command-prefix . ,smerge-basic-map))
   "Keymap for `smerge-mode'.")
 
+(defvar smerge-overlays nil "Overlays managed by smerge-mode")
+
 (easy-menu-define smerge-mode-menu smerge-mode-map
   "Menu for `smerge-mode'."
   '("SMerge"
@@ -159,6 +161,13 @@
      :help "Use Ediff to resolve the conflicts"]
     ))
 
+(easy-menu-define smerge-mode-popup-menu smerge-mode-map
+  "Popup menu for `smerge-mode'."
+  '(nil
+    ["Keep Current" smerge-keep-current-by-mouse :help "Use current (at point) version"]
+    ["Keep All" smerge-keep-all :help "Keep all three versions"]
+    ))
+
 (defconst smerge-font-lock-keywords
   '((smerge-find-conflict
      (1 smerge-mine-face prepend t)
@@ -199,9 +208,18 @@
     (error (format "No `%s'" (aref smerge-match-names n)))))
 
 (defun smerge-auto-leave ()
-  (when (and smerge-auto-leave
-	     (save-excursion (goto-char (point-min))
-			     (not (re-search-forward smerge-begin-re nil t))))
+  (smerge-remove-overlays)
+  (when (and 
+	 ;; 1. Are conflict existed?
+	 ;; As the side effect, overlays are put.
+	 (not (save-excursion 
+		(goto-char (point-min))
+		(let (matched)
+		  (while (smerge-find-conflict nil t)
+		    (setq matched t))
+		  matched)))
+	 ;; 2. Check customize option.
+	 smerge-auto-leave)
     (smerge-mode -1)))
 
 
@@ -301,6 +319,15 @@
       (replace-match (match-string i) t t)
       (smerge-auto-leave))))
 
+(defun smerge-keep-current-by-mouse (event)
+  "Call `smerge-keep-current'at the place where you clicked by a mouse."
+  (interactive "e")
+  (save-excursion
+    (set-buffer (window-buffer (posn-window (event-end event))))
+    (save-excursion
+      (goto-char (posn-point (event-end event)))
+        (smerge-keep-current))))
+
 (defun smerge-diff-base-mine ()
   "Diff 'base' and 'mine' version in current conflict region."
   (interactive)
@@ -316,14 +343,33 @@
   (interactive)
   (smerge-diff 1 3))
 
-(defun smerge-match-conflict ()
+(defun smerge-put-overlay (start end)
+  "Put overlay of smerge-mode between START and END.
+The overlay has its own keymap to show popup menu."
+  (setq overlay (make-overlay start end))
+  (overlay-put overlay 'mouse-face 'highlight)
+  (overlay-put overlay 'keymap (let ((km (make-sparse-keymap)))
+				 (define-key km [down-mouse-3] 
+				   (lambda () (interactive)
+				     (popup-menu smerge-mode-popup-menu)))
+				 km))
+  (overlay-put overlay 'help-echo "down-mouse-3: Use current (at point) version")
+  (push overlay smerge-overlays))
+
+(defun smerge-remove-overlays ()
+  "Delete all overlays made by  `smerge-put-overlay'."
+  (mapcar (lambda (o) (delete-overlay o)) smerge-overlays)
+  (setq smerge-overlays nil))
+
+(defun smerge-match-conflict (&optional put-overlay)
   "Get info about the conflict.  Puts the info in the `match-data'.
 The submatches contain:
  0:  the whole conflict.
  1:  your code.
  2:  the base code.
  3:  other code.
-An error is raised if not inside a conflict."
+An error is raised if not inside a conflict.
+Overlays for smerge popup menu are put if PUT-OVERLAY is non-nil."
   (save-excursion
     (condition-case nil
 	(let* ((orig-point (point))
@@ -370,7 +416,13 @@
 	   (setq base-end   mine-end)
 	   (setq mine-start other-start)
 	   (setq mine-end   other-end)))
-
+	  (when put-overlay
+	    (if (and base-start base-end)
+		(smerge-put-overlay base-start base-end))
+	    (if (and mine-start mine-end)
+		(smerge-put-overlay mine-start mine-end))
+	    (if (and other-start other-end)
+		(smerge-put-overlay other-start other-end)))
 	  (store-match-data (list start end
 				  mine-start mine-end
 				  base-start base-end
@@ -380,14 +432,16 @@
 	  t)
       (search-failed (error "Point not in conflict region")))))
 
-(defun smerge-find-conflict (&optional limit)
+(defun smerge-find-conflict (&optional limit put-overlay)
   "Find and match a conflict region.  Intended as a font-lock MATCHER.
 The submatches are the same as in `smerge-match-conflict'.
 Returns non-nil if a match is found between the point and LIMIT.
-The point is moved to the end of the conflict."
+The point is moved to the end of the conflict.
+If PUT-OVERLAY is non-nil, overlays for smerge popup menu are put 
+as side effect."
   (when (re-search-forward smerge-begin-re limit t)
     (ignore-errors
-      (smerge-match-conflict)
+      (smerge-match-conflict put-overlay)
       (goto-char (match-end 0)))))
 
 (defun smerge-diff (n1 n2)
@@ -522,6 +576,12 @@
   "Minor mode to simplify editing output from the diff3 program.
 \\{smerge-mode-map}"
   nil " SMerge" nil
+  ;; overlays management
+  (if smerge-mode
+      ;; entering smerge-mode
+      (set (make-variable-buffer-local 'smerge-overlays) nil)
+    ;; leaving smerge-mode
+    (smerge-remove-overlays)) 
   (when (and (boundp 'font-lock-mode) font-lock-mode)
     (set (make-local-variable 'font-lock-multiline) t)
     (save-excursion
@@ -529,7 +589,7 @@
 	  (font-lock-add-keywords nil smerge-font-lock-keywords 'append)
 	(font-lock-remove-keywords nil smerge-font-lock-keywords))
       (goto-char (point-min))
-      (while (smerge-find-conflict)
+      (while (smerge-find-conflict nil smerge-mode)
 	(save-excursion
 	  (font-lock-fontify-region (match-beginning 0) (match-end 0) nil))))))

Re: popup menu support for smerge-mode

[Stefan Monnier]

> +(easy-menu-define smerge-mode-popup-menu smerge-mode-map
> +  "Popup menu for `smerge-mode'."
> +  '(nil
> +    ["Keep Current" smerge-keep-current-by-mouse :help "Use current (at point) version"]
> +    ["Keep All" smerge-keep-all :help "Keep all three versions"]
> +    ))

Why not just reuse the existing menu ?

>  (defun smerge-auto-leave ()
> -  (when (and smerge-auto-leave
> -	     (save-excursion (goto-char (point-min))
> -			     (not (re-search-forward smerge-begin-re nil t))))
> +  (smerge-remove-overlays)
> +  (when (and 
> +	 ;; 1. Are conflict existed?
> +	 ;; As the side effect, overlays are put.
> +	 (not (save-excursion 
> +		(goto-char (point-min))
> +		(let (matched)
> +		  (while (smerge-find-conflict nil t)
> +		    (setq matched t))
> +		  matched)))
> +	 ;; 2. Check customize option.
> +	 smerge-auto-leave)
>      (smerge-mode -1)))

This is clearly wrong.  smerge-auto-leave is about leaving smerge-mode
when all conflicts are resolved.  It shouldn't fiddle with overlays at all.
The overlays should be removed by the call to (smerge-mode -1).

> +(defun smerge-put-overlay (start end)
> +  "Put overlay of smerge-mode between START and END.
> +The overlay has its own keymap to show popup menu."
> +  (setq overlay (make-overlay start end))

You're modifying a global `overlay' variable here.  Please let-bind it.

> +  (overlay-put overlay 'mouse-face 'highlight)

Having tried it, I'm not sure whether this is a good idea or not:
when the conflict is large, you end up with the whole window flashing
in `highlight' face in a very annoying way.

> +  (overlay-put overlay 'keymap (let ((km (make-sparse-keymap)))
> +				 (define-key km [down-mouse-3]
> +				   (lambda () (interactive)
> +				     (popup-menu smerge-mode-popup-menu)))
> +				 km))
> +  (overlay-put overlay 'help-echo "down-mouse-3: Use current (at point) version")

The help-echo is wrong: mouse-3 does not select the version under the
cursor (it pops up a menu instead).

> +(defun smerge-remove-overlays ()
> +  "Delete all overlays made by  `smerge-put-overlay'."
> +  (mapcar (lambda (o) (delete-overlay o)) smerge-overlays)
> +  (setq smerge-overlays nil))

When the output of `mapcar' is not used you should use `mapc'.
(lambda (x) (f x)) should always be replaced by 'f.

> +	  (when put-overlay
> +	    (if (and base-start base-end)
> +		(smerge-put-overlay base-start base-end))
> +	    (if (and mine-start mine-end)
> +		(smerge-put-overlay mine-start mine-end))
> +	    (if (and other-start other-end)
> +		(smerge-put-overlay other-start other-end)))

If base-start = base-end you end up with an empty overlay that the
user cannot click on.  It's simpler to just place a single overlay over
the whole conflict (including markers).

Also, I'm not sure it's a good idea to have a `match' function add an
overlay like that.  Matching is not supposed to have such side effects.
It's probably better to let the few callers that need it call
smerge-put-overlay themselves.


	Stefan

Re: popup menu support for smerge-mode

[Masatake YAMATO]

Sorry to be late to answer; and thank you for great reviewing everytime.

Thank you for great reviewing everytime.
And sorry for my poor mistakes.

> > +(easy-menu-define smerge-mode-popup-menu smerge-mode-map
> > +  "Popup menu for `smerge-mode'."
> > +  '(nil
> > +    ["Keep Current" smerge-keep-current-by-mouse :help "Use current (at point) version"]
> > +    ["Keep All" smerge-keep-all :help "Keep all three versions"]
> > +    ))
> 
> Why not just reuse the existing menu ?

Because smerge-mode-popup-menu is much smaller than smerge-mode-menu.
I assume that user may do following two things frequently:
1. keep the version under the cursur. The user can(and want to) point out 
   the version which one wants to keep by mouse. "Keep Other" and "Keep Yours"
   are not needed here.
2. keep all.
 
Of course, other menu items might be useful. But as far as trying, above two
items are `core functions' and are enough many situations. Other functions
in smerge-mode-menu should be invoked from M-x, C-c ^ or menu bar, I think.

> >  (defun smerge-auto-leave ()
> > -  (when (and smerge-auto-leave
> > -	     (save-excursion (goto-char (point-min))
> > -			     (not (re-search-forward smerge-begin-re nil t))))
> > +  (smerge-remove-overlays)
> > +  (when (and 
> > +	 ;; 1. Are conflict existed?
> > +	 ;; As the side effect, overlays are put.
> > +	 (not (save-excursion 
> > +		(goto-char (point-min))
> > +		(let (matched)
> > +		  (while (smerge-find-conflict nil t)
> > +		    (setq matched t))
> > +		  matched)))
> > +	 ;; 2. Check customize option.
> > +	 smerge-auto-leave)
> >      (smerge-mode -1)))
> 
> This is clearly wrong.  smerge-auto-leave is about leaving smerge-mode
> when all conflicts are resolved.  It shouldn't fiddle with overlays at all.
> The overlays should be removed by the call to (smerge-mode -1).

I've moved all overlay related functions from smerge-auto-leave to its 
outside.

> > +(defun smerge-put-overlay (start end)
> > +  "Put overlay of smerge-mode between START and END.
> > +The overlay has its own keymap to show popup menu."
> > +  (setq overlay (make-overlay start end))
> 
> You're modifying a global `overlay' variable here.  Please let-bind it.

I change as you wrote.

> > +  (overlay-put overlay 'mouse-face 'highlight)
> 
> Having tried it, I'm not sure whether this is a good idea or not:
> when the conflict is large, you end up with the whole window flashing
> in `highlight' face in a very annoying way.

I agree. I've changed highlight the area when and where mouse-3 is pressed.
 
> > +  (overlay-put overlay 'keymap (let ((km (make-sparse-keymap)))
> > +				 (define-key km [down-mouse-3]
> > +				   (lambda () (interactive)
> > +				     (popup-menu smerge-mode-popup-menu)))
> > +				 km))
> > +  (overlay-put overlay 'help-echo "down-mouse-3: Use current (at point) version")
> 
> The help-echo is wrong: mouse-3 does not select the version under the
> cursor (it pops up a menu instead).

I change as you wrote.
 
> > +(defun smerge-remove-overlays ()
> > +  "Delete all overlays made by  `smerge-put-overlay'."
> > +  (mapcar (lambda (o) (delete-overlay o)) smerge-overlays)
> > +  (setq smerge-overlays nil))
> 
> When the output of `mapcar' is not used you should use `mapc'.
> (lambda (x) (f x)) should always be replaced by 'f.
I change as you wrote.
 
> > +	  (when put-overlay
> > +	    (if (and base-start base-end)
> > +		(smerge-put-overlay base-start base-end))
> > +	    (if (and mine-start mine-end)
> > +		(smerge-put-overlay mine-start mine-end))
> > +	    (if (and other-start other-end)
> > +		(smerge-put-overlay other-start other-end)))
> 
> If base-start = base-end you end up with an empty overlay that the
> user cannot click on.  

I see. Now empty overlay is never put on.

> It's simpler to just place a single overlay over
> the whole conflict (including markers).

I have not done as you wrote.
I expect "Keep Current" works on the region which is highlighted.


> Also, I'm not sure it's a good idea to have a `match' function add an
> overlay like that.  Matching is not supposed to have such side effects.
> It's probably better to let the few callers that need it call
> smerge-put-overlay themselves.
> 
> 
> 	Stefan
I agree. I've moved all overlay related functions to outside of `match' 
functions.

Masatake YAMATO

cvs diff: warning: unrecognized response `access control disabled, clients can connect from any host' from cvs server
Warning: Remote host denied X11 forwarding.

Index: lisp/smerge-mode.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/smerge-mode.el,v
retrieving revision 1.22
diff -u -r1.22 smerge-mode.el
--- lisp/smerge-mode.el	1 Sep 2003 15:45:14 -0000	1.22
+++ lisp/smerge-mode.el	18 Sep 2003 08:30:56 -0000
@@ -137,6 +137,8 @@
   `((,smerge-command-prefix . ,smerge-basic-map))
   "Keymap for `smerge-mode'.")
 
+(defvar smerge-overlays nil "Overlays managed by smerge-mode")
+
 (easy-menu-define smerge-mode-menu smerge-mode-map
   "Menu for `smerge-mode'."
   '("SMerge"
@@ -159,6 +161,13 @@
      :help "Use Ediff to resolve the conflicts"]
     ))
 
+(easy-menu-define smerge-mode-popup-menu smerge-mode-map
+  "Popup menu for `smerge-mode'."
+  '(nil
+    ["Keep this"   smerge-keep-current-by-mouse :help "Use current (at point) version"]
+    ["Keep All" smerge-keep-all :help "Keep all three versions"]
+    ))
+
 (defconst smerge-font-lock-keywords
   '((smerge-find-conflict
      (1 smerge-mine-face prepend t)
@@ -199,11 +208,13 @@
     (error (format "No `%s'" (aref smerge-match-names n)))))
 
 (defun smerge-auto-leave ()
+  "Turn smerge-mode off and return t if no conflict is found.
+Just return nil if a conflict is found."
   (when (and smerge-auto-leave
 	     (save-excursion (goto-char (point-min))
 			     (not (re-search-forward smerge-begin-re nil t))))
-    (smerge-mode -1)))
-
+    (smerge-mode -1)
+    t))
 
 (defun smerge-keep-all ()
   "Keep all three versions.
@@ -214,7 +225,8 @@
 			 (or (match-string 2) "")
 			 (or (match-string 3) ""))
 		 t t)
-  (smerge-auto-leave))
+  (unless (smerge-auto-leave)
+    (smerge-reset-all-overlays)))
 
 (defun smerge-combine-with-next ()
   "Combine the current conflict with the next one."
@@ -262,7 +274,8 @@
   (interactive)
   (smerge-match-conflict)
   (funcall smerge-resolve-function)
-  (smerge-auto-leave))
+  (unless (smerge-auto-leave)
+    (smerge-reset-all-overlays)))
 
 (defun smerge-keep-base ()
   "Revert to the base version."
@@ -270,7 +283,8 @@
   (smerge-match-conflict)
   (smerge-ensure-match 2)
   (replace-match (match-string 2) t t)
-  (smerge-auto-leave))
+  (unless (smerge-auto-leave)
+    (smerge-reset-all-overlays)))
 
 (defun smerge-keep-other ()
   "Use \"other\" version."
@@ -278,7 +292,8 @@
   (smerge-match-conflict)
   ;;(smerge-ensure-match 3)
   (replace-match (match-string 3) t t)
-  (smerge-auto-leave))
+  (unless (smerge-auto-leave)
+    (smerge-reset-all-overlays)))
 
 (defun smerge-keep-mine ()
   "Keep your version."
@@ -286,7 +301,8 @@
   (smerge-match-conflict)
   ;;(smerge-ensure-match 1)
   (replace-match (match-string 1) t t)
-  (smerge-auto-leave))
+  (unless (smerge-auto-leave)
+    (smerge-reset-all-overlays)))
 
 (defun smerge-keep-current ()
   "Use the current (under the cursor) version."
@@ -299,7 +315,17 @@
       (decf i))
     (if (<= i 0) (error "Not inside a version")
       (replace-match (match-string i) t t)
-      (smerge-auto-leave))))
+      (unless (smerge-auto-leave)
+	(smerge-reset-all-overlays)))))
+
+(defun smerge-keep-current-by-mouse (event)
+  "Call `smerge-keep-current'at the place where you clicked by a mouse."
+  (interactive "e")
+  (save-excursion
+    (set-buffer (window-buffer (posn-window (event-end event))))
+    (save-excursion
+      (goto-char (posn-point (event-end event)))
+      (smerge-keep-current))))
 
 (defun smerge-diff-base-mine ()
   "Diff 'base' and 'mine' version in current conflict region."
@@ -316,6 +342,50 @@
   (interactive)
   (smerge-diff 1 3))
 
+(defun smerge-reset-all-overlays ()
+  "Delete all overlays of smerge-mode and put overlays on the all conflicts again."
+  (smerge-delete-overlays)
+  (save-excursion 
+    (goto-char (point-min))
+    (while (smerge-find-conflict nil)
+      (smerge-put-overlays (cddr (match-data))))))
+
+(defun smerge-put-overlays (match-data)
+  "Put overlays of smerge-mode on the place specified by MATCH-DATA."
+  (let (b e)
+    (while match-data
+      (setq b (car match-data)
+	    e (cadr match-data)
+	    match-data (cddr match-data))
+      (if (and b e (not (eq b e)))
+	  (smerge-put-overlay b e)))))
+
+(defun smerge-put-overlay (start end)
+  "Put overlay of smerge-mode between START and END.
+The overlay has its own keymap to show popup menu."
+  (let ((overlay (make-overlay start end)))
+      (overlay-put overlay 'keymap (let ((km (make-sparse-keymap)))
+				     (define-key km [down-mouse-3] 
+				       (lambda (event) (interactive "e")
+					 (save-excursion
+					   (set-buffer (window-buffer (posn-window (event-end event))))
+					   (save-excursion
+					     (let (o)
+					       (goto-char (posn-point (event-end event)))
+					       (setq o (car (overlays-at (point))))
+					       (overlay-put o 'face 'region)
+					       (sit-for 0) ; redisplay
+					       (popup-menu smerge-mode-popup-menu)
+					       (overlay-put o 'face nil))))))
+				     km))
+      (overlay-put overlay 'help-echo "down-mouse-3: Show popup small menu")
+      (push overlay smerge-overlays)))
+
+(defun smerge-delete-overlays ()
+  "Delete all overlays made by  `smerge-put-overlay'."
+  (mapc 'delete-overlay smerge-overlays)
+  (setq smerge-overlays nil))
+
 (defun smerge-match-conflict ()
   "Get info about the conflict.  Puts the info in the `match-data'.
 The submatches contain:
@@ -522,6 +592,12 @@
   "Minor mode to simplify editing output from the diff3 program.
 \\{smerge-mode-map}"
   nil " SMerge" nil
+  ;; overlays management
+  (if smerge-mode
+      ;; entering smerge-mode
+      (set (make-variable-buffer-local 'smerge-overlays) nil)
+    ;; leaving smerge-mode
+    (smerge-delete-overlays)) 
   (when (and (boundp 'font-lock-mode) font-lock-mode)
     (set (make-local-variable 'font-lock-multiline) t)
     (save-excursion
@@ -529,9 +605,10 @@
 	  (font-lock-add-keywords nil smerge-font-lock-keywords 'append)
 	(font-lock-remove-keywords nil smerge-font-lock-keywords))
       (goto-char (point-min))
-      (while (smerge-find-conflict)
+      (while (smerge-find-conflict nil)
 	(save-excursion
-	  (font-lock-fontify-region (match-beginning 0) (match-end 0) nil))))))
+	  (font-lock-fontify-region (match-beginning 0) (match-end 0) nil)
+	  (smerge-put-overlays (cddr (match-data))))))))
 
 
 (provide 'smerge-mode)

Re: popup menu support for smerge-mode

[Stefan Monnier]

>> Why not just reuse the existing menu ?

> Because smerge-mode-popup-menu is much smaller than smerge-mode-menu.
> I assume that user may do following two things frequently:
> 1. keep the version under the cursur. The user can(and want to) point out 
>    the version which one wants to keep by mouse. "Keep Other" and "Keep Yours"
>    are not needed here.
> 2. keep all.

I'd then suggest to reorder the main menu so that `keep current'
and `keep all' are placed at the top.

> Of course, other menu items might be useful. But as far as trying, above two
> items are `core functions' and are enough many situations. Other functions
> in smerge-mode-menu should be invoked from M-x, C-c ^ or menu bar, I think.

But reusing the same menu has the advantage that if the user wants
to use some other function, she can do so without resorting to
the keyboard.

> I agree. I've changed highlight the area when and where mouse-3 is pressed.

Good idea.
 
>> If base-start = base-end you end up with an empty overlay that the
>> user cannot click on.  
> I see. Now empty overlay is never put on.

The problem was not so much that you end up with an empty overlay.
The problem instead is that there is no place for the user to click on.
I don't know how "average" I am, but I very often need to select an empty
alternative, so with your code, I'd have to resort to the keyboard for
that case.

>> It's simpler to just place a single overlay over
>> the whole conflict (including markers).

> I have not done as you wrote.
> I expect "Keep Current" works on the region which is highlighted.

The highlight does not have to apply to the same region as the
`keymap' overlay.

> +  "Turn smerge-mode off and return t if no conflict is found.
> +Just return nil if a conflict is found."

  "If no conflict left, turn off Smerge mode.
Return non-nil if the mode was indeed turned off."

> +  (unless (smerge-auto-leave)
> +    (smerge-reset-all-overlays)))

On large files, this can take a while and force redisplay of the whole
buffer.  I'd much rather just "remove this one overlay" instead of "remove
all and place them all back, except for this one".

Also maybe it's worth it to create a smerge-replace-match while does
the replace-match&auto-leave&remove-overlay.
 
> +  (save-excursion
> +    (set-buffer (window-buffer (posn-window (event-end event))))

Aka (with-current-buffer (window-buffer (posn-window (event-end event)))

> +      (overlay-put overlay 'keymap (let ((km (make-sparse-keymap)))

Maybe it's better to create the keymap once and store it in
smerge-popup-menu-map.

> +					 (save-excursion
> +					   (set-buffer (window-buffer (posn-window (event-end event))))

Again, I suggest `with-current-buffer'.

> +					   (save-excursion
> +					     (let (o)
> +					       (goto-char (posn-point (event-end event)))
> +					       (setq o (car (overlays-at (point))))
> +					       (overlay-put o 'face 'region)
> +					       (sit-for 0) ; redisplay
> +					       (popup-menu smerge-mode-popup-menu)
> +					       (overlay-put o 'face nil))))))

Since you've moved point to event-end when you call `popup-menu',
I think that you can directly use `smerge-keep-current' in the menu
and throw away `smerge-keep-current-by-mouse'.

Note: in case the user hits C-g or some error occurs somewhere,
it's better to do

  (unwind-protect
      (progn
        (overlay-put o 'face 'region)
        (sit-for 0) ; redisplay
        (popup-menu smerge-mode-popup-menu))
    (overlay-put o 'face nil))))))

> +  (if smerge-mode
> +      ;; entering smerge-mode
> +      (set (make-variable-buffer-local 'smerge-overlays) nil)

If smerge-mode was already ON, this ends up throwing away the list of
overlays already present, so you'll never remove them any more.
Better just do (make-variable-buffer-local 'smerge-overlays), or even
just make-variable-buffer-local at top level.

> -      (while (smerge-find-conflict)
> +      (while (smerge-find-conflict nil)

Looks like a left over from the old patch.


        Stefan

Re: popup menu support for smerge-mode

[Masatake YAMATO]

> >> Why not just reuse the existing menu ?
> 
> > Because smerge-mode-popup-menu is much smaller than smerge-mode-menu.
> > I assume that user may do following two things frequently:
> > 1. keep the version under the cursur. The user can(and want to) point out 
> >    the version which one wants to keep by mouse. "Keep Other" and "Keep Yours"
> >    are not needed here.
> > 2. keep all.
> 
> I'd then suggest to reorder the main menu so that `keep current'
> and `keep all' are placed at the top.
> 
I still insist on the simple popup menu.

The big menu (on menu bar) and the small popup menu have different scope.

The big menu provides rather buffer global operations. 
(e.g. next conflict, previous conflict => buffer global operations)
So "next conflict" and "previous conflict" should be top of the bgi menu.

In other hand, the small popup menu provides operations targeting on ONE
conflict. The scope of the small popup menu is narrower than that of the
big menu.


> > Of course, other menu items might be useful. But as far as trying, above two
> > items are `core functions' and are enough many situations. Other functions
> > in smerge-mode-menu should be invoked from M-x, C-c ^ or menu bar, I think.
> 
> But reusing the same menu has the advantage that if the user wants
> to use some other function, she can do so without resorting to
> the keyboard.
I agree this. So I added "More..." menu item which shows the big menu item
to the small popup menu.

> >> If base-start = base-end you end up with an empty overlay that the
> >> user cannot click on.  
> > I see. Now empty overlay is never put on.
> 
> The problem was not so much that you end up with an empty overlay.
> The problem instead is that there is no place for the user to click on.
> I don't know how "average" I am, but I very often need to select an empty
> alternative, so with your code, I'd have to resort to the keyboard for
> that case.

I've failed to notice this important issue.

In order to over come this situation, I've jsut added "Delete this" to the 
popup menu. `smerge-delete-current' and `smerge-concat-match-strings' do the
job. However, I have no confidence about their implementation.
I've just concatenate the two conflict areas other than the area where the 
mouse cursor is...

>>> It's simpler to just place a single overlay over
>>> the whole conflict (including markers).
>
>> I have not done as you wrote.
>> I expect "Keep Current" works on the region which is highlighted.
>
>The highlight does not have to apply to the same region as the
>`keymap' overlay.

I think applying highlight and keymap to the same region is not so
complex....probably I have not understood the intent of your
suggestions.

>> +  "Turn smerge-mode off and return t if no conflict is found.
>> +Just return nil if a conflict is found."
>
>  "If no conflict left, turn off Smerge mode.
> Return non-nil if the mode was indeed turned off."
I've done this.

>> +  (unless (smerge-auto-leave)
>> +    (smerge-reset-all-overlays)))
>
>On large files, this can take a while and force redisplay of the whole
>buffer.  I'd much rather just "remove this one overlay" instead of "remove
>all and place them all back, except for this one".
>
>Also maybe it's worth it to create a smerge-replace-match while does
>the replace-match&auto-leave&remove-overlay.

I've removed smerge-reset-all-overlays. Instead I use evaporate property
of overlays. When an overlay is created, I put t to evaporate property of 
the overlay. The overlay will be deleted when its region becomes empty.

> > +  (save-excursion
> > +    (set-buffer (window-buffer (posn-window (event-end event))))
> 
> Aka (with-current-buffer (window-buffer (posn-window (event-end event)))
> 
> > +      (overlay-put overlay 'keymap (let ((km (make-sparse-keymap)))
> 
> Maybe it's better to create the keymap once and store it in
> smerge-popup-menu-map.
> 
> > +					 (save-excursion
> > +					   (set-buffer (window-buffer (posn-window (event-end event))))
> 
> Again, I suggest `with-current-buffer'.
> 
> > +					   (save-excursion
> > +					     (let (o)
> > +					       (goto-char (posn-point (event-end event)))
> > +					       (setq o (car (overlays-at (point))))
> > +					       (overlay-put o 'face 'region)
> > +					       (sit-for 0) ; redisplay
> > +					       (popup-menu smerge-mode-popup-menu)
> > +					       (overlay-put o 'face nil))))))
> 
> Since you've moved point to event-end when you call `popup-menu',
> I think that you can directly use `smerge-keep-current' in the menu
> and throw away `smerge-keep-current-by-mouse'.
> 
> Note: in case the user hits C-g or some error occurs somewhere,
> it's better to do
> 
>   (unwind-protect
>       (progn
>         (overlay-put o 'face 'region)
>         (sit-for 0) ; redisplay
>         (popup-menu smerge-mode-popup-menu))
>     (overlay-put o 'face nil))))))
> 
> > +  (if smerge-mode
> > +      ;; entering smerge-mode
> > +      (set (make-variable-buffer-local 'smerge-overlays) nil)
> 
> If smerge-mode was already ON, this ends up throwing away the list of
> overlays already present, so you'll never remove them any more.
> Better just do (make-variable-buffer-local 'smerge-overlays), or even
> just make-variable-buffer-local at top level.
> 
> > -      (while (smerge-find-conflict)
> > +      (while (smerge-find-conflict nil)
> 
> Looks like a left over from the old patch.

I've done all.
Regards,

2003-09-19  Masatake YAMATO  <jet@gyve.org>

	* smerge-mode.el (smerge-overlays): New variable.
	(smerge-mode-popup-menu): New menu.
	(smerge-auto-leave): Return nil if a conflict is found.
	(smerge-keep-current-by-mouse): New function.
	(smerge-put-overlay): New function.
	(smerge-delete-overlays): New function.
	(smerge-mode): Add code to manage overlays.
	(smerge-keep-all, smerge-resolve, smerge-keep-base)
	(smerge-keep-other, smerge-keep-mine, smerge-keep-current): 
	invoke `smerge-reset-all-overlays'.
	(smerge-reset-all-overlays): New function.
	(smerge-put-overlays): New function.
	(smerge-popup-menu-map): New key map.
	(smerge-delete-current): New function.
	(smerge-concat-match-strings): New function.
	(smerge-activate-popup-menu): New function.

cvs diff: warning: unrecognized response `access control disabled, clients can connect from any host' from cvs server
Warning: Remote host denied X11 forwarding.

Index: lisp/smerge-mode.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/smerge-mode.el,v
retrieving revision 1.22
diff -u -r1.22 smerge-mode.el
--- lisp/smerge-mode.el	1 Sep 2003 15:45:14 -0000	1.22
+++ lisp/smerge-mode.el	19 Sep 2003 09:20:13 -0000
@@ -159,6 +159,21 @@
      :help "Use Ediff to resolve the conflicts"]
     ))
 
+(defvar smerge-overlays nil "Overlays managed by smerge-mode")
+(easy-mmode-defmap smerge-popup-menu-map
+  `((,smerge-command-prefix . ,smerge-basic-map)
+    ([down-mouse-3] . smerge-activate-popup-menu))
+  "Keymap for popup menu appeared on conflicts area.")
+(easy-menu-define smerge-mode-popup-menu smerge-popup-menu-map
+  "Popup menu for `smerge-mode'."
+  '(nil
+    ["Keep This"   smerge-keep-current :help "Use current (at point) version"]
+    ["Delete This" smerge-delete-current :help "Delete current (at point) version"]
+    ["Keep All"    smerge-keep-all :help "Keep all three versions"]
+    "---"
+    ["More..." (popup-menu smerge-mode-menu) :help "Show full SMerge mode menu"]
+    ))
+
 (defconst smerge-font-lock-keywords
   '((smerge-find-conflict
      (1 smerge-mine-face prepend t)
@@ -199,11 +214,13 @@
     (error (format "No `%s'" (aref smerge-match-names n)))))
 
 (defun smerge-auto-leave ()
+  "If no conflict left, turn off Smerge mode.
+Return non-nil if the mode was indeed turned off."
   (when (and smerge-auto-leave
 	     (save-excursion (goto-char (point-min))
 			     (not (re-search-forward smerge-begin-re nil t))))
-    (smerge-mode -1)))
-
+    (smerge-mode -1)
+    t))
 
 (defun smerge-keep-all ()
   "Keep all three versions.
@@ -301,6 +318,43 @@
       (replace-match (match-string i) t t)
       (smerge-auto-leave))))
 
+(defun smerge-delete-current ()
+  "Delete the current (under the cursor) version."
+  (interactive)
+  (smerge-match-conflict)
+  (let ((i 3) newstr)
+    (while (or (not (match-end i))
+	       (< (point) (match-beginning i))
+	       (>= (point) (match-end i)))
+      (decf i))
+    (if (<= i 0) (error "Not inside a version")
+      (setq newstr (smerge-concat-match-strings (delete i '(1 2 3))))
+      (replace-match newstr t t)
+      (smerge-auto-leave))))
+
+(defun smerge-concat-match-strings (match-indexes)
+  "Concatenated match-strings taking their buffer position into account.
+MATCH-INDEXES is a list. MATCH-INDEXES should hold two integers(i, j). 
+ (match-string i) and (match-string j) are concatenated."
+  (let ((i (nth 0 match-indexes))
+	(j (nth 1 match-indexes)))
+    (cond
+     ((and (null i) (null j))
+      "")
+     ((or (null i) (null (match-end i)) (= 0 (length (match-string i))))
+      (or (match-string j) ""))
+     ((or (null j) (null (match-end j)) (= 0 (length (match-string j))))
+      (or (match-string i) ""))
+     ((< (match-end i) (match-end j))
+      (concat (match-string i) (match-string j)))
+     ((> (match-end i) (match-end j))
+      (concat (match-string j) (match-string i)))
+     ((<= (length (match-string i)) (length (match-string j)))
+      (match-string j))
+     (t
+      (match-string i)))))
+     
+
 (defun smerge-diff-base-mine ()
   "Diff 'base' and 'mine' version in current conflict region."
   (interactive)
@@ -316,6 +370,53 @@
   (interactive)
   (smerge-diff 1 3))
 
+(defun smerge-reset-all-overlays ()
+  "Delete all overlays of smerge-mode and put overlays on the all conflicts again."
+  (smerge-delete-overlays)
+  (save-excursion 
+    (goto-char (point-min))
+    (while (smerge-find-conflict)
+      (smerge-put-overlays (cddr (match-data))))))
+
+(defun smerge-put-overlays (match-data)
+  "Put overlays of smerge-mode on the place specified by MATCH-DATA."
+  (let (b e)
+    (while match-data
+      (setq b (car match-data)
+	    e (cadr match-data)
+	    match-data (cddr match-data))
+      (if (and b e (not (eq b e)))
+	  (smerge-put-overlay b e)))))
+
+(defun smerge-put-overlay (start end)
+  "Put overlay of smerge-mode between START and END.
+The overlay has its own keymap to show popup menu."
+  (let ((overlay (make-overlay start end)))
+    (overlay-put overlay 'evaporate t)
+    (overlay-put overlay 'help-echo "down-mouse-3: Show popup menu")
+    (overlay-put overlay 'keymap smerge-popup-menu-map)
+    (push overlay smerge-overlays)))
+
+(defun smerge-activate-popup-menu (event)
+  (interactive "e")
+  (with-current-buffer (window-buffer 
+			(posn-window (event-end event)))
+    (save-excursion
+      (let (o)
+	(goto-char (posn-point (event-end event)))
+	(setq o (car (overlays-at (point))))
+	(unwind-protect
+	    (progn
+	      (overlay-put o 'face 'region)
+	      (sit-for 0)		; redisplay
+	      (popup-menu smerge-mode-popup-menu))
+	  (overlay-put o 'face nil))))))
+
+(defun smerge-delete-overlays ()
+  "Delete all overlays made by  `smerge-put-overlay'."
+  (mapc 'delete-overlay smerge-overlays)
+  (setq smerge-overlays nil))
+
 (defun smerge-match-conflict ()
   "Get info about the conflict.  Puts the info in the `match-data'.
 The submatches contain:
@@ -522,6 +623,12 @@
   "Minor mode to simplify editing output from the diff3 program.
 \\{smerge-mode-map}"
   nil " SMerge" nil
+  ;; overlays management
+  (if smerge-mode
+      ;; entering smerge-mode
+      (make-variable-buffer-local 'smerge-overlays)
+    ;; leaving smerge-mode
+    (smerge-delete-overlays)) 
   (when (and (boundp 'font-lock-mode) font-lock-mode)
     (set (make-local-variable 'font-lock-multiline) t)
     (save-excursion
@@ -531,7 +638,8 @@
       (goto-char (point-min))
       (while (smerge-find-conflict)
 	(save-excursion
-	  (font-lock-fontify-region (match-beginning 0) (match-end 0) nil))))))
+	  (font-lock-fontify-region (match-beginning 0) (match-end 0) nil)
+	  (smerge-put-overlays (cddr (match-data))))))))
 
 
 (provide 'smerge-mode)

Re: popup menu support for smerge-mode

[Stefan Monnier]

>>>> It's simpler to just place a single overlay over
>>>> the whole conflict (including markers).
>> 
>>> I have not done as you wrote.
>>> I expect "Keep Current" works on the region which is highlighted.
>> 
>> The highlight does not have to apply to the same region as the
>> `keymap' overlay.

> I think applying highlight and keymap to the same region is not so
> complex....probably I have not understood the intent of your
> suggestions.

You justified using 2 or 3 overlays over each alternative rather than
a single one over the whole conflict by saying that "keep current"
should work on the region which is highlighted.

What I meant to reply is that you could place a single overlay over the
whole conflict with a `keymap' property and then when mouse-3 is pressed
and you add the highlight, you can create a new overlay that covers only
the relevant alternative and place the highlight there: the highlight
and the keymap do not have to apply to the same region.

If you place the overlay over the whole conflict, then the "current"
alternative can be empty without any problem (other than the fact
that the highlighting will not aplly to any character) so you
don't need smerge-delete-current.

BTW, I don't understand this `smerge-delete-current'.  It seems to take
a conflict with a non-empty current alternative and replaces it with
a conflict with an empty current alternative, is that right ?  What I said
I need is to take a conflict where the current alternative is empty and
select that alternative (i.e. just delete the whole conflict).

> I've removed smerge-reset-all-overlays. Instead I use evaporate property
> of overlays. When an overlay is created, I put t to evaporate property of 
> the overlay. The overlay will be deleted when its region becomes empty.

I was thinking of suggesting that but wasn't sure whether it'd work.
Does it ?  If it does, it's probably the simplest alternative, indeed.

> +(defvar smerge-overlays nil "Overlays managed by smerge-mode")
> +(easy-mmode-defmap smerge-popup-menu-map
> +  `((,smerge-command-prefix . ,smerge-basic-map)

What is this line for ?

> +    ([down-mouse-3] . smerge-activate-popup-menu))
> +  "Keymap for popup menu appeared on conflicts area.")
> +(easy-menu-define smerge-mode-popup-menu smerge-popup-menu-map

This adds the popup menu inside smerge-popup-menu-map which I don't
think you want because you popup smerge-mode-popup-menu
from smerge-activate-popup-menu.  I guess it doesn't hurt because
it's placed somewhere in smerge-popup-menu-map where it has no effect ?
Or maybe it adds the popup menu in the menubar when point is in an area
covered by an overlay with the keymap property ?  Ahh... probably not
because you use a `nil' name.  In any case, you should use another name
for the popup-menu-map (or just leave it nil because you don't need it).

> +(defun smerge-reset-all-overlays ()
> +  "Delete all overlays of smerge-mode and put overlays on the all conflicts again."
> +  (smerge-delete-overlays)
> +  (save-excursion 
> +    (goto-char (point-min))
> +    (while (smerge-find-conflict)
> +      (smerge-put-overlays (cddr (match-data))))))

Left over?


        Stefan

Re: popup menu support for smerge-mode

[Masatake YAMATO]

> >>>> It's simpler to just place a single overlay over
> >>>> the whole conflict (including markers).
> >> 
> >>> I have not done as you wrote.
> >>> I expect "Keep Current" works on the region which is highlighted.
> >> 
> >> The highlight does not have to apply to the same region as the
> >> `keymap' overlay.
> 
> > I think applying highlight and keymap to the same region is not so
> > complex....probably I have not understood the intent of your
> > suggestions.
> 
> You justified using 2 or 3 overlays over each alternative rather than
> a single one over the whole conflict by saying that "keep current"
> should work on the region which is highlighted.
> 
> What I meant to reply is that you could place a single overlay over the
> whole conflict with a `keymap' property and then when mouse-3 is pressed
> and you add the highlight, you can create a new overlay that covers only
> the relevant alternative and place the highlight there: the highlight
> and the keymap do not have to apply to the same region.
> 
> If you place the overlay over the whole conflict, then the "current"
> alternative can be empty without any problem (other than the fact
> that the highlighting will not aplly to any character) so you
> don't need smerge-delete-current.
> 
> BTW, I don't understand this `smerge-delete-current'.  It seems to take
> a conflict with a non-empty current alternative and replaces it with
> a conflict with an empty current alternative, is that right ?  What I said
> I need is to take a conflict where the current alternative is empty and
> select that alternative (i.e. just delete the whole conflict).

I see. Now overlays for highlighting and a overlays for keymap are used.
Each overlays for highlighting have a property named owner.
Owner property could take a value; whole, mine, other, base...
The popup menu is switched by the property value.

mine and other uses special menus. I added "Keep alternative" to the
menus.  Other including whole uses the smerge menu appeared on the
menu bar.

smerge-delete-current is gone.

> > I've removed smerge-reset-all-overlays. Instead I use evaporate property
> > of overlays. When an overlay is created, I put t to evaporate property of 
> > the overlay. The overlay will be deleted when its region becomes empty.
> 
> I was thinking of suggesting that but wasn't sure whether it'd work.
> Does it ?  If it does, it's probably the simplest alternative, indeed.

`evaporate' doesn't work I expect. So I introduced suboverlays property to
manage overlay deletion.

Masatake YAMATO

Index: lisp/smerge-mode.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/smerge-mode.el,v
retrieving revision 1.22
diff -u -r1.22 smerge-mode.el
--- lisp/smerge-mode.el	1 Sep 2003 15:45:14 -0000	1.22
+++ lisp/smerge-mode.el	26 Sep 2003 07:45:14 -0000
@@ -159,6 +159,31 @@
      :help "Use Ediff to resolve the conflicts"]
     ))
 
+(defvar smerge-overlays nil "Overlays managed by smerge-mode")
+(easy-mmode-defmap smerge-popup-menu-map
+  `(([down-mouse-3] . smerge-activate-popup-menu))
+  "Keymap for popup menu appeared on conflicts area.")
+(easy-menu-define smerge-mode-mine-popup-menu nil
+  "Popup menu for mine area in `smerge-mode'."
+  '(nil
+    ["Keep This" smerge-keep-current :help "Use current (at point) version"]
+    ;; mine <-> other
+    ["Keep Alternative" smerge-keep-other :help "Use alternative version"] 
+    ["Keep All" smerge-keep-all :help "Keep all three versions"]
+    "---"
+    ["More..." (popup-menu smerge-mode-menu) :help "Show full SMerge mode menu"]
+    ))
+(easy-menu-define smerge-mode-other-popup-menu nil
+  "Popup menu for other area in `smerge-mode'."
+  '(nil
+    ["Keep This"   smerge-keep-current :help "Use current (at point) version"]
+    ;; other <-> mine
+    ["Keep Alternative" smerge-keep-mine :help "Use alternative version"]
+    ["Keep All"    smerge-keep-all :help "Keep all three versions"]
+    "---"
+    ["More..." (popup-menu smerge-mode-menu) :help "Show full SMerge mode menu"]
+    ))
+
 (defconst smerge-font-lock-keywords
   '((smerge-find-conflict
      (1 smerge-mine-face prepend t)
@@ -199,11 +224,13 @@
     (error (format "No `%s'" (aref smerge-match-names n)))))
 
 (defun smerge-auto-leave ()
+  "If no conflict left, turn off Smerge mode.
+Return non-nil if the mode was indeed turned off."
   (when (and smerge-auto-leave
 	     (save-excursion (goto-char (point-min))
 			     (not (re-search-forward smerge-begin-re nil t))))
-    (smerge-mode -1)))
-
+    (smerge-mode -1)
+    t))
 
 (defun smerge-keep-all ()
   "Keep all three versions.
@@ -316,6 +343,92 @@
   (interactive)
   (smerge-diff 1 3))
 
+(defun smerge-put-overlays (match-data)
+  "Put overlays of smerge-mode on the place specified by MATCH-DATA."
+  (let ((m (cddr match-data))
+	(owners '(mine base other base-start other-start))
+	(b-whole (car match-data))
+	(e-whole (cadr match-data))
+	b e o os)
+    (while m
+      (setq b (car m)
+	    e (cadr m)
+	    m (cddr m)
+	    o (car owners)
+	    owners (cdr owners))
+      (when (and b e (not (= b e)))
+	(push (smerge-put-highlight-overlay b e o)
+	      os)))
+    ;; highlight overlays are managed by keymap overlay.
+    ;; When keymap overlay is shrinked or removed, 
+    ;; highlight overlays are removed.
+    (smerge-put-keymap-overlay b-whole e-whole os)))
+    
+(defun smerge-put-highlight-overlay (start end owner)
+  "Put overlay of smerge-mode between START and END.
+The overlay is highlight when it is pressed.
+OWNER is stored to `owner' property of the new overlay."
+  (let ((overlay (make-overlay start end)))
+    (overlay-put overlay 'evaporate t)
+    (overlay-put overlay 'owner owner)
+    (push overlay smerge-overlays)
+    overlay))
+
+(defun smerge-put-keymap-overlay (start end suboverlays)
+  "Put overlay of smerge-mode between START and END.
+The overlay has its own keymap to show popup menu.
+SUBOVERLAYS are overlays managed by this overlay."
+  (let ((overlay (make-overlay start end)))
+    (overlay-put overlay 'evaporate t)
+    (overlay-put overlay 'help-echo "down-mouse-3: Show popup menu")
+    (overlay-put overlay 'local-map smerge-popup-menu-map)
+    (overlay-put overlay 'owner 'whole)
+    (overlay-put overlay 'suboverlays suboverlays)
+    (push overlay smerge-overlays)
+    overlay))
+
+(defun smerge-activate-popup-menu (event)
+  "Show a popup menu for smerge-mode."
+  (interactive "e")
+  (with-current-buffer (window-buffer 
+			(posn-window (event-end event)))
+    (save-excursion
+      (goto-char (posn-point (event-end event)))
+      (let* ((os-at (overlays-at (point)))
+	     (o-whole (car os-at))
+	     (region-whole (- (overlay-end o-whole) (overlay-start o-whole)))
+	     (os-sub  (overlay-get o-whole 'suboverlays))
+	     (o (if (= 1 (length os-at)) o-whole (cadr os-at)))
+	     (owner (overlay-get o 'owner))
+	     (face 'highlight)
+	     (menu smerge-mode-menu))
+	;; Set face and menu
+	(cond
+	 ((eq 'mine owner)
+	  (setq menu smerge-mode-mine-popup-menu
+		face 'region))
+	 ((eq 'other owner)
+	  (setq menu smerge-mode-other-popup-menu
+		face 'region)))
+	;; Highlight and show popup menu
+	(unwind-protect
+	    (progn
+	      (overlay-put o 'face face)
+	      (sit-for 0)		      ;; redisplay
+	      (popup-menu menu))
+	  (overlay-put o 'face nil))
+	;; Delete overlays
+	(when (or 
+	       (not (overlay-buffer o-whole)) ;; dead
+	       (< (- (overlay-end o-whole) (overlay-start o-whole))
+		  region-whole))	      ;; shrinked up
+	  (mapc 'delete-overlay (cons o-whole os-sub)))))))
+
+(defun smerge-delete-overlays ()
+  "Delete all overlays made by  `smerge-put-overlay'."
+  (mapc 'delete-overlay smerge-overlays)
+  (setq smerge-overlays nil))
+
 (defun smerge-match-conflict ()
   "Get info about the conflict.  Puts the info in the `match-data'.
 The submatches contain:
@@ -522,6 +635,12 @@
   "Minor mode to simplify editing output from the diff3 program.
 \\{smerge-mode-map}"
   nil " SMerge" nil
+  ;; overlays management
+  (if smerge-mode
+      ;; entering smerge-mode
+      (make-variable-buffer-local 'smerge-overlays)
+    ;; leaving smerge-mode
+    (smerge-delete-overlays)) 
   (when (and (boundp 'font-lock-mode) font-lock-mode)
     (set (make-local-variable 'font-lock-multiline) t)
     (save-excursion
@@ -531,8 +650,8 @@
       (goto-char (point-min))
       (while (smerge-find-conflict)
 	(save-excursion
-	  (font-lock-fontify-region (match-beginning 0) (match-end 0) nil))))))
-
+	  (font-lock-fontify-region (match-beginning 0) (match-end 0) nil)
+	  (smerge-put-overlays (match-data)))))))
 
 (provide 'smerge-mode)

Re: popup menu support for smerge-mode

[Miles Bader]

Masatake YAMATO <jet@gyve.org> writes:
> > I was thinking of suggesting that but wasn't sure whether it'd work.
> > Does it ?  If it does, it's probably the simplest alternative, indeed.
> 
> `evaporate' doesn't work I expect. So I introduced suboverlays property to
> manage overlay deletion.

I'm not sure exactly what you're saying, but the overlay `evaporate'
feature definitely works (unless it's recently been broken)...

-Miles
-- 
Next to fried food, the South has suffered most from oratory.
  			-- Walter Hines Page

Behavior of evaporate

[Masatake YAMATO]

> > > I was thinking of suggesting that but wasn't sure whether it'd work.
> > > Does it ?  If it does, it's probably the simplest alternative, indeed.
> > 
> > `evaporate' doesn't work I expect. So I introduced suboverlays property to
> > manage overlay deletion.
> 
> I'm not sure exactly what you're saying, but the overlay `evaporate'
> feature definitely works (unless it's recently been broken)...

(progn (setq xxx 1)
       (setq o (make-overlay 0 0))
       (overlay-put o 'modification-hooks (lambda (ov dummy b e l)
					    (setq xxx 0)))
       (overlay-put o 'evaporate t)
       xxx)
=> 1

Result I expected is 0.
So I can know an overlay is dead.
Do I misunderstand?

Masatake YAMATO

Re: Behavior of evaporate

[Miles Bader]

Masatake YAMATO <jet@gyve.org> writes:
> (progn (setq xxx 1)
>        (setq o (make-overlay 0 0))
>        (overlay-put o 'modification-hooks (lambda (ov dummy b e l)
> 					    (setq xxx 0)))
>        (overlay-put o 'evaporate t)
>        xxx)
> => 1
> 
> Result I expected is 0.
> So I can know an overlay is dead.

I don't think modification hooks are called in this case -- since the
overlay had an initial size of 0 (and illegal begin/end values for that
matter), it was never really part of the buffer at all.

The following works as you expect:

   (progn (setq xxx 1)
          (setq o (make-overlay 1 2))
          (overlay-put o 'modification-hooks (list (lambda (ov b e l)
                                                      (setq xxx 0))))
          (overlay-put o 'evaporate t)
          (delete-region 1 2)
          xxx)
=> 0

You can also detect whether an overlay's been evaporated or not by
seeing if it has a null buffer, e.g., (overlay-buffer o) => nil.

-Miles
-- 
`There are more things in heaven and earth, Horatio,
 Than are dreamt of in your philosophy.'

Re: Behavior of evaporate

[David Kastrup]

Masatake YAMATO <jet@gyve.org> writes:

> > > > I was thinking of suggesting that but wasn't sure whether it'd work.
> > > > Does it ?  If it does, it's probably the simplest alternative, indeed.
> > > 
> > > `evaporate' doesn't work I expect. So I introduced suboverlays
> > > property to manage overlay deletion.
> > 
> > I'm not sure exactly what you're saying, but the overlay
> > `evaporate' feature definitely works (unless it's recently been
> > broken)...
> 
> (progn (setq xxx 1)
>        (setq o (make-overlay 0 0))
>        (overlay-put o 'modification-hooks (lambda (ov dummy b e l)
> 					    (setq xxx 0)))
>        (overlay-put o 'evaporate t)
>        xxx)
> => 1
> 
> Result I expected is 0.
> So I can know an overlay is dead.
> Do I misunderstand?

Yes.  From the manual:

`evaporate'
     If this property is non-`nil', the overlay is deleted automatically
     if it ever becomes empty (i.e., if it spans no characters).

But the overlay does not become empty in your example.  It _is_
already empty.

Setting the overlay borders explicitly is not what this property is
useful for catching.  It is there for the case that the overlay
becomes empty by side effect.  Try

(progn (setq xxx 1)
       (goto-char 1)
       (insert "x")
       (setq o (make-overlay 1 2))
       (overlay-put o 'modification-hooks (list (lambda (ov dummy b e
                                                   &optional l)
					    (setq xxx 0))))
       (overlay-put o 'evaporate t)
       (delete-char 1)
       xxx)

And notice that modification-hooks takes a list of functions, not
just a single function.  And that the last argument of the function
is optional.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Behavior of evaporate

[Stefan Monnier]

> (progn (setq xxx 1)
>        (goto-char 1)
>        (insert "x")
>        (setq o (make-overlay 1 2))
>        (overlay-put o 'modification-hooks (list (lambda (ov dummy b e
>                                                    &optional l)
> 					    (setq xxx 0))))
>        (overlay-put o 'evaporate t)
>        (delete-char 1)
>        xxx)

And the above code only tests whether `modification-hooks' works, not
whether `evaporate' works.

As for my initial remark about `evaporate' working, I meant "I'm not sure
if it does what we want here" when I said "I'm not sure it'll work".
I know `evaporate' does work in the sense that it does what it's documented
to do (otherwise I'd have fixed it or filed a bug report).


        Stefan

Re: Behavior of evaporate

[Richard Stallman]

    Yes.  From the manual:

    `evaporate'
	 If this property is non-`nil', the overlay is deleted automatically
	 if it ever becomes empty (i.e., if it spans no characters).

    But the overlay does not become empty in your example.  It _is_
    already empty.

I clarified the manual as follows:

@item evaporate
@kindex evaporate @r{(overlay property)}
If this property is non-@code{nil}, the overlay is deleted automatically
if it becomes empty (i.e., if its length becomes zero).  However,
if the overlay is @emph{already} empty, @code{evaporate} does not
delete it.

Is that entirely correct?

Also, is that really what we want?  Maybe setting the `evaporate'
property non-nil should delete the overlay immediately if it is
already empty.  That would be more consistent.

Re: Behavior of evaporate

[Alex Schroeder]

Richard Stallman <rms@gnu.org> writes:

> Also, is that really what we want?  Maybe setting the `evaporate'
> property non-nil should delete the overlay immediately if it is
> already empty.  That would be more consistent.

Assume you write code:

create overlay, set evaporate property, change begin and end of the overlay

Now change the order:

create overlay, change begin and end of the overlay, set evaporate property

I say that we want the same result in both cases.  Therefore the
current state is what we want.

Alex.
-- 
http://www.emacswiki.org/alex/
There is no substitute for experience.

Re: popup menu support for smerge-mode

[Stefan Monnier]

> mine and other uses special menus. I added "Keep alternative" to the
> menus.  Other including whole uses the smerge menu appeared on the
> menu bar.

Don't forget that conflicts can have a 3-parts shape where there's not just
"mine" and "other" but also the ancestor, in which case there's no single
"Keep alternative".

> +(easy-menu-define smerge-mode-mine-popup-menu nil
> +  "Popup menu for mine area in `smerge-mode'."
> +  '(nil
> +    ["Keep This" smerge-keep-current :help "Use current (at point) version"]
> +    ;; mine <-> other
> +    ["Keep Alternative" smerge-keep-other :help "Use alternative version"] 
> +    ["Keep All" smerge-keep-all :help "Keep all three versions"]
> +    "---"
> +    ["More..." (popup-menu smerge-mode-menu) :help "Show full SMerge mode menu"]
> +    ))
> +(easy-menu-define smerge-mode-other-popup-menu nil
> +  "Popup menu for other area in `smerge-mode'."
> +  '(nil
> +    ["Keep This"   smerge-keep-current :help "Use current (at point) version"]
> +    ;; other <-> mine
> +    ["Keep Alternative" smerge-keep-mine :help "Use alternative version"]
> +    ["Keep All"    smerge-keep-all :help "Keep all three versions"]
> +    "---"
> +    ["More..." (popup-menu smerge-mode-menu) :help "Show full SMerge mode menu"]
> +    ))

I'd rather introduce a new function smerge-keep-alternative which
will determine whether to use `other' or `mine' depending on `current'.
This way there's only one menu rather than two with the same appearance
but different behavior.

> +      (when (and b e (not (= b e)))

I think this can be simplified to `unless (eq e b)' because `e' and `b'
are either both locations or both nil.

> +	;; Delete overlays
> +	(when (or 
> +	       (not (overlay-buffer o-whole)) ;; dead
> +	       (< (- (overlay-end o-whole) (overlay-start o-whole))
> +		  region-whole))	      ;; shrinked up
> +	  (mapc 'delete-overlay (cons o-whole os-sub)))))))

I agree it's easier to remove the overlays from smerge-activate-popup-menu,
but that means they'll just never be removed if the user doesn't use
this new feature.  I think it really needs to be done somewhere else
instead (E.g. at the same place as the auto-leave code is run.  We'll
probably need to introduce a new function `smerge-post-resolution-update'
which will do the auto-leave check and will remove the overlays).


        Stefan

Re: popup menu support for smerge-mode

[Masatake YAMATO]

I've found a time to revise my patch about popup menu support 
for smerge-mode. Stefan, don't angry with me. I hope you do not forget
the old patch... A patch for popup menu for smerge-mode in my local 
machine is too useful to me to throw away:-)

> > mine and other uses special menus. I added "Keep alternative" to the
> > menus.  Other including whole uses the smerge menu appeared on the
> > menu bar.
> 
> Don't forget that conflicts can have a 3-parts shape where there's not just
> "mine" and "other" but also the ancestor, in which case there's no single
> "Keep alternative".

I've added "Revert to the BASE" to the popup menu.

> > +(easy-menu-define smerge-mode-mine-popup-menu nil
> > +  "Popup menu for mine area in `smerge-mode'."
> > +  '(nil
> > +    ["Keep This" smerge-keep-current :help "Use current (at point) version"]
> > +    ;; mine <-> other
> > +    ["Keep Alternative" smerge-keep-other :help "Use alternative version"] 
> > +    ["Keep All" smerge-keep-all :help "Keep all three versions"]
> > +    "---"
> > +    ["More..." (popup-menu smerge-mode-menu) :help "Show full SMerge mode menu"]
> > +    ))
> > +(easy-menu-define smerge-mode-other-popup-menu nil
> > +  "Popup menu for other area in `smerge-mode'."
> > +  '(nil
> > +    ["Keep This"   smerge-keep-current :help "Use current (at point) version"]
> > +    ;; other <-> mine
> > +    ["Keep Alternative" smerge-keep-mine :help "Use alternative version"]
> > +    ["Keep All"    smerge-keep-all :help "Keep all three versions"]
> > +    "---"
> > +    ["More..." (popup-menu smerge-mode-menu) :help "Show full SMerge mode menu"]
> > +    ))
> 
> I'd rather introduce a new function smerge-keep-alternative which
> will determine whether to use `other' or `mine' depending on `current'.
> This way there's only one menu rather than two with the same appearance
> but different behavior.

I wrote `smerge-keep-alternative' and unified the above two menus.

> > +      (when (and b e (not (= b e)))
> 
> I think this can be simplified to `unless (eq e b)' because `e' and `b'
> are either both locations or both nil.

I have used `unless'.

> > +	;; Delete overlays
> > +	(when (or 
> > +	       (not (overlay-buffer o-whole)) ;; dead
> > +	       (< (- (overlay-end o-whole) (overlay-start o-whole))
> > +		  region-whole))	      ;; shrinked up
> > +	  (mapc 'delete-overlay (cons o-whole os-sub)))))))
> 
> I agree it's easier to remove the overlays from smerge-activate-popup-menu,
> but that means they'll just never be removed if the user doesn't use
> this new feature.  I think it really needs to be done somewhere else
> instead (E.g. at the same place as the auto-leave code is run.  We'll
> probably need to introduce a new function `smerge-post-resolution-update'
> which will do the auto-leave check and will remove the overlays).

Removing overlays in smerge-post-resolution-update(or auto-leave) is not easy
because the some part of text area associated with the overlays are removed
during "resolution". Instead I put the code(smerge-delete-overlays-at) to 
delete overlays before "resolution".

Index: lisp/smerge-mode.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/smerge-mode.el,v
retrieving revision 1.24
diff -u -r1.24 smerge-mode.el
--- lisp/smerge-mode.el	6 Oct 2003 16:34:59 -0000	1.24
+++ lisp/smerge-mode.el	10 Mar 2004 04:27:49 -0000
@@ -159,6 +159,22 @@
      :help "Use Ediff to resolve the conflicts"]
     ))
 
+(defvar smerge-overlays nil "Overlays managed by smerge-mode")
+(easy-mmode-defmap smerge-popup-menu-map
+  `(([down-mouse-3] . smerge-activate-popup-menu))
+  "Keymap for popup menu appeared on conflicts area.")
+(easy-menu-define smerge-mode-popup-menu nil
+  "Popup menu for mine area in `smerge-mode'."
+  '(nil
+    ["Keep All" smerge-keep-all :help "Keep all three versions"]
+    ["Revert to the Base" smerge-keep-base :help "Revert to the base version"]
+    ["Keep This" smerge-keep-current :help "Use current (at point) version"]
+    ["Keep Alternative" smerge-keep-alternative :help "Use alternative version"] 
+    "---"
+    ["More..." (popup-menu smerge-mode-menu) :help "Show full SMerge mode menu"]
+    ))
+
+
 (defconst smerge-font-lock-keywords
   '((smerge-find-conflict
      (1 smerge-mine-face prepend t)
@@ -198,18 +214,35 @@
   (unless (match-end n)
     (error (format "No `%s'" (aref smerge-match-names n)))))
 
+(defun smerge-delete-overlays-at (pos)
+  "Delete overlays used in Smerge mode under POS."
+  (let  ((overlays (overlays-at pos))
+	 o
+	 suboverlays)
+    (while overlays
+      (setq o (car overlays)
+	    overlays (cdr overlays)
+	    suboverlays (overlay-get o 'suboverlays))
+      (when suboverlays
+	(setq suboverlays (cons o suboverlays)
+	      overlays nil)))
+    (mapc 'delete-overlay suboverlays)))
+
 (defun smerge-auto-leave ()
+  "If no conflict left, turn off Smerge mode.
+Return non-nil if the mode was indeed turned off."
   (when (and smerge-auto-leave
 	     (save-excursion (goto-char (point-min))
 			     (not (re-search-forward smerge-begin-re nil t))))
-    (smerge-mode -1)))
-
+    (smerge-mode -1)
+    t))
 
 (defun smerge-keep-all ()
   "Keep all three versions.
 Convenient for the kind of conflicts that can arise in ChangeLog files."
   (interactive)
   (smerge-match-conflict)
+  (smerge-delete-overlays-at (point))
   (replace-match (concat (or (match-string 1) "")
 			 (or (match-string 2) "")
 			 (or (match-string 3) ""))
@@ -261,6 +294,7 @@
 some major modes.  Uses `smerge-resolve-function' to do the actual work."
   (interactive)
   (smerge-match-conflict)
+  (smerge-delete-overlays-at (point))
   (funcall smerge-resolve-function)
   (smerge-auto-leave))
 
@@ -269,6 +303,7 @@
   (interactive)
   (smerge-match-conflict)
   (smerge-ensure-match 2)
+  (smerge-delete-overlays-at (point))
   (replace-match (match-string 2) t t)
   (smerge-auto-leave))
 
@@ -277,6 +312,7 @@
   (interactive)
   (smerge-match-conflict)
   ;;(smerge-ensure-match 3)
+  (smerge-delete-overlays-at (point))
   (replace-match (match-string 3) t t)
   (smerge-auto-leave))
 
@@ -285,6 +321,7 @@
   (interactive)
   (smerge-match-conflict)
   ;;(smerge-ensure-match 1)
+  (smerge-delete-overlays-at (point))
   (replace-match (match-string 1) t t)
   (smerge-auto-leave))
 
@@ -298,9 +335,28 @@
 	       (>= (point) (match-end i)))
       (decf i))
     (if (<= i 0) (error "Not inside a version")
+      (smerge-delete-overlays-at (point))
       (replace-match (match-string i) t t)
       (smerge-auto-leave))))
 
+(defun smerge-keep-alternative ()
+  "Use the alternatives (not under the cursor) version."
+  (interactive)
+  (smerge-match-conflict)
+  (let ((i 3))
+    (while (or (not (match-end i))
+	       (< (point) (match-beginning i))
+	       (>= (point) (match-end i)))
+      (decf i))
+    (cond
+     ((<= i 0) (error "Not inside a version"))
+     ((eq i 2) (error "No alternative for the base version"))
+     ((eq i 3) (setq i 1))
+     ((eq i 1) (setq i 3)))
+    (smerge-delete-overlays-at (point))
+    (replace-match (match-string i) t t) 
+    (smerge-auto-leave)))
+
 (defun smerge-diff-base-mine ()
   "Diff 'base' and 'mine' version in current conflict region."
   (interactive)
@@ -316,6 +372,93 @@
   (interactive)
   (smerge-diff 1 3))
 
+(defun smerge-put-overlays (match-data)
+  "Put overlays of smerge-mode on the place specified by MATCH-DATA."
+  (let ((m (cddr match-data))
+	(owners '(mine base other base-start other-start))
+	(b-whole (car match-data))
+	(e-whole (cadr match-data))
+	b e o os)
+    (while m
+      (setq b (car m)
+	    e (cadr m)
+	    m (cddr m)
+	    o (car owners)
+	    owners (cdr owners))
+      (unless (eq e b)
+	(push (smerge-put-highlight-overlay b e o)
+	      os)))
+    ;; highlight overlays are managed by keymap overlay.
+    ;; When keymap overlay is shrinked or removed, 
+    ;; highlight overlays are removed.
+    (smerge-put-keymap-overlay b-whole e-whole os)))
+    
+(defun smerge-put-highlight-overlay (start end owner)
+  "Put overlay of smerge-mode between START and END.
+The overlay is highlight when it is pressed.
+OWNER is stored to `owner' property of the new overlay."
+  (let ((overlay (make-overlay start end)))
+    (overlay-put overlay 'evaporate t)
+    (overlay-put overlay 'owner owner)
+    (push overlay smerge-overlays)
+    overlay))
+
+(defun smerge-put-keymap-overlay (start end suboverlays)
+  "Put overlay of smerge-mode between START and END.
+The overlay has its own keymap to show popup menu.
+SUBOVERLAYS are overlays managed by this overlay."
+  (let ((overlay (make-overlay start end)))
+    (overlay-put overlay 'evaporate t)
+    (overlay-put overlay 'help-echo "down-mouse-3: Show popup menu")
+    (overlay-put overlay 'local-map smerge-popup-menu-map)
+    (overlay-put overlay 'owner 'whole)
+    (overlay-put overlay 'suboverlays suboverlays)
+    (push overlay smerge-overlays)
+    overlay))
+
+(defun smerge-activate-popup-menu (event)
+  "Show a popup menu for smerge-mode."
+  (interactive "e")
+  (with-current-buffer (window-buffer 
+			(posn-window (event-end event)))
+    (save-excursion
+      (goto-char (posn-point (event-end event)))
+      (let ((overlays (overlays-at (point)))
+	    overlay 
+	    face menu)
+	(while overlays
+	  (let* ((o (car overlays))
+		 (owner (overlay-get o 'owner)))
+	    ;; Find mine or other. If such owner is found,
+	    ;; we can overwrite `overlay' local variable.
+	    (cond
+	     ((or (eq 'mine owner) (eq 'other owner))
+	      (setq overlay o
+		    face    'region
+		    menu     smerge-mode-popup-menu
+		    overlays nil))
+	     ;; Find whole. If such owner is found.
+	     ;; we can set `overlay' local variable 
+	     ;;if overlay is not set yet.
+	     ((and (eq 'whole owner) (not overlay))
+	      (setq overlay  o
+		    face    'highlight
+		    menu     smerge-mode-menu))
+	     (t 
+	      (setq overlays (cdr overlays))))))
+	(unwind-protect
+	    (progn
+	      (overlay-put overlay 'face face)
+	      (sit-for 0)		      ;; redisplay
+	      (popup-menu menu))
+	  (overlay-put overlay 'face nil))))))
+
+
+(defun smerge-delete-all-overlays ()
+  "Delete all overlays made by  `smerge-put-overlay'."
+  (mapc 'delete-overlay smerge-overlays)
+  (setq smerge-overlays nil))
+
 (defun smerge-match-conflict ()
   "Get info about the conflict.  Puts the info in the `match-data'.
 The submatches contain:
@@ -522,6 +665,12 @@
   "Minor mode to simplify editing output from the diff3 program.
 \\{smerge-mode-map}"
   nil " SMerge" nil
+  ;; overlays management
+  (if smerge-mode
+      ;; entering smerge-mode
+      (make-variable-buffer-local 'smerge-overlays)
+    ;; leaving smerge-mode
+    (smerge-delete-all-overlays)) 
   (when (and (boundp 'font-lock-mode) font-lock-mode)
     (set (make-local-variable 'font-lock-multiline) t)
     (save-excursion
@@ -531,8 +680,8 @@
       (goto-char (point-min))
       (while (smerge-find-conflict)
 	(save-excursion
-	  (font-lock-fontify-region (match-beginning 0) (match-end 0) nil))))))
-
+	  (font-lock-fontify-region (match-beginning 0) (match-end 0) nil)
+	  (smerge-put-overlays (match-data)))))))
 
 (provide 'smerge-mode)

Re: popup menu support for smerge-mode

[Masatake YAMATO]

[resending]
I've found a time to revise my patch about popup menu support 
for smerge-mode. Stefan, don't angry with me. I hope you do not forget
the old patch... A patch for popup menu for smerge-mode in my local 
machine is too useful to me to throw away:-)

> > mine and other uses special menus. I added "Keep alternative" to the
> > menus.  Other including whole uses the smerge menu appeared on the
> > menu bar.
> 
> Don't forget that conflicts can have a 3-parts shape where there's not just
> "mine" and "other" but also the ancestor, in which case there's no single
> "Keep alternative".

I've added "Revert to the BASE" to the popup menu.

> > +(easy-menu-define smerge-mode-mine-popup-menu nil
> > +  "Popup menu for mine area in `smerge-mode'."
> > +  '(nil
> > +    ["Keep This" smerge-keep-current :help "Use current (at point) version"]
> > +    ;; mine <-> other
> > +    ["Keep Alternative" smerge-keep-other :help "Use alternative version"] 
> > +    ["Keep All" smerge-keep-all :help "Keep all three versions"]
> > +    "---"
> > +    ["More..." (popup-menu smerge-mode-menu) :help "Show full SMerge mode menu"]
> > +    ))
> > +(easy-menu-define smerge-mode-other-popup-menu nil
> > +  "Popup menu for other area in `smerge-mode'."
> > +  '(nil
> > +    ["Keep This"   smerge-keep-current :help "Use current (at point) version"]
> > +    ;; other <-> mine
> > +    ["Keep Alternative" smerge-keep-mine :help "Use alternative version"]
> > +    ["Keep All"    smerge-keep-all :help "Keep all three versions"]
> > +    "---"
> > +    ["More..." (popup-menu smerge-mode-menu) :help "Show full SMerge mode menu"]
> > +    ))
> 
> I'd rather introduce a new function smerge-keep-alternative which
> will determine whether to use `other' or `mine' depending on `current'.
> This way there's only one menu rather than two with the same appearance
> but different behavior.

I wrote `smerge-keep-alternative' and unified the above two menus.

> > +      (when (and b e (not (= b e)))
> 
> I think this can be simplified to `unless (eq e b)' because `e' and `b'
> are either both locations or both nil.

I have used `unless'.

> > +	;; Delete overlays
> > +	(when (or 
> > +	       (not (overlay-buffer o-whole)) ;; dead
> > +	       (< (- (overlay-end o-whole) (overlay-start o-whole))
> > +		  region-whole))	      ;; shrinked up
> > +	  (mapc 'delete-overlay (cons o-whole os-sub)))))))
> 
> I agree it's easier to remove the overlays from smerge-activate-popup-menu,
> but that means they'll just never be removed if the user doesn't use
> this new feature.  I think it really needs to be done somewhere else
> instead (E.g. at the same place as the auto-leave code is run.  We'll
> probably need to introduce a new function `smerge-post-resolution-update'
> which will do the auto-leave check and will remove the overlays).

Removing overlays in smerge-post-resolution-update(or auto-leave) is not easy
because the some part of text area associated with the overlays are removed
during "resolution". Instead I put the code(smerge-delete-overlays-at) to 
delete overlays before "resolution".

Index: lisp/smerge-mode.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/smerge-mode.el,v
retrieving revision 1.24
diff -u -r1.24 smerge-mode.el
--- lisp/smerge-mode.el	6 Oct 2003 16:34:59 -0000	1.24
+++ lisp/smerge-mode.el	10 Mar 2004 04:27:49 -0000
@@ -159,6 +159,22 @@
      :help "Use Ediff to resolve the conflicts"]
     ))
 
+(defvar smerge-overlays nil "Overlays managed by smerge-mode")
+(easy-mmode-defmap smerge-popup-menu-map
+  `(([down-mouse-3] . smerge-activate-popup-menu))
+  "Keymap for popup menu appeared on conflicts area.")
+(easy-menu-define smerge-mode-popup-menu nil
+  "Popup menu for mine area in `smerge-mode'."
+  '(nil
+    ["Keep All" smerge-keep-all :help "Keep all three versions"]
+    ["Revert to the Base" smerge-keep-base :help "Revert to the base version"]
+    ["Keep This" smerge-keep-current :help "Use current (at point) version"]
+    ["Keep Alternative" smerge-keep-alternative :help "Use alternative version"] 
+    "---"
+    ["More..." (popup-menu smerge-mode-menu) :help "Show full SMerge mode menu"]
+    ))
+
+
 (defconst smerge-font-lock-keywords
   '((smerge-find-conflict
      (1 smerge-mine-face prepend t)
@@ -198,18 +214,35 @@
   (unless (match-end n)
     (error (format "No `%s'" (aref smerge-match-names n)))))
 
+(defun smerge-delete-overlays-at (pos)
+  "Delete overlays used in Smerge mode under POS."
+  (let  ((overlays (overlays-at pos))
+	 o
+	 suboverlays)
+    (while overlays
+      (setq o (car overlays)
+	    overlays (cdr overlays)
+	    suboverlays (overlay-get o 'suboverlays))
+      (when suboverlays
+	(setq suboverlays (cons o suboverlays)
+	      overlays nil)))
+    (mapc 'delete-overlay suboverlays)))
+
 (defun smerge-auto-leave ()
+  "If no conflict left, turn off Smerge mode.
+Return non-nil if the mode was indeed turned off."
   (when (and smerge-auto-leave
 	     (save-excursion (goto-char (point-min))
 			     (not (re-search-forward smerge-begin-re nil t))))
-    (smerge-mode -1)))
-
+    (smerge-mode -1)
+    t))
 
 (defun smerge-keep-all ()
   "Keep all three versions.
 Convenient for the kind of conflicts that can arise in ChangeLog files."
   (interactive)
   (smerge-match-conflict)
+  (smerge-delete-overlays-at (point))
   (replace-match (concat (or (match-string 1) "")
 			 (or (match-string 2) "")
 			 (or (match-string 3) ""))
@@ -261,6 +294,7 @@
 some major modes.  Uses `smerge-resolve-function' to do the actual work."
   (interactive)
   (smerge-match-conflict)
+  (smerge-delete-overlays-at (point))
   (funcall smerge-resolve-function)
   (smerge-auto-leave))
 
@@ -269,6 +303,7 @@
   (interactive)
   (smerge-match-conflict)
   (smerge-ensure-match 2)
+  (smerge-delete-overlays-at (point))
   (replace-match (match-string 2) t t)
   (smerge-auto-leave))
 
@@ -277,6 +312,7 @@
   (interactive)
   (smerge-match-conflict)
   ;;(smerge-ensure-match 3)
+  (smerge-delete-overlays-at (point))
   (replace-match (match-string 3) t t)
   (smerge-auto-leave))
 
@@ -285,6 +321,7 @@
   (interactive)
   (smerge-match-conflict)
   ;;(smerge-ensure-match 1)
+  (smerge-delete-overlays-at (point))
   (replace-match (match-string 1) t t)
   (smerge-auto-leave))
 
@@ -298,9 +335,28 @@
 	       (>= (point) (match-end i)))
       (decf i))
     (if (<= i 0) (error "Not inside a version")
+      (smerge-delete-overlays-at (point))
       (replace-match (match-string i) t t)
       (smerge-auto-leave))))
 
+(defun smerge-keep-alternative ()
+  "Use the alternatives (not under the cursor) version."
+  (interactive)
+  (smerge-match-conflict)
+  (let ((i 3))
+    (while (or (not (match-end i))
+	       (< (point) (match-beginning i))
+	       (>= (point) (match-end i)))
+      (decf i))
+    (cond
+     ((<= i 0) (error "Not inside a version"))
+     ((eq i 2) (error "No alternative for the base version"))
+     ((eq i 3) (setq i 1))
+     ((eq i 1) (setq i 3)))
+    (smerge-delete-overlays-at (point))
+    (replace-match (match-string i) t t) 
+    (smerge-auto-leave)))
+
 (defun smerge-diff-base-mine ()
   "Diff 'base' and 'mine' version in current conflict region."
   (interactive)
@@ -316,6 +372,93 @@
   (interactive)
   (smerge-diff 1 3))
 
+(defun smerge-put-overlays (match-data)
+  "Put overlays of smerge-mode on the place specified by MATCH-DATA."
+  (let ((m (cddr match-data))
+	(owners '(mine base other base-start other-start))
+	(b-whole (car match-data))
+	(e-whole (cadr match-data))
+	b e o os)
+    (while m
+      (setq b (car m)
+	    e (cadr m)
+	    m (cddr m)
+	    o (car owners)
+	    owners (cdr owners))
+      (unless (eq e b)
+	(push (smerge-put-highlight-overlay b e o)
+	      os)))
+    ;; highlight overlays are managed by keymap overlay.
+    ;; When keymap overlay is shrinked or removed, 
+    ;; highlight overlays are removed.
+    (smerge-put-keymap-overlay b-whole e-whole os)))
+    
+(defun smerge-put-highlight-overlay (start end owner)
+  "Put overlay of smerge-mode between START and END.
+The overlay is highlight when it is pressed.
+OWNER is stored to `owner' property of the new overlay."
+  (let ((overlay (make-overlay start end)))
+    (overlay-put overlay 'evaporate t)
+    (overlay-put overlay 'owner owner)
+    (push overlay smerge-overlays)
+    overlay))
+
+(defun smerge-put-keymap-overlay (start end suboverlays)
+  "Put overlay of smerge-mode between START and END.
+The overlay has its own keymap to show popup menu.
+SUBOVERLAYS are overlays managed by this overlay."
+  (let ((overlay (make-overlay start end)))
+    (overlay-put overlay 'evaporate t)
+    (overlay-put overlay 'help-echo "down-mouse-3: Show popup menu")
+    (overlay-put overlay 'local-map smerge-popup-menu-map)
+    (overlay-put overlay 'owner 'whole)
+    (overlay-put overlay 'suboverlays suboverlays)
+    (push overlay smerge-overlays)
+    overlay))
+
+(defun smerge-activate-popup-menu (event)
+  "Show a popup menu for smerge-mode."
+  (interactive "e")
+  (with-current-buffer (window-buffer 
+			(posn-window (event-end event)))
+    (save-excursion
+      (goto-char (posn-point (event-end event)))
+      (let ((overlays (overlays-at (point)))
+	    overlay 
+	    face menu)
+	(while overlays
+	  (let* ((o (car overlays))
+		 (owner (overlay-get o 'owner)))
+	    ;; Find mine or other. If such owner is found,
+	    ;; we can overwrite `overlay' local variable.
+	    (cond
+	     ((or (eq 'mine owner) (eq 'other owner))
+	      (setq overlay o
+		    face    'region
+		    menu     smerge-mode-popup-menu
+		    overlays nil))
+	     ;; Find whole. If such owner is found.
+	     ;; we can set `overlay' local variable 
+	     ;;if overlay is not set yet.
+	     ((and (eq 'whole owner) (not overlay))
+	      (setq overlay  o
+		    face    'highlight
+		    menu     smerge-mode-menu))
+	     (t 
+	      (setq overlays (cdr overlays))))))
+	(unwind-protect
+	    (progn
+	      (overlay-put overlay 'face face)
+	      (sit-for 0)		      ;; redisplay
+	      (popup-menu menu))
+	  (overlay-put overlay 'face nil))))))
+
+
+(defun smerge-delete-all-overlays ()
+  "Delete all overlays made by  `smerge-put-overlay'."
+  (mapc 'delete-overlay smerge-overlays)
+  (setq smerge-overlays nil))
+
 (defun smerge-match-conflict ()
   "Get info about the conflict.  Puts the info in the `match-data'.
 The submatches contain:
@@ -522,6 +665,12 @@
   "Minor mode to simplify editing output from the diff3 program.
 \\{smerge-mode-map}"
   nil " SMerge" nil
+  ;; overlays management
+  (if smerge-mode
+      ;; entering smerge-mode
+      (make-variable-buffer-local 'smerge-overlays)
+    ;; leaving smerge-mode
+    (smerge-delete-all-overlays)) 
   (when (and (boundp 'font-lock-mode) font-lock-mode)
     (set (make-local-variable 'font-lock-multiline) t)
     (save-excursion
@@ -531,8 +680,8 @@
       (goto-char (point-min))
       (while (smerge-find-conflict)
 	(save-excursion
-	  (font-lock-fontify-region (match-beginning 0) (match-end 0) nil))))))
-
+	  (font-lock-fontify-region (match-beginning 0) (match-end 0) nil)
+	  (smerge-put-overlays (match-data)))))))
 
 (provide 'smerge-mode)

Re: popup menu support for smerge-mode

[Masatake YAMATO]

Stefan,

http://mail.gnu.org/archive/html/emacs-devel/2003-09/msg00443.html
 
Could you review this patch I posted the last month?
It seems for me that my post is ignored.

Masatake YAMATO