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

[Carl Sorensen <c_sorensen@xxxxxxx>]

On 12/1/09 5:07 PM, "Ian Hulin" <ian@xxxxxxxxxxxx> wrote:

> 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>
>> <mailto:v.villenave@xxxxxxxxx>  wrote:
>> 
>>   
>>  
>>>  
>>> On Mon, Nov 30, 2009 at 11:04 PM, Ian Hulin <ian@xxxxxxxxxxxx>
>>> <mailto: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
> 

How about just writing a new regtest remove-empty-staff-context.ly that
demonstrates the removal of all of the staffs?

Thanks,

Carl


> Cheers,
> Ian