Re: Updating alists (was: Tenuto marking too close to note)

[Jean Abou Samra]

Le 30/12/2021 à 12:36, Lukas-Fabian Moser a écrit :
> It's much worse than that: It's not the first time _I've_ been tripped 
> up by this (previously it was with sort!). I'll just have to write 100 
> copies of
>
> "The exclamation mark in something! does not mean the function is 
> guaranteed to make the desired change in-place."
>
> on my wall.


:-)

For assoc-set! and friends, I actually wonder why this
is since when the function has exhausted the alist in
its search for a matching pair, it could well set the
last cdr to add the new pair at the end. That would make
the interface somewhat more convenient. But libguile/alist.c
writes them as, for example,

SCM_DEFINE (scm_assq_set_x, "assq-set!", 3, 0, 0,
            (SCM alist, SCM key, SCM val),
[...]
{
  SCM handle;

  handle = scm_sloppy_assq (key, alist);
  if (scm_is_pair (handle))
    {
      scm_set_cdr_x (handle, val);
      return alist;
    }
  else
    return scm_acons (key, val, alist);
}

which conses the new pair at the beginning instead.


> But more seriously: Thanks! And also for advertising ice-9 match - 
> it's really a different coding style from the more commonly seen (if 
> pair? ...) constructs.


I have been really liking pattern matching since
I started to more seriously program in OCaml. It
clarifies code by showing what the programmer thinks
in a visual way. More and more languages support
it in some form these days, e.g. Rust has it (OK,
ML-inspired language), and more recently Python gained
it (with one small syntax suggestion from yours
truly).


> * * *
>
> Another question: With the (hopefully) upcoming changes in the 
> script-alist structure (using symbols instead of strings as keys), 
> we're quite close to being able to simply do
>
> \version "2.23.6"
>
> myScripts = \default-script-alist
> myScripts.tenuto.padding = 5
>
> \layout {
>   \context {
>     \Score
>     scriptDefinitions = #myScripts
>   }
> }
>
> {
>   a--
> }
>
> (One might also skip defining a new variable and instead change 
> default-script-alist directly, but it has to be re-read in a \layout 
> block anyway.)
>
> But at the moment, this does not work because the changed key is not 
> being replaced in the alist but added at the beginning. If I see 
> things correctly, this is the remark about "not coalescing multiple 
> overrides of the same property" in nested_property_alist(...) in 
> ily/nested-property.cc.
>
> Re-enabling the outcommented branch in that function (taking care to 
> use assoc_tail instead of assq_tail and correcting the order of 
> arguments to that call) basically works, but there's trouble ahead in 
> the case where one does
>
> variable.entry = 15
> variable.entry.first = #'i-ve-decided-i-want-a-sublist-after-all
>
> which unfortunately is what happens if one \override's a nested 
> property given by a callback (e.g. 
> VerticalAxisGroup.staff-staff-spacing), or worse
>
> variable.entry = #'(some . pair)
> variable.entry.first = #'i-ve-decided-i-want-a-sublist-after-all
>
> since an entry that is a pair looks like the beginning of a sublist.


Both of these cases seem to work the same as in
current versions if I do

diff --git a/lily/nested-property.cc b/lily/nested-property.cc
index 9c158561b1..69d8219f2a 100644
--- a/lily/nested-property.cc
+++ b/lily/nested-property.cc
@@ -49,7 +49,7 @@ partial_list_copy (SCM lst, SCM tail, SCM newtail)
 SCM
 assq_tail (SCM key, SCM alist, SCM based_on = SCM_EOL)
 {
-  for (SCM p = alist; !scm_is_eq (p, based_on); p = scm_cdr (p))
+  for (SCM p = alist; scm_is_pair (p) && scm_is_pair (scm_car (p)) && 
!scm_is_eq (p, based_on); p = scm_cdr (p))
     {
       if (scm_is_eq (scm_caar (p), key))
         return p;
@@ -60,7 +60,7 @@ assq_tail (SCM key, SCM alist, SCM based_on = SCM_EOL)
 SCM
 assv_tail (SCM key, SCM alist, SCM based_on = SCM_EOL)
 {
-  for (SCM p = alist; !scm_is_eq (p, based_on); p = scm_cdr (p))
+  for (SCM p = alist; scm_is_pair (p) && scm_is_pair (scm_car (p)) && 
!scm_is_eq (p, based_on); p = scm_cdr (p))
     {
       if (scm_is_true (scm_eqv_p (scm_caar (p), key)))
         return p;
@@ -75,7 +75,7 @@ assoc_tail (SCM key, SCM alist, SCM based_on = SCM_EOL)
     return assq_tail (key, alist, based_on);
   if (scm_is_number (key) || scm_is_true (scm_char_p (key)))
     return assv_tail (key, alist, based_on);
-  for (SCM p = alist; !scm_is_eq (p, based_on); p = scm_cdr (p))
+  for (SCM p = alist; scm_is_pair (p) && scm_is_pair (scm_car (p)) && 
!scm_is_eq (p, based_on); p = scm_cdr (p))
     {
       if (ly_is_equal (scm_caar (p), key))
         return p;
@@ -156,12 +156,12 @@ nested_property_alist (SCM alist, SCM prop_path, 
SCM value)
     }
   // Outcommented code would coalesce multiple overrides of the same
   // property
-#if 0
-  SCM where = assq_tail (alist, key);
+  //#if 0
+  SCM where = assoc_tail (key, alist);
   if (scm_is_true (where))
     return scm_acons (key, value,
                       partial_list_copy (alist, where, scm_cdr (where)));
-#endif
+  //#endif
   return scm_acons (key, value, alist);
 }


But I really don't have deep enough an insight on this code
to judge, sorry.


> On the other hand, I would expect people who are using
>
> Violin.I = { \someMusic }
> Violin.I = { \someNewMusic }
>
> would indeed like to be their first definition to actually be replaced 
> (even if using \Violin.I in a score happens to produce only the most 
> recent entry).


I can't predict all consequences, but I find
it conceivable.


[David]
> you'll get across-session bleed of assignments when making them
> destructive.  Another option is, of course, to do what amounts to an
> in-place modification of a structural copy.

The outcommented code path mentioned by Lukas does
not mutate the alist pair but copies the spine to
form a new alist sharing a tail with the original.
So yes, there is copy (but shallow and only the spine).
Not that I will teach this to the author of this
code, but for the list at large ...

Regards,
Jean