[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Fix Tracker 918, Add extra RemoveEmpty*StaffContext functions to su
From: |
n . puttock |
Subject: |
Re: Fix Tracker 918, Add extra RemoveEmpty*StaffContext functions to support "Frenched" scores (issue165096) |
Date: |
Thu, 10 Dec 2009 23:43:35 +0000 |
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
- Re: Fix Tracker 918, Add extra RemoveEmpty*StaffContext functions to support "Frenched" scores (issue165096),
n . puttock <=
- Re: Fix Tracker 918, Add extra RemoveEmpty*StaffContext functions to support "Frenched" scores (issue165096), ian, 2009/12/11
- Re: Fix Tracker 918, Add extra RemoveEmpty*StaffContext functions to support "Frenched" scores (issue165096), n . puttock, 2009/12/13
- Re: Fix Tracker 918, Add extra RemoveEmpty*StaffContext functions to support "Frenched" scores (issue165096), ian, 2009/12/14
- Re: Fix Tracker 918, Add extra RemoveEmpty*StaffContext functions to support "Frenched" scores (issue165096), n . puttock, 2009/12/20