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

[reinhold . kainhofer]

Will also correct those spacing issues pointed out by Neil before I'll
push.


http://codereview.appspot.com/4810042/diff/4001/input/regression/set-once.ly
File input/regression/set-once.ly (right):

http://codereview.appspot.com/4810042/diff/4001/input/regression/set-once.ly#newcode6
input/regression/set-once.ly:6: to the previous value rather than the
default."
On 2011/07/21 17:34:07, Neil Puttock wrote:
> this sounds a bit ambiguous; could be misinterpreted

Yes, I'll reformulate it. How about
"@code{\once \set} should change a context property value for just one
timestep and then return to the previous value, rather than resetting
the property to its global default value."

http://codereview.appspot.com/4810042/diff/4001/input/regression/set-once.ly#newcode14
input/regression/set-once.ly:14: < e -1> -"left" |
On 2011/07/21 17:34:07, Neil Puttock wrote:
> you could add an assert function (using ApplyContext) to verify that
the value
> returns to the original \set (if you're feeling paranoid :)

Hehe... No, that would make our life harder should we ever break that,
because then the regtest build will fail for all of use, rather than
simply showing some differences in the images.

http://codereview.appspot.com/4810042/diff/4001/lily/property-iterator.cc
File lily/property-iterator.cc (right):

http://codereview.appspot.com/4810042/diff/4001/lily/property-iterator.cc#newcode44
lily/property-iterator.cc:44: * value after the next timestep */
On 2011/07/21 17:34:07, Neil Puttock wrote:
> don't you mean at the end of the current timestep?

Well, nice catch, I'm actually mixing two different views:
View 1) \set is not assigned to a note, but is written between two
notes, so the following note (to which it shall apply) will be the NEXT
timestep
View 2) \set is evaluated together with the next following note, so
during processing its value is for the CURRENT timestep.

You are right, I should probably stick to view 2 and use current
timestep here.

http://codereview.appspot.com/4810042/