bug#27584: 26.0.50; alist-get: Add optional arg TESTFN

[Eli Zaretskii]

> From: Tino Calancha <tino.calancha@gmail.com>
> Date: Fri, 07 Jul 2017 15:48:01 +0900
> Cc: Nicolas Petton <nicolas@petton.fr>,
>       Stefan Monnier <monnier@IRO.UMontreal.CA>

Thanks.  A few comments about the documentation parts:

> -@defun alist-get key alist &optional default remove
> -This function is like @code{assq}, but instead of returning the entire
> +@defun alist-get key alist &optional default remove testfn
> +This function is like @code{assq} when @var{testfn} is @code{nil},
> +but instead of returning the entire
>  association for @var{key} in @var{alist},
>  @w{@code{(@var{key} . @var{value})}}, it returns just the @var{value}.
> +When @var{testfn} is non-@code{nil}, it returns @var{value} if @var{key}
> +is equal to the car of an element of @var{alist}.  The equality is
> +tested with @var{testfn}.
>  If @var{key} is not found in @var{alist}, it returns @var{default}.

Sometimes, trying to make small changes to existing documentation
makes the documentation less readable and even confusing.  This is one
of those cases: where previously alist-get was only a minor deviation
from assq, and thus just mentioning those deviations would do, now the
deviations are much more significant, and the reference to assq gets
in the way instead of helping.  So I would rewrite the documentation
like this:

  @defun alist-get key alist &optional default remove testfn
  This function is similar to @code{assq}.  It finds the first
  association @w{@code{(@var{key} . @var{value})}} by comparing
  @var{key} with @var{alist} elements, and, if found, returns the
  @var{value} of that association.  If no association is found, the
  function returns @var{default}.  Comparison of @var{key} against
  @var{alist} elements uses the function specified by @var{testfn},
  defaulting to @code{eq}.

  The return value is a generalized variable (@pxref{Generalized
  Variables}) that can be used to change a value with @code{setf}.  When
  using it to set a value, optional argument @var{remove} non-@code{nil}
  means to remove @var{key}'s association from @var{alist} if the new
  value is @code{eql} to @var{default}.
  @end defun

> -@defun assoc-default key alist &optional test default
> +@defun assoc-default key alist &optional test default full
>  This function searches @var{alist} for a match for @var{key}.  For each
>  element of @var{alist}, it compares the element (if it is an atom) or
>  the element's @sc{car} (if it is a cons) against @var{key}, by calling
> @@ -1652,7 +1656,8 @@ Association Lists
>  
>  If an alist element matches @var{key} by this criterion,
>  then @code{assoc-default} returns a value based on this element.
> -If the element is a cons, then the value is the element's @sc{cdr}.
> +If the element is a cons, then the value is the element if @var{full}
> +is non-@code{nil}, or the element's @sc{cdr} if @var{full} is @code{nil}.

Suggest to simplify:

  If the element is a cons, then the value is the element's @sc{cdr}
  if @var{full} is @code{nil} or omitted, or the entire element
  otherwise.

> -(defun map-elt (map key &optional default)
> +(defun map-elt (map key &optional default testfn)
>    "Lookup KEY in MAP and return its associated value.
>  If KEY is not found, return DEFAULT which defaults to nil.
>  
> -If MAP is a list, `eql' is used to lookup KEY.
> +If MAP is a list, TESTFN is used to lookup KEY if non-nil or `eql' if nil.

Since the sentence references more than one argument, the "or `eql' if
nil" part is ambiguous.  Suggest to disambiguate:

  If MAP is a list, `eql' is used to lookup KEY.  Optional argument
  TESTFN, if non-nil, means use its function definition instead of
  `eql'.

> -(defmacro map-put (map key value)
> +(defmacro map-put (map key value &optional testfn)
>    "Associate KEY with VALUE in MAP and return VALUE.
>  If KEY is already present in MAP, replace the associated value
>  with VALUE.
> +When MAP is a list, test equality with TESTFN if non-nil, otherwise use 
> `eql'.

Likewise here.

> -(defun assoc-default (key alist &optional test default)
> +(defun assoc-default (key alist &optional test default full)
>    "Find object KEY in a pseudo-alist ALIST.
>  ALIST is a list of conses or objects.  Each element
>   (or the element's car, if it is a cons) is compared with KEY by
>   calling TEST, with two arguments: (i) the element or its car,
>   and (ii) KEY.
>  If that is non-nil, the element matches; then `assoc-default'
> - returns the element's cdr, if it is a cons, or DEFAULT if the
> - element is not a cons.
> + returns the element, if it is a cons and FULL is non-nil,
> + or the element's cdr, if it is a cons and FULL is nil,
                             ^^
That "it" is ambiguous: does it refer to "element" or to "cdr"?

> -(defun alist-get (key alist &optional default remove)
> -  "Return the value associated with KEY in ALIST, using `assq'.
> +(defun alist-get (key alist &optional default remove testfn)
> +  "Return the value associated with KEY in ALIST.
>  If KEY is not found in ALIST, return DEFAULT.
> +Use TESTFN to lookup in the alist if non-nil.  Otherwise, use `assq'.

Again, "if non-nil" is ambiguous: it could refer to TESTFN or to
alist.

> +@defun assoc-predicate key alist &optional pred
> +This function is like @code{assoc} in that it returns the first
> +association for @var{key} in @var{alist}, but if @code{pred} is
> +non-@code{nil}, then it makes the comparison using @code{pred}
> +instead of @code{equal}.  @code{assoc-predicate} returns @code{nil}
> +if no association in @var{alist} has a @sc{car}, @var{x}, satisfying
> +@code{(funcall pred x key)}.
          ^^^^^^^^^^^^^^^^^^
"pred", "x", and "key" should be in @var here.  I'd also include the
entire @code snippet in @w{..}, so that it won't be split between two
lines.

> ++++
> +** New defun 'assoc-predicate', like 'assoc' with an optional argument
> +PRED, a predicate to compare the elements in the alist.

Please use "function" in NEWS, not "defun".

> +(defun assoc-predicate (key alist &optional pred)
> +  "Like `assoc' but compare keys with TEST."
                                         ^^^^
PRED, not TEST.

Thanks.