Re: Create \temporary for doing overrides without pop-first set (issue 6687044)

[dak]

Reviewers: janek, Trevor Daniels,

Message:
On 2012/10/15 10:45:02, Trevor Daniels wrote:
> I'm not sure this is the right way to go.
> Here's my reasoning:

> a) Unifying the interface to context and grob properties
> seems to be a desirable long-term goal; at least several
> people have spoken in favour of it.  But we do not yet
> have a proposal for doing this.  In particular we do not
> have agreement about desirable syntax and semantics for
> modifying properties, although several suggestions have
> been made.

Let's assume that we "unify" the interface to context and grob
properties.  The _default_ operation on a context property will
_always_ be overwrite rather than push, since context properties track
the change of things like the current clef, the current key signature,
the current whatever.  There is no point in keeping a stack for that
by default, and it would be seriously confusing.

That is what the default operation on a grob property is right now: it
overwrites the current top of the stack.  This matches the default
operation on context properties.  As contrasted to context properties,
grob properties have a save stack.  There is no LilyPond-accessible
manner to access it, it is only available from Scheme.

\temporary is a user interface for saving values, the default is to
_not_ save them.  So after unifying context and grob properties, _not_
saving a previous value by default would make the behavior of \set and
\override correspond.

> b) The present semantics of \override are clearly deficient,
> as evidenced by the need for this patch, in that a proper
> stack of properties is required to avoid called functions
> changing the settings made by the caller.

That is like saying my right leg is clearly deficient, as evidenced by
the need for the left leg if I want to walk somewhere.

The solution is to use it in combination with a left leg, not
amputating it and then devising a new interface for walking.

> c) The scope for writing music functions is now much
> greater thanks to sterling work by David K, and others
> are increasingly taking advantage of them.  Functions
> are frequently posted on the user list in response to
> calls for aid.  So whatever is implemented in response
> to (b) will inevitably filter into the user domain, and
> into the documentation.  Achieving (a) is going to
> take a long time, so any solution to (b) which is
> introduced now will become established.

So what?  If we need to transition to something new, we need to do a
conversion anyway.  If there is _no_ clean way to express the desired
semantics before conversion, the results after conversion will not ask
for the desired semantics either without additional manual changes.

> d) The discussion on -devel about the several ways of
> approaching this issue has not yet come to a consensus.

> So rather than introduce this change into the code base
> now I would prefer to see more discussion and hopefully
> agreement on the best way of approaching (a), in
> particular what the optimum primitive operations should
> be.  That will then inform this discussion about the
> best short-term step to take to resolve (b), as a means
> of facilitating progress towards (a), without the need
> at some stage to withdraw established facilities.

There is only a need to "withdraw" if the new semantics are less
powerful/expressive, namely if we want to decide that there should be
_no_ way to change properties temporarily and restore them afterwards.

Only if we make _that_ decision will \temporary\override become
unconvertible.


Description:
Create \temporary for doing overrides without pop-first set

Normal overrides change the top of the current property stack rather
than pushing to it, by having the pop-first property set in the music.

\temporary will remove this property from its argument's overrides.
For property-changing music that is not an override, a warning will be
generated.

Calling \temporary \override and \revert in succession on the same
property will have a net effect of zero.  Similarly, pairing
\temporary and \undo on the same music containing overrides will have
a net effect of zero.

In more complex arrangements, \revert will revert to the state before
the last (matching) \temporary override of the same property.

Properties are maintained conceptually using one stack per property
per grob per context.  Using \push/\pop instead of \temporary/\undo
was deemed unsuitable for a musician-accessible user interface.

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

Affected files:
  M ly/music-functions-init.ly


Index: ly/music-functions-init.ly
diff --git a/ly/music-functions-init.ly b/ly/music-functions-init.ly
index  
4fa31929d4ef9e2188ba5745612b092c870143e7..92c0170377c1805e7a2eb1aac1f549562aee0d6a  
100644
--- a/ly/music-functions-init.ly
+++ b/ly/music-functions-init.ly
@@ -1242,6 +1242,41 @@ tag =
          (ly:music-property arg 'tags)))
    arg)

+temporary =
+#(define-music-function (parser location music)
+   (ly:music?)
+   (_i "Convert @code{\\override}s in @var{music} to
address@hidden overrides, making them pair up with following
address@hidden reverts in a stack-neutral manner.  An
address@hidden/@/@code{\\revert} sequence created by using
address@hidden and @code{\\undo} on the same music containing overrides
+will cancel out perfectly or cause address@hidden
+
+Non-property-related music is ignored, warnings are generated for any
+property-changing music that isn't an @code{\\override}.")
+   (define warned #f)
+   (for-some-music
+    (lambda (m)
+      (and (or (music-is-of-type? m 'layout-instruction-event)
+               (music-is-of-type? m 'context-specification)
+               (music-is-of-type? m 'apply-context)
+               (music-is-of-type? m 'time-signature-music))
+           (case (ly:music-property m 'name)
+             ((OverrideProperty)
+              (set! (ly:music-property m 'pop-first) '())
+              #t)
+             ((ContextSpeccedMusic)
+              #f)
+             (else
+              (if (not warned)
+                  (begin
+                    (ly:input-warning location (_ "Cannot make ~a  
revertible")
+                                      (ly:music-property m 'name))
+                    (set! warned #t)))
+              #t))))
+    music)
+   music)
+
 time =
 #(define-music-function (parser location beat-structure fraction)
    ((number-list? '()) fraction?)
@@ -1326,14 +1361,17 @@ undo =
    (ly:music?)
    (_i "Convert @code{\\override} and @code{\\set} in @var{music} to
 @code{\\revert} and @code{\\unset}, respectively.  Any reverts and
-unsets already in @var{music} are ignored and not converted.")
+unsets already in @var{music} cause a warning.  Non-property-related music  
is ignored.")
+   (define warned #f)
    (let loop
        ((music music))
      (let
          ((lst
            (fold-some-music
             (lambda (m) (or (music-is-of-type? m 'layout-instruction-event)
-                            (music-is-of-type? m 'context-specification)))
+                            (music-is-of-type? m 'context-specification)
+                            (music-is-of-type? m 'apply-context)
+                            (music-is-of-type? m 'time-signature-music)))
             (lambda (m overrides)
               (case (ly:music-property m 'name)
                 ((OverrideProperty)
@@ -1357,7 +1395,13 @@ unsets already in @var{music} are ignored and not  
converted.")
                               'element (loop (ly:music-property  
m 'element))
                               'context-type (ly:music-property  
m 'context-type))
                   overrides))
-                (else overrides)))
+                (else
+                 (if (not warned)
+                     (begin
+                       (ly:input-warning location (_ "Cannot revert ~a")
+                                         (ly:music-property m 'name))
+                       (set! warned #t)))
+                 overrides)))
             '()
             music)))
        (cond