Re: Make define-builtin-markup{, -list}-command #:category #:properties keywords (issue160048)

[Carl Sorensen]

On 12/3/09 10:54 AM, "David Kastrup" <address@hidden> wrote:

> Carl Sorensen <address@hidden> writes:
> 
>> On 12/3/09 9:12 AM, "David Kastrup" <address@hidden> wrote:
>> 
>>> Graham Percival <address@hidden> writes:
>>>> Are you aware that the world does not revolve around you?  For the
>>>> past few days, the git tree has been broken -- I cannot compile
>>>> the regression tests or documentation.  Not just "the regtests
>>>> look like garbage", but "there is a segfault when attempting to
>>>> compile a regtest and the docs".
>>>> 
>>>> IMNSHO, fixing that problem takes priority over new code.
>>> 
>>> Ok, here is your fix.  Can you now check the memory leak?
>> 
>> Thanks,
>> 
>> A (slightly different) fix has already been pushed.
> 
> I might add, a fix that is much more complex, uses more variables, uses
> more code, and checks conditions multiple times in different code paths.
> 
> Your patch increases the line count.  Mine reduces it.
> 
> That does not mean that my patch can't be improved.
> 
> It definitely needs to get the robust_scm2double(default_outside_staff,
> 0) fix from the version you pushed.  While the regression tests don't
> catch that yet, it definitely is an issue.  And it will be better to
> revert the condition of the if to (last == 0) and consequently
> interchange the then/else branches.  That way the short code path comes
> first, the structure is more apparent, and the reading order corresponds
> better with the execution order.

Great!

Roll a patch and post it for comments on Rietveld.

Carl