Re: emacs-28 354c834: Fix `browse-url-interactive-arg' for certain kinds of events

Re: emacs-28 354c834: Fix `browse-url-interactive-arg' for certain kinds of events

[Stefan Monnier]

>     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

Re: emacs-28 354c834: Fix `browse-url-interactive-arg' for certain kinds of events

[Lars Ingebrigtsen]

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

Re: emacs-28 354c834: Fix `browse-url-interactive-arg' for certain kinds of events

[Po Lu]

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.

Re: emacs-28 354c834: Fix `browse-url-interactive-arg' for certain kinds of events

[Stefan Monnier]

>>> 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.

Re: emacs-28 354c834: Fix `browse-url-interactive-arg' for certain kinds of events

[Lars Ingebrigtsen]

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

Re: emacs-28 354c834: Fix `browse-url-interactive-arg' for certain kinds of events

[Po Lu]

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.

Re: emacs-28 354c834: Fix `browse-url-interactive-arg' for certain kinds of events

[Stefan Monnier]

>> 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