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

[David Kastrup]

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.

-- 
David Kastrup