From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Daniel Koning Newsgroups: gmane.emacs.bugs Subject: bug#6130: 23.1; artist-mode spray-can malfunction Date: Sat, 17 Jan 2015 23:47:29 -0600 Message-ID: References: <8dd014e7478827e94e3a8fe5b2b948e0@lysator.liu.se> <54BA6A0A.4080408@gmx.at> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: ger.gmane.org 1421560095 4269 80.91.229.3 (18 Jan 2015 05:48:15 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sun, 18 Jan 2015 05:48:15 +0000 (UTC) Cc: 6130@debbugs.gnu.org, busk To: martin rudalics Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Sun Jan 18 06:48:14 2015 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1YCiia-0001RX-N7 for geb-bug-gnu-emacs@m.gmane.org; Sun, 18 Jan 2015 06:48:12 +0100 Original-Received: from localhost ([::1]:60971 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YCiiZ-0005vz-Ub for geb-bug-gnu-emacs@m.gmane.org; Sun, 18 Jan 2015 00:48:11 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:59411) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YCiiW-0005vu-2n for bug-gnu-emacs@gnu.org; Sun, 18 Jan 2015 00:48:10 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YCiiR-0000e1-0Q for bug-gnu-emacs@gnu.org; Sun, 18 Jan 2015 00:48:08 -0500 Original-Received: from debbugs.gnu.org ([140.186.70.43]:51583) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YCiiQ-0000dQ-R9 for bug-gnu-emacs@gnu.org; Sun, 18 Jan 2015 00:48:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.80) (envelope-from ) id 1YCiiQ-0002ze-Cg for bug-gnu-emacs@gnu.org; Sun, 18 Jan 2015 00:48:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Daniel Koning Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 18 Jan 2015 05:48:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 6130 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 6130-submit@debbugs.gnu.org id=B6130.142156007111486 (code B ref 6130); Sun, 18 Jan 2015 05:48:02 +0000 Original-Received: (at 6130) by debbugs.gnu.org; 18 Jan 2015 05:47:51 +0000 Original-Received: from localhost ([127.0.0.1]:60444 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1YCiiD-0002zB-Sm for submit@debbugs.gnu.org; Sun, 18 Jan 2015 00:47:50 -0500 Original-Received: from sender1.zohomail.com ([74.201.84.155]:30630) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1YCiiA-0002yz-B8 for 6130@debbugs.gnu.org; Sun, 18 Jan 2015 00:47:48 -0500 Original-Received: from cornelius (99-181-53-37.lightspeed.rcsntx.sbcglobal.net [99.181.53.37]) by mx.zohomail.com with SMTPS id 1421560052737630.5577038187165; Sat, 17 Jan 2015 21:47:32 -0800 (PST) In-Reply-To: <54BA6A0A.4080408@gmx.at> (martin rudalics's message of "Sat, 17 Jan 2015 14:56:26 +0100") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.0.50 (darwin) X-ZohoMailClient: External X-Zoho-Virus-Status: 2 X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 140.186.70.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:98448 Archived-At: martin rudalics writes: >> Here's a patch that handles 1 through 3. > > I think your patch should go into Emacs 24.5. Have you signed a FSF > copyright form? If not, please do that as soon as possible. I've put in a request. I'll write back when I've received and returned the form. >> I also went ahead and searched the lisp/ tree for other places that >> looked risky -- that is, where a position object was assumed to hold a >> window in a context where there was no such guarantee. Nothing jumped >> out at me, but there could be any number of issues with third-party >> code. > > `posnp' also looks strange in this regard. > It does indeed, and I added a line to check for frames. > Please always use two spaces after the end of a sentence. > Please use either > (and (windowp frame-or-window) frame-or-window) > or > (when (windowp frame-or-window) frame-or-window) Done and done. > I'd expect it to be the selected frame but are we 100% sure? Could this > frame not still be the frame selected at the time mouse dragging started > while the selected frame has changed under our feet? Think of weird > things like `focus-follows-mouse' and `mouse-autoselect-window'. (This > remark might be silly but I was too lazy to test its validity right now.) Good eye. Upon testing it out, it does seem to be the case that the event's end position will always reflect the originally selected frame. I'll reword my commentary to reflect that. That said, I don't *think* it's ever possible in practice to change the selected frame in the middle of a drag event through user interaction alone -- either implicitly or by a keypress bound to `other-frame'. I'm basing this on having just tried it in several window systems on both OS X and BSD, including with focus-follows-mouse behavior enabled (and with the corresponding Emacs variable turned on). I was only able to test it by means of an event-reading loop which programmatically called `other-frame' after every down-mouse event. >> +Return the window that @var{position} is in. If the frame with input >> +focus does not have any window at @var{position}, return the frame >> +instead. > > Hmmm... here you use "frame with input focus". This looks better but > I'm still not entirely convinced. Come to think of it, this is wrong either way, because the implication of "the frame with input focus" is the frame with input focus at the time of evaluation, whereas the position list reflects the state at the time it was generated. How about: "If @var{position} represents a location outside the frame where the event was initiated, [...]" >> + (let* ((spacing (when (display-graphic-p frame) >> + (or (with-current-buffer (window-buffer window) > > Here `window' is the selected window. IMHO > > (frame-selected-window frame) > > sounds more accurate, given what I said about the selected frame above. I haven't changed these lines except for taking them out of a now-superfluous let*, but I agree that they look wrong. (Technically `window' is either the window from the position list or nil. If it is nil, though, `window-buffer' does return the buffer of the selected window. The calculation should be made w/r/t the selected window of the position list's frame, so I'll make that change.) >> diff --git a/lisp/textmodes/artist.el b/lisp/textmodes/artist.el > > I didn't look into these but just trust your experience here. This one looks like a major change because of all the diffed lines, but that's just due to adding an extra layer of indentation. The only semantic change is wrapping the `track-mouse' form in an `unwind-protect'. Here's the revised patch. diff --git a/doc/lispref/commands.texi b/doc/lispref/commands.texi index 36c7445..6fdc8e2 100644 --- a/doc/lispref/commands.texi +++ b/doc/lispref/commands.texi @@ -1489,8 +1489,10 @@ prefix @samp{drag-}. For example, dragging the mouse with button 2 held down generates a @code{drag-mouse-2} event. The second and third elements of the event give the starting and ending position of the drag, as mouse position lists (@pxref{Click Events}). You can access -the second element of any mouse event in the same way, with no need to -distinguish drag events from others. +the second element of any mouse event in the same way. However, the +drag event may end outside the boundaries of the frame that was +initially selected. In that case, the third element's position list +contains that frame in place of a window. The @samp{drag-} prefix follows the modifier key prefixes such as @samp{C-} and @samp{M-}. @@ -1635,7 +1637,10 @@ represented by lists that look like this: @noindent @var{position} is a mouse position list (@pxref{Click Events}), -specifying the current position of the mouse cursor. +specifying the current position of the mouse cursor. As with the +end-position of a drag event, this position list may represent a +location outside the boundaries of the initially selected frame, in +which case the list contains that frame in place of a window. The special form @code{track-mouse} enables generation of motion events within its body. Outside of @code{track-mouse} forms, Emacs @@ -1850,6 +1855,14 @@ into another window. That produces a pair of events like these: -453816)) @end smallexample +The frame with input focus might not take up the entire screen, and +the user might move the mouse outside the scope of the frame. Inside +the @code{track-mouse} special form, that produces an event like this: + +@smallexample +(mouse-movement (# nil (563 . 205) 532301936)) +@end smallexample + To handle a SIGUSR1 signal, define an interactive function, and bind it to the @code{signal usr1} event sequence: @@ -2014,7 +2027,9 @@ Events}); and @code{nil} otherwise. various parts of it: @defun posn-window position -Return the window that @var{position} is in. +Return the window that @var{position} is in. If @var{position} +represents a location outside the frame where the event was initiated, +return that frame instead. @end defun @defun posn-area position diff --git a/lisp/subr.el b/lisp/subr.el index 0534585..2eb293a 100644 --- a/lisp/subr.el +++ b/lisp/subr.el @@ -1083,7 +1083,7 @@ The return value is a positive integer." (defun posnp (obj) "Return non-nil if OBJ appears to be a valid `posn' object." - (and (windowp (car-safe obj)) + (and (or (windowp (car-safe obj)) (framep (car-safe obj))) (atom (car-safe (setq obj (cdr obj)))) ;AREA-OR-POS. (integerp (car-safe (car-safe (setq obj (cdr obj))))) ;XOFFSET. (integerp (car-safe (cdr obj))))) ;TIMESTAMP. @@ -1142,24 +1142,28 @@ For a scroll-bar event, the result column is 0, and the row corresponds to the vertical position of the click in the scroll bar. POSITION should be a list of the form returned by the `event-start' and `event-end' functions." - (let* ((pair (posn-x-y position)) - (window (posn-window position)) - (area (posn-area position))) + (let* ((pair (posn-x-y position)) + (frame-or-window (posn-window position)) + (frame (if (framep frame-or-window) + frame-or-window + (window-frame frame-or-window))) + (window (when (windowp frame-or-window) frame-or-window)) + (area (posn-area position))) (cond - ((null window) + ((null frame-or-window) '(0 . 0)) ((eq area 'vertical-scroll-bar) (cons 0 (scroll-bar-scale pair (1- (window-height window))))) ((eq area 'horizontal-scroll-bar) (cons (scroll-bar-scale pair (window-width window)) 0)) (t - (let* ((frame (if (framep window) window (window-frame window))) - ;; FIXME: This should take line-spacing properties on - ;; newlines into account. - (spacing (when (display-graphic-p frame) - (or (with-current-buffer (window-buffer window) - line-spacing) - (frame-parameter frame 'line-spacing))))) + ;; FIXME: This should take line-spacing properties on + ;; newlines into account. + (let* ((spacing (when (display-graphic-p frame) + (or (with-current-buffer + (window-buffer (frame-selected-window frame)) + line-spacing) + (frame-parameter frame 'line-spacing))))) (cond ((floatp spacing) (setq spacing (truncate (* spacing (frame-char-height frame))))) diff --git a/lisp/textmodes/artist.el b/lisp/textmodes/artist.el index 8a2383c..85d9410 100644 --- a/lisp/textmodes/artist.el +++ b/lisp/textmodes/artist.el @@ -4963,52 +4963,55 @@ The event, EV, is the mouse event." (artist-funcall init-fn x1 y1) (if (not artist-rubber-banding) (artist-no-rb-set-point1 x1 y1)) - (track-mouse - (while (or (mouse-movement-p ev) - (member 'down (event-modifiers ev))) - (setq ev-start-pos (artist-coord-win-to-buf - (posn-col-row (event-start ev)))) - (setq x1 (car ev-start-pos)) - (setq y1 (cdr ev-start-pos)) - - ;; Cancel previous timer - (if timer - (cancel-timer timer)) - - (if (not (eq initial-win (posn-window (event-start ev)))) - ;; If we moved outside the window, do nothing - nil - - ;; Still in same window: - ;; - ;; Check if user presses or releases shift key - (if (artist-shift-has-changed shift-state ev) - - ;; First check that the draw-how is the same as we - ;; already have. Otherwise, ignore the changed shift-state. - (if (not (eq draw-how - (artist-go-get-draw-how-from-symbol - (if (not shift-state) shifted unshifted)))) - (message "Cannot switch to shifted operation") - - ;; progn is "implicit" since this is the else-part - (setq shift-state (not shift-state)) - (setq op (if shift-state shifted unshifted)) - (setq draw-how (artist-go-get-draw-how-from-symbol op)) - (setq draw-fn (artist-go-get-draw-fn-from-symbol op)))) - - ;; Draw the new shape - (setq shape (artist-funcall draw-fn x1 y1)) - (artist-move-to-xy x1 y1) - - ;; Start the timer to call `draw-fn' repeatedly every - ;; `interval' second - (if (and interval draw-fn) - (setq timer (run-at-time interval interval draw-fn x1 y1)))) - - ;; Read next event - (setq ev (read-event)))) - + (unwind-protect + (track-mouse + (while (or (mouse-movement-p ev) + (member 'down (event-modifiers ev))) + (setq ev-start-pos (artist-coord-win-to-buf + (posn-col-row (event-start ev)))) + (setq x1 (car ev-start-pos)) + (setq y1 (cdr ev-start-pos)) + + ;; Cancel previous timer + (if timer + (cancel-timer timer)) + + (if (not (eq initial-win (posn-window (event-start ev)))) + ;; If we moved outside the window, do nothing + nil + + ;; Still in same window: + ;; + ;; Check if user presses or releases shift key + (if (artist-shift-has-changed shift-state ev) + + ;; First check that the draw-how is the same as we + ;; already have. Otherwise, ignore the changed shift-state. + (if (not (eq draw-how + (artist-go-get-draw-how-from-symbol + (if (not shift-state) shifted unshifted)))) + (message "Cannot switch to shifted operation") + + ;; progn is "implicit" since this is the else-part + (setq shift-state (not shift-state)) + (setq op (if shift-state shifted unshifted)) + (setq draw-how (artist-go-get-draw-how-from-symbol op)) + (setq draw-fn (artist-go-get-draw-fn-from-symbol op)))) + + ;; Draw the new shape + (setq shape (artist-funcall draw-fn x1 y1)) + (artist-move-to-xy x1 y1) + + ;; Start the timer to call `draw-fn' repeatedly every + ;; `interval' second + (if (and interval draw-fn) + (setq timer (run-at-time interval interval draw-fn x1 y1)))) + + ;; Read next event + (setq ev (read-event)))) + ;; Cleanup: get rid of any active timer. + (if timer + (cancel-timer timer))) ;; Cancel any timers (if timer (cancel-timer timer))