Re: feature/android a496509ced 1/2: Update Android port

Re: feature/android a496509ced 1/2: Update Android port

[Stefan Monnier]

Hi Po Lu,

Po Lu via Mailing list for Emacs changes [2023-01-19 09:20:42] wrote:
> branch: feature/android
> commit a496509cedb17109d0e6297a74e2ff8ed526333c
> Author: Po Lu <luangruo@yahoo.com>
> Commit: Po Lu <luangruo@yahoo.com>
>
>     Update Android port
[...]
>     * lisp/subr.el (event-start):
>     (event-end): Handle touchscreen events.
>     * lisp/touch-screen.el (touch-screen-handle-timeout):
>     (touch-screen-handle-point-update):
>     (touch-screen-handle-point-up):
>     (touch-screen-track-tap):
>     (touch-screen-track-drag):
>     (touch-screen-drag-mode-line-1):
>     (touch-screen-drag-mode-line): New functions.
>     ([mode-line touchscreen-begin]):
>     ([bottom-divider touchscreen-begin]): Bind new events.
>     
>     * lisp/wid-edit.el (widget-event-point):
>     (widget-keymap):
>     (widget-event-start):
>     (widget-button--check-and-call-button):
>     (widget-button-click): Improve touchscreen support.
[...]
> diff --git a/lisp/subr.el b/lisp/subr.el
> index f909b63aab..345816dbd2 100644
> --- a/lisp/subr.el
> +++ b/lisp/subr.el
> @@ -1636,7 +1636,13 @@ nil or (STRING . POSITION)'.
>  `posn-timestamp': The time the event occurred, in milliseconds.
>  
>  For more information, see Info node `(elisp)Click Events'."
> -  (or (and (consp event) (nth 1 event))
> +  (or (and (consp event)
> +           ;; Ignore touchscreen events.  They store the posn in a
> +           ;; different format, and can have multiple posns.
> +           (not (memq (car event) '(touchscreen-begin
> +                                    touchscreen-update
> +                                    touchscreen-end)))
> +           (nth 1 event))
>        (event--posn-at-point)))
>  
>  (defun event-end (event)
> @@ -1644,7 +1650,11 @@ For more information, see Info node `(elisp)Click Events'."
>  EVENT should be a click, drag, or key press event.
>  
>  See `event-start' for a description of the value returned."
> -  (or (and (consp event) (nth (if (consp (nth 2 event)) 2 1) event))
> +  (or (and (consp event)
> +           (not (memq (car event) '(touchscreen-begin
> +                                    touchscreen-update
> +                                    touchscreen-end)))
> +           (nth (if (consp (nth 2 event)) 2 1) event))
>        (event--posn-at-point)))
>  
>  (defsubst event-click-count (event)
[...]
> diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
> index 60bd2baa6f..4c52d82798 100644
> --- a/lisp/wid-edit.el
> +++ b/lisp/wid-edit.el
> @@ -65,8 +65,11 @@
>  ;;; Compatibility.
>  
>  (defun widget-event-point (event)
> -  "Character position of the end of event if that exists, or nil."
> -  (posn-point (event-end event)))
> +  "Character position of the end of event if that exists, or nil.
> +EVENT can either be a mouse event or a touch screen event."
> +  (if (eq (car-safe event) 'touchscreen-begin)
> +      (posn-point (cdadr event))
> +    (posn-point (event-end event))))
>  
>  (defun widget-button-release-event-p (event)
>    "Non-nil if EVENT is a mouse-button-release event object."
[...]
> @@ -1072,8 +1076,18 @@ Note that such modes will need to require wid-edit.")
>    "If non-nil, `widget-button-click' moves point to a button after invoking it.
>  If nil, point returns to its original position after invoking a button.")
>  
> +(defun widget-event-start (event)
> +  "Return the start of EVENT.
> +If EVENT is not a touchscreen event, simply return its
> +`event-start'.  Otherwise, it is a touchscreen event, so return
> +the posn of its touchpoint."
> +  (if (eq (car event) 'touchscreen-begin)
> +      (cdadr event)
> +    (event-start event)))

Could you explain why for touchscreen events you made `event-end` and
`event-start` return "posn-at-point" (which seems positively useless),
forcing you in turn to add ad-hoc handling in `widget-event-point` and
to introduce a new `widget-event-start`?

I understand that touchscreen events may have multiple posns, but it
seems that making `event-end` and `event-start` do what
`widget-event-point` and `widget-event-start` do would be better than
what we have now, no?


        Stefan

Re: feature/android a496509ced 1/2: Update Android port

[Po Lu]

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

> Could you explain why for touchscreen events you made `event-end` and
> `event-start` return "posn-at-point" (which seems positively useless),
> forcing you in turn to add ad-hoc handling in `widget-event-point` and
> to introduce a new `widget-event-start`?

Actually, the cdadr of a touchscreen-begin or touchscreen-end event is
its mouse position list.

> I understand that touchscreen events may have multiple posns, but it
> seems that making `event-end` and `event-start` do what
> `widget-event-point` and `widget-event-start` do would be better than
> what we have now, no?

I recall moving the code for processing touch screen events from
widget.el to subr.el.  I probably neglected to remove the old code from
widget.el in the process.

Thanks.

Re: feature/android a496509ced 1/2: Update Android port

[Stefan Monnier]

Po Lu [2023-08-16 20:21:33] wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> Could you explain why for touchscreen events you made `event-end` and
>> `event-start` return "posn-at-point" (which seems positively useless),
>> forcing you in turn to add ad-hoc handling in `widget-event-point` and
>> to introduce a new `widget-event-start`?
>
> Actually, the cdadr of a touchscreen-begin or touchscreen-end event is
> its mouse position list.

Does that mean that `widget-event-start` doesn't always return a `posn`?
If so, could you add some comments explaining what's going on and why
it's done this way?

>> I understand that touchscreen events may have multiple posns, but it
>> seems that making `event-end` and `event-start` do what
>> `widget-event-point` and `widget-event-start` do would be better than
>> what we have now, no?
> I recall moving the code for processing touch screen events from
> widget.el to subr.el.

AFAICT the code for `event-start/end` in `master` has not been changed
since the above commit.


        Stefan

Re: feature/android a496509ced 1/2: Update Android port

[Po Lu]

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

> Does that mean that `widget-event-start` doesn't always return a `posn`?
> If so, could you add some comments explaining what's going on and why
> it's done this way?

Oh no, I meant that touchscreen-begin events actually appear like so:

  (touchscreen-begin (NUM . EVENT-START))

where the cdadr, EVENT-START, is the mouse position list, and NUM is the
touch identification number.

Re: feature/android a496509ced 1/2: Update Android port

[Stefan Monnier]

>> Does that mean that `widget-event-start` doesn't always return a `posn`?
>> If so, could you add some comments explaining what's going on and why
>> it's done this way?
>
> Oh no, I meant that touchscreen-begin events actually appear like so:
>
>   (touchscreen-begin (NUM . EVENT-START))
>
> where the cdadr, EVENT-START, is the mouse position list, and NUM is the
> touch identification number.

I don't really understand how this answers my question.
Is "the mouse position list" an "posn"?
If yes, then why doesn't `event-start/end` return it?


        Stefan

Re: feature/android a496509ced 1/2: Update Android port

[Po Lu]

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

> I don't really understand how this answers my question.
> Is "the mouse position list" an "posn"?

Yes.  We refer to "posns" as mouse position lists.

> If yes, then why doesn't `event-start/end` return it?

It does, so I've removed the redundant code from widget.el.

Re: feature/android a496509ced 1/2: Update Android port

[Stefan Monnier]

>> I don't really understand how this answers my question.
>> Is "the mouse position list" an "posn"?
> Yes.  We refer to "posns" as mouse position lists.

Ah, great, thanks.

>> If yes, then why doesn't `event-start/end` return it?
> It does, so I've removed the redundant code from widget.el.

Where?

The code I see on `master` is:

    (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."
      (declare (side-effect-free t))
      (or (and (consp event)
               (not (memq (car event) '(touchscreen-begin
                                        touchscreen-update
                                        touchscreen-end)))
               (nth (if (consp (nth 2 event)) 2 1) event))
          (event--posn-at-point)))

so for `touchscreen-begin` we return (event--posn-at-point).
What am I missing?


        Stefan

Re: feature/android a496509ced 1/2: Update Android port

[Po Lu]

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

>>> I don't really understand how this answers my question.
>>> Is "the mouse position list" an "posn"?
>> Yes.  We refer to "posns" as mouse position lists.
>
> Ah, great, thanks.
>
>>> If yes, then why doesn't `event-start/end` return it?
>> It does, so I've removed the redundant code from widget.el.
>
> Where?
>
> The code I see on `master` is:
>
>     (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."
>       (declare (side-effect-free t))
>       (or (and (consp event)
>                (not (memq (car event) '(touchscreen-begin
>                                         touchscreen-update
>                                         touchscreen-end)))
>                (nth (if (consp (nth 2 event)) 2 1) event))
>           (event--posn-at-point)))
>
> so for `touchscreen-begin` we return (event--posn-at-point).
> What am I missing?
>
>
>         Stefan

That's simply an omission on my fault, which I apologize for.  I'll fix
it ASAP.

Re: feature/android a496509ced 1/2: Update Android port

[Stefan Monnier]

> That's simply an omission on my fault,

Ah, good.

> which I apologize for.

No harm.

> I'll fix it ASAP.

Thanks,


        Stefan