bug#34708: alist-get has unclear documentation

[Michael Heerdegen <michael_heerdegen@web.de>]

Drew Adams <drew.adams@oracle.com> writes:

> > > > 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?
>
> An _optional_ arg.  Why wouldn't we want it?

Because it would be one more argument that only has a meaning in place
expressions.

> BTW, how does `alist-get' deal with a value that is
> a list: `(a 1)' instead of `(a . 1)'?  I guess it
> just considers the VALUE to be `(1)'.

Yes.

> If so, is `eql' really appropriate for most such cases (which are
> quite common)?
>
> And even for `(a . (1 2))', aka `(a 1 2)', is `eql' appropriate?
> Since the cdr of a list is more typically a list, why would we choose
> `eql' as the default value-comparison predicate?  To compare lists the
> default predicate should be `equal' (or possibly but not likely `eq'),
> no?

Yes, in these cases eql is not useful.

> > > > (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.
>
> Then why did (does?) the doc say "if the new value
> is `eql' to DEFAULT"?  It sounds like it removes
> only the entries with a given key AND a given value.

It removes the first entry with given KEY and value eql DEFAULT.  BTW; I
suggest to have a look at the code in gv.el.

> Anyway, if that's all REMOVE does (removes all
> occurrences), and if it can be a predicate, then it
> sounds like it just does `cl-delete-if'.
>
> If so, what's an example of why/when someone would
> want to use `setf' and `alist-get' to remove entries,
> as opposed to just using `cl-delete-if'?

You can also cl-delete-if, yes, there semantics overlap.  OTOH
cl-delete-if doesn't support generalized variables.

> Then isn't it a bit misleading for the function name
> and doc to suggest that this is a general way of using
> alists?

I don't get that impression from the doc.  For the name: what else would
you suggest?  This _is_ the general way of getting associations from an
alist.  You can also use it in place expressions where it is convenient,
you don't have to, of course.  It's in the nature of place expressions
that there use is a bit limited compared to what you could do with
general lisp.  I see no problem here.

> So far, I guess I don't see the use case for making it a generalized
> variable.  It's easy enough to set alist values, and doing so with
> `setf' and `alist-get' sounds more complicated, not less.
>
> For getting, I think I get it: folks apparently don't want to get the
> full element and then dig out the value (cdr) from it.  (Is there more
> to it than that?)  For setting and removing, I don't get the
> advantage, so far.

You only seem to think of one case: I have a variable x which holds an
alist which I want to manipulate.  There are more cases: the alist place
could be a real nontrivial place.  And you can not only use setf on
alist-get places, but all macros that handle places, e.g. incf or callf
or letf or ...  There are surely cases where using alist-get in a place
expression can be handy.  Nowhere is said that you should use alist-get
only and always when dealing with alists, especially for manipulation.

> > 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.
>
> Why?  The doc says that it returns DEFAULT only
> if KEY is not found in ALIST.  But entry (a . -1)
> finds `a' associated with -1.  What am I missing?

Think from the viewpoint of places: I have set the place to 1.  Then I
expect that I get 1 when evaluating the place expression afterwards.

> (setq my-alist  (cl-delete-if
>                   (lambda (entry)
>                     (and (eql (car entry 'a))
>                          (eql (cdr entry 1))))
>                   my-alist))
>
> more straightforward than this:
>
> (setf (alist-get 'a my-alist 1 'remove) 1)?

Depends on your taste, I guess?

> `alist-get' specifies an alist entry (a single one, BTW).  `setf' of
> `alist-get' should set/create an alist entry (a single one, BTW).
> Otherwise, we're abusing the intention of one or both of these
> "functions".  No?

I indeed see that point different, yes.  The remove syntax when using it
as a place is not super sexy (no one says you have to use it for that
btw), but I don't see what you write as a requirement.  When not using
REMOVE it's all very straightforward in my opinion.


Michael.