Hi Ian, LGTM, apart from some formatting issues and a few incorrect \version numbers.
Can you sort out the naming of the new regression tests? For consistency with the existing test, I'd advise amending them as follows: hara-kiri-drumstaff.ly hara-kiri-rhythmicstaff.ly hara-kiri-tabstaff.ly Cheers, Neil http://codereview.appspot.com/165096/diff/1/2 File input/regression/hara-kiri-drums.ly (right): http://codereview.appspot.com/165096/diff/1/2#newcode1 input/regression/hara-kiri-drums.ly:1: \version "2.13.8" 2.13.9 http://codereview.appspot.com/165096/diff/1/2#newcode2 input/regression/hara-kiri-drums.ly:2: \header { texidoc = I know you've just copied the existing example, but it is a bit messy; it would be preferable to tidy things up here: \header { texidoc = "Hara-kiri ... http://codereview.appspot.com/165096/diff/1/2#newcode14 input/regression/hara-kiri-drums.ly:14: trailing whitespace http://codereview.appspot.com/165096/diff/1/2#newcode18 input/regression/hara-kiri-drums.ly:18: ragged-right= ##t ragged-right = http://codereview.appspot.com/165096/diff/1/2#newcode26 input/regression/hara-kiri-drums.ly:26: \new DrumStaff \drummode { sn4 sn sn sn \break s1 \break sn4 sn sn sn \break sn sn sn sn} \drummode { sn4 sn sn sn \break s1 break sn4 sn sn sn \break sn4 sn sn sn } etc. http://codereview.appspot.com/165096/diff/1/3 File input/regression/hara-kiri-percent-repeat.ly (left): http://codereview.appspot.com/165096/diff/1/3#oldcode8 input/regression/hara-kiri-percent-repeat.ly:8: \new Staff { c''1 c'' \break c'' c'' } << \new Staff etc. http://codereview.appspot.com/165096/diff/1/3 File input/regression/hara-kiri-percent-repeat.ly (right): http://codereview.appspot.com/165096/diff/1/3#newcode1 input/regression/hara-kiri-percent-repeat.ly:1: \version "2.13.8" 2.13.9 http://codereview.appspot.com/165096/diff/1/3#newcode4 input/regression/hara-kiri-percent-repeat.ly:4: texidoc = "Staves, RhythmicStaves, TabStaves and DrumStaves with percent repeats are not suppressed." line too long http://codereview.appspot.com/165096/diff/1/3#newcode10 input/regression/hara-kiri-percent-repeat.ly:10: \new TabStaff \repeat percent 4 {c1} \repeat percent 4 { c1 } http://codereview.appspot.com/165096/diff/1/3#newcode11 input/regression/hara-kiri-percent-repeat.ly:11: \new DrumStaff \drummode { \repeat percent 4 {hh1} } { hh1 } http://codereview.appspot.com/165096/diff/1/3#newcode16 input/regression/hara-kiri-percent-repeat.ly:16: \context { \RemoveEmptyStaffContext } indent two spaces only: \layout { \context { \RemoveEmptyStaffContext } http://codereview.appspot.com/165096/diff/1/4 File input/regression/hara-kiri-rhythmicstaves.ly (right): http://codereview.appspot.com/165096/diff/1/4#newcode1 input/regression/hara-kiri-rhythmicstaves.ly:1: \version "2.13.5" 2.13.9 http://codereview.appspot.com/165096/diff/1/4#newcode2 input/regression/hara-kiri-rhythmicstaves.ly:2: \header { texidoc = same formatting nitpicks as hara-kiri-percent-repeat.ly http://codereview.appspot.com/165096/diff/1/4#newcode14 input/regression/hara-kiri-rhythmicstaves.ly:14: trailing whitespace http://codereview.appspot.com/165096/diff/1/5 File input/regression/hara-kiri-tabs.ly (right): http://codereview.appspot.com/165096/diff/1/5#newcode1 input/regression/hara-kiri-tabs.ly:1: \version "2.13.5" 2.13.9 http://codereview.appspot.com/165096/diff/1/5#newcode3 input/regression/hara-kiri-tabs.ly:3: \header { texidoc = same formatting nitpicks as hara-kiri-percent-repeat.ly http://codereview.appspot.com/165096/diff/1/5#newcode15 input/regression/hara-kiri-tabs.ly:15: This example was done with a pianostaff, which has fixed distance This can be removed, since it's not true (and hasn't been for a long time) http://codereview.appspot.com/165096/diff/1/5#newcode18 input/regression/hara-kiri-tabs.ly:18: trailing whitespace http://codereview.appspot.com/165096/diff/1/6 File ly/engraver-init.ly (left): http://codereview.appspot.com/165096/diff/1/6#oldcode1013 ly/engraver-init.ly:1013: RemoveEmptyRhythmicStaffContext= \context { RemoveEmptyRhythmicStaffContext = \context { http://codereview.appspot.com/165096/diff/1/6 File ly/engraver-init.ly (right): http://codereview.appspot.com/165096/diff/1/6#newcode1012 ly/engraver-init.ly:1012: % Add RemoveEmpty*StaffContext function definitions here Remove this. http://codereview.appspot.com/165096/diff/1/6#newcode1014 ly/engraver-init.ly:1014: RemoveEmptyDrumStaffContext= \context { RemoveEmptyDrumStaffContext = \context { http://codereview.appspot.com/165096/diff/1/6#newcode1028 ly/engraver-init.ly:1028: RemoveEmptyTabStaffContext= \context { RemoveEmptyTabStaffContext = \context { http://codereview.appspot.com/165096 _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
