bug#35238: 27.0.50; Clarify eventp behaviour with booleans

bug#35238: 27.0.50; Clarify eventp behaviour with booleans

[Basil L. Contovounesios]

[-- Attachment #1: Type: text/plain, Size: 347 bytes --]

Severity: minor
Tags: patch

As mentioned in a past commit[1], nil is not an event.
Since the car of a mouse click event is considered its type,
shouldn't (nil) also be rejected as an event?

  (eventp '(nil)) ; => t

Should t be counted as an event?

  (eventp t)    ; => t
  (eventp '(t)) ; => t

If not, would the following change be welcome?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: eventp.diff --]
[-- Type: text/x-diff, Size: 817 bytes --]

diff --git a/lisp/subr.el b/lisp/subr.el
index bdf98979c4..8c4df5bc6c 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -1227,12 +1227,14 @@ listify-key-sequence
 			  c)))
 	    key)))
 
-(defun eventp (obj)
-  "True if the argument is an event object."
-  (when obj
-    (or (integerp obj)
-        (and (symbolp obj) obj (not (keywordp obj)))
-        (and (consp obj) (symbolp (car obj))))))
+(defun eventp (object)
+  "Return non-nil if OBJECT is an input event or event object."
+  (or (integerp object)
+      (and (symbolp (if (consp object)
+                        (setq object (car object))
+                      object))
+           (not (booleanp object))
+           (not (keywordp object)))))
 
 (defun event-modifiers (event)
   "Return a list of symbols representing the modifier keys in event EVENT.

[-- Attachment #3: Type: text/plain, Size: 532 bytes --]


[1: e18941095a]: * lisp/term/x-win.el (x-menu-bar-open): ...
  2012-08-10 10:47:12 -0400
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=e18941095a56075d6eb908a65aafcd1697fea2ae

Thanks,

-- 
Basil

In GNU Emacs 27.0.50 (build 9, x86_64-pc-linux-gnu, X toolkit, Xaw3d scroll bars)
 of 2019-04-11 built on thunk
Repository revision: 0627a8d7bc6ffa29d7a503fd36e760778ecb9fa1
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12003000
System Description: Debian GNU/Linux buster/sid

bug#35238: 27.0.50; Clarify eventp behaviour with booleans

[Stefan Monnier]

> As mentioned in a past commit[1], nil is not an event.
> Since the car of a mouse click event is considered its type,
> shouldn't (nil) also be rejected as an event?

Not sure it's worth the trouble (there are already lots of other objects
that aren't events but for which eventp returns non-nil).

> Should t be counted as an event?
>
>   (eventp t)    ; => t
>   (eventp '(t)) ; => t

Not sure if we ever generate such an event, but what would be the
benefit of rejecting it?


        Stefan

bug#35238: 27.0.50; Clarify eventp behaviour with booleans

[Basil L. Contovounesios]

[-- Attachment #1: Type: text/plain, Size: 436 bytes --]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> As mentioned in a past commit[1], nil is not an event.
>> Since the car of a mouse click event is considered its type,
>> shouldn't (nil) also be rejected as an event?
>
> Not sure it's worth the trouble (there are already lots of other objects
> that aren't events but for which eventp returns non-nil).

Sure, but the following would at least make eventp treat nil
consistently:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: eventp.diff --]
[-- Type: text/x-diff, Size: 781 bytes --]

diff --git a/lisp/subr.el b/lisp/subr.el
index bdf98979c4..407bd6379d 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -1227,12 +1227,13 @@ listify-key-sequence
 			  c)))
 	    key)))
 
-(defun eventp (obj)
-  "True if the argument is an event object."
-  (when obj
-    (or (integerp obj)
-        (and (symbolp obj) obj (not (keywordp obj)))
-        (and (consp obj) (symbolp (car obj))))))
+(defun eventp (object)
+  "Return non-nil if OBJECT is an input event or event object."
+  (or (integerp object)
+      (and (symbolp (if (consp object)
+                        (setq object (car object))
+                      object))
+           (not (keywordp object)))))
 
 (defun event-modifiers (event)
   "Return a list of symbols representing the modifier keys in event EVENT.

[-- Attachment #3: Type: text/plain, Size: 274 bytes --]


>> Should t be counted as an event?
>>
>>   (eventp t)    ; => t
>>   (eventp '(t)) ; => t
>
> Not sure if we ever generate such an event, but what would be the
> benefit of rejecting it?

I don't know, it could even be detrimental; I was just curious.

Thanks,

-- 
Basil

bug#35238: 27.0.50; Clarify eventp behaviour with booleans

[Stefan Monnier]

> Sure, but the following would at least make eventp treat nil
> consistently:

No objection on my side (nor to the previous patch, FWIW),


        Stefan

bug#35238: 27.0.50; Clarify eventp behaviour with booleans

[Eli Zaretskii]

> From: "Basil L. Contovounesios" <contovob@tcd.ie>
> Date: Fri, 12 Apr 2019 12:11:01 +0100
> Cc: 35238@debbugs.gnu.org
> 
> -(defun eventp (obj)
> -  "True if the argument is an event object."
> -  (when obj
> -    (or (integerp obj)
> -        (and (symbolp obj) obj (not (keywordp obj)))
> -        (and (consp obj) (symbolp (car obj))))))
> +(defun eventp (object)
> +  "Return non-nil if OBJECT is an input event or event object."
> +  (or (integerp object)
> +      (and (symbolp (if (consp object)
> +                        (setq object (car object))
> +                      object))
> +           (not (keywordp object)))))

Please make sure the ELisp manual is kept in sync.  Also, does this
need to be called out in NEWS?

bug#35238: 27.0.50; Clarify eventp behaviour with booleans

[Basil L. Contovounesios]

Eli Zaretskii <eliz@gnu.org> writes:

> Please make sure the ELisp manual is kept in sync.

While skimming the manual, I came across recent-keys in (info "(elisp)
Recording Input").  Stefan extended this function in Emacs 25.1[1] with
an optional argument INCLUDE-CMDS which causes it to output "events of
the form (nil . COMMAND)".

(The manual wasn't updated to document this alternative behaviour, but I
can do that in addition to / instead of the eventp change if one of ye
doesn't beat me to it.)

The question is, should eventp continue returning non-nil for these
events of the form (nil . COMMAND)?  Or should recent-keys be changed to
return a different form of event?  If my assumption is correct that the
former option is preferred, I will check that the manual documents such
events consistently instead of changing the behaviour of eventp.

Note that (seq-every-p #'eventp (recent-keys t)) is currently non-nil,
and the only caller that specifies INCLUDE-CMDS non-nil is view-lossage,
which doesn't rely on eventp.

[1: eca1ea9655]: * lisp/help.el (view-lossage): ...
  2014-11-09 21:58:52 -0500
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=eca1ea96559e04e18a62a61208d501c557dd4cab

>  Also, does this need to be called out in NEWS?

Depends on what actually ends up needing to be changed, I think.
My original intention to treat events of the form (nil ...) as nil now
seems less plausible, so the only changes I foresee are docfixes.

Thanks,

-- 
Basil

bug#35238: 27.0.50; Clarify eventp behaviour with booleans

[Stefan Monnier]

> The question is, should eventp continue returning non-nil for these
> events of the form (nil . COMMAND)?

I don't think we need to consider them as events.
More specifically, recent-keys uses this representation for the
execution of a command because it expects that no event would have such
a shape.


        Stefan

bug#35238: 27.0.50; Clarify eventp behaviour with booleans

[Basil L. Contovounesios]

[-- Attachment #1: Type: text/plain, Size: 398 bytes --]

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

>> The question is, should eventp continue returning non-nil for these
>> events of the form (nil . COMMAND)?
>
> I don't think we need to consider them as events.
> More specifically, recent-keys uses this representation for the
> execution of a command because it expects that no event would have such
> a shape.

OK, then how's the following?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Clarify-what-constitutes-an-event-bug-35238.patch --]
[-- Type: text/x-diff, Size: 4778 bytes --]

From 62f82c31406b9ea3ca59258bba9663184b8d5734 Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob@tcd.ie>
Date: Sun, 21 Apr 2019 23:02:01 +0100
Subject: [PATCH] Clarify what constitutes an event (bug#35238)

* doc/lispref/commands.texi (Input Events): Specify that events are
non-nil and remove vestiges of bug#10190.
* doc/lispref/os.texi (Recording Input): Document optional argument
of recent-keys.
* lisp/subr.el (eventp): Check that the car of conses is non-nil.
* etc/NEWS: Announce it as an incompatible change.
* src/keyboard.c (Frecent_keys): Clarify that returned "events" are
not real events.
---
 doc/lispref/commands.texi |  9 +++------
 doc/lispref/os.texi       |  7 ++++++-
 etc/NEWS                  |  5 +++++
 lisp/subr.el              | 14 ++++++++------
 src/keyboard.c            |  2 +-
 5 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/doc/lispref/commands.texi b/doc/lispref/commands.texi
index cd44c1c87e..5ea0be2667 100644
--- a/doc/lispref/commands.texi
+++ b/doc/lispref/commands.texi
@@ -1047,12 +1047,9 @@ Input Events
 This function returns non-@code{nil} if @var{object} is an input event
 or event type.
 
-Note that any symbol might be used as an event or an event type.
-@code{eventp} cannot distinguish whether a symbol is intended by Lisp
-code to be used as an event.  Instead, it distinguishes whether the
-symbol has actually been used in an event that has been read as input in
-the current Emacs session.  If a symbol has not yet been so used,
-@code{eventp} returns @code{nil}.
+Note that any non-@code{nil} symbol might be used as an event or an
+event type; @code{eventp} cannot distinguish whether a symbol is
+intended by Lisp code to be used as an event.
 @end defun
 
 @menu
diff --git a/doc/lispref/os.texi b/doc/lispref/os.texi
index 59cd5a8fe8..fef954eb7a 100644
--- a/doc/lispref/os.texi
+++ b/doc/lispref/os.texi
@@ -2197,7 +2197,7 @@ Recording Input
 @subsection Recording Input
 @cindex recording input
 
-@defun recent-keys
+@defun recent-keys &optional include-cmds
 This function returns a vector containing the last 300 input events from
 the keyboard or mouse.  All input events are included, whether or not
 they were used as parts of key sequences.  Thus, you always get the last
@@ -2205,6 +2205,11 @@ Recording Input
 (These are excluded because they are less interesting for debugging; it
 should be enough to see the events that invoked the macros.)
 
+If @var{include-cmds} is non-@code{nil}, complete key sequences in the
+result vector are interleaved with pseudo-events of the form
+@code{(nil . @var{COMMAND})}, where @var{COMMAND} is the binding of
+the key sequence (@pxref{Command Overview}).
+
 A call to @code{clear-this-command-keys} (@pxref{Command Loop Info})
 causes this function to return an empty vector immediately afterward.
 @end defun
diff --git a/etc/NEWS b/etc/NEWS
index b13ab47768..76943391bc 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1526,6 +1526,11 @@ performs (setq-local indent-line-function #'indent-relative).
 ** 'make-process' no longer accepts a non-nil ':stop' key.  This has
 never worked reliably, and now causes an error.
 
++++
+** 'eventp' no longer returns non-nil for lists whose car is nil.
+This is consistent with the fact that nil, though a symbol, is not a
+valid event type.
+
 \f
 * Lisp Changes in Emacs 27.1
 
diff --git a/lisp/subr.el b/lisp/subr.el
index f68f9dd419..be21dc67a0 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -1238,12 +1238,14 @@ listify-key-sequence
 			  c)))
 	    key)))
 
-(defun eventp (obj)
-  "True if the argument is an event object."
-  (when obj
-    (or (integerp obj)
-        (and (symbolp obj) obj (not (keywordp obj)))
-        (and (consp obj) (symbolp (car obj))))))
+(defun eventp (object)
+  "Return non-nil if OBJECT is an input event or event object."
+  (or (integerp object)
+      (and (if (consp object)
+               (setq object (car object))
+             object)
+           (symbolp object)
+           (not (keywordp object)))))
 
 (defun event-modifiers (event)
   "Return a list of symbols representing the modifier keys in event EVENT.
diff --git a/src/keyboard.c b/src/keyboard.c
index dff8f6b2fc..6d3030644a 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -9966,7 +9966,7 @@ If CHECK-TIMERS is non-nil, timers that are ready to run will do so.  */)
 DEFUN ("recent-keys", Frecent_keys, Srecent_keys, 0, 1, 0,
        doc: /* Return vector of last few events, not counting those from keyboard macros.
 If INCLUDE-CMDS is non-nil, include the commands that were run,
-represented as events of the form (nil . COMMAND).  */)
+represented as pseudo-events of the form (nil . COMMAND).  */)
   (Lisp_Object include_cmds)
 {
   bool cmds = !NILP (include_cmds);
-- 
2.20.1


[-- Attachment #3: Type: text/plain, Size: 20 bytes --]


Thanks,

-- 
Basil

bug#35238: 27.0.50; Clarify eventp behaviour with booleans

[Stefan Monnier]

> OK, then how's the following?

LGTM


        Stefan

bug#35238: 27.0.50; Clarify eventp behaviour with booleans

[Basil L. Contovounesios]

tags 35238 fixed
close 35238
quit

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> OK, then how's the following?
>
> LGTM

Thanks, I pushed to master[1] and am therefore closing this report.

[1: c972da907d]: Clarify what constitutes an event (bug#35238)
  2019-05-07 18:00:20 +0100
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=c972da907d494b6d5efd423aa3b5d0b23f7b7801

-- 
Basil