If this isn't covered already, then could you add a regtest for this? Mike presumably ...
11 years, 10 months ago
(2012-07-17 05:42:55 UTC)
#1
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.
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 ...
11 years, 10 months ago
(2012-07-18 04:39:06 UTC)
#3
Am 17.07.2012 17:51, schrieb Keith OHara: > On Mon, 16 Jul 2012 22:42:55 -0700, <graham@percival-music.ca> ...
11 years, 9 months ago
(2012-07-20 18:38:35 UTC)
#4
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
On Fri, 20 Jul 2012 11:38:34 -0700, Marc Hohl <marc@hohlart.de> wrote: > I think that ...
11 years, 9 months ago
(2012-07-20 21:41:28 UTC)
#5
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.
Am 20.07.2012 23:43, schrieb Keith OHara: > On Fri, 20 Jul 2012 11:38:34 -0700, Marc ...
11 years, 9 months ago
(2012-07-21 08:07:10 UTC)
#6
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
Issue 6406051: span-bar.cc: Sort bar-line extents in case alignAboveContext is active
(Closed)
Created 11 years, 10 months ago by Keith
Modified 11 years, 9 months ago
Reviewers: Graham Percival, marc
Base URL:
Comments: 1