Re: Clef support for cue notes (issue2726043)

[v . villenave]

Hi Reinhold,

it certainly looks good! I haven't tested your patch set though, so
these are just a couple of nitpicks off the top of my head.


http://codereview.appspot.com/2726043/diff/1/input/regression/cue-clef.ly
File input/regression/cue-clef.ly (right):

http://codereview.appspot.com/2726043/diff/1/input/regression/cue-clef.ly#newcode4
input/regression/cue-clef.ly:4: texidoc = "Clefs for cue notes: Normal
clefs should be printed, and in addition
Is it "Normal" that this word is capitalized?

http://codereview.appspot.com/2726043/diff/1/lily/pitch-scheme.cc
File lily/pitch-scheme.cc (right):

http://codereview.appspot.com/2726043/diff/1/lily/pitch-scheme.cc#newcode175
lily/pitch-scheme.cc:175: }
Just out of curiosity: have you checked that it isn't affected by #466?

http://codereview.appspot.com/2726043/diff/1/ly/engraver-init.ly
File ly/engraver-init.ly (right):

http://codereview.appspot.com/2726043/diff/1/ly/engraver-init.ly#newcode79
ly/engraver-init.ly:79:
Are you sure that we want to \consist the Cue_clef_engraver by default?
Most of the time, it won't be needed at all... (I'm searching for a way
to conditionally load it when needed, but I can't find any though.)

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

http://codereview.appspot.com/2726043/diff/1/ly/music-functions-init.ly#newcode219
ly/music-functions-init.ly:219: (make-cue-clef-set type))
I'm not sure this is the proper indentation for .ly code.

http://codereview.appspot.com/2726043/