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