lilypond-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Calculate wordwrapped/justified lines using the stencil stacking pri


From: dak
Subject: Re: Calculate wordwrapped/justified lines using the stencil stacking primitives (issue 13827044)
Date: Tue, 24 Sep 2013 08:53:08 +0000

Reviewers: janek,

Message:
On 2013/09/24 07:46:17, janek wrote:
could you give an example of how it was failing before?

Regtests change, and the new regtests seem pretty consistent.  I was not
going to measure this out in detail.  The comments of the old code also
spell out a failure case.  Check out
\markuplist
#(map (lambda (n)
       #{ \markup
            \override #'(line-width . 60)
            \column {
              \override #`(text-direction . ,(if (zero? (remainder n 2))
RIGHT LEFT))
              \justify { $@(map number->string (iota (quotient n 2))) }
              \draw-hline
            }
       #})
  (iota 160 60))

and you'll see that the old code has problems in the patterns ending on
85 and 104.  Do you have an actual problem with how the new code is
written in comparison to the old code?  I sometimes have a hard time
understanding why people object against refactoring code even when the
old code has been prodded until it happens to turn out the correct thing
most of the time without anybody exactly knowing why.

In this case, it happens to fail sometimes.  The failure case _has_
actually been described in comments, but the old design of the code made
it probably too cumbersome for anybody to cater for that case.

But assuming that this code would not actually fix anything but lead to
equivalent results: would you object to it?  Can you point out what part
in particular you object to?  Because if there is stuff you don't want
to have in the code base unless it actually fixes something, chances are
that you don't want to have it in the code base either way, and it
should be done differently.

"It works" is not the ultimate excuse for everything.  Because if no-one
can understand _why_ it works, then even in the case that it works in
100% of all cases now, future changes might break it unintentionally
when nobody can understand the reason it works.

Issue 3552 is such a case where I changed code not because of any known
breakage, but because the risk of future breakage in connection with
superficially unrelated and/or user code.

Description:
Calculate wordwrapped/justified lines using the stencil stacking
primitives

In that manner, the spacing decisions are sure to be correct.

Please review this at https://codereview.appspot.com/13827044/

Affected files (+87, -63 lines):
  M scm/define-markup-commands.scm





reply via email to

[Prev in Thread] Current Thread [Next in Thread]