Hi,
I've implemented Neil's comments, re-run regression tests locally and
uploaded amended patches to Rietveld.
I think this should be ready to push now.
Cheers,
Ian
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"
On 2009/12/10 23:43:35, Neil Puttock wrote:
2.13.9
Done.
http://codereview.appspot.com/165096/diff/1/2#newcode14
input/regression/hara-kiri-drums.ly:14:
On 2009/12/10 23:43:35, Neil Puttock wrote:
trailing whitespace
Done.
http://codereview.appspot.com/165096/diff/1/2#newcode18
input/regression/hara-kiri-drums.ly:18: ragged-right= ##t
On 2009/12/10 23:43:35, Neil Puttock wrote:
ragged-right =
Done.
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}
On 2009/12/10 23:43:35, Neil Puttock wrote:
\drummode {
sn4 sn sn sn \break
s1 break
sn4 sn sn sn \break
sn4 sn sn sn
}
etc.
Done.
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'' }
On 2009/12/10 23:43:35, Neil Puttock wrote:
<<
\new Staff
etc.
Done.
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#newcode4
input/regression/hara-kiri-percent-repeat.ly:4: texidoc = "Staves,
RhythmicStaves, TabStaves and DrumStaves with percent repeats are not
suppressed."
On 2009/12/10 23:43:35, Neil Puttock wrote:
line too long
Doc comment split into two shorter lines.
Done.
http://codereview.appspot.com/165096/diff/1/3#newcode11
input/regression/hara-kiri-percent-repeat.ly:11: \new DrumStaff
\drummode { \repeat percent 4 {hh1} }
On 2009/12/10 23:43:35, Neil Puttock wrote:
{ hh1 }
Done.
http://codereview.appspot.com/165096/diff/1/3#newcode16
input/regression/hara-kiri-percent-repeat.ly:16: \context {
\RemoveEmptyStaffContext }
On 2009/12/10 23:43:35, Neil Puttock wrote:
indent two spaces only:
\layout {
\context { \RemoveEmptyStaffContext }
Done.
http://codereview.appspot.com/165096/diff/1/4
File input/regression/hara-kiri-rhythmicstaves.ly (right):
http://codereview.appspot.com/165096/diff/1/4#newcode2
input/regression/hara-kiri-rhythmicstaves.ly:2: \header { texidoc =
On 2009/12/10 23:43:35, Neil Puttock wrote:
same formatting nitpicks as hara-kiri-percent-repeat
Done.
http://codereview.appspot.com/165096/diff/1/4#newcode4
input/regression/hara-kiri-rhythmicstaves.ly:4: " Hara-kiri staves kill
themselves if they are empty. This
"kill themselves" > "are suppressed" (fewer characters, better style)
http://codereview.appspot.com/165096/diff/1/4#newcode5
input/regression/hara-kiri-rhythmicstaves.ly:5: example really contains
three rhythmic staves, but as they progress, empty ones
"they progress" > "it progresses" (fewer characters, clearer meaning)
http://codereview.appspot.com/165096/diff/1/4#newcode14
input/regression/hara-kiri-rhythmicstaves.ly:14:
On 2009/12/10 23:43:35, Neil Puttock wrote:
trailing whitespace
Done.
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"
On 2009/12/10 23:43:35, Neil Puttock wrote:
2.13.9
Done.
http://codereview.appspot.com/165096/diff/1/5#newcode1
input/regression/hara-kiri-tabs.ly:1: \version "2.13.5"
File now renamed hara-kiri-tabstaff.ly
http://codereview.appspot.com/165096/diff/1/5#newcode3
input/regression/hara-kiri-tabs.ly:3: \header { texidoc =
On 2009/12/10 23:43:35, Neil Puttock wrote:
same formatting nitpicks as hara-kiri-percent-repeat.ly
Done.
http://codereview.appspot.com/165096/diff/1/5#newcode5
input/regression/hara-kiri-tabs.ly:5: " Hara-kiri staves kill themselves
if they are empty. This
"kill themselves" > "are suppressed"
http://codereview.appspot.com/165096/diff/1/5#newcode6
input/regression/hara-kiri-tabs.ly:6: example really contains three tab
staves, but as they progress, empty ones
"they progress" > "it progresses"
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
On 2009/12/10 23:43:35, Neil Puttock wrote:
This can be removed, since it's not true (and hasn't been for a long
time)
Done.
http://codereview.appspot.com/165096/diff/1/5#newcode18
input/regression/hara-kiri-tabs.ly:18:
On 2009/12/10 23:43:35, Neil Puttock wrote:
trailing whitespace
Done.
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 {
On 2009/12/10 23:43:35, Neil Puttock wrote:
RemoveEmptyRhythmicStaffContext = \context {
Done.
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
On 2009/12/10 23:43:35, Neil Puttock wrote:
Remove this.
Old habits. I worked on a project for a long time where we were
required to put a comment in code to link the bug-tracking system to a
particular change in a source file.
Done.
http://codereview.appspot.com/165096/diff/1/6#newcode1014
ly/engraver-init.ly:1014: RemoveEmptyDrumStaffContext= \context {
On 2009/12/10 23:43:35, Neil Puttock wrote:
RemoveEmptyDrumStaffContext = \context {
Done.
http://codereview.appspot.com/165096/diff/1/6#newcode1028
ly/engraver-init.ly:1028: RemoveEmptyTabStaffContext= \context {
On 2009/12/10 23:43:35, Neil Puttock wrote:
RemoveEmptyTabStaffContext = \context {
Done.
http://codereview.appspot.com/165096
_______________________________________________
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
