Re: Please test. Re: [LyX/master] Improve undo of consecutive insertions/deletions

[Richard Heck]

On 03/17/2015 05:08 AM, Jean-Marc Lasgouttes wrote:
Le 16/03/2015 18:43, Jean-Marc Lasgouttes a écrit :
commit ed3d9544a331a7c28730089e7b059eea592584c4
Author: Jean-Marc Lasgouttes <lasgout...@lyx.org>
Date:   Thu Mar 12 14:47:39 2015 +0100
     Improve undo of consecutive insertions/deletions

This commit implements an idea of Scott about making undo of typed 
text more useful. I'd like everybody to try it out and tell me whether 
the behavior is useful. There are other alternatives, like saving at 
word level. I am not sure whether there are better ideas implemented 
in other programs/platforms.

I see the description below, but can you say what the practical effect 
of the change is supposed to be? I.e., what will we as users see?

Richard



     The old scheme was:
      * multiple insertions are undone by groups of 20
      * multiple deletions are undone in one big block

     The new scheme is to stop merging undo elements after 2 seconds 
of elapsed time.

     Moreover, the merging of undo elements stops when the cursor has 
moved. Potentially, this could allow to remove many of the 
finishUndo() calls.

     Fixes bug #9204.

diff --git a/src/Text.cpp b/src/Text.cpp
index ea9fba7..c438f77 100644
--- a/src/Text.cpp
+++ b/src/Text.cpp
@@ -198,7 +198,7 @@ void mergeParagraph(BufferParams const & bparams,


  Text::Text(InsetText * owner, bool use_default_layout)
-    : owner_(owner), autoBreakRows_(false), undo_counter_(0)
+    : owner_(owner), autoBreakRows_(false)
  {
      pars_.push_back(Paragraph());
      Paragraph & par = pars_.back();
@@ -212,7 +212,7 @@ Text::Text(InsetText * owner, bool 
use_default_layout)


  Text::Text(InsetText * owner, Text const & text)
-    : owner_(owner), autoBreakRows_(text.autoBreakRows_), 
undo_counter_(0)
+    : owner_(owner), autoBreakRows_(text.autoBreakRows_)
  {
      pars_ = text.pars_;
      ParagraphList::iterator const end = pars_.end();
@@ -1081,15 +1081,6 @@ void Text::charInserted(Cursor & cur)
  {
      Paragraph & par = cur.paragraph();

-    // Here we call finishUndo for every 20 characters inserted.
-    // This is from my experience how emacs does it. (Lgb)
-    if (undo_counter_ < 20) {
-        ++undo_counter_;
-    } else {
-        cur.finishUndo();
-        undo_counter_ = 0;
-    }
-
      // register word if a non-letter was entered
      if (cur.pos() > 1
          && !par.isWordSeparator(cur.pos() - 2)
diff --git a/src/Text.h b/src/Text.h
index e6759ef..4c8ab94 100644
--- a/src/Text.h
+++ b/src/Text.h
@@ -382,8 +382,6 @@ private:
      bool autoBreakRows_;
      /// position of the text in the buffer.
      DocIterator macrocontext_position_;
-    ///
-    unsigned int undo_counter_;
  };


diff --git a/src/Undo.cpp b/src/Undo.cpp
index 2a01c8b..7a5b176 100644
--- a/src/Undo.cpp
+++ b/src/Undo.cpp
@@ -33,6 +33,7 @@
  #include "support/debug.h"
  #include "support/gettext.h"
  #include "support/lassert.h"
+#include "support/lyxtime.h"

  #include <algorithm>
  #include <deque>
@@ -72,7 +73,7 @@ struct UndoElement
                  bool lc, size_t gid) :
          kind(kin), cur_before(cb), cell(cel), from(fro), end(en),
          pars(pl), array(ar), bparams(0),
-        lyx_clean(lc), group_id(gid)
+        lyx_clean(lc), group_id(gid), time(current_time())
          {
          }
      ///
@@ -80,11 +81,11 @@ struct UndoElement
                  bool lc, size_t gid) :
          kind(ATOMIC_UNDO), cur_before(cb), cell(), from(0), end(0),
          pars(0), array(0), bparams(new BufferParams(bp)),
-        lyx_clean(lc), group_id(gid)
+        lyx_clean(lc), group_id(gid), time(current_time())
      {
      }
      ///
-    UndoElement(UndoElement const & ue)
+    UndoElement(UndoElement const & ue) : time(current_time())
      {
          kind = ue.kind;
          cur_before = ue.cur_before;
@@ -127,6 +128,8 @@ struct UndoElement
      bool lyx_clean;
      /// the element's group id
      size_t group_id;
+    /// timestamp
+    time_t time;
  private:
      /// Protect construction
      UndoElement();
@@ -325,9 +328,13 @@ void Undo::Private::doRecordUndo(UndoKind kind,
          && samePar(stack.top().cell, cell)
          && stack.top().kind == kind
          && stack.top().from == from
-        && stack.top().end == end) {
+        && stack.top().end == end
+        && stack.top().cur_after == cur_before
+        && current_time() - stack.top().time <= 2) {
          // reset cur_after; it will be filled correctly by 
endUndoGroup.
          stack.top().cur_after = CursorData();
+        // update the timestamp of the undo element
+        stack.top().time = current_time();
          return;
      }