ok, done.

A/

On Tue, Oct 14, 2014 at 5:33 PM, Richard Heck <[email protected]> wrote:
> On 10/14/2014 11:17 AM, Alfredo Braunstein wrote:
>>
>> On Tue, Oct 14, 2014 at 8:51 AM, Alfredo Braunstein <[email protected]>
>> wrote:
>>>
>>> On Mon, Oct 13, 2014 at 6:27 PM, Jean-Marc Lasgouttes
>>> <[email protected]> wrote:
>>>>
>>>> Le 13/10/2014 18:15, Alfredo Braunstein a écrit :
>>>>>>>
>>>>>>> The fix however seems bogus to me:
>>>>>>
>>>>>>
>>>>>>
>>>>>>> -                cur.forwardPos();
>>>>>>> +                cur.top().forwardPos();
>>>>>>
>>>>>>
>>>>>>
>>>>>> Why bogus? Why do you want to revert it?
>>>>>
>>>>>
>>>>> To me it seems wrong (for the reasons explained below). But maybe I
>>>>> don't understand it, could you explain? Was your intention to make the
>>>>> same change also to the other instance of cur.forwardPos() in that
>>>>> function? Right now it doesn't achieve what the commit message says
>>>>> and doesn't stop the crash...
>>>>
>>>>
>>>> If I remember correctly, I thought the change was obviously needed, but
>>>> I
>>>> knew deep inside that it was not fixing the real bug. It may be that I
>>>> was
>>>> wrong and that we want to go inside the inset if it can contains
>>>> changes.
>>>>
>>>>>> I think that the functions should be rewritten from scratch :)
>>>
>>> What about the following? I've factored out the change-finding part
>>> (that could enter into insets) from the change-selection part, that
>>> should not, and rewritten most of it. From my limited testing it seems
>>> fine (more welcome of course). As a plus, the logic is tons simpler
>>> and we save 25 lines.
>>>
>>> A/
>>>
>>>   Text.cpp    |    7 ---
>>>   lyxfind.cpp |  121
>>> +++++++++++++++++++++++-------------------------------------
>>>   lyxfind.h   |    5 ++
>>>   3 files changed, 54 insertions(+), 79 deletions(-)
>>
>> Any kind soul that would help me test this? The testing should be done
>> on change tracking in general (i.e. next/previous change while
>> merging), and specifically "accept/reject change" while stepping on a
>> changed part. The scope is to determine if behaviour with the patch
>> applied is better than what we have and if there are no obvious
>> problems (e.g. crashes, like #9145).
>
>
> Unfortunately, I do not have time right now. If no one else steps forward,
> though, commit to master and wait for complaints. I'm not sure how things
> got done back in the day, but that is often how we proceed nowadays, except
> with really big stuff (like JMarc's str-metrics rewrite). It's a function of
> our resource starvation.
>
> Richard
>

Reply via email to