Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1637)

Issue 6406051: span-bar.cc: Sort bar-line extents in case alignAboveContext is active (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 years, 11 months ago by Keith
Modified:
6 years, 11 months ago
Reviewers:
Graham Percival, marc
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Patch Set 1 #

Patch Set 2 : regtest #

Patch Set 3 : restore sort from 1846 #

Total comments: 1

Patch Set 4 : only need one sort #

Unified diffs Side-by-side diffs Delta from patch set Stats (+16 lines, -11 lines) Patch
M input/regression/alignment-order.ly View 1 2 1 chunk +13 lines, -11 lines 0 comments Download
M lily/span-bar.cc View 1 2 1 chunk +3 lines, -0 lines 0 comments Download

Messages

Total messages: 6
Graham Percival
If this isn't covered already, then could you add a regtest for this? Mike presumably ...
6 years, 11 months ago (2012-07-17 05:42:55 UTC) #1
Keith
On Mon, 16 Jul 2012 22:42:55 -0700, <graham@percival-music.ca> wrote: > regtest I'm expanding `alignment-order.ly` after ...
6 years, 11 months ago (2012-07-17 15:51:20 UTC) #2
Keith
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 ...
6 years, 11 months ago (2012-07-18 04:39:06 UTC) #3
marc
Am 17.07.2012 17:51, schrieb Keith OHara: > On Mon, 16 Jul 2012 22:42:55 -0700, <graham@percival-music.ca> ...
6 years, 11 months ago (2012-07-20 18:38:35 UTC) #4
Keith
On Fri, 20 Jul 2012 11:38:34 -0700, Marc Hohl <marc@hohlart.de> wrote: > I think that ...
6 years, 11 months ago (2012-07-20 21:41:28 UTC) #5
marc
6 years, 11 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
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b