Improve UndoGroupHelper and use it more

Now the helper class contains logic that checks whether buffer are
known before closing them. This avoids potential crashes.

Use it in different places to siplify code. It is not clear at this
point whether it should be used everywhere.

Followup to bug #10847.

(cherry picked from commit cd9e42dc3529980257d2b0fe6fd623fd2b99a1e6)

diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index ffe522e0f9845a7838526037969df4aedc4856bf..ca3c049b4dbd096c03cfba779a7d8a2db7b1e44b 100644 (file)
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -2650,7 +2650,8 @@ void Buffer::dispatch(FuncRequest const & func, DispatchResult & dr)
        string const argument = to_utf8(func.argument());
        // We'll set this back to false if need be.
        bool dispatched = true;
-       undo().beginUndoGroup();
+       // This handles undo groups automagically
+       UndoGroupHelper ugh(this);
 
        switch (func.action()) {
        case LFUN_BUFFER_TOGGLE_READ_ONLY:
@@ -2902,7 +2903,6 @@ void Buffer::dispatch(FuncRequest const & func, DispatchResult & dr)
                break;
        }
        dr.dispatched(dispatched);
-       undo().endUndoGroup();
 }
 
 

diff --git a/src/Undo.cpp b/src/Undo.cpp
index 49f636cbb9eb87fbe99a2b88b224b92f483be6fb..01394bc27f79ff2a309be37d55f58b5a7644949c 100644 (file)
--- a/src/Undo.cpp
+++ b/src/Undo.cpp
@@ -18,6 +18,7 @@
 #include "Undo.h"
 
 #include "Buffer.h"
+#include "BufferList.h"
 #include "BufferParams.h"
 #include "buffer_funcs.h"
 #include "Cursor.h"
@@ -661,7 +662,8 @@ UndoGroupHelper::UndoGroupHelper(Buffer * buf) : d(new UndoGroupHelper::Impl)
 UndoGroupHelper::~UndoGroupHelper()
 {
        for (Buffer * buf : d->buffers_)
-               buf->undo().endUndoGroup();
+               if (theBufferList().isLoaded(buf) || theBufferList().isInternal(buf))
+                       buf->undo().endUndoGroup();
        delete d;
 }
 

diff --git a/src/frontends/qt4/GuiApplication.cpp b/src/frontends/qt4/GuiApplication.cpp
index 35b639c9ac490e27e09ee25c123afce961eb9840..0ce130eb69cf9146ef0a0888721b7dfdf0095a54 100644 (file)
--- a/src/frontends/qt4/GuiApplication.cpp
+++ b/src/frontends/qt4/GuiApplication.cpp
@@ -1395,9 +1395,9 @@ DispatchResult const & GuiApplication::dispatch(FuncRequest const & cmd)
        if (current_view_ && current_view_->currentBufferView()) {
                current_view_->currentBufferView()->cursor().saveBeforeDispatchPosXY();
                buffer = &current_view_->currentBufferView()->buffer();
-               if (buffer)
-                       buffer->undo().beginUndoGroup();
        }
+       // This handles undo groups automagically
+       UndoGroupHelper ugh(buffer);
 
        DispatchResult dr;
        // redraw the screen at the end (first of the two drawing steps).
@@ -1406,10 +1406,6 @@ DispatchResult const & GuiApplication::dispatch(FuncRequest const & cmd)
        dispatch(cmd, dr);
        updateCurrentView(cmd, dr);
 
-       // the buffer may have been closed by one action
-       if (theBufferList().isLoaded(buffer) || theBufferList().isInternal(buffer))
-               buffer->undo().endUndoGroup();
-
        d->dispatch_result_ = dr;
        return d->dispatch_result_;
 }
@@ -1873,8 +1869,8 @@ void GuiApplication::dispatch(FuncRequest const & cmd, DispatchResult & dr)
                // FIXME: this LFUN should also work without any view.
                Buffer * buffer = (current_view_ && current_view_->documentBufferView())
                                  ? &(current_view_->documentBufferView()->buffer()) : 0;
-               if (buffer)
-                       buffer->undo().beginUndoGroup();
+               // This handles undo groups automagically
+               UndoGroupHelper ugh(buffer);
                while (!arg.empty()) {
                        string first;
                        arg = split(arg, first, ';');
@@ -1882,9 +1878,6 @@ void GuiApplication::dispatch(FuncRequest const & cmd, DispatchResult & dr)
                        func.setOrigin(cmd.origin());
                        dispatch(func);
                }
-               // the buffer may have been closed by one action
-               if (theBufferList().isLoaded(buffer) || theBufferList().isInternal(buffer))
-                       buffer->undo().endUndoGroup();
                break;
        }
 

diff --git a/src/frontends/qt4/GuiDocument.cpp b/src/frontends/qt4/GuiDocument.cpp
index 09203781f0080a32dee27dfc67113478258d5552..560883ec6a545e6c00c7d62fb9bf601c58faa748 100644 (file)
--- a/src/frontends/qt4/GuiDocument.cpp
+++ b/src/frontends/qt4/GuiDocument.cpp
@@ -4327,7 +4327,8 @@ void GuiDocument::dispatchParams()
        // We need a non-const buffer object.
        Buffer & buf = const_cast<BufferView *>(bufferview())->buffer();
        // There may be several undo records; group them (bug #8998)
-       buf.undo().beginUndoGroup();
+       // This handles undo groups automagically
+       UndoGroupHelper ugh(&buf);
 
        // This must come first so that a language change is correctly noticed
        setLanguage();
@@ -4394,10 +4395,6 @@ void GuiDocument::dispatchParams()
        // If we used an LFUN, we would not need these two lines:
        BufferView * bv = const_cast<BufferView *>(bufferview());
        bv->processUpdateFlags(Update::Force | Update::FitCursor);
-
-       // Don't forget to close the group. Note that it is important
-       // to check that there is no early return in the method.
-       buf.undo().endUndoGroup();
 }
 
 

diff --git a/src/insets/InsetGraphics.cpp b/src/insets/InsetGraphics.cpp
index 94182a73269fa5585adf4cce8de06e6d5d149b59..a46d3fbc64059611d2dac01f2ba0b32a01df9b8c 100644 (file)
--- a/src/insets/InsetGraphics.cpp
+++ b/src/insets/InsetGraphics.cpp
@@ -1150,7 +1150,8 @@ void unifyGraphicsGroups(Buffer & b, string const & argument)
        InsetGraphicsParams params;
        InsetGraphics::string2params(argument, b, params);
 
-       b.undo().beginUndoGroup();
+       // This handles undo groups automagically
+       UndoGroupHelper ugh(&b);
        Inset & inset = b.inset();
        InsetIterator it  = inset_iterator_begin(inset);
        InsetIterator const end = inset_iterator_end(inset);
@@ -1165,7 +1166,6 @@ void unifyGraphicsGroups(Buffer & b, string const & argument)
                        }
                }
        }
-       b.undo().endUndoGroup();
 }
 
 

diff --git a/src/insets/InsetLabel.cpp b/src/insets/InsetLabel.cpp
index 789a774ac6b993ecc2114cf9091c3ad56ecdb913..7f3651f3a382e9fae870ef94a6d58ff9b97e0698 100644 (file)
--- a/src/insets/InsetLabel.cpp
+++ b/src/insets/InsetLabel.cpp
@@ -97,12 +97,12 @@ void InsetLabel::updateLabelAndRefs(docstring const & new_label,
        if (label == old_label)
                return;
 
-       buffer().undo().beginUndoGroup();
+       // This handles undo groups automagically
+       UndoGroupHelper ugh(&buffer());
        if (cursor)
                cursor->recordUndo();
        setParam("name", label);
        updateReferences(old_label, label);
-       buffer().undo().endUndoGroup();
 }
 
 

diff --git a/status.23x b/status.23x
index 30eff6cbc0057832c986bb4c751873555eeaf75c..7f5727714bd89822dd11e14e1a05c78402f409ac 100644 (file)
--- a/status.23x
+++ b/status.23x
@@ -44,6 +44,7 @@ What's new
 
 * USER INTERFACE
 
+- Improve Undo for operations that act on several buffers (bug 10823).
 
 
 * INTERNALS