On Tue, Oct 14, 2014 at 8:51 AM, Alfredo Braunstein <abrau...@lyx.org> wrote: > On Mon, Oct 13, 2014 at 6:27 PM, Jean-Marc Lasgouttes > <lasgout...@lyx.org> 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). Alfredo
