Re: Additional cleanup around xterm-mouse

[Eli Zaretskii <eliz@gnu.org>]

> Date: Sat, 02 Jan 2021 14:20:06 -0800
> From: Jared Finder <jared@finder.org>
> Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> > LGTM.  I didn't review the documentation changes, since they are
> > incomplete and somewhat outdated; I will do that when you post the
> > final result.
> 
> Thanks!  I did a pass over the comments now.  Updated (hopefully final) 
> patch attached.

Thanks.

> For backward compatibility purposes, the above logic is contained in a
> new internal-only function: 'read-potential-mouse-event'.

Should 'read-potential-mouse-event' be named
'read--potential-mouse-event', to signal its being an internal
function?

> +(defconst read-key-full-map
> +  (let ((map (make-sparse-keymap)))
> +    (define-key map [t] 'dummy)

Can we have either a doc string or a comment explaining the need and
the use of this function?

> +If the optional argument DISABLE-FALLBACKS is non-nil, all
> +unbound fallbacks usually done by `read-key-sequence' are
> +disabled such as discarding mouse down events.  This is generally
> +what you want as `read-key' temporarily removes all bindings.

The last sentence here is confusing, and even alarming: what is meant
"removing all bindings" there?  This needs to be explained or
reworded.

> +Otherwise, only downcasing of the last event is disabled."

I think this also needs a clearer wording.  First, "Otherwise" is
ambiguous: do you mean if DISABLE-FALLBACKS is nil or omitted, or do
you mean something else?  And second, the "downcasing of the last
event" part was not described anywhere, so it is a mystery for the
reader.

> +         ;; FIXME: Audit existing usage of `read-key' to see if they
                                     ^^^^^
"uses", I think.  It will also better fit with "they".

> +;; FIXME: Once there's a safe way to transition away from read-event,
> +;; this function should be deleted.

When the function is deleted, we will also need to make other changes,
like replace its use with something else, right?  Then please say that
here.

>  DEFUN ("read-event", Fread_event, Sread_event, 0, 3, 0,
>         doc: /* Read an event object from the input stream.
> +
> +If you want to read non-character events consider calling `read-key'
                                           ^
A comma is missing there.

> +instead.  `read-key' will decode events via `input-decode-map' that
> +`read-event' will not.  On a terminal this includes function keys such
> +as <f7> and <right>, or mouse events generated by `xterm-mouse-mode'.

Please use upper case for function keys: <F7> and <RIGHT>.