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

[Carl Sorensen]

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

> 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.

Fair enough.  I apologize for replying too soon and too short.

> 
> 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?

Yes, and then I posted another patch on Rietveld, because I recognized my
mistakes.  I did not see any comment from you on that patch.

That's why I responded inappropriately.  Again, I'm not justifying, just
explaining.  I'll review the issue some more.

Carl