From: "Basil L. Contovounesios" via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
To: Michael Heerdegen <michael_heerdegen@web.de>
Cc: Adam Porter <adam@alphapapa.net>,
Nicolas Petton <nicolas@petton.fr>,
47368@debbugs.gnu.org
Subject: bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN
Date: Wed, 15 Sep 2021 13:50:43 +0100 [thread overview]
Message-ID: <87o88uroik.fsf@tcd.ie> (raw)
In-Reply-To: <87wnnisy9c.fsf@tcd.ie> (Basil L. Contovounesios's message of "Tue, 14 Sep 2021 21:22:39 +0100")
[-- Attachment #1: Type: text/plain, Size: 512 bytes --]
Basil L. Contovounesios [2021-09-14 21:22 +0100] wrote:
> Michael Heerdegen [2021-09-14 16:40 +0200] wrote:
>
>> Adam Porter <adam@alphapapa.net> writes:
>>
>>> Friendly ping. :) The cutting of the Emacs 28 release branch
>>> approaches, and I'd really like to have this fixed in Emacs 28.
>>
>> Basil, are you maybe able to care about this one?
>
> What was the conclusion? That map-elt should use equal for alists?
> If so, sure, I'll try to propose a patch soon.
How's the attached?
Thanks,
--
Basil
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Consistently-test-alist-keys-with-equal-in-map.el.patch --]
[-- Type: text/x-diff, Size: 6299 bytes --]
From a62bb01dd43a9957fe5f04552390a0b3e46e32f9 Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob@tcd.ie>
Date: Wed, 15 Sep 2021 00:17:26 +0100
Subject: [PATCH] Consistently test alist keys with equal in map.el
* etc/NEWS: Announce new default behavior of map-elt and map-delete
on alists.
* lisp/emacs-lisp/map.el: Bump to version 3.2.
(map-elt): Test alist keys with equal by default. Betray a little
bit more information in the docstring on which functions are used
for which map types. (Bug#47368)
(map-put): Update docstring accordingly.
(map--plist-delete): Consistently test plist keys with eq.
(map-delete): Consistently test alist keys with equal.
* test/lisp/emacs-lisp/map-tests.el (test-map-elt-testfn): Update
for new map-elt behavior.
(test-map-put!-alist, test-map-delete-alist): New tests.
---
etc/NEWS | 6 ++++++
lisp/emacs-lisp/map.el | 18 +++++++++-------
test/lisp/emacs-lisp/map-tests.el | 34 ++++++++++++++++++++++++++-----
3 files changed, 46 insertions(+), 12 deletions(-)
diff --git a/etc/NEWS b/etc/NEWS
index 0c9dded458..b2ffbff2ef 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -3558,6 +3558,12 @@ and well-behaved enough to lose the "internal" marker.
** map.el
+---
+*** Alist keys are now consistently compared with 'equal' by default.
+Until now, 'map-elt' and 'map-delete' compared alist keys with 'eq' by
+default. They now use 'equal' instead, for consistency with
+'map-put!' and 'map-contains-key'.
+
*** Pcase 'map' pattern added keyword symbols abbreviation.
A pattern like '(map :sym)' binds the map's value for ':sym' to 'sym',
equivalent to '(map (:sym sym))'.
diff --git a/lisp/emacs-lisp/map.el b/lisp/emacs-lisp/map.el
index 77431f0c59..e0af448eaf 100644
--- a/lisp/emacs-lisp/map.el
+++ b/lisp/emacs-lisp/map.el
@@ -5,7 +5,7 @@
;; Author: Nicolas Petton <nicolas@petton.fr>
;; Maintainer: emacs-devel@gnu.org
;; Keywords: extensions, lisp
-;; Version: 3.1
+;; Version: 3.2
;; Package-Requires: ((emacs "26"))
;; This file is part of GNU Emacs.
@@ -103,10 +103,14 @@ map--plist-p
(and (consp list) (atom (car list))))
(cl-defgeneric map-elt (map key &optional default testfn)
- "Lookup KEY in MAP and return its associated value.
+ "Look up KEY in MAP and return its associated value.
If KEY is not found, return DEFAULT which defaults to nil.
-TESTFN is deprecated. Its default depends on the MAP argument.
+TESTFN is the function to use for comparing keys. It is
+deprecated because its default and valid values depend on the MAP
+argument. Generally, alist keys are compared with `equal', plist
+keys with `eq', and hash-table keys with the hash-table's test
+function.
In the base definition, MAP can be an alist, plist, hash-table,
or array."
@@ -136,7 +140,7 @@ map-elt
:list (if (map--plist-p map)
(let ((res (plist-member map key)))
(if res (cadr res) default))
- (alist-get key map default nil testfn))
+ (alist-get key map default nil (or testfn #'equal)))
:hash-table (gethash key map default)
:array (if (map-contains-key map key)
(aref map key)
@@ -147,7 +151,7 @@ map-put
If KEY is already present in MAP, replace the associated value
with VALUE.
When MAP is an alist, test equality with TESTFN if non-nil,
-otherwise use `eql'.
+otherwise use `equal'.
MAP can be an alist, plist, hash-table, or array."
(declare (obsolete "use map-put! or (setf (map-elt ...) ...) instead" "27.1"))
@@ -157,7 +161,7 @@ map--plist-delete
(let ((tail map) last)
(while (consp tail)
(cond
- ((not (equal key (car tail)))
+ ((not (eq key (car tail)))
(setq last tail)
(setq tail (cddr last)))
(last
@@ -177,7 +181,7 @@ map-delete
;; FIXME: Signal map-not-inplace i.s.o returning a different list?
(if (map--plist-p map)
(map--plist-delete map key)
- (setf (alist-get key map nil t) nil)
+ (setf (alist-get key map nil t #'equal) nil)
map))
(cl-defmethod map-delete ((map hash-table) key)
diff --git a/test/lisp/emacs-lisp/map-tests.el b/test/lisp/emacs-lisp/map-tests.el
index c0f0dbc92b..afade8e295 100644
--- a/test/lisp/emacs-lisp/map-tests.el
+++ b/test/lisp/emacs-lisp/map-tests.el
@@ -85,11 +85,13 @@ test-map-elt-default
(should (= 5 (map-elt map 0 5)))))
(ert-deftest test-map-elt-testfn ()
- (let ((map (list (cons "a" 1) (cons "b" 2)))
- ;; Make sure to use a non-eq "a", even when compiled.
- (noneq-key (string ?a)))
- (should-not (map-elt map noneq-key))
- (should (map-elt map noneq-key nil #'equal))))
+ (let* ((a (string ?a))
+ (map `((,a . 0) (,(string ?b) . 1))))
+ (should (= (map-elt map a) 0))
+ (should (= (map-elt map "a") 0))
+ (should (= (map-elt map (string ?a)) 0))
+ (should (= (map-elt map "b") 1))
+ (should (= (map-elt map (string ?b)) 1))))
(ert-deftest test-map-elt-with-nil-value ()
(should-not (map-elt '((a . 1) (b)) 'b 2)))
@@ -129,6 +131,19 @@ test-map-put!-new-keys
(setf (map-elt map size) 'v)
(should (eq (map-elt map size) 'v))))))
+(ert-deftest test-map-put!-alist ()
+ "Test `map-put!' test function on alists."
+ (let ((key (string ?a))
+ (val 0)
+ map)
+ (should-error (map-put! map key val) :type 'map-not-inplace)
+ (setq map (list (cons key val)))
+ (map-put! map key (1- val))
+ (should (equal map '(("a" . -1))))
+ (map-put! map (string ?a) (1+ val))
+ (should (equal map '(("a" . 1))))
+ (should-error (map-put! map (string ?a) val #'eq) :type 'map-not-inplace)))
+
(ert-deftest test-map-put-alist-new-key ()
"Regression test for Bug#23105."
(let ((alist (list (cons 0 'a))))
@@ -197,6 +212,15 @@ test-map-delete-empty
(with-empty-maps-do map
(should (eq map (map-delete map t)))))
+(ert-deftest test-map-delete-alist ()
+ "Test `map-delete' test function on alists."
+ (let* ((a (string ?a))
+ (map `((,a) (,(string ?b)))))
+ (setq map (map-delete map a))
+ (should (equal map '(("b"))))
+ (setq map (map-delete map (string ?b)))
+ (should-not map)))
+
(ert-deftest test-map-nested-elt ()
(let ((vec [a b [c d [e f]]]))
(should (eq (map-nested-elt vec '(2 2 0)) 'e)))
--
2.33.0
next prev parent reply other threads:[~2021-09-15 12:50 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-24 22:52 bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN dalanicolai
2021-03-24 23:24 ` Basil L. Contovounesios
2021-03-25 2:39 ` Michael Heerdegen
2021-03-25 14:48 ` dalanicolai
2021-03-25 15:33 ` bug#47368: [External] : " Drew Adams
2021-03-26 18:47 ` Basil L. Contovounesios
2021-03-26 20:04 ` Drew Adams
2021-03-26 20:23 ` Basil L. Contovounesios
2021-03-26 22:40 ` Drew Adams
2021-03-26 22:59 ` Basil L. Contovounesios
2021-03-26 3:59 ` Michael Heerdegen
2021-03-26 7:38 ` dalanicolai
2021-03-26 13:31 ` Stefan Monnier
2021-03-26 15:32 ` dalanicolai
2021-03-26 18:57 ` Basil L. Contovounesios
2021-03-26 23:18 ` Michael Heerdegen
2021-03-27 20:37 ` Basil L. Contovounesios
2021-03-26 23:23 ` Michael Heerdegen
2021-03-26 18:58 ` Basil L. Contovounesios
2021-07-21 15:34 ` Adam Porter
2021-07-22 2:15 ` Michael Heerdegen
2021-07-31 2:15 ` Michael Heerdegen
2021-09-13 14:06 ` Adam Porter
2021-09-14 14:40 ` Michael Heerdegen
2021-09-14 20:22 ` Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-09-15 0:48 ` Michael Heerdegen
2021-09-15 9:26 ` Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-09-15 12:39 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-09-15 21:53 ` Michael Heerdegen
2021-09-15 12:50 ` Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors [this message]
2021-09-15 21:55 ` Michael Heerdegen
2021-09-21 12:42 ` Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87o88uroik.fsf@tcd.ie \
--to=bug-gnu-emacs@gnu.org \
--cc=47368@debbugs.gnu.org \
--cc=adam@alphapapa.net \
--cc=contovob@tcd.ie \
--cc=michael_heerdegen@web.de \
--cc=nicolas@petton.fr \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).