Subject: bug#34708: alist-get has unclear documentation

From: Michael Heerdegen <[email-masked]>
Date: Mon, 11 Mar 2019 17:19:51 +0100

Drew Adams <[email-masked]> writes:

> > (1) "When using it to set a value, optional argument REMOVE non-nil
> > means to remove KEY from ALIST if the new value is `eql' to DEFAULT."
> >
> > I wonder if there are use cases where the user wants something different
> > than `eql'?  E.g. `equal' when the associations are strings?  Note that
> > this is something different than TESTFN which is for comparing keys.
>
> I think so, yes.  Why wouldn't we want to allow that?

To not add one more argument?

If we do that, I guess I would rather allow that the non-nil value of
REMOVE is allowed to be a function.  A related use case would be to
allow to delete the association of a key independently from associated
value.

> > (2) The remove feature has a strange corner case.  Normally the
> > first found association is removed,
>
> So "normally" it's really "remove one".
>
> Why is this?  What's the point of REMOVE - why is
> it needed (added to the design) at all?  Is it to
> provide a way to remove all entries with a given
> key or only the first such?

The first.

> If we want to provide for pretty much everything
> that one typically does with an alist (without
> `alist-get') then don't we need to provide for the
> ability to do any kind of removal - or other
> operations - on alist elements that have a given key?
>
> Should "set" and "remove" operations not (at least
> be able to) obtain the _full_ sequence (in entry
> order) of such matching elements, and then perform
> specific operations on that sequence (such as setting
> or removing the first one, the Nth one, or all of
> them)?
>
> If we were not trying to allow `alist-get' to be
> usable as a generalized variable then I suppose
> we wouldn't need to worry about any of this.

We tried.  I think the result should be consistent and convenient, but
we don't need to implement all realizations of all operations for the
generalized variable.

One thing I don't find consistent is the case where the alist already
has multiple occurrences of a key.  E.g.

(setq my-alist '((a . 1) (b . 2) (a . -1)))
(setf (alist-get 'a my-alist 1 'remove) 1)
my-alist ==> ((b . 2) (a . -1))

(alist-get 'a my-alist 1)
  ==> -1    (!)

One would expect 1, of course.

> It would be good to see a statement/spec of what
> `alist-get' is trying to accomplish, especially
> wrt setting, testing (diff predicates), and
> removing.

Yes, this is what my patch will try to accomplish.

Michael.