Re: Fix Tracker 918, Add extra RemoveEmpty*StaffContext functions to support "Frenched" scores (issue165096)

[ian]

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