Renamed this function to not cause confusion with the new option, as this function will effectively update the selection as a whole.

Let us not rename it (see comment above)

This forces uniqueness. Otherwise appending would allow duplicate values and I couldn't think of a reason to allow it.

Let's remove this new function, see comment above

I would say:

You can dynamically update the selection by using the:update_selection assign.:update_selection must be a 1-arity function that receives the current selection and returns the new one:

I don't think we need the second example

My suggestion is to replace this line with:

update_selection(update_fn.(selection), selection, options, mode, value_mapper)

Whereupdate_selection is the oldupdate_selection function, we don't rename it and we don't add the newupdate_selection function.

We won't have dupe detection but I think it's overkill. We don't have it anyway when you pass the selection explicitly with:value, why have it in the case of:update_selection? The caller can easily check for dupes and remove them if they want to

I ran into a problem with this and still need to think of a way to figure it out. Theupdate_fn.(selection) may leave mixed entries (current + new) and it fails when trying to ran the existing ones with thevalue_mapper again. This is why the function I wrote splits it and only "normalize" the new entries.

Sorry that was not clear, my function fixes the problem by splitting it and selectively runningvalue_mapper only on new values. If we are to revert and use the ogupdate_selection we must address thevalue_mapper that may encounter already mapped values.

Thanks for clarifying. Ok, but I don't think the problem is actually fixed by your function. Take this example:

current_selection=[%{value:1,label:"one"},%{value:2,label:"two"}]value_mapper=fnn->%{value:n,label:to_string(n)}endupdate_fn=fncurrent_selection->current_selection++[2,3]end

With this arguments, yourupdate_selection function
will fail to spot that 2 is already in the selection, because it's looking for unmapped values (2,3) in the list of mapped values (line 547).

I can't think of an elegant solution for this. Can you?

My impulse would be to say: it's the responsibility of the writer ofupdate_fn to leave the selection in a state that can be mapped using thevalue_mapper they provided.
Same for dupe detection.

Ok I thought about it and I think another, perhaps better option would to not callvalue_mapper at all when updating the selection programmatically. This makes sense because the caller can map the selection themselves if they want to (i.e. they can provide the selection already in the right format).value_mapper was intended anyway for the case where the user has no way to map the selection (i.e. when it's coming from existing values in the form).

So something like:

update_selection(update_fn.(selection), selection, options, mode, & &1)

Let us not rename it (see comment above)

Let's remove this new function, see comment above

Love the tests! ❤️

Love the tests! ❤️