bug#34708: alist-get has unclear documentation

bug#34708: alist-get has unclear documentation

[Miguel V. S. Frasson]

Hi

In most recent subr.el (git repository, lisp folder), this is the
definition of alist-get:

(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'.

This is a generalized variable suitable for use with `setf'.
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."
  (ignore remove) ;;Silence byte-compiler.
  (let ((x (if (not testfn)
               (assq key alist)
             (assoc key alist testfn))))
    (if x (cdr x) default)))

* Last paragraph starts with `This'. What is `this'? ALIST? TESTFN?
alist-get itself? Since this doc-string is there for a long time, it
may be the case it makes sense and I didn't understand it, but again
in this case, others will not understand as well, unclear doc-string.

* How do I use `this' or `it' to set a value? Function is alist-*get*
but somehow I can set values. A simple example on doc-string and/or
info node would explain everything.

* Action of REMOVE is described, but it doesn't correspond to code.
REMOVE is ignored.

* Probably Elisp info follows misleading doc-string.

Miguel


-- 
Miguel Vinicius Santini Frasson
mvsfrasson@gmail.com

bug#34708: alist-get has unclear documentation

[Michael Heerdegen]

"Miguel V. S. Frasson" <mvsfrasson@gmail.com> writes:

> * Last paragraph starts with `This'. What is `this'? ALIST? TESTFN?
> alist-get itself?

Only one of those makes sense to me.  Do you know what a generalized
variable is?

> * How do I use `this' or `it' to set a value? Function is alist-*get*
> but somehow I can set values. A simple example on doc-string and/or
> info node would explain everything.

That could make sense, since it's an important use case and generalized
variables are probably not something everyone is used to.

> * Action of REMOVE is described, but it doesn't correspond to code.
> REMOVE is ignored.

That's ok: the generalized variable is implemented in gv.el.


Michael.

bug#34708: alist-get has unclear documentation

[Miguel V. S. Frasson]

[-- Attachment #1: Type: text/plain, Size: 1318 bytes --]

Hi Michael

Em sáb, 2 de mar de 2019 06:25, Michael Heerdegen <michael_heerdegen@web.de>
escreveu:

> "Miguel V. S. Frasson" <mvsfrasson@gmail.com> writes:
>
> > * Last paragraph starts with `This'. What is `this'? ALIST? TESTFN?
> > alist-get itself?
>
> Only one of those makes sense to me.  Do you know what a generalized
> variable is?
>

I think so, ALIST, but 'this' should be the last thing that was referred to
or talked about. The point is that the documentation must be clear, and it
is not in this case.

I know about generalized variables but really never used myself.

> * How do I use `this' or `it' to set a value? Function is alist-*get*
> > but somehow I can set values. A simple example on doc-string and/or
> > info node would explain everything.
>
> That could make sense, since it's an important use case and generalized
> variables are probably not something everyone is used to.
>

I can't imagine how to *set* anything with alist-get. It seams to me that
it just use the value of ALIST for look up, so talk about generalized
variables is meaningless to me here.

> * Action of REMOVE is described, but it doesn't correspond to code.
> > REMOVE is ignored.
>
> That's ok: the generalized variable is implemented in gv.el.
>
>
> Michael.
>

Miguel

>

[-- Attachment #2: Type: text/html, Size: 2679 bytes --]

bug#34708: alist-get has unclear documentation

[Michael Heerdegen]

"Miguel V. S. Frasson" <mvsfrasson@gmail.com> writes:

> I can't imagine how to *set* anything with alist-get. It seams to me
> that it just use the value of ALIST for look up, so talk about
> generalized variables is meaningless to me here.

You use it like this: say variable V is bound to an alist, then you can
do (setf (alist-get key V) value).  After that, (alist-get key V) will
evaluate to VALUE, so you have "set" that place.  In the general case, V
can also be a generalized variable, e.g. (car SOMETHING-ELSE).

To replace the word "this" with something better is not so easy.  We
could write "The name of this function can be used to build expressions
that can be used as a generalized variable", but I doubt it will make
things clearer for somebody not familiar with the concept of generalized
variables.  Using this function name to build place expressions is not
different from using other function names that allow to be used for
generalized variables.

I would rather go with an example, which I think is justified because
using this function name in place expressions is the canonical way to
modify alists and people need to use it (there is no `alist-put') no
matter if they are familiar with generalized variables.

Michael.

bug#34708: alist-get has unclear documentation

[Eric Abrahamsen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> "Miguel V. S. Frasson" <mvsfrasson@gmail.com> writes:
>
>> I can't imagine how to *set* anything with alist-get. It seams to me
>> that it just use the value of ALIST for look up, so talk about
>> generalized variables is meaningless to me here.
>
> You use it like this: say variable V is bound to an alist, then you can
> do (setf (alist-get key V) value).  After that, (alist-get key V) will
> evaluate to VALUE, so you have "set" that place.  In the general case, V
> can also be a generalized variable, e.g. (car SOMETHING-ELSE).
>
> To replace the word "this" with something better is not so easy.  We
> could write "The name of this function can be used to build expressions
> that can be used as a generalized variable", but I doubt it will make
> things clearer for somebody not familiar with the concept of generalized
> variables.  Using this function name to build place expressions is not
> different from using other function names that allow to be used for
> generalized variables.

One other phrase you often see here is "setf-able place". I don't know
if that's formally acceptable in docstrings, but it would be much more
comprehensible to say "this form is a setf-able place", and would give
the key hint (setf) as well. It's true it's pretty weird to refer to a
function call as a variable.

> I would rather go with an example, which I think is justified because
> using this function name in place expressions is the canonical way to
> modify alists and people need to use it (there is no `alist-put') no
> matter if they are familiar with generalized variables.

Most definitely, this needs examples. I also agree that the REMOVE usage
needs an example -- I made it work eventually, but it took a fair bit of
experimentation.

Eric

bug#34708: alist-get has unclear documentation

[Miguel V. S. Frasson]

[-- Attachment #1: Type: text/plain, Size: 1531 bytes --]

Hi

Thanks for the explanation. Now it is clear. I use alists a lot. I will use
it better.

Miguel





Em sáb, 2 de mar de 2019 15:10, Michael Heerdegen <michael_heerdegen@web.de>
escreveu:

> "Miguel V. S. Frasson" <mvsfrasson@gmail.com> writes:
>
> > I can't imagine how to *set* anything with alist-get. It seams to me
> > that it just use the value of ALIST for look up, so talk about
> > generalized variables is meaningless to me here.
>
> You use it like this: say variable V is bound to an alist, then you can
> do (setf (alist-get key V) value).  After that, (alist-get key V) will
> evaluate to VALUE, so you have "set" that place.  In the general case, V
> can also be a generalized variable, e.g. (car SOMETHING-ELSE).
>
> To replace the word "this" with something better is not so easy.  We
> could write "The name of this function can be used to build expressions
> that can be used as a generalized variable", but I doubt it will make
> things clearer for somebody not familiar with the concept of generalized
> variables.  Using this function name to build place expressions is not
> different from using other function names that allow to be used for
> generalized variables.
>
> I would rather go with an example, which I think is justified because
> using this function name in place expressions is the canonical way to
> modify alists and people need to use it (there is no `alist-put') no
> matter if they are familiar with generalized variables.
>
> Michael.
>
>
>

[-- Attachment #2: Type: text/html, Size: 2245 bytes --]

bug#34708: alist-get has unclear documentation

[Eric Abrahamsen]

"Miguel V. S. Frasson" <mvsfrasson@gmail.com> writes:

> Hi
>
> Thanks for the explanation. Now it is clear. I use alists a lot. I will use
> it better.

This function is so handy I've pretty much stopped using plists at all.
This one does everything you need.

bug#34708: alist-get has unclear documentation

[Phil Sainty]

On 3/03/19 8:06 AM, Eric Abrahamsen wrote:
> Most definitely, this needs examples. I also agree that the REMOVE
> usage needs an example -- I made it work eventually, but it took a
> fair bit of experimentation.

Agreed.  I think the remove syntax is all but unreadable:

(setf (alist-get KEY LIST t t) t)

to remove items from LIST with key eq to KEY.

Unless accompanied by comments, I do not think that meaning obvious
at all.

This variant gives a better idea...

(setf (alist-get KEY LIST :remove :remove) :remove)

I struggle to imagine a scenario where I wouldn't use some more
explicit syntax for deletion, though; even when the decision to
remove or not is being arrived at dynamically.


-Phil

bug#34708: alist-get has unclear documentation

[Miguel V. S. Frasson]

[-- Attachment #1: Type: text/plain, Size: 1707 bytes --]

Hi

I think the sentence below is a good and short explanation for the
doc-string.

The return value can be conveniently used as a generalized variable (a
place) to set the value associated with KEY in ALIST, like in the example
 (setf (alist-get key alist) new-value)

Miguel


Em sáb, 2 de mar de 2019 15:10, Michael Heerdegen <michael_heerdegen@web.de>
escreveu:

> "Miguel V. S. Frasson" <mvsfrasson@gmail.com> writes:
>
> > I can't imagine how to *set* anything with alist-get. It seams to me
> > that it just use the value of ALIST for look up, so talk about
> > generalized variables is meaningless to me here.
>
> You use it like this: say variable V is bound to an alist, then you can
> do (setf (alist-get key V) value).  After that, (alist-get key V) will
> evaluate to VALUE, so you have "set" that place.  In the general case, V
> can also be a generalized variable, e.g. (car SOMETHING-ELSE).
>
> To replace the word "this" with something better is not so easy.  We
> could write "The name of this function can be used to build expressions
> that can be used as a generalized variable", but I doubt it will make
> things clearer for somebody not familiar with the concept of generalized
> variables.  Using this function name to build place expressions is not
> different from using other function names that allow to be used for
> generalized variables.
>
> I would rather go with an example, which I think is justified because
> using this function name in place expressions is the canonical way to
> modify alists and people need to use it (there is no `alist-put') no
> matter if they are familiar with generalized variables.
>
> Michael.
>
>
>

[-- Attachment #2: Type: text/html, Size: 2345 bytes --]

bug#34708: alist-get has unclear documentation

[Michael Heerdegen]

"Miguel V. S. Frasson" <mvsfrasson@gmail.com> writes:

> Hi
>
> I think the sentence below is a good and short explanation for the
> doc-string.
>
> The return value can be conveniently used as a generalized variable (a
> place) to set the value associated with KEY in ALIST, like in the
> example (setf (alist-get key alist) new-value)

Thanks for the idea.  I don't think we should explain it like this
however, because when evaluating

  (setf (alist-get key alist) new-value)

the function `alist-get' is never called, so there is no return value.
Of course what is sexy about place expressions is that it looks like you
would directly set the result of a function call, but what happens is
that setf doesn't evaluate the call but analyses it and builds and
evaluates code that leads to this result.

Eric suggested to say "this form is a setf-able place" but this also
doesn't answer the question what this (form) is.  `alist-get' is not a
form, it's the name of a function.  In my opinion it would be cleaner to
say something like "the name of this function can be used to build place
expressions" or "can be used in place expressions" or so.  Better ideas
welcome.


Michael.

bug#34708: alist-get has unclear documentation

[Michael Heerdegen]

Phil Sainty <psainty@orcon.net.nz> writes:

> Agreed.  I think the remove syntax is all but unreadable:
>
> (setf (alist-get KEY LIST t t) t)
>
> to remove items from LIST with key eq to KEY.
>
> Unless accompanied by comments, I do not think that meaning obvious
> at all.
>
> This variant gives a better idea...
>
> (setf (alist-get KEY LIST :remove :remove) :remove)

Yes, the syntax is a bit weird.  I think I would prefer to write it as

  (setf (alist-get key my-alist nil 'remove) nil)

The syntax also makes some sense: If you set the association of KEY to
the default that `alist-get' would return when the entry would not be
not found, the entry can be removed.  But I agree we should add such an
example, since not everybody wants to meditate over why this makes sense
in order to remember the syntax.


Michael.

bug#34708: alist-get has unclear documentation

[Drew Adams]

> > I think the sentence below is a good and short explanation for the
> > doc-string.
> >
> > The return value can be conveniently used as a generalized variable

Lose "conveniently", please.

> > (a place) to set the value associated with KEY in ALIST, like in the
> > example (setf (alist-get key alist) new-value)
> 
> Thanks for the idea.  I don't think we should explain it like this
> however, because when evaluating
> 
>   (setf (alist-get key alist) new-value)
> 
> the function `alist-get' is never called, so there is no return value.

Yes.  (But that text didn't say it was called, and
it didn't mention a return value.)

`setf' is a macro.  Its PLACE arg serves as a
_specification_ of a place (a "generalized
variable") whose value is to be set.  And "set"
means create or update.

It's not really about `alist-get' here; it's
about `setf'.  `alist-get' itself has nothing
to do with using a generalized variable.

> Of course what is sexy about place expressions is that it looks like
> you would directly set the result of a function call, but what happens is
> that setf doesn't evaluate the call but analyses it and builds and
> evaluates code that leads to this result.

Yes.  But that's "just" plumbing.  It's not
important to explain that here, I think.

In terms of describing the role of `alist-get'
as a `setf' place it's not relevant, at a first
approximation.

That `setf' doesn't call `alist-get' but instead
analyses the spec and builds code that does the
right thing is not necessary for getting the main
point that `alist-get' can be used with `setf' to
specify an alist element to create or update.

> Eric suggested to say "this form is a setf-able place" but this also
> doesn't answer the question what this (form) is.  `alist-get' is not a
> form, it's the name of a function.  In my opinion it would be cleaner
> to say something like "the name of this function can be used to build
> place expressions" or "can be used in place expressions" or so.
> Better ideas welcome.

Yes wrt the substance (content).  But an active
phrasing is often better than the passive "__ can
be used".  Say what this does by saying what you
can do with it.

 You can use function `alist-get' in a PLACE-expression
 argument to `setf'.  In this role it specifies an
 alist element whose value `setf' sets:

   (setf (alist-get KEY ALIST) NEW-VALUE)

 Here, `setf' sets the value part of an element
 of ALIST whose key is KEY to NEW-VALUE.

It's important to not give the impression that
there must be an _existing_ element with KEY.
Showing an example can help dispel that mistake.

   (setq foo ())
   (setf (alist-get 'a foo) 1
         (alist-get 'b foo) 2)

   C-h v foo ; ==> ((b . 2) (a . 1))

bug#34708: alist-get has unclear documentation

[Eric Abrahamsen]

On 03/03/19 07:51 AM, Drew Adams wrote:

[...]

>> Eric suggested to say "this form is a setf-able place" but this also
>> doesn't answer the question what this (form) is. `alist-get' is not a
>> form, it's the name of a function. In my opinion it would be cleaner
>> to say something like "the name of this function can be used to build
>> place expressions" or "can be used in place expressions" or so.
>> Better ideas welcome.

Well I used "form" because it _is_ the form -- (alist-get KEY ALIST) --
that is the setf-able place, not the function/macro/name of the
function. At least as far as the coder is concerned.

> Yes wrt the substance (content). But an active
> phrasing is often better than the passive "__ can
> be used". Say what this does by saying what you
> can do with it.
>
> You can use function `alist-get' in a PLACE-expression
> argument to `setf'. In this role it specifies an
> alist element whose value `setf' sets:

> (setf (alist-get KEY ALIST) NEW-VALUE)
>
> Here, `setf' sets the value part of an element
> of ALIST whose key is KEY to NEW-VALUE.

But I like the above better anyway!

bug#34708: alist-get has unclear documentation

[Eric Abrahamsen]

On 03/03/19 07:51 AM, Drew Adams wrote:

[...]

> It's important to not give the impression that
> there must be an _existing_ element with KEY.
> Showing an example can help dispel that mistake.
>
> (setq foo ())
> (setf (alist-get 'a foo) 1
> (alist-get 'b foo) 2)
>
> C-h v foo; ==> ((b. 2) (a. 1))

I think it would be nice to have an example that shows both a common
use-case (as an accumulator), and how to use REMOVE. I started off with
the accumulator part:

(let (word word-freq)
  (while (setq word (pop word-list))
    (cl-incf (alist-get word word-freq 0 #'equal) 1)))

But so far haven't come up with a non-contrived way to work REMOVE in
there...

bug#34708: alist-get has unclear documentation

[Michael Heerdegen]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> I think it would be nice to have an example that shows both a common
> use-case (as an accumulator), and how to use REMOVE. I started off with
> the accumulator part:
>
> (let (word word-freq)
>   (while (setq word (pop word-list))
>     (cl-incf (alist-get word word-freq 0 #'equal) 1)))
>
> But so far haven't come up with a non-contrived way to work REMOVE in
> there...

And I guess you didn't really want to specify a function as REMOVE arg,
right?

Michael.

bug#34708: alist-get has unclear documentation

[Eric Abrahamsen]

On 03/04/19 17:38 PM, Michael Heerdegen wrote:
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> I think it would be nice to have an example that shows both a common
>> use-case (as an accumulator), and how to use REMOVE. I started off with
>> the accumulator part:
>>
>> (let (word word-freq)
>>   (while (setq word (pop word-list))
>>     (cl-incf (alist-get word word-freq 0 #'equal) 1)))
>>
>> But so far haven't come up with a non-contrived way to work REMOVE in
>> there...
>
> And I guess you didn't really want to specify a function as REMOVE arg,
> right?

Don't you think it's confusing enough already? :)

Though maybe there could be a second example showing "advanced topics in
REMOVE".

While we're here, you mentioned up-thread that REMOVE isn't intuitive if
you don't understand what the DEFAULT arg is doing. Obviously I don't
understand that: would you elaborate?

Thanks,
Eric

bug#34708: alist-get has unclear documentation

[Michael Heerdegen]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> While we're here, you mentioned up-thread that REMOVE isn't intuitive
> if you don't understand what the DEFAULT arg is doing. Obviously I
> don't understand that: would you elaborate?

Ok, I try to repeat with different words.  Too many words, sorry.  What
I wanted to say was actually simple.

In most cases what (setf (alist-get KEY ...) ...) should do is clear:
change the association of an existing key, or add a key-value
association.

In case of using the DEFAULT argument - we don't yet speak of setf at all
here -

  (alist-get key alist default)

returns DEFAULT if KEY is not found.  But it also returns DEFAULT if KEY
is found in ALIST and happens to be associated with DEFAULT.

This makes a call like

  (setf (alist-get key alist default) default)

ambiguous: the "goal" (which is making (alist-get key alist default)
eval to DEFAULT) can be reached in two ways: (1) by making KEY being
associated with DEFAULT in ALIST and (2) by removing any existing
association for KEY.

You can choose which behavior you want via the REMOVE argument which
comes after the DEFAULT arg: specifying REMOVE non-nil gives you (2) -
remove it - else you get (1).

BTW, a simpler way to delete an association in an alist is

  (setf (alist-get key alist) nil)

This will do most of the time unless it's important that the alist
doesn't grow too much.  Maybe it's because of this that the remove
facility is a bit hidden.

Michael.

bug#34708: alist-get has unclear documentation

[Eric Abrahamsen]

On 03/04/19 19:22 PM, Michael Heerdegen wrote:

[...]

> This makes a call like
>
> (setf (alist-get key alist default) default)
>
> ambiguous: the "goal" (which is making (alist-get key alist default)
> eval to DEFAULT) can be reached in two ways: (1) by making KEY being
> associated with DEFAULT in ALIST and (2) by removing any existing
> association for KEY.
>
> You can choose which behavior you want via the REMOVE argument which
> comes after the DEFAULT arg: specifying REMOVE non-nil gives you (2) -
> remove it - else you get (1).

Thanks for spelling all this out! I guess my confusion is the
interaction of REMOVE with DEFAULT. Why does REMOVE only do anything if
the value being set is equal to the DEFAULT? If they are not equal,
REMOVE is ignored, and the value is set. How does that make sense?

bug#34708: alist-get has unclear documentation

[Michael Heerdegen]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> Thanks for spelling all this out! I guess my confusion is the
> interaction of REMOVE with DEFAULT. Why does REMOVE only do anything
> if the value being set is equal to the DEFAULT? If they are not equal,
> REMOVE is ignored, and the value is set. How does that make sense?

If you do (setf GV V) with some place expression GV and some value V,
you expect that afterwards GV evaluates to V.

If (setf (alist-get key alist nil 'remove) t) would remove the
association of KEY,

  (alist-get key alist nil 'remove)

or

  (alist-get key alist nil)

would not eval to nil, although you have set the place to t.

With other words: removing elements from an alist is something that
doesn't fit 100% to place expressions, so the syntax and semantics you
get are not 100% straightforward.  Not super sexy, but consistent.


Michael.

bug#34708: alist-get has unclear documentation

[Eric Abrahamsen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> Thanks for spelling all this out! I guess my confusion is the
>> interaction of REMOVE with DEFAULT. Why does REMOVE only do anything
>> if the value being set is equal to the DEFAULT? If they are not equal,
>> REMOVE is ignored, and the value is set. How does that make sense?
>
> If you do (setf GV V) with some place expression GV and some value V,
> you expect that afterwards GV evaluates to V.
>
> If (setf (alist-get key alist nil 'remove) t) would remove the
> association of KEY,
>
>   (alist-get key alist nil 'remove)
>
> or
>
>   (alist-get key alist nil)
>
> would not eval to nil, although you have set the place to t.
>
> With other words: removing elements from an alist is something that
> doesn't fit 100% to place expressions, so the syntax and semantics you
> get are not 100% straightforward.  Not super sexy, but consistent.

Okay, I guess that makes sense, thanks. But we still need some more
examples in the docstring!

bug#34708: alist-get has unclear documentation

[Drew Adams]

Getting from an alist the value for a given key is
straightforward, obvious.  So presumably, wrt getting,
`alist-get' doesn't have to say more than that's what
it does.

But _setting_ a value for a given key in an alist is
NOT straightforward.  (An alist is not a hash table.)

Often (typically) it means just consing a new alist
entry on the front of the alist.  (That's pretty much
the main reason to use an alist.)

But you could alternatively first remove one or more
existing entry for that key from the alist and then
add the requested key+value entry.  And if you remove
_all_ such entries first then you don't necessarily
need to add the new entry to the beginning (though
you almost always would, other things being equal).

The ambiguity wrt setting means that the part of
the `alist-get' doc that talks about using it with
`setf', to set the value of the key, needs to be
very clear and correct wrt the implementation.  If
the implementation just tacks on a new entry at the
list beginning, then say so.  If it does something
else then say what that is.

It's not admissible to just say that it sets the
key to the given value.  Why?  Because of how alists
are used.  Code can very easily make use of an alist
if it knows how it is being maintained.  `alist-get'
is a general function, and its doc needs to say
something about how `setf' sets the value (what the
result is).

Similarly wrt removing an alist entry for a given
key.  Does it actually remove all such entries, or
does it just tack on a new entry with value nil at
the beginning of the list?

These things need to be specified in the doc.
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.

A user doesn't necessarily care about the "how"
details, but s?he deserves to know the "what":
What is the result of setting the value of a key
or removing a key?

bug#34708: alist-get has unclear documentation

[Michael Heerdegen]

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

> 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.

(2) The remove feature has a strange corner case.  Normally the first
found association is removed, 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?


Michael.

bug#34708: alist-get has unclear documentation

[Drew Adams]

> > 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.

bug#34708: alist-get has unclear documentation

[Michael Heerdegen]

Drew Adams <drew.adams@oracle.com> 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.

bug#34708: alist-get has unclear documentation

[Drew Adams]

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

An _optional_ arg.  Why wouldn't we want it?

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)'.  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?

> 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.

That'd be OK.  If the second test function would be
only for removal then letting REMOVE be a function
would be fine.  Presumably it would be a predicate,
testing each full entry (the cons that holds both
key and value), not just the 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.

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.

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

I may be missing something.  I'm not familiar with
the whole bug thread and I'm looking at the existing
(old) doc string.

> > 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.

Then isn't it a bit misleading for the function name
and doc to suggest that this is a general way of using
alists?  There is already some misunderstanding out
there about alists, with some folks thinking that there
should only ever be a single entry with a given key
(which is true of a hash table).  Won't this augment
such confusion?

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.

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

But if you don't find it inconsistent then that's
a problem, because many (most, I expect) uses of
alists do have some multiple occurrences of a key.

In any case, what you show is an example of why I
wouldn't think that `setf' with `alist-get' is
simpler.  It may be less verbose sometimes, but it
doesn't seem as clear.

If, as the doc says, it removes only the entries
with a given key AND a given value, then isn't this:

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

Or if, as I think you're saying, it removes all the
entries with a given key, regardless of the values,
then just this:

(setq my-alist  (cl-delete-if
                  (lambda (entry) (eql (car entry 'a)))
                  my-alist))

I find the `setf' with `remove' and double 1's to be
confusing.  It looks like it removes all entries for
key `a' that have value 1, and then it _creates_ an
entry (a . 1).  I know that it doesn't do that, but
to me that's what it looks like it's saying.

If there really is a good use case for `alist-get'
to be a generalized variable, and for that to let
you remove entries and not just set/create entries,
then it seems like a better syntax could be found.

FWIW, to me the whole remove thing seems to fly in
the face of what `alist-get' and `setf' are about.
With REMOVE `setf' is NOT setting an alist element.
Instead, it's changing the alist structure - it's
not acting on elements of the list.

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

> > 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.

Great.  Thx.

bug#34708: alist-get has unclear documentation

[Michael Heerdegen]

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.

bug#34708: alist-get has unclear documentation

[Michael Heerdegen]

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

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

BTW with a different viewpoint, when you use

 (setcdr (assoc 'a my-alist) 17)

on the above degenerated alist you also change _both_ of the 'a
associations, so one could argue that the `alist-get' setter behaves
correctly when removing both: there are not two 'a associations in
MY-ALIST but the same one has just been added two times, so it's correct
to remove both of them.

Michael.

bug#34708: alist-get has unclear documentation

[Drew Adams]

> > 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.

But they are the more common cases, I expect.
The cdr of a list is typically a list, not an
atom.

And even for the elements of an alist I expect
that is more typical: a list of values (as VALUE
of a KEY) is more general and typically more
useful than a single, atomic value.

If this is the case then it's an argument for
the default being set to `equal', not `eql'.

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

[FYI: "The first such", above, meant the first
entry with a given key - "such" refers to
"entries with a given key".  It doesn't mean the
first entry with a given key and value.  I guess
I should have made that more explicit.]

My question in that case is why REMOVE is made
to work this way?  Normally, alist element
lookup/identification is by key only (or value
only).  To look up both key and value it's
`member', not `assoc' (or `rassoc').

That's an operation on general list elements;
it's not so much an alist operation.  Why is
removing an association different in this
regard from adding or changing an association?
Why does it need both key and value?

I'm still having trouble grasping the point of
having REMOVE in this construct.  Seems like a
ball of confusion and behavior not specifically
related to alists.

> > 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.

At any rate, that example was predicated on my wrong
guess that REMOVE removes all, not just the first,
identified instance, which you've now clarified
is not the case.

> > 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.

I find it misleading, but people can see things
differently.

Getting an association by both key and value
is _not_ the general way of getting an alist
association.  Getting it by key only is the
usual way.  Getting it by value only is another,
less common way.

`alist-get' by itself seems to act normally wrt
alists.  But its use with `setf' and REMOVE
doesn't seem, to me, to reflect the usual alist
manipulation.
 
> > 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.

How is that a different case?  But maybe I
don't have understand what you have in mind
by a real nontrivial place.  Presumably
you're talking about an alist with complex
elements?  How does that change things? Could
you give an example that shows what you mean?

> 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.

OK.  An example would still help.  The `setf'
examples we've seen so far don't seem very
helpful, to me.  But maybe an example with
some other `*f' place-manipulation functions
would help understanding.

I think it's mostly REMOVE that bothers me.
I expect an alist "place" operation to set
(create or update) an alist entry, not to
remove an entry, and especially not by
specifying that entry by both key and value.

> > > 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.

You have _not_ set the place to 1, have you?
The second 1, combined with REMOVE, doesn't
set the place at all, does it?  Doesn't REMOVE
just remove the place altogether?

I think that the REMOVE "feature" gets in the
way of understanding here.  I think you're
agreeing, in a sense, by saying that the sexp
gives the _impression_ that it sets the place
to 1 without actually doing so.  No?

> > `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.

Let's consider just REMOVE then.

bug#34708: alist-get has unclear documentation

[Drew Adams]

> > > (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.
> 
> BTW with a different viewpoint, when you use
>  (setcdr (assoc 'a my-alist) 17)
> 
> on the above degenerated alist you also change _both_ of the 'a
> associations, so one could argue that the `alist-get' setter behaves
> correctly when removing both: there are not two 'a associations in
> MY-ALIST but the same one has just been added two times, so it's correct
> to remove both of them.

OK.  Put it differently: it's worth documenting
that updating an alist entry with `setf' is a
"destructive" operation: it can change list
structure.  Dunno whether that is already said
somewhere, but even if it is, a reminder wouldn't
hurt.

bug#34708: alist-get has unclear documentation

[Michael Heerdegen]

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

> OK.  Put it differently: it's worth documenting that updating an alist
> entry with `setf' is a "destructive" operation: it can change list
> structure.  Dunno whether that is already said somewhere, but even if
> it is, a reminder wouldn't hurt.

But isn't that trivial?  How else could we add associations?

Michael.

bug#34708: alist-get has unclear documentation

[Michael Heerdegen]

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

> > Yes, in these cases eql is not useful.
>
> But they are the more common cases, I expect.
> The cdr of a list is typically a list, not an
> atom.
>
> If this is the case then it's an argument for
> the default being set to `equal', not `eql'.

An argument against changing the default is that `equal' is slower.

> My question in that case is why REMOVE is made
> to work this way?  Normally, alist element
> lookup/identification is by key only (or value
> only).  To look up both key and value it's
> `member', not `assoc' (or `rassoc').

As I said: otherwise it would not make sense with `setf'.

I think you have a wrong expectation of what the implementation of
alist-get as a place expression is trying to achieve: It doesn't want to
implement each and every way of modification operations for alists.  It
simply implements the most straightforward way of what setting such a
place could mean.  First of all the expected semantics of setf should
hold.

In one, special, case - when setting the place to the default - setting
the place can be achieved by removing an association from the alist
(because after removing the association DEFAULT is returned by
evaluating the place expression), but it's only done if explicitly
requested by providing an extra argument.  If you don't like using it
like that, just use something else.

> That's an operation on general list elements; it's not so much an
> alist operation.  Why is removing an association different in this
> regard from adding or changing an association?  Why does it need both
> key and value?

As I said: because setf requires you to specify a value, and it only
makes sense to remove an association if this value is the DEFAULT.  Else
the value in (setf (alist-get ...) val) would be ignored.  That would be
nonsense.

> > 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.
>
> How is that a different case?  But maybe I
> don't have understand what you have in mind
> by a real nontrivial place.  Presumably
> you're talking about an alist with complex
> elements?

No, I was talking about complex (nested) place expressions referring to
an alist.


Michael.

bug#34708: alist-get has unclear documentation

[Drew Adams]

> > OK.  Put it differently: it's worth documenting that updating an alist
> > entry with `setf' is a "destructive" operation: it can change list
> > structure.  Dunno whether that is already said somewhere, but even if
> > it is, a reminder wouldn't hurt.
> 
> But isn't that trivial?  How else could we add associations?

Yes, it's true of `setf' in general.  It's still
worth repeating, I think (just one opinion).
One person's "trivial" is another's "gotcha".

But apparently we don't even point this out in
the Elisp manual doc for `setf', alas.

And even for `setcdr' etc., the manual doesn't
repeat it.  It mentions it only in the parent
node, `Modifying Existing List Structure'.

`setf' is not mentioned in that node, BTW.
Perhaps some mention would make sense, saying
that when the place to be modified is a list
the list structure can be modified - it is a
so-called "destructive" operation.

"Destructive" is mentioned in node `Rearrangement',
however.  Why it's not mentioned in nodes `Setcar'
and `Setcdr' I don't know.  (Sure, those nodes do
make clear that list structure can be modified.
But they don't specifically call them "destructive"
operations.)

I suggest we document explicitly for `alist-get'
that using it as a generalized variable is a
"destructive" operation - in both the doc string
and node `Association Lists'.

Whenever we say of some function that it "is a
generalized variable suitable for use with
`setf'", I think we should add that using it to
modify a list is a "destructive" operation, i.e.,
it can change list structure.

We should be a bit more consistent.  We say
`delq' is a "destructive" operation, but we
don't say that explicitly about `delete' with
a list (node `Sets and Lists').  (We do say it
indirectly.)

I'm not crazy about the term "destructive" for
changes to list structure, BTW.  But that's the
term Emacs Lisp doc has chosen to use.  As such,
we should call it out everywhere it's applicable
- or nowhere.  It is not the case, for example,
that `setcdr' is more "destructive" than `setf'
+ `alist-get'.

Some get so excited about such "destruction"
that they make a big deal about strongly
discouraging users from using such functions.
I don't.  But I do think it's worth explicitly
making users aware of which functions can
change list structure (as well as what that
means - the consequences).

bug#34708: alist-get has unclear documentation

[Drew Adams]

> > > Yes, in these cases eql is not useful.
> >
> > But they are the more common cases, I expect.
> > The cdr of a list is typically a list, not an
> > atom.
> >
> > If this is the case then it's an argument for
> > the default being set to `equal', not `eql'.
> 
> An argument against changing the default is that `equal' is slower.

I don't think such an argument would be a strong
reason.

And it's not even true for atoms.  It is true for
lists.  But in that case `equal' is much more
useful, I think (and I think you agreed, above),
and is less surprising (a potential gotcha).

> > My question in that case is why REMOVE is made
> > to work this way?  Normally, alist element
> > lookup/identification is by key only (or value
> > only).  To look up both key and value it's
> > `member', not `assoc' (or `rassoc').
> 
> As I said: otherwise it would not make sense with `setf'.

I missed where you said that, I guess.  But what
is your reason?  Why doesn't it make sense for
an alist to identify an element by just its key?
Why expect both key and value for only this case
(removal)?  I missed your reason for that.  (But
I see you go into this a bit below, so see below.)

> I think you have a wrong expectation of what the implementation of
> alist-get as a place expression is trying to achieve: It doesn't want to
> implement each and every way of modification operations for alists.  It
> simply implements the most straightforward way of what setting such a
> place could mean.  First of all the expected semantics of setf should
> hold.

How does "the expected semantics of setf"
conflict with what I'm saying?  How does it
require the implementation (design really) of
`alist-get' that you support?  Maybe you're
right, but I don't see an argument supporting
that.

> > That's an operation on general list elements; it's not so much an
> > alist operation.  Why is removing an association different in this
> > regard from adding or changing an association?  Why does it need both
> > key and value?
> 
> As I said: because setf requires you to specify a value, and it only
> makes sense to remove an association if this value is the DEFAULT.  Else
> the value in (setf (alist-get ...) val) would be ignored.  That would be
> nonsense.

This is why I said that removal is not setting
an `alist-get' place.  Setting the value to
DEFAULT would be setting the place.  But removing
the entry is not.

Why have REMOVE instead of just letting users
set the value (explicitly) to DEFAULT?  That
would really follow "the expected semantics of
setf", with nothing funny going on, no confusion.

`alist-get' is a function, not just a gv place.
Argument REMOVE makes no sense for the function
(does it?).  All the other args do make sense
for the function.  Shouldn't all of the args to
a function make sense for it?  Should we add
args that are only used by `setf'?  Has that
been done before?

Adding REMOVE seems like a mistake, to me.  I
suppose I'm kind of acting as devil's advocate
here, but I really do think, so far, that this
is something odd and confusing.  I'm looking
for a good reason.  If I hear it then it will
likely all make sense to me.

> > > 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.
> >
> > How is that a different case?  But maybe I
> > don't have understand what you have in mind
> > by a real nontrivial place.  Presumably
> > you're talking about an alist with complex
> > elements?
> 
> No, I was talking about complex (nested) place expressions referring to
> an alist.

I still don't follow; sorry.  I guess you mean
an alist embedded in some other list structure?
Could you give an example, showing the advantage
of having REMOVE?

If you think I'm not helping then I'll keep quiet.
I'm hoping that by trying to understand the
rationale I'm helping a bit.

bug#34708: alist-get has unclear documentation

[Michael Heerdegen]

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

> Why have REMOVE instead of just letting users
> set the value (explicitly) to DEFAULT?

That's the default behavior without using REMOVE.  Removing associations
is also a valid use case and in my eyes still consistent with the place
semantics.

> That would really follow "the expected semantics of setf", with
> nothing funny going on, no confusion.
>
> `alist-get' is a function, not just a gv place.  Argument REMOVE makes
> no sense for the function (does it?).  All the other args do make
> sense for the function.  Shouldn't all of the args to a function make
> sense for it?  Should we add args that are only used by `setf'?  Has
> that been done before?

Ok, REMOVE is more or less your only criticism.  I too find it a bit
odd, especially the additional argument, but not too odd and useful
enough to keep it.  It's very debatable, agreed.


Michael.

bug#34708: alist-get has unclear documentation

[Michael Heerdegen]

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

> > But isn't that trivial?  How else could we add associations?
>
> Yes, it's true of `setf' in general.  It's still worth repeating, I
> think (just one opinion).  One person's "trivial" is another's
> "gotcha".

Actually, it's not even trivial at all.  Setting alist-get as place
could also build a new alist, leaving the one stored in the ALIST place
intact, and set place ALIST to the new list.  But it potentially
modifies the list stored in place ALIST.  That's not inevitable, so it
could be worth telling that.

Michael.

bug#34708: alist-get has unclear documentation

[Michael Heerdegen]

[-- Attachment #1: Type: text/plain, Size: 644 bytes --]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Actually, it's not even trivial at all.  Setting alist-get as place
> could also build a new alist, leaving the one stored in the ALIST place
> intact, and set place ALIST to the new list.  But it potentially
> modifies the list stored in place ALIST.  That's not inevitable, so it
> could be worth telling that.

I tried that - see the draft at the end of the message.  And I tried to
cover everything that has been noted here to some degree while still
keeping the docstring fluent and at an acceptable length.

I did not touch the manual, maybe someone else wants to give that a try?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-WIP-Improve-documentation-of-alist-get.patch --]
[-- Type: text/x-diff, Size: 1608 bytes --]

From 3b8ef1ee9d9a5001e2de930ec34e3bdd3d4f87ad Mon Sep 17 00:00:00 2001
From: Michael Heerdegen <michael_heerdegen@web.de>
Date: Tue, 12 Mar 2019 15:13:55 +0100
Subject: [PATCH] WIP: Improve documentation of alist-get

(Bug#34708)...
---
 lisp/subr.el | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/lisp/subr.el b/lisp/subr.el
index 4024c68e68..8ea8fb602c 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -756,9 +756,31 @@ alist-get
 If KEY is not found in ALIST, return DEFAULT.
 Use TESTFN to lookup in the alist if non-nil.  Otherwise, use `assq'.

-This is a generalized variable suitable for use with `setf'.
+You can use `alist-get' in PLACE expressions.  This will modify
+an existing association (more precisely, the first one if
+multiple exist), or add a new element to the beginning of ALIST,
+destructively modifying the list stored in ALIST.
+
+Example:
+
+   (setq foo '((a . 0)))
+   (setf (alist-get 'a foo) 1
+         (alist-get 'b foo) 2)
+
+   foo ==> ((b . 2) (a . 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."
+means to remove KEY from ALIST if the new value is `eql' to
+DEFAULT (more precisely the first found association will be
+deleted from the alist).
+
+Example:
+
+  (setq foo '((a . 1) (b . 2)))
+  (setf (alist-get 'b foo nil 'remove) nil)
+
+  foo ==> ((a . 1))"
   (ignore remove) ;;Silence byte-compiler.
   (let ((x (if (not testfn)
                (assq key alist)
--
2.20.1


[-- Attachment #3: Type: text/plain, Size: 11 bytes --]



Michael.

bug#34708: alist-get has unclear documentation

[Eric Abrahamsen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Michael Heerdegen <michael_heerdegen@web.de> writes:
>
>> Actually, it's not even trivial at all.  Setting alist-get as place
>> could also build a new alist, leaving the one stored in the ALIST place
>> intact, and set place ALIST to the new list.  But it potentially
>> modifies the list stored in place ALIST.  That's not inevitable, so it
>> could be worth telling that.
>
> I tried that - see the draft at the end of the message.  And I tried to
> cover everything that has been noted here to some degree while still
> keeping the docstring fluent and at an acceptable length.
>
> I did not touch the manual, maybe someone else wants to give that a try?
>
> From 3b8ef1ee9d9a5001e2de930ec34e3bdd3d4f87ad Mon Sep 17 00:00:00 2001
> From: Michael Heerdegen <michael_heerdegen@web.de>
> Date: Tue, 12 Mar 2019 15:13:55 +0100
> Subject: [PATCH] WIP: Improve documentation of alist-get
>
> (Bug#34708)...
> ---
>  lisp/subr.el | 26 ++++++++++++++++++++++++--
>  1 file changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/lisp/subr.el b/lisp/subr.el
> index 4024c68e68..8ea8fb602c 100644
> --- a/lisp/subr.el
> +++ b/lisp/subr.el
> @@ -756,9 +756,31 @@ alist-get
>  If KEY is not found in ALIST, return DEFAULT.
>  Use TESTFN to lookup in the alist if non-nil.  Otherwise, use `assq'.
>
> -This is a generalized variable suitable for use with `setf'.
> +You can use `alist-get' in PLACE expressions.  This will modify
> +an existing association (more precisely, the first one if
> +multiple exist), or add a new element to the beginning of ALIST,
> +destructively modifying the list stored in ALIST.
> +
> +Example:
> +
> +   (setq foo '((a . 0)))
> +   (setf (alist-get 'a foo) 1
> +         (alist-get 'b foo) 2)
> +
> +   foo ==> ((b . 2) (a . 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."
> +means to remove KEY from ALIST if the new value is `eql' to
> +DEFAULT (more precisely the first found association will be
> +deleted from the alist).
> +
> +Example:
> +
> +  (setq foo '((a . 1) (b . 2)))
> +  (setf (alist-get 'b foo nil 'remove) nil)
> +
> +  foo ==> ((a . 1))"
>    (ignore remove) ;;Silence byte-compiler.
>    (let ((x (if (not testfn)
>                 (assq key alist)

I like it! Thanks for your work on this.

bug#34708: alist-get has unclear documentation

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Yes, the syntax is a bit weird.  I think I would prefer to write it as
>
>   (setf (alist-get key my-alist nil 'remove) nil)

BTW, I also want to point you to map.el.  It has `map-elt' which is also
setf'able and also works for other kinds of maps (hash-tables in
particular), and a distinct `map-remove'.  I guess it's time to
advertise map.el functions a bit more (in the manual).

Michael.

bug#34708: alist-get has unclear documentation

[Michael Heerdegen]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> I like it! Thanks for your work on this.

Patch installed (master).  Can we close this report?

I left the manual section about alist-get as is.  It is not perfect, but
what I really would like to have instead in the long run is a chapter
about map.el functions.  This is not subject of this report, however,
and I'm not sure if map.el is mature enough to do that now.

Thanks,

Michael.

bug#34708: alist-get has unclear documentation

[Michael Heerdegen]

> Patch installed (master).  Can we close this report?
>
> I left the manual section about alist-get as is.  It is not perfect, but
> what I really would like to have instead in the long run is a chapter
> about map.el functions.  This is not subject of this report, however,
> and I'm not sure if map.el is mature enough to do that now.

Ok, closing.  Thanks everyone for the discussion.

Michael.

bug#34708: Thanks

[Miguel V. S. Frasson]

[-- Attachment #1: Type: text/plain, Size: 260 bytes --]

Dear all in this discussion

I learned a from this discussion, specially about how to use alist-get. The
only missing lesson was how to remove association of a key from alist using
setf and alist-get.

Could someone give a quick example?

Best regards

Miguel

[-- Attachment #2: Type: text/html, Size: 473 bytes --]

bug#34708: Thanks

[Michael Heerdegen]

"Miguel V. S. Frasson" <mvsfrasson@gmail.com> writes:

> I learned a from this discussion, specially about how to use
> alist-get. The only missing lesson was how to remove association of a
> key from alist using setf and alist-get.
>
> Could someone give a quick example?

The modified docstring of alist-get has now an example.  It says:

[...]
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 (more precisely the first found association will be
deleted from the alist).

Example:

  (setq foo '((a . 1) (b . 2)))
  (setf (alist-get 'b foo nil 'remove) nil)

  foo => ((a . 1))


Does this suffice?

BTW, it doesn't say you have to use this feature.  If you find it
confusing, use delq+assoc+setq, or alternatively, what map.el provides.


Regards,

Michael.