Create \temporary for doing overrides without pop-first set

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.

[janek, 2012-10-15 06:30:12]

Hi,

from what i understand, the current situation is:
1) we'd like to see the interface for changing properties redesigned
(http://lists.gnu.org/archive/html/lilypond-devel/2012-10/msg00518.html),
2) the above won't happen sooner than several months from now,
3) we are not sure whether we want to have \temporary or maybe change \override
itself to be just push,
4) anyway, we need to change current behavior because it's buggy.

So, i suggest to wait for that discussion to end (i could use more replies to
http://lists.gnu.org/archive/html/lilypond-devel/2012-10/msg00519.html).

But the patch itself, shall we choose to use \temporary, LGTM.

http://codereview.appspot.com/6687044/diff/4001/ly/music-functions-init.ly
File ly/music-functions-init.ly (right):

http://codereview.appspot.com/6687044/diff/4001/ly/music-functions-init.ly#ne...
ly/music-functions-init.ly:1250: @emph{corresponding} reverts in a stack-neutral
manner.  An
Could you write explicitely both here and in the commit message that "as of
2.17.5, \override is a pop+push"?

Had i not participated in previous discussion, i would have no idea why some
overrides could be pop-first and what that could actually mean to the music.

[Trevor Daniels, 2012-10-15 10:45:02]

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.

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.  This is of
minor importance now as few functions so that, but is
likely to increase in importance as more complex functions
are devised.

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.  

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. 

Trevor

[dak, 2012-10-15 12:51:25]

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.

[wl_gnu.org, 2012-10-15 13:02:38]

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

What about having a `simple' and an `advanced' interface?  This is,
let's stay with the current commands, enriched by \temporary and
friends as suggested by David.  At the same time, we could provide
\push, \pop, and \clear (with *this* names) for advanced users who
want to write music functions without side effects.


    Werner

[dak, 2012-10-15 13:10:14]

On 2012/10/15 13:02:38, wl_gnu.org wrote:
> > 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.
> 
> What about having a `simple' and an `advanced' interface?  This is,
> let's stay with the current commands, enriched by \temporary and
> friends as suggested by David.  At the same time, we could provide
> \push, \pop, and \clear (with *this* names) for advanced users who
> want to write music functions without side effects.

\clear with the proposed cut-through dynamics is rather the maximum possible
side effect.  It makes only some sense when it is intended to counter the
sideeffects of making "push" the default interface rather than "overwrite", with
the expectation that "push" will often get used when "overwrite" would have been
appropriate.  push and pop would not provide anything new.

I have a really hard time seeing who we are supposed to be doing a favor here.

[janek, 2012-10-15 14:03:07]

On Mon, Oct 15, 2012 at 2:51 PM,  <dak@gnu.org> wrote:
> 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.

Hmm.  I've looked at available context properties, and in my opinion
it could be useful to be able to perform "temporary \sets" on some of
them:

associatedVoice
beamExceptions
clefOctavation
crescendoSpanner
crescendoText
fontSize
instrumentName
instrumentTransposition
ottavation
PedalSustainStyle
tupletSpannerDuration

[talk] also, i'd find it nice if it was possible to write
\set Staff.color = #red
and have LilyPond override #'color property for all grobs belonging to
that Staff.  Currently doing this requires a music function, so it's
out of reach for beginners.
If we ever decided to do some "inheritance" of this sort, it would
make even more sense to be able to set context properties temporarily.
[/talk]

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

lol! :D
While that was fun, i'm pretty sure that wasn't what Trevor had in mind :)

cheers,
Janek

[dak, 2012-10-15 14:21:08]

On 2012/10/15 14:03:07, janek wrote:
> On Mon, Oct 15, 2012 at 2:51 PM,  <mailto:dak@gnu.org> wrote:
>
> > 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.
> 
> Hmm.  I've looked at available context properties, and in my opinion
> it could be useful to be able to perform "temporary \sets" on some of
> them:
> 
> associatedVoice
> beamExceptions
> clefOctavation
> crescendoSpanner
> crescendoText
> fontSize
> instrumentName
> instrumentTransposition
> ottavation

Where would be the point to return to a previous ottavation rather
than just setting the desired one?

> PedalSustainStyle
> tupletSpannerDuration

Well, I did not claim that there was _no_ conceivable context property
where a temporary replacement would make sense and/or that we would
never want to implement a stack here.  The question was about the
_default_ behavior, and enough things are _tracked_ in their
_progress_ in context properties that a default stack behavior does
not make sense.

I have no idea what you are trying to argue here: of course, if
context and grob properties are supposed to be unified, _both_ will
_have_ a stack implementation if temporary changes in grob properties
are desired.

But that does not mean that the default mode of setting a property
should be pushing.

> [talk] also, i'd find it nice if it was possible to write
> \set Staff.color = #red
> and have LilyPond override #'color property for all grobs belonging to
> that Staff.  Currently doing this requires a music function, so it's
> out of reach for beginners.
> If we ever decided to do some "inheritance" of this sort, it would
> make even more sense to be able to set context properties temporarily.
> [/talk]

This is not really related to implementing \temporary.  It is more
related to
<URL:http://code.google.com/p/lilypond/issues/detail?id=2832#c3>.

[dak, 2012-10-15 14:28:44]

http://codereview.appspot.com/6687044/diff/4001/ly/music-functions-init.ly
File ly/music-functions-init.ly (right):

http://codereview.appspot.com/6687044/diff/4001/ly/music-functions-init.ly#ne...
ly/music-functions-init.ly:1250: @emph{corresponding} reverts in a stack-neutral
manner.  An
On 2012/10/15 06:30:12, janek wrote:
> Could you write explicitely both here and in the commit message that "as of
> 2.17.5, \override is a pop+push"?
> 
> Had i not participated in previous discussion, i would have no idea why some
> overrides could be pop-first and what that could actually mean to the music.

_All_ \override are "pop-first".  This is not the place to document it.  I might
try to rephrase this, but something like "as of 2.17.5, \override is a pop+push"
has no place whatsoever in the documentation string of a function.

The documentation string is not the place for hand-waving suggestions of
possible changes.  It is supposed to document the current state, period.

[Trevor Daniels, 2012-10-15 14:46:50]

On 2012/10/15 14:21:08, dak wrote:
> On 2012/10/15 14:03:07, janek wrote:
> > Hmm.  I've looked at available context properties, and in my opinion
> > it could be useful to be able to perform "temporary \sets" on some of
> > them:
> > 
> > associatedVoice
> > beamExceptions
> > clefOctavation
> > crescendoSpanner
> > crescendoText
> > fontSize
> > instrumentName
> > instrumentTransposition
> > ottavation
> 
> Where would be the point to return to a previous ottavation rather
> than just setting the desired one?
> 
> > PedalSustainStyle
> > tupletSpannerDuration
> 
> Well, I did not claim that there was _no_ conceivable context property
> where a temporary replacement would make sense and/or that we would
> never want to implement a stack here.  The question was about the
> _default_ behavior, and enough things are _tracked_ in their
> _progress_ in context properties that a default stack behavior does
> not make sense.

This is where our opinions differ.  I and others believe a default
stack behaviour does make sense.  For most uses there is no
difference between \override followed by \revert for both
behaviours of \override.  It only differs when there are two
or more consecutive \overrides for the same property, and in
that case we will usually want the stack behaviour.  In the
few cases where we really want to clear the stack we suggest
implementing \clear.
  
> I have no idea what you are trying to argue here: of course, if
> context and grob properties are supposed to be unified, _both_ will
> _have_ a stack implementation if temporary changes in grob properties
> are desired.

Exactly.  So let's prepare the way for that now.  What we don't 
want is to implement the stack behaviour by a quite separate 
command which makes the interface more complicated and unintuitive
and which will be discarded later.

> But that does not mean that the default mode of setting a property
> should be pushing.

Why not?  What harm will it do?

[dak, 2012-10-15 15:30:14]

On 2012/10/15 14:46:50, Trevor Daniels wrote:
> On 2012/10/15 14:21:08, dak wrote:
> 
> > But that does not mean that the default mode of setting a property
> > should be pushing.
> 
> Why not?  What harm will it do?

A stack filled with garbage from pushes that happened without particular intent
is not more useful than no stack at all.  In neither case will you be able to
reliably return to a desired point of saving state before.

At any rate, I've had it and won't waste more time on this issue.

[wl_gnu.org, 2012-10-15 18:24:10]

>> The solution is to use it in combination with a left leg, not
>> amputating it and then devising a new interface for walking.
> 
> lol! :D
> While that was fun, i'm pretty sure that wasn't what Trevor had in
> mind :)

Here's the fitting cartoon.

  http://bilder.rofl.to/pic/beppo-der-clown

The translated text:

  Beppo has to laugh a little bit after realizing the foolishness of
  what he has just done.


     Werner

[janek, 2012-10-15 20:27:16]

On Mon, Oct 15, 2012 at 4:28 PM,  <dak@gnu.org> wrote:
> On 2012/10/15 06:30:12, janek wrote:
>> Could you write explicitely both here and in the commit message that
>> "as of 2.17.5, \override is a pop+push"?
>> Had i not participated in previous discussion, i would have no idea
>> why some overrides could be pop-first and what that could actually
>> mean to the music.
>
> _All_ \override are "pop-first".

Yes, i understand it (thanks to your explanation!).  I meant to
describe what i would think if i hadn't participated in the
discussion.

> This is not the place to document it.
> I might try to rephrase this, but something like "as of 2.17.5,
> \override is a pop+push" has no place whatsoever in the documentation
> string of a function.

ok, let's just have a note in the commit message.

Following Werner's advice, i drop all my objections to this patch.
Let's have it included; we can discuss changes to the stack interface
(i.e. whether \override should be just a push) separately in the
future.

LGTM!

thanks for your patience, David!  and for your watchfulness, Werner!
all the best,
Janek

[janek, 2012-10-21 16:24:36]

There was some controversy with this patch, but it was counted down and noone
found anything wrong with the actual code.  Some people are not sure whether we
want to have \temporary or something else, but they can change it in the future
if they want.  For now, this patch adds something that is missing and allows to
fix some bugs, so i'm going to push it on Tuesday or so; there's no point in
wasting David's work.

cheers,
janek

[janek, 2012-10-23 14:15:11]

pushed as 5c4b80afe97acbe20199a3aa71a0d63172112f23

David, please close this issue.

thanks!
Janek