From mboxrd@z Thu Jan 1 00:00:00 1970 Path: main.gmane.org!not-for-mail From: "Stefan Monnier" Newsgroups: gmane.emacs.devel Subject: Re: popup menu support for smerge-mode Date: Sun, 24 Aug 2003 10:42:38 -0400 Sender: emacs-devel-bounces+emacs-devel=quimby.gnus.org@gnu.org Message-ID: <200308241442.h7OEgcxV020578@rum.cs.yale.edu> References: <20030110.211645.48817912.jet@gyve.org> <200301111941.h0BJfxm04057@rum.cs.yale.edu> <20030824.150334.55506789.jet@gyve.org> NNTP-Posting-Host: deer.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: sea.gmane.org 1061736217 1285 80.91.224.253 (24 Aug 2003 14:43:37 GMT) X-Complaints-To: usenet@sea.gmane.org NNTP-Posting-Date: Sun, 24 Aug 2003 14:43:37 +0000 (UTC) Cc: emacs-devel@gnu.org, monnier@cs.yale.edu, monnier+gnu/emacs@rum.cs.yale.edu Original-X-From: emacs-devel-bounces+emacs-devel=quimby.gnus.org@gnu.org Sun Aug 24 16:43:34 2003 Return-path: Original-Received: from quimby.gnus.org ([80.91.224.244]) by deer.gmane.org with esmtp (Exim 3.35 #1 (Debian)) id 19qw5e-0001T8-00 for ; Sun, 24 Aug 2003 16:43:34 +0200 Original-Received: from monty-python.gnu.org ([199.232.76.173]) by quimby.gnus.org with esmtp (Exim 3.12 #1 (Debian)) id 19qwAm-0005dh-00 for ; Sun, 24 Aug 2003 16:48:52 +0200 Original-Received: from localhost ([127.0.0.1] helo=monty-python.gnu.org) by monty-python.gnu.org with esmtp (Exim 4.20) id 19qw5Q-0003on-04 for emacs-devel@quimby.gnus.org; Sun, 24 Aug 2003 10:43:20 -0400 Original-Received: from list by monty-python.gnu.org with tmda-scanned (Exim 4.20) id 19qw5K-0003m1-ON for emacs-devel@gnu.org; Sun, 24 Aug 2003 10:43:14 -0400 Original-Received: from mail by monty-python.gnu.org with spam-scanned (Exim 4.20) id 19qw4o-0003S5-Df for emacs-devel@gnu.org; Sun, 24 Aug 2003 10:43:13 -0400 Original-Received: from [128.36.229.169] (helo=rum.cs.yale.edu) by monty-python.gnu.org with esmtp (Exim 4.20) id 19qw4o-0003RR-4k for emacs-devel@gnu.org; Sun, 24 Aug 2003 10:42:42 -0400 Original-Received: from rum.cs.yale.edu (localhost [127.0.0.1]) by rum.cs.yale.edu (8.12.8/8.12.8) with ESMTP id h7OEgfKc020580; Sun, 24 Aug 2003 10:42:41 -0400 Original-Received: (from monnier@localhost) by rum.cs.yale.edu (8.12.8/8.12.8/Submit) id h7OEgcxV020578; Sun, 24 Aug 2003 10:42:38 -0400 X-Mailer: exmh version 2.4 06/23/2000 with nmh-1.0.4 Original-To: Masatake YAMATO X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.2 Precedence: list List-Id: Emacs development discussions. List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+emacs-devel=quimby.gnus.org@gnu.org Xref: main.gmane.org gmane.emacs.devel:16122 X-Report-Spam: http://spam.gmane.org/gmane.emacs.devel:16122 > +(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