Re: [frogs] FW: Issue 918 in lilypond: Enhancement: \RemoveEmpty*StaffContext for all appropriate contexts

[Ian Hulin <ian@xxxxxxxxxxxx>]

Hi Carl,

Your last message came through in stereo stereo. . .

Carl Sorensen wrote:

Thanks, Ian.

The patch looks good to me as far as it goes.

On 11/30/09 4:31 PM, "Valentin Villenave" <v.villenave@xxxxxxxxx> wrote:

  

On Mon, Nov 30, 2009 at 11:04 PM, Ian Hulin <ian@xxxxxxxxxxxx> wrote:
    

Patch for review on Rietveld� http://codereview.appspot.com/164045
      

Don't forget to document the new shortcuts, and to add an item to
changes.tely :)
    

Strictly speaking, there's not a requirement for developers to write their
own documentation (although we appreciate it).

However, there *is* a requirement for developers to write their own
regtests.  I didn't see any regtests (either new or modified) in the patch.
  

That's because there don't seem to be any specifically for these functions.  There are two regression tests in which RemoveEmptyStaffContext is used:
 hara-kiri-percent-repeat.ly and hara-kiri-pianostaff.ly.
Nothing tests RemoveRhythmicStaffContext
I can add  DrumStaff, TabStaff and RhythmicStaff  and the corresponding RemoveEmpty*StaffContext statements quite easily in  hara-kiri-percent-repeat.ly, but I'll need to add three new tests, hara-kiri-drums.ly, hara-kiri-rhythmicstaves.ly and hara-kiri-tabs.ly

Cheers,
Ian