Hi Neil,
address@hidden wrote:
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
Just info for fellow Frogs.
You can do this by using (for example)
$ cd <your git repo directory>
$ git mv input/regression/hara-kiri-drums.ly
input/regression/hara-kiri-drumstaff.ly
$ git mv input/regression/hara-kiri-tabs.ly
input/regression/hara-kiri-tabstaff.ly
$ git mv
input/regression/hara-kiri-rhythmicstaves.ly input/regression/hara-kiri-rhythmicstaff.ly
Using git mv updates your file system and the git info at the same time.
You can use
$ git mv -n <source> <dest>
to test what you are about to do before actually doing the rename.
I'll do the other changes once I've renamed files and put up an amended
patch set on Rietveld.
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"
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
______________________________________________ This email has
been scanned by Netintelligence
http://www.netintelligence.com/email
|