Implement \once as music function able to operate on complex stuff.

Implement \once as music function able to operate on complex stuff.

[Reinhold, 2011-11-03 10:47:59]

LGTM, although regtests are missing (for \once applied to multiple settings at
one, stored in a variable).

[dak, 2011-11-03 11:25:49]

On 2011/11/03 10:47:59, Reinhold wrote:
> LGTM, although regtests are missing (for \once applied to multiple settings at
> one, stored in a variable).

There you are.

[Reinhold, 2011-11-03 11:45:00]

http://codereview.appspot.com/5322065/diff/3001/input/regression/complex-once.ly
File input/regression/complex-once.ly (right):

http://codereview.appspot.com/5322065/diff/3001/input/regression/complex-once...
input/regression/complex-once.ly:11: \unHideNotes  g a  \once\hideNotes b c |
While this is the easy approach, it builds on the fact that hideNotes is
internally implemented as a music expression with several \override commands. If
that internal representation ever changes, we won't have a proper regtest any
more...
I would prefer to define the multiple property operations command explicitly in
the regtest, so we have complete control and no risk that a totally unrelated
change might break a regtest (without us even noticing).

I.e. I would explicitly do
multipleOperations = {
  \override #'NoteHead #'color = #red
  \override #'Stem #'color = #blue
}

and then \once\multipleOperations.

[dak, 2011-11-03 12:57:26]

On 2011/11/03 11:45:00, Reinhold wrote:
>
http://codereview.appspot.com/5322065/diff/3001/input/regression/complex-once.ly
> File input/regression/complex-once.ly (right):
> 
>
http://codereview.appspot.com/5322065/diff/3001/input/regression/complex-once...
> input/regression/complex-once.ly:11: \unHideNotes  g a  \once\hideNotes b c |
> While this is the easy approach, it builds on the fact that hideNotes is
> internally implemented as a music expression with several \override commands.
If
> that internal representation ever changes, we won't have a proper regtest any
> more...
> I would prefer to define the multiple property operations command explicitly
in
> the regtest, so we have complete control and no risk that a totally unrelated
> change might break a regtest (without us even noticing).
> 
> I.e. I would explicitly do
> multipleOperations = {
>   \override #'NoteHead #'color = #red
>   \override #'Stem #'color = #blue
> }
> 
> and then \once\multipleOperations.

I disagree.  The whole point of redefining \once is to make it useful for
_existing_ functions/variables known to work by meddling with properties.

Regtests also serve as illustrations of functionality.  Defining an explicit
sandbox for the regtest to work on is completely eliminating its educational
value.

I consider it very unlikely that changing \hideNotes in a manner where it would
either
a) work with the previous definition of \once
b) keep working with the new definition of \once while failing to work
   with a sandbox like the above
is feasible at all.

So I consider the regtest as _fully_ doing its intended job.  The whole point of
the change is that you can use \once on existing commands like \hideNotes. 
That's the desired and intended behavior.

[dak, 2011-11-03 14:15:33]

As an illustration: take a look at ly/property-init.ly.  It contains dozens of
commands that will cooperate with the new \once.  All of them (well, the
reverting commands don't, just because \once\revert is not [yet?] workable in
general).  That's its principal selling point.

And I don't want to hide that.  It's much more important than testing that it
will work with some custom-defined command.

[Neil Puttock, 2011-11-03 14:21:05]

LGTM.

http://codereview.appspot.com/5322065/diff/3001/input/regression/complex-once.ly
File input/regression/complex-once.ly (right):

http://codereview.appspot.com/5322065/diff/3001/input/regression/complex-once...
input/regression/complex-once.ly:7: \layout { ragged-right = ##t }
redundant

http://codereview.appspot.com/5322065/diff/3001/input/regression/complex-once...
input/regression/complex-once.ly:11: \unHideNotes  g a  \once\hideNotes b c |
remove extra spaces

\once \hideNotes

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

http://codereview.appspot.com/5322065/diff/3001/ly/music-functions-init.ly#ne...
ly/music-functions-init.ly:583: (_i "Set @code{once} to @code{#t} on all layout
instruction events in @var{music}")
add full stop/period

http://codereview.appspot.com/5322065/diff/3001/ly/music-functions-init.ly#ne...
ly/music-functions-init.ly:589: (ly:music-warning m (_ "Can't apply \\once to
timed music"))))
cannot (or can not, whatever we use)

[dak, 2011-11-03 14:47:54]

http://codereview.appspot.com/5322065/diff/3001/input/regression/complex-once.ly
File input/regression/complex-once.ly (right):

http://codereview.appspot.com/5322065/diff/3001/input/regression/complex-once...
input/regression/complex-once.ly:7: \layout { ragged-right = ##t }
On 2011/11/03 14:21:05, Neil Puttock wrote:
> redundant

I remembered reading somewhere that every test should have this.  No idea why. 
No idea where.  Can't find it in CG, no idea where else.  Probably copied it
from some existing test.  Will remove.

http://codereview.appspot.com/5322065/diff/3001/input/regression/complex-once...
input/regression/complex-once.ly:11: \unHideNotes  g a  \once\hideNotes b c |
On 2011/11/03 14:21:05, Neil Puttock wrote:
> remove extra spaces
> 
> \once \hideNotes

Will do.

[pkx166h, 2011-11-03 23:42:09]

Passes make and no reg test diffs.

James