GNU bug report logs - #35206
[PATCH] Misleading `list-get' argument description

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 35206 in the body.
You can then email your comments to 35206 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: Mattias Engdegård <mattiase <at> acm.org>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Misleading `list-get' argument description
Date: Tue, 9 Apr 2019 12:34:09 +0200

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

The doc string for `list-get' says

  Use TESTFN to lookup in the alist if non-nil.  Otherwise, use `assq'.

which is misleading since it's an equality predicate, not a look-up function, and the default is `eq', not `assq'.
How about changing it to

  Equality is defined by TESTFN or by `eq' if nil or omitted.

[0001-Clarify-the-TESTFN-argument-to-alist-get.patch (application/octet-stream, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mattias Engdegård <mattiase <at> acm.org>
Cc: 35206 <at> debbugs.gnu.org
Subject: Re: bug#35206: [PATCH] Misleading `list-get' argument description
Date: Tue, 09 Apr 2019 14:00:29 +0300

> From: Mattias Engdegård <mattiase <at> acm.org>
> Date: Tue, 9 Apr 2019 12:34:09 +0200
> 
> The doc string for `list-get' says
> 
>   Use TESTFN to lookup in the alist if non-nil.  Otherwise, use `assq'.
> 
> which is misleading since it's an equality predicate, not a look-up function, and the default is `eq', not `assq'.
> How about changing it to
> 
>   Equality is defined by TESTFN or by `eq' if nil or omitted.

IMO, this has a problem similar to the original text: it has nothing
to "connect" it to the main part of the doc string, which is this:

  Return the value associated with KEY in ALIST.

Neither "look up" nor "equality" is directly and obviously related to
"associated with".  E.g., you probably like "equality" better because
your mental model of "associated with" is a test for equality;
however, someone else might prefer the current text because they think
of "looking up" in the same situation.  But the text doesn't make the
relation explicit, and that is IMO its main problem.

Can you think of a change that would resolve this problem?

Thanks.

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

From: Mattias Engdegård <mattiase <at> acm.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 35206 <at> debbugs.gnu.org
Subject: Re: bug#35206: [PATCH] Misleading `list-get' argument description
Date: Tue, 9 Apr 2019 13:26:09 +0200

9 apr. 2019 kl. 13.00 skrev Eli Zaretskii <eliz <at> gnu.org>:
> 
> Neither "look up" nor "equality" is directly and obviously related to
> "associated with".  E.g., you probably like "equality" better because
> your mental model of "associated with" is a test for equality;
> however, someone else might prefer the current text because they think
> of "looking up" in the same situation.  But the text doesn't make the
> relation explicit, and that is IMO its main problem.

Good point. What about:

Comparison of KEY against the car of each ALIST element
is made using TESTFN, or `eq' if nil or omitted.

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

From: Mattias Engdegård <mattiase <at> acm.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 35206 <at> debbugs.gnu.org
Subject: Re: bug#35206: [PATCH] Misleading `list-get' argument description
Date: Tue, 9 Apr 2019 13:45:34 +0200

> 
> Comparison of KEY against the car of each ALIST element
> is made using TESTFN, or `eq' if nil or omitted.

If no knowledge whatsoever of alists can be assumed on the part of the reader, perhaps this would be better:

   "Return the value associated with KEY in ALIST.
+The value is the cdr of the first element in ALIST whose car is equal to KEY.
 If KEY is not found in ALIST, return DEFAULT.
-Use TESTFN to lookup in the alist if non-nil.  Otherwise, use `assq'.
+Equality is defined by TESTFN or by `eq' if nil or omitted.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mattias Engdegård <mattiase <at> acm.org>
Cc: 35206 <at> debbugs.gnu.org
Subject: Re: bug#35206: [PATCH] Misleading `list-get' argument description
Date: Tue, 09 Apr 2019 17:41:28 +0300

> From: Mattias Engdegård <mattiase <at> acm.org>
> Date: Tue, 9 Apr 2019 13:45:34 +0200
> Cc: 35206 <at> debbugs.gnu.org
> 
> > 
> > Comparison of KEY against the car of each ALIST element
> > is made using TESTFN, or `eq' if nil or omitted.
> 
> If no knowledge whatsoever of alists can be assumed on the part of the reader, perhaps this would be better:
> 
>    "Return the value associated with KEY in ALIST.
> +The value is the cdr of the first element in ALIST whose car is equal to KEY.
>  If KEY is not found in ALIST, return DEFAULT.
> -Use TESTFN to lookup in the alist if non-nil.  Otherwise, use `assq'.
> +Equality is defined by TESTFN or by `eq' if nil or omitted.

This is much better, IMO.  It is better than your previous proposal,
because the text is simpler and thus more clear.

However, a slight rewording would improve it even more:

  Find an element of ALIST whose `car' equals KEY and return its `cdr'.
  ...
  Equality with KEY is tested by TESTFN, defaulting to `eq'.

IMO, this isn't about assuming knowledge, this is about being as
explicit as reasonably possible about what the function does.
(Strictly speaking, both your suggestion and mine still assume some
knowledge about alists, because we never explain what is an alist, nor
what is an "element" of an alist.)

Thanks.

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

From: Mattias Engdegård <mattiase <at> acm.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 35206-done <at> debbugs.gnu.org
Subject: Re: bug#35206: [PATCH] Misleading `list-get' argument description
Date: Tue, 9 Apr 2019 16:59:48 +0200

9 apr. 2019 kl. 16.41 skrev Eli Zaretskii <eliz <at> gnu.org>:
> 
>  Find an element of ALIST whose `car' equals KEY and return its `cdr'.
>  ...
>  Equality with KEY is tested by TESTFN, defaulting to `eq'.

Thank you, I pushed that change (except that I used "the first element" instead of "an element" for precision).

> IMO, this isn't about assuming knowledge, this is about being as
> explicit as reasonably possible about what the function does.
> (Strictly speaking, both your suggestion and mine still assume some
> knowledge about alists, because we never explain what is an alist, nor
> what is an "element" of an alist.)

That's all right -- I was mainly concerned with the quite misleading TESTFN description, and looked at the docs of similar functions (assq, assoc).

Previous Next

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