all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* Re: emacs-28 354c834: Fix `browse-url-interactive-arg' for certain kinds of events
       [not found] ` <20211120075814.63AB3209FD@vcs0.savannah.gnu.org>
@ 2021-11-20 13:20   ` Stefan Monnier
  2021-11-21  8:29     ` Lars Ingebrigtsen
  0 siblings, 1 reply; 7+ messages in thread
From: Stefan Monnier @ 2021-11-20 13:20 UTC (permalink / raw)
  To: Po Lu; +Cc: emacs-devel

>     Fix `browse-url-interactive-arg' for certain kinds of events
>     
>     * lisp/net/browse-url.el (browse-url-interactive-arg): Don't
>     call `mouse-set-point' unless event is actually a mouse event.
> ---
>  lisp/net/browse-url.el | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lisp/net/browse-url.el b/lisp/net/browse-url.el
> index 3af37e4..50d11b4 100644
> --- a/lisp/net/browse-url.el
> +++ b/lisp/net/browse-url.el
> @@ -730,7 +730,8 @@ position clicked before acting.
>  This function returns a list (URL NEW-WINDOW-FLAG)
>  for use in `interactive'."
>    (let ((event (elt (this-command-keys) 0)))
> -    (and (listp event) (mouse-set-point event)))
> +    (when (mouse-event-p event)
> +      (mouse-set-point event)))
>    (list (read-string prompt (or (and transient-mark-mode mark-active
>  				     ;; rfc2396 Appendix E.
>  				     (replace-regexp-in-string

Shouldn't this be fixed in `mouse-set-point` (or even `event-end` and
`event-start`) instead, and then we should remove the `listp` test since
`mouse-set-point` nowadays should work with any kind of event.


        Stefan




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: emacs-28 354c834: Fix `browse-url-interactive-arg' for certain kinds of events
  2021-11-20 13:20   ` emacs-28 354c834: Fix `browse-url-interactive-arg' for certain kinds of events Stefan Monnier
@ 2021-11-21  8:29     ` Lars Ingebrigtsen
  2021-11-21  9:19       ` Po Lu
  0 siblings, 1 reply; 7+ messages in thread
From: Lars Ingebrigtsen @ 2021-11-21  8:29 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Po Lu, emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Shouldn't this be fixed in `mouse-set-point` (or even `event-end` and
> `event-start`) instead, and then we should remove the `listp` test since
> `mouse-set-point` nowadays should work with any kind of event.

I think it would be convenient if `mouse-set-point' did nothing if the
event given isn't a mouse event -- we have a bunch of commands that are
both mousey and non-mousey.

In addition, perhaps there should be a new interactive spec for "set the
mouse point if called from the mouse"?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: emacs-28 354c834: Fix `browse-url-interactive-arg' for certain kinds of events
  2021-11-21  8:29     ` Lars Ingebrigtsen
@ 2021-11-21  9:19       ` Po Lu
  2021-11-21 14:37         ` Stefan Monnier
  0 siblings, 1 reply; 7+ messages in thread
From: Po Lu @ 2021-11-21  9:19 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Stefan Monnier, emacs-devel

Lars Ingebrigtsen <larsi@gnus.org> writes:

>> Shouldn't this be fixed in `mouse-set-point` (or even `event-end` and
>> `event-start`) instead, and then we should remove the `listp` test since
>> `mouse-set-point` nowadays should work with any kind of event.

> I think it would be convenient if `mouse-set-point' did nothing if the
> event given isn't a mouse event -- we have a bunch of commands that are
> both mousey and non-mousey.

Either way, I think we should not change the behavior of
`mouse-set-point' in the release branch.

It is documented to only accept mouse click events.



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: emacs-28 354c834: Fix `browse-url-interactive-arg' for certain kinds of events
  2021-11-21  9:19       ` Po Lu
@ 2021-11-21 14:37         ` Stefan Monnier
  2021-11-21 17:04           ` Lars Ingebrigtsen
  2021-11-22  0:45           ` Po Lu
  0 siblings, 2 replies; 7+ messages in thread
From: Stefan Monnier @ 2021-11-21 14:37 UTC (permalink / raw)
  To: Po Lu; +Cc: Lars Ingebrigtsen, emacs-devel

>>> Shouldn't this be fixed in `mouse-set-point` (or even `event-end` and
>>> `event-start`) instead, and then we should remove the `listp` test since
>>> `mouse-set-point` nowadays should work with any kind of event.
>
>> I think it would be convenient if `mouse-set-point' did nothing if the
>> event given isn't a mouse event -- we have a bunch of commands that are
>> both mousey and non-mousey.

It's been that way already for quite a while.  Try to call it with an
event like `?a`.  This is handled in `event-end`.  It's just that this
special handling did not account for events like `(menu-bar)`.

> Either way, I think we should not change the behavior of
> `mouse-set-point' in the release branch.

I'd agree with that, I'm talking about what goes on `master`.

So, more concretely I propose the patch below for `master`.


        Stefan


diff --git a/lisp/net/browse-url.el b/lisp/net/browse-url.el
index f85f5f6149..c33517ea10 100644
--- a/lisp/net/browse-url.el
+++ b/lisp/net/browse-url.el
@@ -737,8 +737,7 @@ browse-url-interactive-arg
 This function returns a list (URL NEW-WINDOW-FLAG)
 for use in `interactive'."
   (let ((event (elt (this-command-keys) 0)))
-    (when (mouse-event-p event)
-      (mouse-set-point event)))
+    (mouse-set-point event))
   (list (read-string prompt (or (and transient-mark-mode mark-active
 				     ;; rfc2396 Appendix E.
 				     (replace-regexp-in-string
diff --git a/lisp/subr.el b/lisp/subr.el
index 7ba764880e..4e06aefd06 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -1553,22 +1553,22 @@ event-start
 `posn-timestamp': The time the event occurred, in milliseconds.
 
 For more information, see Info node `(elisp)Click Events'."
-  (if (consp event) (nth 1 event)
-    ;; Use `window-point' for the case when the current buffer
-    ;; is temporarily switched to some other buffer (bug#50256)
-    (or (posn-at-point (window-point))
-        (list (selected-window) (window-point) '(0 . 0) 0))))
+  (or (and (consp event) (nth 1 event))
+      ;; Use `window-point' for the case when the current buffer
+      ;; is temporarily switched to some other buffer (bug#50256)
+      (posn-at-point (window-point))
+      (list (selected-window) (window-point) '(0 . 0) 0)))
 
 (defun event-end (event)
   "Return the ending position of EVENT.
 EVENT should be a click, drag, or key press event.
 
 See `event-start' for a description of the value returned."
-  (if (consp event) (nth (if (consp (nth 2 event)) 2 1) event)
-    ;; Use `window-point' for the case when the current buffer
-    ;; is temporarily switched to some other buffer (bug#50256)
-    (or (posn-at-point (window-point))
-        (list (selected-window) (window-point) '(0 . 0) 0))))
+  (or (and (consp event) (nth (if (consp (nth 2 event)) 2 1) event))
+      ;; Use `window-point' for the case when the current buffer
+      ;; is temporarily switched to some other buffer (bug#50256)
+      (posn-at-point (window-point))
+      (list (selected-window) (window-point) '(0 . 0) 0)))
 
 (defsubst event-click-count (event)
   "Return the multi-click count of EVENT, a click or drag event.




^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: emacs-28 354c834: Fix `browse-url-interactive-arg' for certain kinds of events
  2021-11-21 14:37         ` Stefan Monnier
@ 2021-11-21 17:04           ` Lars Ingebrigtsen
  2021-11-22  0:45           ` Po Lu
  1 sibling, 0 replies; 7+ messages in thread
From: Lars Ingebrigtsen @ 2021-11-21 17:04 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Po Lu, emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> It's been that way already for quite a while.  Try to call it with an
> event like `?a`.  This is handled in `event-end`.  It's just that this
> special handling did not account for events like `(menu-bar)`.

Right; I thought I remembered this just working without any checks, so I
was surprised that we had to add checks now.  (But I assumed I just
misremembered.)

> So, more concretely I propose the patch below for `master`.

Looks good to me.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: emacs-28 354c834: Fix `browse-url-interactive-arg' for certain kinds of events
  2021-11-21 14:37         ` Stefan Monnier
  2021-11-21 17:04           ` Lars Ingebrigtsen
@ 2021-11-22  0:45           ` Po Lu
  2021-11-22 19:25             ` Stefan Monnier
  1 sibling, 1 reply; 7+ messages in thread
From: Po Lu @ 2021-11-22  0:45 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Lars Ingebrigtsen, emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> So, more concretely I propose the patch below for `master`.

I think it needs to be announced in NEWS, as it changes previously
documented behaviour.

Thanks.



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: emacs-28 354c834: Fix `browse-url-interactive-arg' for certain kinds of events
  2021-11-22  0:45           ` Po Lu
@ 2021-11-22 19:25             ` Stefan Monnier
  0 siblings, 0 replies; 7+ messages in thread
From: Stefan Monnier @ 2021-11-22 19:25 UTC (permalink / raw)
  To: Po Lu; +Cc: Lars Ingebrigtsen, emacs-devel

>> So, more concretely I propose the patch below for `master`.
> I think it needs to be announced in NEWS, as it changes previously
> documented behaviour.

The actual change in semantics was made in 2012, so I think it's a bit
late for that ;-)


        Stefan




^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-11-22 19:25 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20211120075812.24103.86181@vcs0.savannah.gnu.org>
     [not found] ` <20211120075814.63AB3209FD@vcs0.savannah.gnu.org>
2021-11-20 13:20   ` emacs-28 354c834: Fix `browse-url-interactive-arg' for certain kinds of events Stefan Monnier
2021-11-21  8:29     ` Lars Ingebrigtsen
2021-11-21  9:19       ` Po Lu
2021-11-21 14:37         ` Stefan Monnier
2021-11-21 17:04           ` Lars Ingebrigtsen
2021-11-22  0:45           ` Po Lu
2021-11-22 19:25             ` Stefan Monnier

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.