GNU bug report logs - #58720
29.0.50; Improve error reporting of EUDC plist functions

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 58720 in the body.
You can then email your comments to 58720 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: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: bug-gnu-emacs <at> gnu.org
Subject: 29.0.50; Improve error reporting of EUDC plist functions
Date: Sat, 22 Oct 2022 20:39:46 +0300

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

Severity: wishlist
Tags: patch

In https://bugs.gnu.org/58531#19 it was suggested that
eudc-plist-member, eudc-plist-get, and eudc-lax-plist-get could signal a
more informative wrong-type-argument instead of a generic static error
when passed a degenerate plist argument.  The attached patch makes this
so.

An alternative is to simply call plist-member and plist-get directly,
relying on them for any error reporting, and not worry about arguments
that degenerate further down the list than the point of interest.

WDYT?  How important is it to name and shame degenerate plists as
eagerly as possible in EUDC?

Thanks,

-- 
Basil

[0001-Improve-error-reporting-of-EUDC-plist-functions.patch (text/x-diff, inline)]

From e9e447214b2b0d19af8b7dc43abbcd4c7944a065 Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Date: Sat, 22 Oct 2022 20:25:54 +0300
Subject: [PATCH] Improve error reporting of EUDC plist functions

* lisp/net/eudc.el (eudc--plist-member): Signal a more informative
wrong-type-argument instead of a generic error (bug#58531#19).
* test/lisp/net/eudc-tests.el (eudc--plist-member)
(eudc-plist-member, eudc-plist-get, eudc-lax-plist-get): Update
tests accordingly.
---
 lisp/net/eudc.el            |  5 ++---
 test/lisp/net/eudc-tests.el | 24 ++++++++++++------------
 2 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/lisp/net/eudc.el b/lisp/net/eudc.el
index 0283b04574..5f9e78fc7f 100644
--- a/lisp/net/eudc.el
+++ b/lisp/net/eudc.el
@@ -108,9 +108,8 @@ eudc--using-bbdb-3-or-newer-p
 
 (defun eudc--plist-member (plist prop &optional predicate)
   "Like `plist-member', but signal on invalid PLIST."
-  ;; Could also use `plistp', but that would change the error.
-  (or (zerop (% (length plist) 2))
-      (error "Malformed plist"))
+  (or (plistp plist)
+      (signal 'wrong-type-argument `(plistp ,plist)))
   (plist-member plist prop predicate))
 
 (defun eudc-plist-member (plist prop)
diff --git a/test/lisp/net/eudc-tests.el b/test/lisp/net/eudc-tests.el
index 219c250bf0..915006a97c 100644
--- a/test/lisp/net/eudc-tests.el
+++ b/test/lisp/net/eudc-tests.el
@@ -26,9 +26,9 @@ eudc--plist-member
   (dolist (obj '(a (a . a) (a a . a)))
     (should-error (eudc--plist-member obj nil) :type 'wrong-type-argument))
   (dolist (plist '((nil) (a) (a a a)))
-    (dolist (key '(nil a))
-      (should (equal (should-error (eudc--plist-member plist key))
-                     '(error "Malformed plist")))))
+    (let ((err `(wrong-type-argument plistp ,(copy-sequence plist))))
+      (dolist (key '(nil a))
+        (should (equal err (should-error (eudc--plist-member plist key)))))))
   (let ((-nil (string ?n ?i ?l))
         (-a (string ?a)))
     (should-not (eudc--plist-member () nil))
@@ -56,9 +56,9 @@ eudc-plist-member
   (dolist (obj '(a (a . a) (a a . a)))
     (should-error (eudc-plist-member obj nil) :type 'wrong-type-argument))
   (dolist (plist '((nil) (a) (a a a)))
-    (dolist (key '(nil a))
-      (should (equal (should-error (eudc-plist-member plist key))
-                     '(error "Malformed plist")))))
+    (let ((err `(wrong-type-argument plistp ,(copy-sequence plist))))
+      (dolist (key '(nil a))
+        (should (equal err (should-error (eudc-plist-member plist key)))))))
   (let ((-nil (string ?n ?i ?l))
         (-a (string ?a)))
     (should-not (eudc-plist-member () nil))
@@ -86,9 +86,9 @@ eudc-plist-get
   (dolist (obj '(a (a . a) (a a . a)))
     (should-error (eudc-plist-get obj nil) :type 'wrong-type-argument))
   (dolist (plist '((nil) (a) (a a a)))
-    (dolist (key '(nil a))
-      (should (equal (should-error (eudc-plist-get plist key))
-                     '(error "Malformed plist")))))
+    (let ((err `(wrong-type-argument plistp ,(copy-sequence plist))))
+      (dolist (key '(nil a))
+        (should (equal err (should-error (eudc-plist-get plist key)))))))
   (let ((-nil (string ?n ?i ?l))
         (-a (string ?a)))
     (should-not (eudc-plist-get () nil))
@@ -120,9 +120,9 @@ eudc-lax-plist-get
   (dolist (obj '(a (a . a) (a a . a)))
     (should-error (eudc-lax-plist-get obj nil) :type 'wrong-type-argument))
   (dolist (plist '((nil) (a) (a a a)))
-    (dolist (key '(nil a))
-      (should (equal (should-error (eudc-lax-plist-get plist key))
-                     '(error "Malformed plist")))))
+    (let ((err `(wrong-type-argument plistp ,(copy-sequence plist))))
+      (dolist (key '(nil a))
+        (should (equal err (should-error (eudc-lax-plist-get plist key)))))))
   (let ((-nil (string ?n ?i ?l))
         (-a (string ?a)))
     (should-not (eudc-lax-plist-get () nil))
-- 
2.35.1

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

From: Thomas Fitzsimmons <fitzsim <at> fitzsim.org>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: 58720 <at> debbugs.gnu.org
Subject: Re: bug#58720: 29.0.50; Improve error reporting of EUDC plist
 functions
Date: Sat, 22 Oct 2022 14:06:00 -0400

Hi Basil,

"Basil L. Contovounesios" <contovob <at> tcd.ie> writes:

> Severity: wishlist
> Tags: patch
>
> In https://bugs.gnu.org/58531#19 it was suggested that
> eudc-plist-member, eudc-plist-get, and eudc-lax-plist-get could signal a
> more informative wrong-type-argument instead of a generic static error
> when passed a degenerate plist argument.  The attached patch makes this
> so.
>
> An alternative is to simply call plist-member and plist-get directly,
> relying on them for any error reporting, and not worry about arguments
> that degenerate further down the list than the point of interest.
>
> WDYT?  How important is it to name and shame degenerate plists as
> eagerly as possible in EUDC?

This patch looks good to me.  I haven't tested it, but if you have, feel
free to push to master.  (EUDC is not packaged in GNU ELPA, so it's OK
to use new functions like plistp without providing backward
compatibility.)

Thanks!
Thomas

Message #13 received at 58720-done <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Thomas Fitzsimmons <fitzsim <at> fitzsim.org>
Cc: 58720-done <at> debbugs.gnu.org
Subject: Re: bug#58720: 29.0.50; Improve error reporting of EUDC plist
 functions
Date: Sun, 23 Oct 2022 00:51:00 +0300

close 58720 29.1
quit

Thomas Fitzsimmons [2022-10-22 14:06 -0400] wrote:

> "Basil L. Contovounesios" <contovob <at> tcd.ie> writes:
>
>> In https://bugs.gnu.org/58531#19 it was suggested that
>> eudc-plist-member, eudc-plist-get, and eudc-lax-plist-get could signal a
>> more informative wrong-type-argument instead of a generic static error
>> when passed a degenerate plist argument.  The attached patch makes this
>> so.
>>
>> An alternative is to simply call plist-member and plist-get directly,
>> relying on them for any error reporting, and not worry about arguments
>> that degenerate further down the list than the point of interest.
>>
>> WDYT?  How important is it to name and shame degenerate plists as
>> eagerly as possible in EUDC?
>
> This patch looks good to me.  I haven't tested it, but if you have, feel
> free to push to master.  (EUDC is not packaged in GNU ELPA, so it's OK
> to use new functions like plistp without providing backward
> compatibility.)

Thanks.  Pushed and closing.

Improve error reporting of EUDC plist functions
9db7b11cf7 2022-10-23 00:44:52 +0300
https://git.sv.gnu.org/cgit/emacs.git/commit/?id=9db7b11cf7

-- 
Basil

Previous Next

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