lilypond-devel
[Top][All Lists]
Advanced

[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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]