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

[David Kastrup]

Carl Sorensen <address@hidden> writes:

> On 12/3/09 10:54 AM, "David Kastrup" <address@hidden> wrote:
>
>> Carl Sorensen <address@hidden> writes:
>>
>>> 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.

I am still in need of actual tests (or at least working recipes for
reproducing the problems reported with earlier versions) of the memory
situation of the patch I put a week ago on Rietveld.  Putting another
there would be a distraction.

I have been told by Graham that the segfault from script-column.cc was
what precluded reviews of my older patch set.  So I addressed that.
Even though there was no report in the bug tracker to go by.

But I certainly will not invest a lot of work, upload, branch creation,
discussion whatever because you are not interested in making your fix
cleaner after being told how.

Your first commit did not even compile.  If you don't jump through any
review hoops for your code, why should I for _your_ code?

It is enough work being made to jump through hoops for my own code.

-- 
David Kastrup