GNU bug report logs - #24133
25.1.50; Some checkdoc.el functions use call-interactively incorrectly

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 24133 in the body.
You can then email your comments to 24133 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Matthew Malcomson <hardenedapple <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 25.1.50; Some checkdoc.el functions use call-interactively incorrectly
Date: Tue, 2 Aug 2016 08:08:24 +0100

There are a few checkdoc.el functions that pass `current-prefix-arg'
as the third argument (KEYS) to `call-interactively'.
When running any of these with a prefix argument (whether numeric or
a cons cell) gives an error about the argument to `call-interactively'
not being a vector.
e.g.

emacs -Q

C-u M-x checkdoc-ispell<RET>

checkdoc-ispell: Wrong type argument: vectorp, (4)


I see this with any emacs version I've tried.

Message #8 received at 24133 <at> debbugs.gnu.org (full text, mbox):

From: Robert Cochran <robert-emacs <at> cochranmail.com>
To: Matthew Malcomson <hardenedapple <at> gmail.com>
Cc: 24133 <at> debbugs.gnu.org
Subject: Re: bug#24133: 25.1.50;
 Some checkdoc.el functions use call-interactively incorrectly
Date: Fri, 19 Aug 2016 18:24:40 -0700

[Message part 1 (text/plain, inline)]

Matthew Malcomson <hardenedapple <at> gmail.com> writes:

> There are a few checkdoc.el functions that pass `current-prefix-arg'
> as the third argument (KEYS) to `call-interactively'.
> When running any of these with a prefix argument (whether numeric or
> a cons cell) gives an error about the argument to `call-interactively'
> not being a vector.
> e.g.
>
> emacs -Q
>
> C-u M-x checkdoc-ispell<RET>
>
> checkdoc-ispell: Wrong type argument: vectorp, (4)
>
>
> I see this with any emacs version I've tried.

As it turns out, whoever wrote that wasn't paying close enough
attention, as the third argument is supposed to be for events (to
satisfy commands that ask for "e", "k", "K", or "U").

It seems that `call-interactively' propagates the prefix argument to the
called function automatically (it did with a test command that printed
its arg in my git emacs -Q), so you should be able to remove the last 2
arguments to each call and it should Just Work(tm).

I have a patch, attached, that does just that.

-----

[0001-Fix-uses-of-call-interactively-in-lisp-emacs-lisp-ch.patch (text/x-patch, inline)]

From 77b83195d234efe1e8876733067548d7e9c17271 Mon Sep 17 00:00:00 2001
From: Robert Cochran <robert-git <at> cochranmail.com>
Date: Fri, 19 Aug 2016 18:03:24 -0700
Subject: [PATCH] Fix uses of (call-interactively) in
 lisp/emacs-lisp/checkdoc.el

* lisp/emacs-lisp/checkdoc.el (checkdoc-ispell)
  (checkdoc-ispell-current-buffer)
  (checkdoc-ispell-interactive)
  (checkdoc-ispell-message-text)
  (checkdoc-ispell-start)
  (checkdoc-ispell-continue)
  (checkdoc-ispell-comments)
  (checkdoc-ispell-defun):
  Do not pass 'current-prefix-arg' to 'call-interactively' as an event
  vector; merely allow it to propagate forward to the interactive call.

Passing the prefix argument as the 3rd argument to 'call-interactively'
causes the prefix argument to be interpreted as events, which is not
only wrong, but also causes a type error, as 'current-prefix-arg' can
never be a vector as 'call-interactively' expects. 'call-interactively'
automatically passes its prefix argument to the called function, so just
do that, eliminating faulty behavior.
---
 lisp/emacs-lisp/checkdoc.el | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/lisp/emacs-lisp/checkdoc.el b/lisp/emacs-lisp/checkdoc.el
index 3a81ade..769c2fe 100644
--- a/lisp/emacs-lisp/checkdoc.el
+++ b/lisp/emacs-lisp/checkdoc.el
@@ -1062,7 +1062,7 @@ checkdoc-ispell
 Prefix argument is the same as for `checkdoc'"
   (interactive)
   (let ((checkdoc-spellcheck-documentation-flag t))
-    (call-interactively #'checkdoc nil current-prefix-arg)))
+    (call-interactively #'checkdoc)))
 
 ;;;###autoload
 (defun checkdoc-ispell-current-buffer ()
@@ -1071,7 +1071,7 @@ checkdoc-ispell-current-buffer
 Prefix argument is the same as for `checkdoc-current-buffer'"
   (interactive)
   (let ((checkdoc-spellcheck-documentation-flag t))
-    (call-interactively #'checkdoc-current-buffer nil current-prefix-arg)))
+    (call-interactively #'checkdoc-current-buffer)))
 
 ;;;###autoload
 (defun checkdoc-ispell-interactive ()
@@ -1080,7 +1080,7 @@ checkdoc-ispell-interactive
 Prefix argument is the same as for `checkdoc-interactive'"
   (interactive)
   (let ((checkdoc-spellcheck-documentation-flag t))
-    (call-interactively #'checkdoc-interactive nil current-prefix-arg)))
+    (call-interactively #'checkdoc-interactive)))
 
 ;;;###autoload
 (defun checkdoc-ispell-message-interactive ()
@@ -1099,7 +1099,7 @@ checkdoc-ispell-message-text
 Prefix argument is the same as for `checkdoc-message-text'"
   (interactive)
   (let ((checkdoc-spellcheck-documentation-flag t))
-    (call-interactively #'checkdoc-message-text nil current-prefix-arg)))
+    (call-interactively #'checkdoc-message-text)))
 
 ;;;###autoload
 (defun checkdoc-ispell-start ()
@@ -1108,7 +1108,7 @@ checkdoc-ispell-start
 Prefix argument is the same as for `checkdoc-start'"
   (interactive)
   (let ((checkdoc-spellcheck-documentation-flag t))
-    (call-interactively #'checkdoc-start nil current-prefix-arg)))
+    (call-interactively #'checkdoc-start)))
 
 ;;;###autoload
 (defun checkdoc-ispell-continue ()
@@ -1117,7 +1117,7 @@ checkdoc-ispell-continue
 Prefix argument is the same as for `checkdoc-continue'"
   (interactive)
   (let ((checkdoc-spellcheck-documentation-flag t))
-    (call-interactively #'checkdoc-continue nil current-prefix-arg)))
+    (call-interactively #'checkdoc-continue)))
 
 ;;;###autoload
 (defun checkdoc-ispell-comments ()
@@ -1126,7 +1126,7 @@ checkdoc-ispell-comments
 Prefix argument is the same as for `checkdoc-comments'"
   (interactive)
   (let ((checkdoc-spellcheck-documentation-flag t))
-    (call-interactively #'checkdoc-comments nil current-prefix-arg)))
+    (call-interactively #'checkdoc-comments)))
 
 ;;;###autoload
 (defun checkdoc-ispell-defun ()
@@ -1135,7 +1135,7 @@ checkdoc-ispell-defun
 Prefix argument is the same as for `checkdoc-defun'"
   (interactive)
   (let ((checkdoc-spellcheck-documentation-flag t))
-    (call-interactively #'checkdoc-defun nil current-prefix-arg)))
+    (call-interactively #'checkdoc-defun)))
 
 ;;; Error Management
 ;;
-- 
2.7.4

[Message part 3 (text/plain, inline)]

-----

HTH,
-- 
~Robert Cochran

GPG Fingerprint - E778 2DD4 FEA6 6A68 6F26  AD2D E5C3 EB36 4886 8871

Message #11 received at 24133 <at> debbugs.gnu.org (full text, mbox):

From: Robert Cochran <robert-emacs <at> cochranmail.com>
To: Robert Cochran <robert-emacs <at> cochranmail.com>
Cc: Matthew Malcomson <hardenedapple <at> gmail.com>, 24133 <at> debbugs.gnu.org
Subject: Re: bug#24133: 25.1.50;
 Some checkdoc.el functions use call-interactively incorrectly
Date: Fri, 26 Aug 2016 09:14:48 -0700

Ping!

Has anyone had a chance to look this over yet?
-- 
~Robert Cochran

GPG Fingerprint - E778 2DD4 FEA6 6A68 6F26  AD2D E5C3 EB36 4886 8871

Message #14 received at 24133 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Robert Cochran <robert-emacs <at> cochranmail.com>
Cc: Matthew Malcomson <hardenedapple <at> gmail.com>, 24133 <at> debbugs.gnu.org
Subject: Re: bug#24133: 25.1.50;
 Some checkdoc.el functions use call-interactively incorrectly
Date: Fri, 26 Aug 2016 22:58:59 -0400

Robert Cochran <robert-emacs <at> cochranmail.com> writes:

> It seems that `call-interactively' propagates the prefix argument to the
> called function automatically (it did with a test command that printed
> its arg in my git emacs -Q), so you should be able to remove the last 2
> arguments to each call and it should Just Work(tm).
>
> I have a patch, attached, that does just that.

Looks good, except for some minor format problems in the commit message
(refer to etc/CONTRIBUTE "** Commit messages", and
https://www.gnu.org/prep/standards/html_node/Style-of-Change-Logs.html).

> Subject: [PATCH] Fix uses of (call-interactively) in
>  lisp/emacs-lisp/checkdoc.el
>
> * lisp/emacs-lisp/checkdoc.el (checkdoc-ispell)
>   (checkdoc-ispell-current-buffer)

The lines in a changelog entry should be not indented (i.e., they should
in the same column as the "*").

>   (checkdoc-ispell-interactive)
>   (checkdoc-ispell-message-text)
>   (checkdoc-ispell-start)
>   (checkdoc-ispell-continue)
>   (checkdoc-ispell-comments)
>   (checkdoc-ispell-defun):
>   Do not pass 'current-prefix-arg' to 'call-interactively' as an event
>   vector; merely allow it to propagate forward to the interactive call.
>

The explanatory part of the message should go before the changelog
entry, not after.

> Passing the prefix argument as the 3rd argument to 'call-interactively'
> causes the prefix argument to be interpreted as events, which is not
> only wrong, but also causes a type error, as 'current-prefix-arg' can
> never be a vector as 'call-interactively' expects. 'call-interactively'

Sentences should end with a double space (set
`sentence-end-double-space' to t).

> automatically passes its prefix argument to the called function, so just
> do that, eliminating faulty behavior.

Message #17 received at 24133 <at> debbugs.gnu.org (full text, mbox):

From: Robert Cochran <robert-emacs <at> cochranmail.com>
To: npostavs <at> users.sourceforge.net
Cc: Matthew Malcomson <hardenedapple <at> gmail.com>,
 Robert Cochran <robert-emacs <at> cochranmail.com>, 24133 <at> debbugs.gnu.org
Subject: Re: bug#24133: 25.1.50;
 Some checkdoc.el functions use call-interactively incorrectly
Date: Sat, 27 Aug 2016 11:38:06 -0700

[Message part 1 (text/plain, inline)]

I believe I've fixed all of the mentioned issues in this new
patch. Apologies for having not gotten it correct the first time; I'll
keep in mind to better review before sending in the future.

-----

[0001-Fix-uses-of-call-interactively-in-lisp-emacs-lisp-ch.patch (text/x-patch, inline)]

From 335663f3d9ba0de2f970f2c32626aa5acef17118 Mon Sep 17 00:00:00 2001
From: Robert Cochran <robert-git <at> cochranmail.com>
Date: Fri, 19 Aug 2016 18:03:24 -0700
Subject: [PATCH] Fix uses of (call-interactively) in
 lisp/emacs-lisp/checkdoc.el

Passing the prefix argument as the 3rd argument to 'call-interactively'
causes the prefix argument to be interpreted as events, which is not
only wrong, but also causes a type error, as 'current-prefix-arg' can
never be a vector as 'call-interactively' expects.  'call-interactively'
automatically passes its prefix argument to the called function, so just
do that, eliminating faulty behavior.

* lisp/emacs-lisp/checkdoc.el (checkdoc-ispell)
(checkdoc-ispell-current-buffer)
(checkdoc-ispell-interactive)
(checkdoc-ispell-message-text)
(checkdoc-ispell-start)
(checkdoc-ispell-continue)
(checkdoc-ispell-comments)
(checkdoc-ispell-defun):
Do not pass 'current-prefix-arg' to 'call-interactively' as an event
vector; merely allow it to propagate forward to the interactive call.
---
 lisp/emacs-lisp/checkdoc.el | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/lisp/emacs-lisp/checkdoc.el b/lisp/emacs-lisp/checkdoc.el
index 3a81ade..769c2fe 100644
--- a/lisp/emacs-lisp/checkdoc.el
+++ b/lisp/emacs-lisp/checkdoc.el
@@ -1062,7 +1062,7 @@ checkdoc-ispell
 Prefix argument is the same as for `checkdoc'"
   (interactive)
   (let ((checkdoc-spellcheck-documentation-flag t))
-    (call-interactively #'checkdoc nil current-prefix-arg)))
+    (call-interactively #'checkdoc)))
 
 ;;;###autoload
 (defun checkdoc-ispell-current-buffer ()
@@ -1071,7 +1071,7 @@ checkdoc-ispell-current-buffer
 Prefix argument is the same as for `checkdoc-current-buffer'"
   (interactive)
   (let ((checkdoc-spellcheck-documentation-flag t))
-    (call-interactively #'checkdoc-current-buffer nil current-prefix-arg)))
+    (call-interactively #'checkdoc-current-buffer)))
 
 ;;;###autoload
 (defun checkdoc-ispell-interactive ()
@@ -1080,7 +1080,7 @@ checkdoc-ispell-interactive
 Prefix argument is the same as for `checkdoc-interactive'"
   (interactive)
   (let ((checkdoc-spellcheck-documentation-flag t))
-    (call-interactively #'checkdoc-interactive nil current-prefix-arg)))
+    (call-interactively #'checkdoc-interactive)))
 
 ;;;###autoload
 (defun checkdoc-ispell-message-interactive ()
@@ -1099,7 +1099,7 @@ checkdoc-ispell-message-text
 Prefix argument is the same as for `checkdoc-message-text'"
   (interactive)
   (let ((checkdoc-spellcheck-documentation-flag t))
-    (call-interactively #'checkdoc-message-text nil current-prefix-arg)))
+    (call-interactively #'checkdoc-message-text)))
 
 ;;;###autoload
 (defun checkdoc-ispell-start ()
@@ -1108,7 +1108,7 @@ checkdoc-ispell-start
 Prefix argument is the same as for `checkdoc-start'"
   (interactive)
   (let ((checkdoc-spellcheck-documentation-flag t))
-    (call-interactively #'checkdoc-start nil current-prefix-arg)))
+    (call-interactively #'checkdoc-start)))
 
 ;;;###autoload
 (defun checkdoc-ispell-continue ()
@@ -1117,7 +1117,7 @@ checkdoc-ispell-continue
 Prefix argument is the same as for `checkdoc-continue'"
   (interactive)
   (let ((checkdoc-spellcheck-documentation-flag t))
-    (call-interactively #'checkdoc-continue nil current-prefix-arg)))
+    (call-interactively #'checkdoc-continue)))
 
 ;;;###autoload
 (defun checkdoc-ispell-comments ()
@@ -1126,7 +1126,7 @@ checkdoc-ispell-comments
 Prefix argument is the same as for `checkdoc-comments'"
   (interactive)
   (let ((checkdoc-spellcheck-documentation-flag t))
-    (call-interactively #'checkdoc-comments nil current-prefix-arg)))
+    (call-interactively #'checkdoc-comments)))
 
 ;;;###autoload
 (defun checkdoc-ispell-defun ()
@@ -1135,7 +1135,7 @@ checkdoc-ispell-defun
 Prefix argument is the same as for `checkdoc-defun'"
   (interactive)
   (let ((checkdoc-spellcheck-documentation-flag t))
-    (call-interactively #'checkdoc-defun nil current-prefix-arg)))
+    (call-interactively #'checkdoc-defun)))
 
 ;;; Error Management
 ;;
-- 
2.7.4

[Message part 3 (text/plain, inline)]

-----

Thanks,
--
~Robert Cochran

GPG Fingerprint - E778 2DD4 FEA6 6A68 6F26  AD2D E5C3 EB36 4886 8871

Message #20 received at 24133 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Robert Cochran <robert-emacs <at> cochranmail.com>
Cc: Matthew Malcomson <hardenedapple <at> gmail.com>, 24133 <at> debbugs.gnu.org
Subject: Re: bug#24133: 25.1.50;
 Some checkdoc.el functions use call-interactively incorrectly
Date: Thu, 01 Sep 2016 22:55:58 -0400

Robert Cochran <robert-emacs <at> cochranmail.com> writes:

> I believe I've fixed all of the mentioned issues in this new
> patch.

Yup, looks ready for master.  What's your copyright status?

Message #23 received at 24133 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: npostavs <at> users.sourceforge.net
Cc: hardenedapple <at> gmail.com, robert-emacs <at> cochranmail.com,
 24133 <at> debbugs.gnu.org
Subject: Re: bug#24133: 25.1.50;
 Some checkdoc.el functions use call-interactively incorrectly
Date: Fri, 02 Sep 2016 10:01:34 +0300

> From: npostavs <at> users.sourceforge.net
> Date: Thu, 01 Sep 2016 22:55:58 -0400
> Cc: Matthew Malcomson <hardenedapple <at> gmail.com>, 24133 <at> debbugs.gnu.org
> 
> Robert Cochran <robert-emacs <at> cochranmail.com> writes:
> 
> > I believe I've fixed all of the mentioned issues in this new
> > patch.
> 
> Yup, looks ready for master.  What's your copyright status?

Robert's assignment is on file.

Message #26 received at 24133 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: hardenedapple <at> gmail.com, robert-emacs <at> cochranmail.com,
 24133 <at> debbugs.gnu.org
Subject: Re: bug#24133: 25.1.50;
 Some checkdoc.el functions use call-interactively incorrectly
Date: Sat, 03 Sep 2016 12:04:43 -0400

tags 24133 fixed
close 24133 25.2
quit

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: npostavs <at> users.sourceforge.net
>> Date: Thu, 01 Sep 2016 22:55:58 -0400
>> Cc: Matthew Malcomson <hardenedapple <at> gmail.com>, 24133 <at> debbugs.gnu.org
>> 
>> Robert Cochran <robert-emacs <at> cochranmail.com> writes:
>> 
>> > I believe I've fixed all of the mentioned issues in this new
>> > patch.
>> 
>> Yup, looks ready for master.  What's your copyright status?
>
> Robert's assignment is on file.

I've pushed as ea015641 (I added colons to the commit message which I
hadn't noticed were missing before).

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.