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(-)
diff --git a/src/Text.cpp b/src/Text.cpp
index bd4e23d..a2ae1ae 100644
--- a/src/Text.cpp
+++ b/src/Text.cpp
@@ -1276,8 +1276,7 @@ void Text::acceptOrRejectChanges(Cursor & cur, ChangeOp
op)
LBUFERR(this == cur.text());
if (!cur.selection()) {
- bool const changed = cur.paragraph().isChanged(cur.pos());
- if (!(changed && findNextChange(&cur.bv())))
+ if (!selectChange(cur))
return;
}
@@ -1293,7 +1292,6 @@ void Text::acceptOrRejectChanges(Cursor & cur, ChangeOp
op)
bool endsBeforeEndOfPar = (endPos < pars_[endPit].size());
// first, accept/reject changes within each individual paragraph (do
not consider end-of-par)
-
for (pit_type pit = begPit; pit <= endPit; ++pit) {
pos_type parSize = pars_[pit].size();
@@ -1369,11 +1367,8 @@ void Text::acceptOrRejectChanges(Cursor & cur, ChangeOp
op)
}
// finally, invoke the DEPM
-
deleteEmptyParagraphMechanism(begPit, endPit,
cur.buffer()->params().track_changes);
- //
-
cur.finishUndo();
cur.clearSelection();
setCursorIntern(cur, begPit, begPos);
diff --git a/src/lyxfind.cpp b/src/lyxfind.cpp
index 6b9556e..f3b3eef 100644
--- a/src/lyxfind.cpp
+++ b/src/lyxfind.cpp
@@ -390,96 +390,70 @@ bool lyxreplace(BufferView * bv,
}
-namespace {
-bool findChange(DocIterator & cur, bool next)
+bool findNextChange(DocIterator & cur)
{
- if (!next)
- cur.backwardPos();
- for (; cur; next ? cur.forwardPos() : cur.backwardPos())
- if (cur.inTexted() && cur.paragraph().isChanged(cur.pos())) {
- if (!next)
- // if we search backwards, take a step forward
- // to correctly set the anchor
- cur.top().forwardPos();
+ for (; cur; cur.forwardPos())
+ if (cur.inTexted() && cur.paragraph().isChanged(cur.pos()))
return true;
- }
-
return false;
}
-bool findChange(BufferView * bv, bool next)
+bool findPreviousChange(DocIterator & cur)
{
- Cursor cur(*bv);
- cur.setCursor(next ? bv->cursor().selectionEnd()
- : bv->cursor().selectionBegin());
-
- // Are we within a change ? Then first search forward (backward),
- // clear the selection and search the other way around (see the end
- // of this function). This will avoid changes to be selected half.
- bool search_both_sides = false;
- Cursor tmpcur = cur;
- // Find enclosing text cursor
- while (tmpcur.inMathed())
- tmpcur.pop_back();
- Change change_next_pos
- = tmpcur.paragraph().lookupChange(tmpcur.pos());
- if (change_next_pos.changed()) {
- if (cur.inMathed()) {
- cur = tmpcur;
- search_both_sides = true;
- } else if (tmpcur.pos() > 0 && tmpcur.inTexted()) {
- Change change_prev_pos
- = tmpcur.paragraph().lookupChange(tmpcur.pos()
- 1);
- if (change_next_pos.isSimilarTo(change_prev_pos))
- search_both_sides = true;
- }
+ for (cur.backwardPos(); cur; cur.backwardPos()) {
+ if (cur.inTexted() && cur.paragraph().isChanged(cur.pos()))
+ return true;
}
+ return false;
+}
- // find the next change
- if (!findChange(cur, next))
- return false;
-
- bv->mouseSetCursor(cur, false);
-
- CursorSlice & tip = cur.top();
-
- if (!next && tip.pos() > 0)
- // take a step into the change
- tip.backwardPos();
- Change orig_change = tip.paragraph().lookupChange(tip.pos());
+bool selectChange(Cursor & cur, bool forward)
+{
+ if (!cur.inTexted() || !cur.paragraph().isChanged(cur.pos()))
+ return false;
+ Change ch = cur.paragraph().lookupChange(cur.pos());
- if (next) {
- for (; tip.pit() < tip.lastpit() || tip.pos() < tip.lastpos();
tip.forwardPos()) {
- Change change = tip.paragraph().lookupChange(tip.pos());
- if (!change.isSimilarTo(orig_change))
- break;
- }
- } else {
- for (; tip.pit() > 0 || tip.pos() > 0;) {
- tip.backwardPos();
- Change change = tip.paragraph().lookupChange(tip.pos());
- if (!change.isSimilarTo(orig_change)) {
- // take a step forward to correctly set the
selection
- tip.forwardPos();
- break;
- }
- }
+ CursorSlice tip1 = cur.top();
+ for (; tip1.pit() < tip1.lastpit() || tip1.pos() < tip1.lastpos();
tip1.forwardPos()) {
+ Change ch2 = tip1.paragraph().lookupChange(tip1.pos());
+ if (!ch2.isSimilarTo(ch))
+ break;
}
-
- if (!search_both_sides) {
- // Now set the selection.
- bv->mouseSetCursor(cur, true);
- } else {
- bv->mouseSetCursor(cur, false);
- findChange(bv, !next);
+ CursorSlice tip2 = cur.top();
+ for (; tip2.pit() > 0 || tip2.pos() > 0;) {
+ tip2.backwardPos();
+ Change ch2 = tip2.paragraph().lookupChange(tip2.pos());
+ if (!ch2.isSimilarTo(ch)) {
+ // take a step forward to correctly set the selection
+ tip2.forwardPos();
+ break;
+ }
}
-
+ if (forward)
+ swap(tip1, tip2);
+ cur.top() = tip1;
+ cur.bv().mouseSetCursor(cur, false);
+ cur.top() = tip2;
+ cur.bv().mouseSetCursor(cur, true);
return true;
}
+
+
+namespace {
+
+
+bool findChange(BufferView * bv, bool forward)
+{
+ Cursor cur(*bv);
+ cur.setCursor(forward ? bv->cursor().selectionEnd()
+ : bv->cursor().selectionBegin());
+ forward ? findNextChange(cur) : findPreviousChange(cur);
+ return selectChange(cur, forward);
}
+}
bool findNextChange(BufferView * bv)
{
@@ -493,6 +467,7 @@ bool findPreviousChange(BufferView * bv)
}
+
namespace {
typedef vector<pair<string, string> > Escapes;
diff --git a/src/lyxfind.h b/src/lyxfind.h
index 31f81e8..94efba8 100644
--- a/src/lyxfind.h
+++ b/src/lyxfind.h
@@ -26,6 +26,7 @@ namespace lyx {
class Buffer;
+class Cursor;
class BufferView;
class DocIterator;
class FuncRequest;
@@ -72,6 +73,10 @@ bool findNextChange(BufferView * bv);
/// find the previous change in the buffer
bool findPreviousChange(BufferView * bv);
+/// select change under the cursor
+bool selectChange(Cursor & cur, bool forward = true);
+
+
class FindAndReplaceOptions {
public:
typedef enum {
