bug#34708: alist-get has unclear documentation

[Drew Adams <drew.adams@oracle.com>]

> > Alists can be handled in more than one way when
> > setting and deleting keys.  The doc needs to tell
> > us what `setf' with `alist-get' does to realize
> > these things.
> 
> I agree.
> I'm in the middle of preparing a patch.  While doing that, two questions
> arose:
> 
> (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?

> (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?

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.

Likewise, if we weren't trying to provide something
for alists that lets you use alists as alists (!)
then I suppose we wouldn't need to worry about any
of it.  IOW, if we were limiting `alist-get' to
hash table-like behavior (e.g. a single entry per
key) then none of these questions would need answers.

But if we are really trying to provide _alist_
manipulation then yes, I think we need to consider
such things - find reasonable ways to provide for
all of the usual alist manipulations.

I'm not sure what the motivation for `alist-get'
is/was.  I suppose it was to provide a simpler way
to interact with (manipulate) alists.

It's not clear to me that there is a simpler way
than what we've always been doing, IF, that is, we
try to provide for setting, testing, and removing,
in all their generality, and not just getting.

IOW, I'm thinking that _getting_ an alist entry is
pretty straightforward (though even in that case
someone might want to get the 2nd or Nth such,
and not just the first), but that setting and
removing are not necessarily so straightforward.

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.

> but if you somehow manage to add the same
> cons (in the sense of `eq') to the same alist, all
> of them are removed.  Compare e.g.
> 
> (progn
>   (setq my-alist '((a . 1) (a . 1) (b . 2)))
>   (setf (alist-get 'a my-alist nil 'remove) nil))
>   ;; my-alist ==> ((a . 1) (b . 2))
>   vs.
> (progn
>   (setq my-alist '((a . 1) (b . 2)))
>   (push (car my-alist) my-alist)
>   ;; my-alist ==> (#1=(a . 1) #1# (b . 2))
>   (setf (alist-get 'a my-alist nil 'remove) nil))
>   ;; my-alist ==> ((b . 2))
> 
> This is because the code uses delq to delete a found cons, and delq
> removes all `eq' elements.
> 
> Is it worth to document or change that?

Sounds like an implementation/design artifact.
If that will stay as part of the design then yes,
I'd say such behavior needs to be documented.