lilypond-devel
[Top][All Lists]
Advanced

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

Re: [Lilypond-auto] Issue 3256 in lilypond: Patch: Eliminates side poisi


From: address@hidden
Subject: Re: [Lilypond-auto] Issue 3256 in lilypond: Patch: Eliminates side poisition calculations for system-start-text grobs.
Date: Sat, 30 Mar 2013 08:40:40 +0200

On 30 mars 2013, at 08:27, address@hidden wrote:

> Updates:
>       Labels: -Patch-push
> 
> Comment #8 on issue 3256 by address@hidden: Patch: Eliminates side poisition 
> calculations for system-start-text grobs.
> http://code.google.com/p/lilypond/issues/detail?id=3256
> 
> This patch has broken incipits, like in input/regression/incipit.ly.  Its 
> commit message is _totally_ different from the issue description or the 
> Rietveld issue description.  It does not contain the issue number either.  
> You have not marked the issue as fixed, and the commit id is not to be found 
> anywhere.
> 
> In consequence, it is not possible to track the commit to this particular 
> issue and review.  It is not the first time, either.  Please keep the commit 
> messages and the issue and review descriptions the same and mark the issues 
> as fixed _including_ the commit id _right_ _after_ _pushing_.
> 
> The commit in question is
> commit 6e4e69f2735a764eab2e6f70f83546461da0203b
> Author: Mike Solomon <address@hidden>
> Date:   Fri Mar 29 05:55:13 2013 +0100
> 
>    Uses special X alignment for instrument names.
> 
>    Previously, instrument names used the side-position-interface for
>    calculations in its alignment function.  This made no sense, as it never
>    had any side-support elements, requiring a kludge to get them to
>    align properly.
> 
>    Here, we remove that kludge and align them by aligning them to their
>    right end and adding padding.  We also change the name of the grob array
>    in which vertical axis groups are stored to
>    system-start-text-align-elements so that the grob is not accidentally
>    treated as an axis-group implementing an elements array.
> 
> I propose reverting it since using the instrumentName for prefatory material 
> is documented and previously working behavior and it is not clear what this 
> will break in addition to incipits.  So it is not really fit to be included 
> in a stable release at this point of time.
> 


It is not clear what this patch does and doesn't break until people use it.  
Unfortunately, the regtest checker does not pick up on certain differences, so 
it is important for users testing the software to signal these.  I fix them 
right away.  You are absolutely right that it is not clear what this will break 
in addition to incipits, but I'd rather deal with that as it crops up as it is 
impossible to know given our current regtest checking suite. With Phil's pixel 
comparator this sort of problem will go away, but that'd need to get integrated 
into the build system.

Fixes are quick, so I just need to know the problem.  I can then fix it ASAP.

Cheers,
MS


reply via email to

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