span-bar.cc: Sort bar-line extents in case alignAboveContext is active

[Graham Percival, 2012-07-17 05:42:55]

If this isn't covered already, then could you add a regtest for this?

Mike presumably had some reason for removing it in the first place, so if this
doesn't produce a visible change in any regtests then it's likely that somebody
will make this change again in the future.

[Keith, 2012-07-17 15:51:20]

On Mon, 16 Jul 2012 22:42:55 -0700, <graham@percival-music.ca> wrote:

> regtest
I'm expanding `alignment-order.ly` after the patchy test.

[Keith, 2012-07-18 04:39:06]

http://codereview.appspot.com/6406051/diff/6002/lily/span-bar.cc
File lily/span-bar.cc (right):

http://codereview.appspot.com/6406051/diff/6002/lily/span-bar.cc#newcode92
lily/span-bar.cc:92: vector_sort (extents, Interval::left_less);
This sort was originally added six years ago for the same problem 

commit 459216bffe7aa64cd1e9f31f1fda28f90f6e9c1c
Author: Han-Wen Nienhuys <hanwen@xs4all.nl>
Date:   Mon May 30 09:45:14 2005 +0000

    * lily/span-bar.cc (print): sort bar line extents. Fixes problem
    with disappearing span bars when alignAboveContext is active

[marc, 2012-07-20 18:38:35]

Am 17.07.2012 17:51, schrieb Keith OHara:
> On Mon, 16 Jul 2012 22:42:55 -0700, <graham@percival-music.ca> wrote:
>
>> regtest
> I'm expanding `alignment-order.ly` after the patchy test.
I think that my current redefinition already includes the sorting
(see lines 578/579 of 
http://codereview.appspot.com/6305115/diff/30001/scm/bar-line.scm)
so perhaps this patch could be postponed until

http://codereview.appspot.com/6305115/


is accepted? Then only the regtest would be needed.

I don't know if this is acceptable?

Regards,

Marc

[Keith, 2012-07-20 21:41:28]

On Fri, 20 Jul 2012 11:38:34 -0700, Marc Hohl <marc@hohlart.de> wrote:

> I think that my current redefinition already includes the sorting
> (see lines 578/579 of
> http://codereview.appspot.com/6305115/diff/30001/scm/bar-line.scm)

I don't read Scheme, but it does seem to function properly.

> so perhaps this patch could be postponed until
>
> http://codereview.appspot.com/6305115/
>
> is accepted? Then only the regtest would be needed.

It is easier for me to push the fix along with the regtest, than to remember to
push the regtest later.  Also, pushing the bug fix separately from your
re-implementation gives us a bug-free point in the history of the code, to which
we can return if we discover a problem with the re-implementation.

[marc, 2012-07-21 08:07:10]

Am 20.07.2012 23:43, schrieb Keith OHara:
> On Fri, 20 Jul 2012 11:38:34 -0700, Marc Hohl <marc@hohlart.de> wrote:
>
>> I think that my current redefinition already includes the sorting
>> (see lines 578/579 of
>> http://codereview.appspot.com/6305115/diff/30001/scm/bar-line.scm)
>
> I don't read Scheme, but it does seem to function properly.
>
>> so perhaps this patch could be postponed until
>>
>> http://codereview.appspot.com/6305115/
>>
>> is accepted? Then only the regtest would be needed.
>
> It is easier for me to push the fix along with the regtest, than to 
> remember to push the regtest later.  Also, pushing the bug fix 
> separately from your re-implementation gives us a bug-free point in 
> the history of the code, to which we can return if we discover a 
> problem with the re-implementation.
>
>
Ok, sounds reasonable.

Marc