* bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN @ 2021-03-24 22:52 dalanicolai 2021-03-24 23:24 ` Basil L. Contovounesios 2021-07-21 15:34 ` Adam Porter 0 siblings, 2 replies; 32+ messages in thread From: dalanicolai @ 2021-03-24 22:52 UTC (permalink / raw) To: 47368 [-- Attachment #1: Type: text/plain, Size: 3687 bytes --] Message-ID: <87czvonp73.fsf@dalanicolai-at-gmail.com> --text follows this line-- The docstring of the map-elt function from the map.el package (version 3.0) mentions that TESTFN is deprecated because "its default depends on the MAP argument". However when I try e.g. (map-elt '(("A1" . 3)) "A1") it returns nil. When I add the correct TESTFN (map-elt '(("A1" . 3)) "A1" nil 'string=) it does correctly return 3. So it seems to me that TESTFN is not yet deprecated, or that otherwise I am understanding it incorrectly. In that case I would label this as a documentation bug. In GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.25, cairo version 1.16.0) of 2021-02-18 built on daniel-fedora Repository revision: 185121da6978553d538d37d6d0e67dc52e13311f Repository branch: feature/native-comp Windowing system distributor 'The X.Org Foundation', version 11.0.12010000 System Description: Fedora 34 (Workstation Edition Prerelease) Configured using: 'configure --with-nativecomp' Configured features: CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NATIVE_COMP NOTIFY INOTIFY PDUMPER PNG RSVG SOUND THREADS TIFF TOOLKIT_SCROLL_BARS X11 XDBE XIM XPM GTK3 ZLIB Important settings: value of $LANG: en_US.UTF-8 value of $XMODIFIERS: @im=none locale-coding-system: utf-8-unix Major mode: Lisp Interaction Minor modes in effect: tooltip-mode: t global-eldoc-mode: t eldoc-mode: t electric-indent-mode: t mouse-wheel-mode: t tool-bar-mode: t menu-bar-mode: t file-name-shadow-mode: t global-font-lock-mode: t font-lock-mode: t blink-cursor-mode: t auto-composition-mode: t auto-encryption-mode: t auto-compression-mode: t line-number-mode: t transient-mark-mode: t Load-path shadows: None found. Features: (shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs rfc822 mml mml-sec epa derived epg epg-config gnus-util rmail rmail-loaddefs auth-source cl-seq eieio eieio-core cl-macs eieio-loaddefs password-cache json text-property-search time-date subr-x mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils map seq byte-opt gv bytecomp byte-compile cconv help-fns radix-tree cl-print debug backtrace help-mode easymenu find-func cl-loaddefs cl-lib iso-transl tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type mwheel term/x-win x-win term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe tabulated-list replace newcomment text-mode elisp-mode lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch timer select scroll-bar mouse jit-lock font-lock syntax facemenu font-core term/tty-colors frame minibuffer cl-generic cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european ethiopic indian cyrillic chinese composite charscript charprop case-table epa-hook jka-cmpr-hook help simple abbrev obarray cl-preloaded nadvice button loaddefs faces cus-face pcase macroexp files window text-properties overlay sha1 md5 base64 format env code-pages mule custom widget hashtable-print-readable backquote threads dbusbind inotify lcms2 dynamic-setting system-font-setting font-render-setting cairo move-toolbar gtk x-toolkit x multi-tty make-network-process nativecomp emacs) Memory information: ((conses 16 74272 8338) (symbols 48 7113 0) (strings 32 20890 1678) (string-bytes 1 713441) (vectors 16 13521) (vector-slots 8 292167 12582) (floats 8 25 33) (intervals 56 238 0) (buffers 992 13)) [-- Attachment #2: Type: text/html, Size: 4190 bytes --] ^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN 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-07-21 15:34 ` Adam Porter 1 sibling, 1 reply; 32+ messages in thread From: Basil L. Contovounesios @ 2021-03-24 23:24 UTC (permalink / raw) To: dalanicolai; +Cc: 47368 dalanicolai <dalanicolai@gmail.com> writes: > The docstring of the map-elt function from the map.el package (version > 3.0) mentions that TESTFN is deprecated because "its default depends on > the MAP argument". However when I try e.g. > > (map-elt '(("A1" . 3)) "A1") > > it returns nil. This is expected, as alist keys are tested with eq by default. That's what the docstring is trying to warn about: alists default to testing with eq, but can also use eql, equal, or anything else. Hash tables, OTOH, are limited to the test function that they were created with. So TESTFN doesn't always work as expected depending on the map type. > When I add the correct TESTFN > > (map-elt '(("A1" . 3)) "A1" nil 'string=) > > it does correctly return 3. > > So it seems to me that TESTFN is not yet deprecated, or that otherwise I > am understanding it incorrectly. Deprecation means "this is not recommended" and "support for this may be removed in a future version". So to me TESTFN seems to be working as intended. > In that case I would label this as a documentation bug. What would you like to see clarified in the documentation? Thanks, -- Basil ^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN 2021-03-24 23:24 ` Basil L. Contovounesios @ 2021-03-25 2:39 ` Michael Heerdegen 2021-03-25 14:48 ` dalanicolai ` (3 more replies) 0 siblings, 4 replies; 32+ messages in thread From: Michael Heerdegen @ 2021-03-25 2:39 UTC (permalink / raw) To: Basil L. Contovounesios; +Cc: dalanicolai, 47368 "Basil L. Contovounesios" <contovob@tcd.ie> writes: > dalanicolai <dalanicolai@gmail.com> writes: > > > The docstring of the map-elt function from the map.el package (version > > 3.0) mentions that TESTFN is deprecated because "its default depends on > > the MAP argument". However when I try e.g. > > > > (map-elt '(("A1" . 3)) "A1") > > > > it returns nil. > > This is expected, as alist keys are tested with eq by default. > > That's what the docstring is trying to warn about: alists default to > testing with eq, but can also use eql, equal, or anything else. Is it that obvious? We have `assoc' and `assq' built-in - to me it's not obvious that "alist keys are tested with eq by default". It's the default for `alist-get', ok, which is used by the implementation, but not everybody will know that. I would add a sentence about that. Michael. ^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN 2021-03-25 2:39 ` Michael Heerdegen @ 2021-03-25 14:48 ` dalanicolai 2021-03-25 15:33 ` bug#47368: [External] : " Drew Adams ` (2 subsequent siblings) 3 siblings, 0 replies; 32+ messages in thread From: dalanicolai @ 2021-03-25 14:48 UTC (permalink / raw) To: Michael Heerdegen; +Cc: Basil L. Contovounesios, 47368 [-- Attachment #1: Type: text/plain, Size: 2198 bytes --] For clarity I will insert the first lines of the docstrings here: map-elt is a Lisp closure in ‘map.el’. > > (map-elt MAP KEY &optional DEFAULT) > > Probably introduced at or before Emacs version 26.1. > > Lookup 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. > > In the base definition, MAP can be an alist, plist, hash-table, > or array. > First I agree with Michael that this docstring assumes a lot of knowledge from the programmer. The sentence "its default depends on the MAP argument" could be more explicit, to make the docstring friendly to new elisp programmers. e.g. mention eq for alists (and also the TESTFN's for the other ones) Second calling the TESTFN deprecated is misleading if it a basic requirement for the basic thing I am trying to achieve (i.e. matching a string). So it should probably mention that it is not required if you want to use the MAP's default TESTFN, but otherwise it is required (while deprecated sounds to me like there shouldn't be a need to use it) Thanks for your answer! On Thu, 25 Mar 2021 at 03:39, Michael Heerdegen <michael_heerdegen@web.de> wrote: > "Basil L. Contovounesios" <contovob@tcd.ie> writes: > > > dalanicolai <dalanicolai@gmail.com> writes: > > > > > The docstring of the map-elt function from the map.el package (version > > > 3.0) mentions that TESTFN is deprecated because "its default depends on > > > the MAP argument". However when I try e.g. > > > > > > (map-elt '(("A1" . 3)) "A1") > > > > > > it returns nil. > > > > This is expected, as alist keys are tested with eq by default. > > > > That's what the docstring is trying to warn about: alists default to > > testing with eq, but can also use eql, equal, or anything else. > > Is it that obvious? We have `assoc' and `assq' built-in - to me it's > not obvious that "alist keys are tested with eq by default". It's the > default for `alist-get', ok, which is used by the implementation, but > not everybody will know that. I would add a sentence about that. > > Michael. > [-- Attachment #2: Type: text/html, Size: 3135 bytes --] ^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#47368: [External] : bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN 2021-03-25 2:39 ` Michael Heerdegen 2021-03-25 14:48 ` dalanicolai @ 2021-03-25 15:33 ` Drew Adams 2021-03-26 18:47 ` Basil L. Contovounesios 2021-03-26 3:59 ` Michael Heerdegen 2021-03-26 18:58 ` Basil L. Contovounesios 3 siblings, 1 reply; 32+ messages in thread From: Drew Adams @ 2021-03-25 15:33 UTC (permalink / raw) To: Michael Heerdegen, Basil L. Contovounesios Cc: dalanicolai, 47368@debbugs.gnu.org > > This is expected, as alist keys are tested with eq by default. Since when? Where? Expected by whom, and by what code? > > That's what the docstring is trying to warn about: alists default to > > testing with eq, but can also use eql, equal, or anything else. > > Is it that obvious? We have `assoc' and `assq' built-in - to me it's > not obvious that "alist keys are tested with eq by default". It's the > default for `alist-get', ok, which is used by the implementation, but > not everybody will know that. I would add a sentence about that. +1. Alists are general. They can be used in many ways. Their keys can be tested in multiple ways. Neither code nor doc should assume anything about how an alist is composed or treated - nothing beyond the fact that at least some of the list elements are likely to be conses. ^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#47368: [External] : bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN 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 0 siblings, 1 reply; 32+ messages in thread From: Basil L. Contovounesios @ 2021-03-26 18:47 UTC (permalink / raw) To: Drew Adams; +Cc: Michael Heerdegen, dalanicolai, 47368 [ BTW, your MUA seems to have mangled the email subject, and I received the mail twice.] Drew Adams <drew.adams@oracle.com> writes: >> > This is expected, as alist keys are tested with eq by default. > > Since when? Since the introduction of map.el in Emacs 25. > Where? Expected by whom, and by what code? By the function being discussed. -- Basil ^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#47368: [External] : bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN 2021-03-26 18:47 ` Basil L. Contovounesios @ 2021-03-26 20:04 ` Drew Adams 2021-03-26 20:23 ` Basil L. Contovounesios 0 siblings, 1 reply; 32+ messages in thread From: Drew Adams @ 2021-03-26 20:04 UTC (permalink / raw) To: Basil L. Contovounesios Cc: Michael Heerdegen, dalanicolai, 47368@debbugs.gnu.org > >> > This is expected, as alist keys are tested with eq by default. > > > > Since when? > > Since the introduction of map.el in Emacs 25. The general statement you made, that alist keys are treated with eq by default, is false, AFAIK. That may be true of `map-elt', but it's not true in general (AFAIK). > > Where? Expected by whom, and by what code? > > By the function being discussed. Does the doc string of the function being discussed, `map-elt' say this? Does it say anything at all about how keys are compared? I have only Emacs 27.1 - the latest release available on MS Windows, and there the doc string says NADA about how keys are compared. Nothing about what it means for "if KEY is not found". At the very least, if such is still the case then the doc needs to updated to specify how keys are compared, IMHO. I'm hoping that doc more recent than Emacs 27.1 already takes care of this. You say, for example: That's what the docstring is trying to warn about: alists default to testing with eq, but can also use eql, equal, or anything else. I don't see (in 27.1) where the doc string warns about any such thing. Nothing about eq being the default, and nothing about testing being also possible with the others you mention. Not only that, but the doc string says that TESTFN is deprecated, but there's no other mention of TESTFN. What's TESTFN? Where is it specified? It's not part of the function signature that's shown. How can you refer to it if there's no indication anywhere here of what it is? This makes no sense to me. And why is there this line at the end of the doc string? Undocumented What on earth is that supposed to mean to a reader of the `map-elt' doc? ^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#47368: [External] : bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN 2021-03-26 20:04 ` Drew Adams @ 2021-03-26 20:23 ` Basil L. Contovounesios 2021-03-26 22:40 ` Drew Adams 0 siblings, 1 reply; 32+ messages in thread From: Basil L. Contovounesios @ 2021-03-26 20:23 UTC (permalink / raw) To: Drew Adams; +Cc: Michael Heerdegen, dalanicolai, 47368@debbugs.gnu.org Drew Adams <drew.adams@oracle.com> writes: >> >> > This is expected, as alist keys are tested with eq by default. >> > >> > Since when? >> >> Since the introduction of map.el in Emacs 25. > > The general statement you made, that alist keys are treated with eq by > default, is false, AFAIK. That may be true of `map-elt', but it's not > true in general (AFAIK). This bug report is about map-elt, not alists in general. >> > Where? Expected by whom, and by what code? >> >> By the function being discussed. > > Does the doc string of the function being discussed, `map-elt' say > this? Does it say anything at all about how keys are compared? It used to, before the TESTFN argument was deprecated. But the whole point of this discussion is that one size doesn't fit all, since map-elt is a generic function that can be adapted to heterogeneous types and semantics, both within Emacs core and external packages. > I have only Emacs 27.1 - the latest release available on MS Windows, > and there the doc string says NADA about how keys are compared. > Nothing about what it means for "if KEY is not found". > > At the very least, if such is still the case then the doc needs to > updated to specify how keys are compared, IMHO. > > I'm hoping that doc more recent than Emacs 27.1 already takes care of > this. You say, for example: > > That's what the docstring is trying to warn about: > alists default to testing with eq, but can also use > eql, equal, or anything else. > > I don't see (in 27.1) where the doc string warns about > any such thing. "TESTFN is deprecated. Its default depends on the MAP argument." > Nothing about eq being the default, and nothing about testing being > also possible with the others you mention. > > Not only that, but the doc string says that TESTFN > is deprecated, but there's no other mention of TESTFN. > > What's TESTFN? Where is it specified? It's not part > of the function signature that's shown. How can you > refer to it if there's no indication anywhere here of > what it is? This makes no sense to me. All of these points are already being discussed in this thread. Patches with improvements are always welcome. > And why is there this line at the end of the doc string? > > Undocumented > > What on earth is that supposed to mean to a reader of > the `map-elt' doc? The limitations of and suggestions for improvements to the documentation generated for generic functions are already discussed elsewhere, and are not specific to the current issue. -- Basil ^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#47368: [External] : bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN 2021-03-26 20:23 ` Basil L. Contovounesios @ 2021-03-26 22:40 ` Drew Adams 2021-03-26 22:59 ` Basil L. Contovounesios 0 siblings, 1 reply; 32+ messages in thread From: Drew Adams @ 2021-03-26 22:40 UTC (permalink / raw) To: Basil L. Contovounesios Cc: Michael Heerdegen, dalanicolai, 47368@debbugs.gnu.org > > Nothing about eq being the default, and nothing about > > testing being also possible with the others you mention. > > > > Not only that, but the doc string says that TESTFN > > is deprecated, but there's no other mention of TESTFN. > > > > What's TESTFN? Where is it specified? It's not part > > of the function signature that's shown. How can you > > refer to it if there's no indication anywhere here of > > what it is? This makes no sense to me. > > All of these points are already being discussed in this thread. And yet in this thread you asked, seeming to suggest that the doc here is fine as is: What would you like to see clarified in the documentation? Seems pretty clear that this doc has more than one problem, as I guess (hope) you acknowledge now. If the function handles different kinds of data (alists, hash tables, ... whatnot), and if its behavior can depend on an equality predicate that it can't know (can't even be passed as an arg), then _that_, at the very least, should be stated in the doc. I'd think that if a test function _can_ be used for at least some types of data, such as alists, then it should be allowed as an optional arg. Just because a function is generic, that doesn't mean it has to always act with lowest-common-denominator behavior. A hash table has a given comparer. But so what? Is `eq' the _default_ comparer, or is it the only one for some data types? "its default depends on the MAP argument". Maybe that too isn't generic enough? If `eq' is all that's allowed for some types then it makes no sense to speak of a "default" those cases. You said: That's what the docstring is trying to warn about: alists default to testing with eq, but can also use eql, equal, or anything else. Hash tables, OTOH, are limited to the test function that they were created with. So TESTFN doesn't always work as expected depending on the map type. Why not allow TESTFN, and state that it applies only to some types (naming some of them)? Why cripple it just because it's limited for some types? Are you sure deprecating TESTFN was a great idea? If it will remain deprecated, then at least add a statement like what you wrote (above), to the doc. And let users know how to use other than `eq' with an alist etc., since you tell them that `eq' is only the default and they can use other comparers. And especially if the types are limited to alist, hash table, and array, just _how_ a key is found should be specified. It makes no sense (to me) for the doc to say only "If KEY is found". FWIW, a cursory look at the code suggests the type can also be a plist, but the doc currently mentions only the other 3 types. That too seems wrong. ^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#47368: [External] : bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN 2021-03-26 22:40 ` Drew Adams @ 2021-03-26 22:59 ` Basil L. Contovounesios 0 siblings, 0 replies; 32+ messages in thread From: Basil L. Contovounesios @ 2021-03-26 22:59 UTC (permalink / raw) To: Drew Adams; +Cc: Michael Heerdegen, dalanicolai, 47368@debbugs.gnu.org Drew Adams <drew.adams@oracle.com> writes: >> > Nothing about eq being the default, and nothing about >> > testing being also possible with the others you mention. >> > >> > Not only that, but the doc string says that TESTFN >> > is deprecated, but there's no other mention of TESTFN. >> > >> > What's TESTFN? Where is it specified? It's not part >> > of the function signature that's shown. How can you >> > refer to it if there's no indication anywhere here of >> > what it is? This makes no sense to me. >> >> All of these points are already being discussed in this thread. > > And yet in this thread you asked, seeming to suggest > that the doc here is fine as is: > > What would you like to see clarified in the documentation? > > Seems pretty clear that this doc has more than one > problem, as I guess (hope) you acknowledge now. You seem to have misunderstood my cited text. The issues you have mentioned have already been acknowledged. What remains to be done is decide on how to improve map-elt's interface, so that its documentation can be updated accordingly. > FWIW, a cursory look at the code suggests the type > can also be a plist, but the doc currently mentions > only the other 3 types. That too seems wrong. This is already fixed on master, and in version 3.0 of the map.el package on GNU ELPA: https://elpa.gnu.org/packages/map.html -- Basil ^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN 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 3:59 ` Michael Heerdegen 2021-03-26 7:38 ` dalanicolai 2021-03-26 13:31 ` Stefan Monnier 2021-03-26 18:58 ` Basil L. Contovounesios 3 siblings, 2 replies; 32+ messages in thread From: Michael Heerdegen @ 2021-03-26 3:59 UTC (permalink / raw) To: Stefan Monnier; +Cc: Basil L. Contovounesios, dalanicolai, 47368 Hi Stefan, we are discussing here the limitation for `map-elt' calls with alists caused by deprecating the TESTFN argument (done by you a while ago). What's a good way to solve this? Obviously the map abstraction doesn't fit so super well for alists because unlike the other map type alists don't know "their" test function. But disallowing alists that don't test with `eq' seems an unnecessary restriction. Can we say that the argument is allowed only for alists? Regards, Michael. I <michael_heerdegen@web.de> wrote: > > > The docstring of the map-elt function from the map.el package (version > > > 3.0) mentions that TESTFN is deprecated because "its default depends on > > > the MAP argument". However when I try e.g. > > > > > > (map-elt '(("A1" . 3)) "A1") > > > > > > it returns nil. > > > > This is expected, as alist keys are tested with eq by default. > > > > That's what the docstring is trying to warn about: alists default to > > testing with eq, but can also use eql, equal, or anything else. > > Is it that obvious? We have `assoc' and `assq' built-in - to me it's > not obvious that "alist keys are tested with eq by default". It's the > default for `alist-get', ok, which is used by the implementation, but > not everybody will know that. I would add a sentence about that. ^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN 2021-03-26 3:59 ` Michael Heerdegen @ 2021-03-26 7:38 ` dalanicolai 2021-03-26 13:31 ` Stefan Monnier 1 sibling, 0 replies; 32+ messages in thread From: dalanicolai @ 2021-03-26 7:38 UTC (permalink / raw) To: Michael Heerdegen; +Cc: Basil L. Contovounesios, Stefan Monnier, 47368 [-- Attachment #1: Type: text/plain, Size: 1704 bytes --] You probably already noticed it, but I only notice just now that the TESTFN option also has been removed from the calling convention with `(advertised-calling-convention (map key &optional default) "27.1")`. (Just to add to my previous answer) On Fri, 26 Mar 2021 at 04:59, Michael Heerdegen <michael_heerdegen@web.de> wrote: > Hi Stefan, > > we are discussing here the limitation for `map-elt' calls with alists > caused by deprecating the TESTFN argument (done by you a while ago). > > What's a good way to solve this? Obviously the map abstraction doesn't > fit so super well for alists because unlike the other map type alists > don't know "their" test function. But disallowing alists that don't > test with `eq' seems an unnecessary restriction. Can we say that the > argument is allowed only for alists? > > Regards, > > Michael. > > > I <michael_heerdegen@web.de> wrote: > > > > > The docstring of the map-elt function from the map.el package > (version > > > > 3.0) mentions that TESTFN is deprecated because "its default depends > on > > > > the MAP argument". However when I try e.g. > > > > > > > > (map-elt '(("A1" . 3)) "A1") > > > > > > > > it returns nil. > > > > > > This is expected, as alist keys are tested with eq by default. > > > > > > That's what the docstring is trying to warn about: alists default to > > > testing with eq, but can also use eql, equal, or anything else. > > > > Is it that obvious? We have `assoc' and `assq' built-in - to me it's > > not obvious that "alist keys are tested with eq by default". It's the > > default for `alist-get', ok, which is used by the implementation, but > > not everybody will know that. I would add a sentence about that. > > [-- Attachment #2: Type: text/html, Size: 2422 bytes --] ^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN 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 ` (2 more replies) 1 sibling, 3 replies; 32+ messages in thread From: Stefan Monnier @ 2021-03-26 13:31 UTC (permalink / raw) To: Michael Heerdegen; +Cc: Basil L. Contovounesios, dalanicolai, 47368 > What's a good way to solve this? Obviously the map abstraction doesn't > fit so super well for alists because unlike the other map type alists > don't know "their" test function. But disallowing alists that don't > test with `eq' seems an unnecessary restriction. Can we say that the > argument is allowed only for alists? How 'bout always using `equal`? Stefan ^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN 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:23 ` Michael Heerdegen 2 siblings, 0 replies; 32+ messages in thread From: dalanicolai @ 2021-03-26 15:32 UTC (permalink / raw) To: Stefan Monnier; +Cc: Michael Heerdegen, Basil L. Contovounesios, 47368 [-- Attachment #1: Type: text/plain, Size: 1655 bytes --] Well, as a hobbyist programmer, this makes more sense to me anyway. If users don't want the behavior of using `equal` then they could just as well use `alist-get`. I hope you don't mind if I write down some thoughts (and macro's) I have on my mind. Personally, I think it would be great for growing the community, if emacs-lisp got more approachable (in the direction of python). I actually got here because, as an exercise, I was trying to implement Norvig's sudoku solver <https://norvig.com/sudoku.html> in emacs-lisp, which was a quite frustrating exercise (this is no complaint, but just a fact). I think it would be great if emacs-lisp could look, and become readable and usable, more like that (in which map.el and seq.el are doing a very nice job of course). So that emacs-lisp could actually become a nice and friendly teaching language, which is equally fun to script in as in python. Actually, I got the feeling that it would be nice to have list-comprehension like syntax also. Therefore, I tried to create some general `array` and `table` macro's here <https://github.com/dalanicolai/list-factory/blob/main/list-factory.el> (which is my first macro exercise ever). As it is just an exercise it is undocumented, but you can very easily get the idea from looking at the (commented out) tests at the bottom of that file. Also, from looking at map.el, I assume I should probably implement it using cl-defgeneric. Haha... sorry for the elaborate answer. You can neglect most of this message, but maybe someone is interested and shares some of these ideas (or likes the idea of these macro's). I just couldn't resist to share these thoughts... [-- Attachment #2: Type: text/html, Size: 1892 bytes --] ^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN 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 2 siblings, 2 replies; 32+ messages in thread From: Basil L. Contovounesios @ 2021-03-26 18:57 UTC (permalink / raw) To: Stefan Monnier; +Cc: Michael Heerdegen, dalanicolai, Nicolas Petton, 47368 Stefan Monnier <monnier@iro.umontreal.ca> writes: >> What's a good way to solve this? Obviously the map abstraction doesn't >> fit so super well for alists because unlike the other map type alists >> don't know "their" test function. But disallowing alists that don't >> test with `eq' seems an unnecessary restriction. Can we say that the >> argument is allowed only for alists? > > How 'bout always using `equal`? No objections here. CCing Nicolas in case he has any comments. Just some code archaeology for more context: - Pre-Emacs-25 map-elt used assoc. - Emacs 25 map-elt used alist-get without TESTFN. - Emacs 26 map-elt gained TESTFN at the same time that alist-get did, in https://bugs.gnu.org/27584. - Emacs 27 deprecated map-elt's TESTFN. -- Basil ^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN 2021-03-26 18:57 ` Basil L. Contovounesios @ 2021-03-26 23:18 ` Michael Heerdegen 2021-03-27 20:37 ` Basil L. Contovounesios 1 sibling, 0 replies; 32+ messages in thread From: Michael Heerdegen @ 2021-03-26 23:18 UTC (permalink / raw) To: Basil L. Contovounesios Cc: dalanicolai, Stefan Monnier, Nicolas Petton, 47368 "Basil L. Contovounesios" <contovob@tcd.ie> writes: > Just some code archaeology for more context: > > - Pre-Emacs-25 map-elt used assoc. > - Emacs 25 map-elt used alist-get without TESTFN. > - Emacs 26 map-elt gained TESTFN at the same time that alist-get did, > in https://bugs.gnu.org/27584. > - Emacs 27 deprecated map-elt's TESTFN. And `map-put!' has been added recently. For alists it compares with `equal', and it's used by the setter for the general `map-elt' variable, so we currently don't have consistence. Michael. ^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN 2021-03-26 18:57 ` Basil L. Contovounesios 2021-03-26 23:18 ` Michael Heerdegen @ 2021-03-27 20:37 ` Basil L. Contovounesios 1 sibling, 0 replies; 32+ messages in thread From: Basil L. Contovounesios @ 2021-03-27 20:37 UTC (permalink / raw) To: Nicolas Petton; +Cc: Michael Heerdegen, dalanicolai, Stefan Monnier, 47368 Trying the CC again, this time with a hopefully correct address. Sorry about the noise. "Basil L. Contovounesios" <contovob@tcd.ie> writes: > Stefan Monnier <monnier@iro.umontreal.ca> writes: > >>> What's a good way to solve this? Obviously the map abstraction doesn't >>> fit so super well for alists because unlike the other map type alists >>> don't know "their" test function. But disallowing alists that don't >>> test with `eq' seems an unnecessary restriction. Can we say that the >>> argument is allowed only for alists? >> >> How 'bout always using `equal`? > > No objections here. CCing Nicolas in case he has any comments. > > Just some code archaeology for more context: > > - Pre-Emacs-25 map-elt used assoc. > - Emacs 25 map-elt used alist-get without TESTFN. > - Emacs 26 map-elt gained TESTFN at the same time that alist-get did, > in https://bugs.gnu.org/27584. > - Emacs 27 deprecated map-elt's TESTFN. -- Basil ^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN 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:23 ` Michael Heerdegen 2 siblings, 0 replies; 32+ messages in thread From: Michael Heerdegen @ 2021-03-26 23:23 UTC (permalink / raw) To: Stefan Monnier; +Cc: Basil L. Contovounesios, dalanicolai, 47368 Stefan Monnier <monnier@iro.umontreal.ca> writes: > How 'bout always using `equal`? I think it would be an improvement. But are alists the only exception - are we sure there are no useful implementations of map types were the TESTFN is not constant? Like a dictionary where we sometimes want to look up a word case-sensitively, and if none found, case-insensitively? Michael. ^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN 2021-03-25 2:39 ` Michael Heerdegen ` (2 preceding siblings ...) 2021-03-26 3:59 ` Michael Heerdegen @ 2021-03-26 18:58 ` Basil L. Contovounesios 3 siblings, 0 replies; 32+ messages in thread From: Basil L. Contovounesios @ 2021-03-26 18:58 UTC (permalink / raw) To: Michael Heerdegen; +Cc: dalanicolai, 47368 Michael Heerdegen <michael_heerdegen@web.de> writes: > "Basil L. Contovounesios" <contovob@tcd.ie> writes: > >> dalanicolai <dalanicolai@gmail.com> writes: >> >> > The docstring of the map-elt function from the map.el package (version >> > 3.0) mentions that TESTFN is deprecated because "its default depends on >> > the MAP argument". However when I try e.g. >> > >> > (map-elt '(("A1" . 3)) "A1") >> > >> > it returns nil. >> >> This is expected, as alist keys are tested with eq by default. >> >> That's what the docstring is trying to warn about: alists default to >> testing with eq, but can also use eql, equal, or anything else. > > Is it that obvious? We have `assoc' and `assq' built-in - to me it's > not obvious that "alist keys are tested with eq by default". It's the > default for `alist-get', ok, which is used by the implementation, but > not everybody will know that. I would add a sentence about that. There used to be such a sentence until the argument was deprecated. I agree that whatever we decide on should be made clear. -- Basil ^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN 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-07-21 15:34 ` Adam Porter 2021-07-22 2:15 ` Michael Heerdegen 1 sibling, 1 reply; 32+ messages in thread From: Adam Porter @ 2021-07-21 15:34 UTC (permalink / raw) To: 47368 Hi, If I may chime in here, I'd like for map-elt to be usable with alists having string keys. It may not generally be very common in Elisp to use alists with string keys, but in certain contexts, like preparing JSON maps for encoding, it is. In one of my packages, I came up with this workaround, which may be tolerable when only used once in the whole package, but wouldn't be nice to use more often: (cl-letf (((symbol-function 'alist-get) (lambda (key alist &optional _default _remove _testfn) (cdr (assoc-string key alist))))) (let ((alist (list (cons "foo" "FOO") (cons "bar" "BAR")))) (map-elt alist "foo"))) ;;=> "FOO" In one of my other packages, I would have to use it more often, so I guess I'll use alist-get for now. In general, I think that using `equal' is a good solution. It seems like map-elt is intended to abstract over some Lisp implementation details (to some extent, anyway), so using `equal' instead of `eq' seems sensible, since I think it will usually DWIM. If I really need to compare Lisp object identity rather than equality, I'll probably know how, and I probably won't need to do that as often, anyway. Could this change be made for Emacs 28 and tagged for a 3.1 release of map.el? Thanks, Adam ^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN 2021-07-21 15:34 ` Adam Porter @ 2021-07-22 2:15 ` Michael Heerdegen 2021-07-31 2:15 ` Michael Heerdegen 0 siblings, 1 reply; 32+ messages in thread From: Michael Heerdegen @ 2021-07-22 2:15 UTC (permalink / raw) To: Adam Porter; +Cc: Nicolas Petton, 47368 Adam Porter <adam@alphapapa.net> writes: > Could this change be made for Emacs 28 and tagged for a 3.1 release of > map.el? Let's try to CC Nicolas again, in the past he always showed up sooner or later, and he wanted to stay involved. Michael. ^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN 2021-07-22 2:15 ` Michael Heerdegen @ 2021-07-31 2:15 ` Michael Heerdegen 2021-09-13 14:06 ` Adam Porter 0 siblings, 1 reply; 32+ messages in thread From: Michael Heerdegen @ 2021-07-31 2:15 UTC (permalink / raw) To: Adam Porter; +Cc: Nicolas Petton, 47368 Michael Heerdegen <michael_heerdegen@web.de> writes: > Let's try to CC Nicolas again, in the past he always showed up sooner > or later, and he wanted to stay involved. Hmm, seems his address "nicolas@petton.fr" is not reachable ATM? I can try again later or try to contact him in some other way. Michael. ^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN 2021-07-31 2:15 ` Michael Heerdegen @ 2021-09-13 14:06 ` Adam Porter 2021-09-14 14:40 ` Michael Heerdegen 0 siblings, 1 reply; 32+ messages in thread From: Adam Porter @ 2021-09-13 14:06 UTC (permalink / raw) To: Michael Heerdegen; +Cc: Nicolas Petton, 47368 On Fri, Jul 30, 2021 at 9:18 PM Michael Heerdegen <michael_heerdegen@web.de> wrote: > > Michael Heerdegen <michael_heerdegen@web.de> writes: > > > Let's try to CC Nicolas again, in the past he always showed up sooner > > or later, and he wanted to stay involved. > > Hmm, seems his address "nicolas@petton.fr" is not reachable ATM? I can > try again later or try to contact him in some other way. Friendly ping. :) The cutting of the Emacs 28 release branch approaches, and I'd really like to have this fixed in Emacs 28. ^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN 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 0 siblings, 1 reply; 32+ messages in thread From: Michael Heerdegen @ 2021-09-14 14:40 UTC (permalink / raw) To: Basil L. Contovounesios; +Cc: Adam Porter, Nicolas Petton, 47368 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? I'm on vacation and don't have too much time. Michael. ^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN 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 12:50 ` Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 2 replies; 32+ messages in thread From: Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-09-14 20:22 UTC (permalink / raw) To: Michael Heerdegen; +Cc: Adam Porter, Nicolas Petton, 47368 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. -- Basil ^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN 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 12:50 ` Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors 1 sibling, 2 replies; 32+ messages in thread From: Michael Heerdegen @ 2021-09-15 0:48 UTC (permalink / raw) To: Basil L. Contovounesios Cc: Adam Porter, Nicolas Petton, Stefan Monnier, 47368 "Basil L. Contovounesios" <contovob@tcd.ie> writes: > 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? That was the least controversial solution discussed, at least to my impression. But would that include to change the default of `alist-get', too? I guess these should behave consistently. [ Personally I would still be interested in an answer to the question: why don't we consider to keep the TESTFN arg at least for the list map type case? ] Michael. ^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN 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 1 sibling, 0 replies; 32+ messages in thread From: Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-09-15 9:26 UTC (permalink / raw) To: Michael Heerdegen; +Cc: Adam Porter, Nicolas Petton, Stefan Monnier, 47368 Michael Heerdegen [2021-09-15 02:48 +0200] wrote: > "Basil L. Contovounesios" <contovob@tcd.ie> writes: >> What was the conclusion? That map-elt should use equal for alists? > > That was the least controversial solution discussed, at least to my > impression. > > But would that include to change the default of `alist-get', too? I > guess these should behave consistently. That sounds a bit too radical to me within the scope of this bug report, and for Emacs 28. Since alist-get has the type in its name, and allows for a testfn argument, I personally don't see any conflict between map-elt and alist-get having different default behaviour on alists. > [ Personally I would still be interested in an answer to the question: > why don't we consider to keep the TESTFN arg at least for the list map > type case? ] I don't feel too strongly either way, and it's not my decision, but I guess the point is that if you already know that your map is an alist, then you can "customise" the behaviour of map-elt by using a different accessor function, since map.el wants to provide a type-agnostic API. Thanks, -- Basil ^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN 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 1 sibling, 1 reply; 32+ messages in thread From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-09-15 12:39 UTC (permalink / raw) To: Michael Heerdegen Cc: Basil L. Contovounesios, Adam Porter, Nicolas Petton, 47368 > [ Personally I would still be interested in an answer to the question: > why don't we consider to keep the TESTFN arg at least for the list map > type case? ] This would only make sense if we add corresponding args to several other functions of `map.el`, and it would bring us back to the problem that this arg would be ignored for most map types and couldn't be obeyed without paying a high cost. If you know it's a list and you want to use a specific test, then don't use `map-elt`. Stefan ^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN 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 0 siblings, 0 replies; 32+ messages in thread From: Michael Heerdegen @ 2021-09-15 21:53 UTC (permalink / raw) To: Stefan Monnier Cc: Basil L. Contovounesios, Adam Porter, Nicolas Petton, 47368 Stefan Monnier <monnier@iro.umontreal.ca> writes: > If you know it's a list and you want to use a specific test, then don't > use `map-elt`. That's the part I missed. Makes sense. Michael. ^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN 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 12:50 ` Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors 2021-09-15 21:55 ` Michael Heerdegen 1 sibling, 1 reply; 32+ messages in thread From: Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-09-15 12:50 UTC (permalink / raw) To: Michael Heerdegen; +Cc: Adam Porter, Nicolas Petton, 47368 [-- 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 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN 2021-09-15 12:50 ` Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 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 0 siblings, 1 reply; 32+ messages in thread From: Michael Heerdegen @ 2021-09-15 21:55 UTC (permalink / raw) To: Basil L. Contovounesios; +Cc: Adam Porter, Nicolas Petton, 47368 "Basil L. Contovounesios" <contovob@tcd.ie> writes: > How's the attached? From what I understand, looks all good. Regards, Michael. ^ permalink raw reply [flat|nested] 32+ messages in thread
* bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN 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 0 siblings, 0 replies; 32+ messages in thread From: Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-09-21 12:42 UTC (permalink / raw) To: Michael Heerdegen; +Cc: Adam Porter, 47368-done, Nicolas Petton close 47368 28.1 quit Michael Heerdegen [2021-09-15 23:55 +0200] wrote: > "Basil L. Contovounesios" <contovob@tcd.ie> writes: >> How's the attached? > From what I understand, looks all good. Thanks. There were no further comments, so I pushed the patch, and am therefore closing this report. Consistently test alist keys with equal in map.el 14495e33af 2021-09-21 13:32:49 +0100 https://git.sv.gnu.org/cgit/emacs.git/commit/?id=14495e33afa0b8038c494f1e6e90065683ccbd07 -- Basil ^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2021-09-21 12:42 UTC | newest] Thread overview: 32+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 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 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
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.