* bug#24133: 25.1.50; Some checkdoc.el functions use call-interactively incorrectly
@ 2016-08-02 7:08 Matthew Malcomson
2016-08-20 1:24 ` Robert Cochran
0 siblings, 1 reply; 8+ messages in thread
From: Matthew Malcomson @ 2016-08-02 7:08 UTC (permalink / raw)
To: 24133
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#24133: 25.1.50; Some checkdoc.el functions use call-interactively incorrectly
2016-08-02 7:08 bug#24133: 25.1.50; Some checkdoc.el functions use call-interactively incorrectly Matthew Malcomson
@ 2016-08-20 1:24 ` Robert Cochran
2016-08-26 16:14 ` Robert Cochran
2016-08-27 2:58 ` npostavs
0 siblings, 2 replies; 8+ messages in thread
From: Robert Cochran @ 2016-08-20 1:24 UTC (permalink / raw)
To: Matthew Malcomson; +Cc: 24133
[-- Attachment #1: Type: text/plain, Size: 1015 bytes --]
Matthew Malcomson <hardenedapple@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.
-----
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: checkdoc.el call-interactively fixup patch --]
[-- Type: text/x-patch, Size: 4080 bytes --]
From 77b83195d234efe1e8876733067548d7e9c17271 Mon Sep 17 00:00:00 2001
From: Robert Cochran <robert-git@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
[-- Attachment #3: Type: text/plain, Size: 102 bytes --]
-----
HTH,
--
~Robert Cochran
GPG Fingerprint - E778 2DD4 FEA6 6A68 6F26 AD2D E5C3 EB36 4886 8871
^ permalink raw reply related [flat|nested] 8+ messages in thread
* bug#24133: 25.1.50; Some checkdoc.el functions use call-interactively incorrectly
2016-08-20 1:24 ` Robert Cochran
@ 2016-08-26 16:14 ` Robert Cochran
2016-08-27 2:58 ` npostavs
1 sibling, 0 replies; 8+ messages in thread
From: Robert Cochran @ 2016-08-26 16:14 UTC (permalink / raw)
To: Robert Cochran; +Cc: Matthew Malcomson, 24133
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#24133: 25.1.50; Some checkdoc.el functions use call-interactively incorrectly
2016-08-20 1:24 ` Robert Cochran
2016-08-26 16:14 ` Robert Cochran
@ 2016-08-27 2:58 ` npostavs
2016-08-27 18:38 ` Robert Cochran
1 sibling, 1 reply; 8+ messages in thread
From: npostavs @ 2016-08-27 2:58 UTC (permalink / raw)
To: Robert Cochran; +Cc: Matthew Malcomson, 24133
Robert Cochran <robert-emacs@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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#24133: 25.1.50; Some checkdoc.el functions use call-interactively incorrectly
2016-08-27 2:58 ` npostavs
@ 2016-08-27 18:38 ` Robert Cochran
2016-09-02 2:55 ` npostavs
0 siblings, 1 reply; 8+ messages in thread
From: Robert Cochran @ 2016-08-27 18:38 UTC (permalink / raw)
To: npostavs; +Cc: Matthew Malcomson, 24133
[-- Attachment #1: Type: text/plain, Size: 200 bytes --]
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.
-----
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Patch to fix checkdoc's misuse of call-interactively --]
[-- Type: text/x-patch, Size: 4063 bytes --]
From 335663f3d9ba0de2f970f2c32626aa5acef17118 Mon Sep 17 00:00:00 2001
From: Robert Cochran <robert-git@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
[-- Attachment #3: Type: text/plain, Size: 104 bytes --]
-----
Thanks,
--
~Robert Cochran
GPG Fingerprint - E778 2DD4 FEA6 6A68 6F26 AD2D E5C3 EB36 4886 8871
^ permalink raw reply related [flat|nested] 8+ messages in thread
* bug#24133: 25.1.50; Some checkdoc.el functions use call-interactively incorrectly
2016-08-27 18:38 ` Robert Cochran
@ 2016-09-02 2:55 ` npostavs
2016-09-02 7:01 ` Eli Zaretskii
0 siblings, 1 reply; 8+ messages in thread
From: npostavs @ 2016-09-02 2:55 UTC (permalink / raw)
To: Robert Cochran; +Cc: Matthew Malcomson, 24133
Robert Cochran <robert-emacs@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?
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#24133: 25.1.50; Some checkdoc.el functions use call-interactively incorrectly
2016-09-02 2:55 ` npostavs
@ 2016-09-02 7:01 ` Eli Zaretskii
2016-09-03 16:04 ` npostavs
0 siblings, 1 reply; 8+ messages in thread
From: Eli Zaretskii @ 2016-09-02 7:01 UTC (permalink / raw)
To: npostavs; +Cc: hardenedapple, 24133
> From: npostavs@users.sourceforge.net
> Date: Thu, 01 Sep 2016 22:55:58 -0400
> Cc: Matthew Malcomson <hardenedapple@gmail.com>, 24133@debbugs.gnu.org
>
> Robert Cochran <robert-emacs@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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#24133: 25.1.50; Some checkdoc.el functions use call-interactively incorrectly
2016-09-02 7:01 ` Eli Zaretskii
@ 2016-09-03 16:04 ` npostavs
0 siblings, 0 replies; 8+ messages in thread
From: npostavs @ 2016-09-03 16:04 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: hardenedapple, 24133
tags 24133 fixed
close 24133 25.2
quit
Eli Zaretskii <eliz@gnu.org> writes:
>> From: npostavs@users.sourceforge.net
>> Date: Thu, 01 Sep 2016 22:55:58 -0400
>> Cc: Matthew Malcomson <hardenedapple@gmail.com>, 24133@debbugs.gnu.org
>>
>> Robert Cochran <robert-emacs@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).
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-09-03 16:04 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-02 7:08 bug#24133: 25.1.50; Some checkdoc.el functions use call-interactively incorrectly Matthew Malcomson
2016-08-20 1:24 ` Robert Cochran
2016-08-26 16:14 ` Robert Cochran
2016-08-27 2:58 ` npostavs
2016-08-27 18:38 ` Robert Cochran
2016-09-02 2:55 ` npostavs
2016-09-02 7:01 ` Eli Zaretskii
2016-09-03 16:04 ` npostavs
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).