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
