Re: Fix 153: \once\set properly restores the context property (issue4810042)

[reinhold . kainhofer]

Reviewers: Trevor Daniels, joeneeman, Neil Puttock,

Message:
On 2011/07/21 16:15:41, joeneeman wrote:
> If you do

> \set #'foo = #1
> c
> \once \set #'foo = #2
> c
> \once \set #'foo = #3
> c

> then the final value of foo should be 1. But with this code, I think
it will be
> 2.

Why should it be 2? The finalization hook to reset the value to the
previous one is processed at the end of the current moment, long before
the next \once\set will be processed (at the begin of the next moment).
So when the second \once\set is processed, the value of #'foo has
already been reset to #1 and the finalization  hook installed for the
secnd \once\set will reset the property to 1.

BTW, things like

\set #'foo = #1
c
\once \set #'foo = #2
\once \set #'foo = #3
c
c

also work properly (at the end the value will be 1), because apparently
the finalization hooks are processed in the correct (reverse) order.

Anyway, thanks for thinking about this case. I have updated the regtest
to include it.



Description:
Fix 153: \once\set properly restores the context property

\once\set works by installing a finalization hook in the iterator.
To restore the context property value before the \once\set, we simply
cache the old value and pass it to the finalization hook as third
argument. The finalizatino hook then sends a SetProperty event with
the old value rather than an UnsetProperty event, which reverts the
value to the default.

Please review this at http://codereview.appspot.com/4810042/

Affected files:
  A input/regression/set-once.ly
  M lily/include/property-iterator.hh
  M lily/property-iterator.cc


Index: input/regression/set-once.ly
diff --git a/input/regression/set-once.ly b/input/regression/set-once.ly
new file mode 100644
index  
0000000000000000000000000000000000000000..46faf94a1632605cd4cdd1c5a029aa0bd0113087
--- /dev/null
+++ b/input/regression/set-once.ly
@@ -0,0 +1,29 @@
+\version "2.15.5"
+
+\header {
+
+  texidoc = "@code{\once\set} should revert the setting after the next  
moment
+to the previous value rather than the default."
+
+}
+\relative {
+  \set fingeringOrientations = #'(left)
+  < e -1>1 |
+  \once \set fingeringOrientations = #'(right)
+  < e -1> |
+  < e -1> -"left" |
+
+  \once \set fingeringOrientations = #'(right)
+  < e -1>
+  \once \set fingeringOrientations = #'(up)
+  <e -1>
+  <e -1>-"left"
+
+  \set fingeringOrientations = #'(left)
+  <e -1>
+  \once \set fingeringOrientations = #'(up)
+  \once \set fingeringOrientations = #'(right)
+  <e -1>-"right"
+
+  <e -1>-"left"
+}
Index: lily/include/property-iterator.hh
diff --git a/lily/include/property-iterator.hh  
b/lily/include/property-iterator.hh
index  
8aa0cec11d23a64a3315dd60f1b13415a2f14193..c53a4d9648162c1a2aa07c78fbb9f8760b0ca1c0  
100644
--- a/lily/include/property-iterator.hh
+++ b/lily/include/property-iterator.hh
@@ -29,7 +29,7 @@ class Property_iterator : public Simple_music_iterator
 {
 public:
   DECLARE_SCHEME_CALLBACK (constructor, ());
-  DECLARE_SCHEME_CALLBACK (once_finalization, (SCM, SCM));
+  DECLARE_SCHEME_CALLBACK (once_finalization, (SCM, SCM, SCM));
   DECLARE_CLASSNAME(Property_iterator);

 protected:
Index: lily/property-iterator.cc
diff --git a/lily/property-iterator.cc b/lily/property-iterator.cc
index  
62f316b7f1b7220e12d9e8bf773aa5921337fd00..f84c67decc379727bb3ca46c69f05c85b2077e9f  
100644
--- a/lily/property-iterator.cc
+++ b/lily/property-iterator.cc
@@ -31,13 +31,26 @@ bool check_grob (Music *mus, SCM sym);
    translation unit, and set the property.
 */
 void
-Property_iterator::process (Moment m)
+Property_iterator::process (Moment mom)
 {
-  send_stream_event (get_outlet (), "SetProperty", get_music ()->origin (),
-                    ly_symbol2scm ("symbol"), get_music ()->get_property 
("symbol"),
-                    ly_symbol2scm ("value"), get_music ()->get_property 
("value"));
+  Context *o = get_outlet ();
+  Music *m = get_music ();
+  SCM previous_value = o->get_property (m->get_property ("symbol"));
+  send_stream_event (o, "SetProperty", m->origin (),
+                    ly_symbol2scm ("symbol"), m->get_property ("symbol"),
+                    ly_symbol2scm ("value"), m->get_property ("value"));
+
+  /* For \once\set install a finalization hook to reset the property to  
the old
+   * value after the next timestep */
+  if (to_boolean (m->get_property ("once")))
+    {
+      Global_context *tg = get_outlet ()->get_global_context ();
+      tg->add_finalization (scm_list_n (once_finalization_proc,
+                                       o->self_scm (), m->self_scm (),
+                                       ly_quote_scm (previous_value), 
SCM_UNDEFINED));
+    }

-  Simple_music_iterator::process (m);
+  Simple_music_iterator::process (mom);
 }

 void
@@ -50,30 +63,25 @@ Property_unset_iterator::process (Moment m)
   Simple_music_iterator::process (m);
 }

-MAKE_SCHEME_CALLBACK (Property_iterator, once_finalization, 2);
+MAKE_SCHEME_CALLBACK (Property_iterator, once_finalization, 3);
 SCM
-Property_iterator::once_finalization (SCM ctx, SCM music)
+Property_iterator::once_finalization (SCM ctx, SCM music, SCM  
previous_value)
 {
   Music *m = unsmob_music (music);
   Context *c = unsmob_context (ctx);

-  send_stream_event (c, "UnsetProperty", m->origin (),
-                    ly_symbol2scm ("symbol"), m->get_property ("symbol"));
+  // Do not use UnsetProperty, which sets the default, but rather
+  // restore the value before the \once\set command
+  send_stream_event (c, "SetProperty", m->origin (),
+                    ly_symbol2scm ("symbol"), m->get_property ("symbol"),
+                    ly_symbol2scm ("value"), previous_value);
+
   return SCM_UNSPECIFIED;
 }

 void
 Property_iterator::do_quit ()
 {
-  if (to_boolean (get_music ()->get_property ("once")))
-    {
-      SCM trans = get_outlet ()->self_scm ();
-      SCM music = get_music ()->self_scm ();
-
-      Global_context *tg = get_outlet ()->get_global_context ();
-      tg->add_finalization (scm_list_n (once_finalization_proc,
-                                       trans, music, SCM_UNDEFINED));
-    }
 }

 bool