Re: Fixes issue 1881 (cyclic dependency with beam calculations) (issue 5038042)

[address@hidden]

On Sep 17, 2011, at 1:06 PM, Peekay Ex wrote:

> Mike,
> 
> On Sat, Sep 17, 2011 at 10:27 AM, address@hidden
> <address@hidden> wrote:
>> On Sep 17, 2011, at 11:07 AM, address@hidden wrote:
>> 
>>> LGTM.
>>> 
>>> BTW, I have a few queries about stem::length:
>>> 
>>> 76   (let* ((d (ly:grob-property grob 'direction))
>>> 
>>> You don't use 'direction; is it still necessary to get it to trigger
>>> other calculations?
>>> 
>> 
>> I doubt it - it's likely vestigial.  I'll work on a patch that addresses the 
>> issues below and remove this line.
>> 
>>> 79          (beam (ly:grob-object grob 'beam)))
>>> 
>>> Why do you need to access 'beam?  AFAICT, the callback will never be
>>> triggered on beamed notes, so it's redundant.  It also makes the last
>>> line look like a thinko:
>>> 
>>> 82         (ly:grob-property grob 'length))))
>>> 
>>> This would be a calculation-in-progress, since you're already inside the
>>> callback.
>>> 
>> 
>> Agreed - I can likely get rid of all this stuff.  I like the term `thinko' :)
>> 
>> I'll try reducing this as much as possible and will post a patch.
>> 
> 
> Are 5038042 and 5057041 both for 1881? or just 5038042 and if they are
> all the same 1881 can I just apply both patches to do the checks or do
> I need to reg test each one separately?
> 
> Thanks

Hey James,

They should be applied separately - 5038042 fixes 1881, and 5057041 prunes down 
bloated code.  There is a chance that 5057041 is effected by 5038042 (I haven't 
tested them together yet) though I doubt it.  After their countdowns, I'd push 
5038042 first, rerun regtests on 5057041, and then either push 5057041 or 
modify it if necessary.

Cheers,
MS