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: 18 Sep 2003 11:27:58 -0400 Sender: emacs-devel-bounces+emacs-devel=quimby.gnus.org@gnu.org Message-ID: References: <200301111941.h0BJfxm04057@rum.cs.yale.edu> <20030824.150334.55506789.jet@gyve.org> <200308241442.h7OEgcxV020578@rum.cs.yale.edu> <20030918.175404.163031143.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 1063899674 446 80.91.224.253 (18 Sep 2003 15:41:14 GMT) X-Complaints-To: usenet@sea.gmane.org NNTP-Posting-Date: Thu, 18 Sep 2003 15:41:14 +0000 (UTC) Cc: monnier+gnu/emacs@rum.cs.yale.edu, monnier+gnu/emacs@cs.yale.edu, emacs-devel@gnu.org Original-X-From: emacs-devel-bounces+emacs-devel=quimby.gnus.org@gnu.org Thu Sep 18 17:41:12 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 1A00u8-0008AK-00 for ; Thu, 18 Sep 2003 17:41:12 +0200 Original-Received: from monty-python.gnu.org ([199.232.76.173]) by quimby.gnus.org with esmtp (Exim 3.35 #1 (Debian)) id 1A00yB-0007Pz-00 for ; Thu, 18 Sep 2003 17:45:23 +0200 Original-Received: from localhost ([127.0.0.1] helo=monty-python.gnu.org) by monty-python.gnu.org with esmtp (Exim 4.22) id 1A00rQ-00077U-IO for emacs-devel@quimby.gnus.org; Thu, 18 Sep 2003 11:38:24 -0400 Original-Received: from list by monty-python.gnu.org with tmda-scanned (Exim 4.22) id 1A00r1-00076t-VD for emacs-devel@gnu.org; Thu, 18 Sep 2003 11:37:59 -0400 Original-Received: from mail by monty-python.gnu.org with spam-scanned (Exim 4.22) id 1A00qz-00076F-3R for emacs-devel@gnu.org; Thu, 18 Sep 2003 11:37:58 -0400 Original-Received: from [132.204.24.67] (helo=mercure.iro.umontreal.ca) by monty-python.gnu.org with esmtp (Exim 4.22) id 1A00hk-0005kW-MY for emacs-devel@gnu.org; Thu, 18 Sep 2003 11:28:24 -0400 Original-Received: from vor.iro.umontreal.ca (vor.iro.umontreal.ca [132.204.24.42]) by mercure.iro.umontreal.ca (8.12.9/8.12.9) with ESMTP id h8IFRxHD000398; Thu, 18 Sep 2003 11:27:59 -0400 Original-Received: by vor.iro.umontreal.ca (Postfix, from userid 20848) id D00643C547; Thu, 18 Sep 2003 11:27:58 -0400 (EDT) Original-To: Masatake YAMATO In-Reply-To: <20030918.175404.163031143.jet@gyve.org> Original-Lines: 113 User-Agent: Gnus/5.09 (Gnus v5.9.0) Emacs/21.3.50 X-MailScanner-DIRO: Found to be clean 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:16465 X-Report-Spam: http://spam.gmane.org/gmane.emacs.devel:16465 >> 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