* bug#59710: Wrong type argument when editing a multisession variable
@ 2022-11-30 1:49 Juanma Barranquero
2022-12-02 13:07 ` Eli Zaretskii
0 siblings, 1 reply; 7+ messages in thread
From: Juanma Barranquero @ 2022-11-30 1:49 UTC (permalink / raw)
To: 59710; +Cc: lars ingebrigtsen
[-- Attachment #1: Type: text/plain, Size: 2704 bytes --]
Package: emacs
Version: 29.0.60
Severity: normal
X-Debbugs-Cc: "Lars Ingebrigtsen" <larsi@gnus.org>
;;; init.el
(require 'multisession)
(define-multisession-variable test-var nil "Test var" :package "test")
(setf (multisession-value test-var) t)
;;; end of init.el
emacs
M-x list-multisession-values <RET>
e
Debugger entered--Lisp error: (wrong-type-argument symbolp (intern (cdr
id)))
multisession-edit-value(("test" . "test-var"))
funcall-interactively(multisession-edit-value ("test" . "test-var"))
command-execute(multisession-edit-value)
The problem comes from this change
commit bd586121ac21e046f60f75eeb0200866c38d6f9f
Author: Lars Ingebrigtsen <larsi@gnus.org>
Date: 2022-01-22 11:56:13 +0100
Make the test for existing multisession variables more sensible
* lisp/emacs-lisp/multisession.el (multisession-edit-value):
Unconfuse the code.
diff --git a/lisp/emacs-lisp/multisession.el
b/lisp/emacs-lisp/multisession.el
index 4a293796a8..25307594c6 100644
--- a/lisp/emacs-lisp/multisession.el
+++ b/lisp/emacs-lisp/multisession.el
@@ -437,8 +437,8 @@ multisession-edit-value
(let* ((object (or
;; If the multisession variable already exists, use
;; it (so that we update it).
- (and (boundp (intern-soft (cdr id)))
- (symbol-value (intern (cdr id))))
+ (and (intern-soft (cdr id))
+ (bound-and-true-p (intern (cdr id))))
;; Create a new object.
(make-multisession
:package (car id)
because `bound-and-true-p' is a macro that requires as argument a symbol,
which (intern ...) is not.
ELISP> (bound-and-true-p (intern "whatever"))
*** Eval error *** Wrong type argument: symbolp, (intern "whatever")
so I'm afraid this change was never tested.
The fix is reverting the change, doing perhaps this
diff --git a/lisp/emacs-lisp/multisession.el
b/lisp/emacs-lisp/multisession.el
index 9d6e8c0d88..78d4137317 100644
--- a/lisp/emacs-lisp/multisession.el
+++ b/lisp/emacs-lisp/multisession.el
@@ -447,8 +447,9 @@ multisession-edit-value
(let* ((object (or
;; If the multisession variable already exists, use
;; it (so that we update it).
- (and (intern-soft (cdr id))
- (bound-and-true-p (intern (cdr id))))
+ (if-let (sym (intern-soft (cdr id)))
+ (and (boundp sym) (symbol-value sym))
+ nil)
;; Create a new object.
(make-multisession
:package (car id)
[-- Attachment #2: Type: text/html, Size: 4644 bytes --]
^ permalink raw reply related [flat|nested] 7+ messages in thread
* bug#59710: Wrong type argument when editing a multisession variable
2022-11-30 1:49 bug#59710: Wrong type argument when editing a multisession variable Juanma Barranquero
@ 2022-12-02 13:07 ` Eli Zaretskii
2022-12-02 13:30 ` Juanma Barranquero
0 siblings, 1 reply; 7+ messages in thread
From: Eli Zaretskii @ 2022-12-02 13:07 UTC (permalink / raw)
To: Juanma Barranquero; +Cc: larsi, 59710
> Cc: "lars ingebrigtsen" <larsi@gnus.org>
> From: Juanma Barranquero <lekktu@gmail.com>
> Date: Wed, 30 Nov 2022 02:49:15 +0100
>
> Debugger entered--Lisp error: (wrong-type-argument symbolp (intern (cdr id)))
> multisession-edit-value(("test" . "test-var"))
> funcall-interactively(multisession-edit-value ("test" . "test-var"))
> command-execute(multisession-edit-value)
>
> The problem comes from this change
>
> commit bd586121ac21e046f60f75eeb0200866c38d6f9f
> Author: Lars Ingebrigtsen <larsi@gnus.org>
> Date: 2022-01-22 11:56:13 +0100
>
> Make the test for existing multisession variables more sensible
>
> * lisp/emacs-lisp/multisession.el (multisession-edit-value):
> Unconfuse the code.
>
> diff --git a/lisp/emacs-lisp/multisession.el b/lisp/emacs-lisp/multisession.el
> index 4a293796a8..25307594c6 100644
> --- a/lisp/emacs-lisp/multisession.el
> +++ b/lisp/emacs-lisp/multisession.el
> @@ -437,8 +437,8 @@ multisession-edit-value
> (let* ((object (or
> ;; If the multisession variable already exists, use
> ;; it (so that we update it).
> - (and (boundp (intern-soft (cdr id)))
> - (symbol-value (intern (cdr id))))
> + (and (intern-soft (cdr id))
> + (bound-and-true-p (intern (cdr id))))
> ;; Create a new object.
> (make-multisession
> :package (car id)
>
> because `bound-and-true-p' is a macro that requires as argument a symbol, which (intern ...) is not.
>
> ELISP> (bound-and-true-p (intern "whatever"))
> *** Eval error *** Wrong type argument: symbolp, (intern "whatever")
>
> so I'm afraid this change was never tested.
>
> The fix is reverting the change, doing perhaps this
>
> diff --git a/lisp/emacs-lisp/multisession.el b/lisp/emacs-lisp/multisession.el
> index 9d6e8c0d88..78d4137317 100644
> --- a/lisp/emacs-lisp/multisession.el
> +++ b/lisp/emacs-lisp/multisession.el
> @@ -447,8 +447,9 @@ multisession-edit-value
> (let* ((object (or
> ;; If the multisession variable already exists, use
> ;; it (so that we update it).
> - (and (intern-soft (cdr id))
> - (bound-and-true-p (intern (cdr id))))
> + (if-let (sym (intern-soft (cdr id)))
> + (and (boundp sym) (symbol-value sym))
> + nil)
> ;; Create a new object.
> (make-multisession
> :package (car id)
This makes sense to me, so please go ahead and install (assuming that the
multisession tests still all pass after this change).
Thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#59710: Wrong type argument when editing a multisession variable
2022-12-02 13:07 ` Eli Zaretskii
@ 2022-12-02 13:30 ` Juanma Barranquero
2022-12-02 15:06 ` Eli Zaretskii
0 siblings, 1 reply; 7+ messages in thread
From: Juanma Barranquero @ 2022-12-02 13:30 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: larsi, 59710-done
[-- Attachment #1: Type: text/plain, Size: 766 bytes --]
On Fri, Dec 2, 2022 at 2:07 PM Eli Zaretskii <eliz@gnu.org> wrote:
> This makes sense to me, so please go ahead and install (assuming that the
> multisession tests still all pass after this change).
Yes, they pass. Installed in commit e5b0141b0d of 2022-12-02.
BTW, I wonder if it would make sense to make bound-and-true-p to check that
it gets a symbol:
diff --git i/lisp/bindings.el w/lisp/bindings.el
index c1ad5f7520..6ee730af58 100644
--- i/lisp/bindings.el
+++ w/lisp/bindings.el
@@ -671,4 +671,6 @@ bound-and-true-p
Note that if `lexical-binding' is in effect, this function isn't
meaningful if it refers to a lexically bound variable."
+ (unless (symbolp var)
+ (error "Wrong type argument: symbolp, %S" var))
`(and (boundp (quote ,var)) ,var))
[-- Attachment #2: Type: text/html, Size: 1246 bytes --]
^ permalink raw reply related [flat|nested] 7+ messages in thread
* bug#59710: Wrong type argument when editing a multisession variable
2022-12-02 13:30 ` Juanma Barranquero
@ 2022-12-02 15:06 ` Eli Zaretskii
2022-12-02 15:40 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
0 siblings, 1 reply; 7+ messages in thread
From: Eli Zaretskii @ 2022-12-02 15:06 UTC (permalink / raw)
To: Juanma Barranquero, Stefan Monnier, Lars Ingebrigtsen; +Cc: 59710
> From: Juanma Barranquero <lekktu@gmail.com>
> Date: Fri, 2 Dec 2022 14:30:52 +0100
> Cc: larsi@gnus.org, 59710-done@debbugs.gnu.org
>
> BTW, I wonder if it would make sense to make bound-and-true-p to check that it gets a symbol:
>
> diff --git i/lisp/bindings.el w/lisp/bindings.el
> index c1ad5f7520..6ee730af58 100644
> --- i/lisp/bindings.el
> +++ w/lisp/bindings.el
> @@ -671,4 +671,6 @@ bound-and-true-p
> Note that if `lexical-binding' is in effect, this function isn't
> meaningful if it refers to a lexically bound variable."
> + (unless (symbolp var)
> + (error "Wrong type argument: symbolp, %S" var))
> `(and (boundp (quote ,var)) ,var))
I have no opinion on that. Lars, Stefan: WDYT?
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#59710: Wrong type argument when editing a multisession variable
2022-12-02 15:06 ` Eli Zaretskii
@ 2022-12-02 15:40 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-12-03 0:25 ` Juanma Barranquero
0 siblings, 1 reply; 7+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-12-02 15:40 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: Juanma Barranquero, Lars Ingebrigtsen, 59710
>> @@ -671,4 +671,6 @@ bound-and-true-p
>> Note that if `lexical-binding' is in effect, this function isn't
>> meaningful if it refers to a lexically bound variable."
>> + (unless (symbolp var)
>> + (error "Wrong type argument: symbolp, %S" var))
>> `(and (boundp (quote ,var)) ,var))
>
> I have no opinion on that. Lars, Stefan: WDYT?
I wish Someone™ implemented something like
[Fortifying macros](https://dl.acm.org/doi/10.1145/1863543.1863577)
to solve this in a more general way :-)
I have no objections against such a change.
Stefan
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#59710: Wrong type argument when editing a multisession variable
2022-12-02 15:40 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2022-12-03 0:25 ` Juanma Barranquero
2022-12-03 0:28 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
0 siblings, 1 reply; 7+ messages in thread
From: Juanma Barranquero @ 2022-12-03 0:25 UTC (permalink / raw)
To: Stefan Monnier; +Cc: Eli Zaretskii, Lars Ingebrigtsen, 59710
[-- Attachment #1: Type: text/plain, Size: 177 bytes --]
In fact, I think it would be marginally better
(unless (symbolp var)
(signal 'wrong-type-argument (list 'symbolp var)))
Assuming ok to install... where? emacs-29 or master?
[-- Attachment #2: Type: text/html, Size: 433 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#59710: Wrong type argument when editing a multisession variable
2022-12-03 0:25 ` Juanma Barranquero
@ 2022-12-03 0:28 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
0 siblings, 0 replies; 7+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-12-03 0:28 UTC (permalink / raw)
To: Juanma Barranquero; +Cc: Eli Zaretskii, Lars Ingebrigtsen, 59710
Juanma Barranquero [2022-12-03 01:25:13] wrote:
> In fact, I think it would be marginally better
>
> (unless (symbolp var)
> (signal 'wrong-type-argument (list 'symbolp var)))
>
> Assuming ok to install... where? emacs-29 or master?
master
Tefan
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-12-03 0:28 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-11-30 1:49 bug#59710: Wrong type argument when editing a multisession variable Juanma Barranquero
2022-12-02 13:07 ` Eli Zaretskii
2022-12-02 13:30 ` Juanma Barranquero
2022-12-02 15:06 ` Eli Zaretskii
2022-12-02 15:40 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-12-03 0:25 ` Juanma Barranquero
2022-12-03 0:28 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
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).