commit cd9e42dc3529980257d2b0fe6fd623fd2b99a1e6
Author: Jean-Marc Lasgouttes <[email protected]>
Date:   Wed Dec 13 11:10:49 2017 +0100

    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.
---
 src/Buffer.cpp                       |    4 ++--
 src/Undo.cpp                         |    4 +++-
 src/frontends/qt4/GuiApplication.cpp |   15 ++++-----------
 src/frontends/qt4/GuiDocument.cpp    |    7 ++-----
 src/insets/InsetGraphics.cpp         |    4 ++--
 src/insets/InsetLabel.cpp            |    4 ++--
 6 files changed, 15 insertions(+), 23 deletions(-)

diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index 558c351..e063171 100644
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -2596,7 +2596,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:
@@ -2846,7 +2847,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 b388b9c..eb77228 100644
--- 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"
@@ -667,7 +668,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 5d7f559..b53126f 100644
--- a/src/frontends/qt4/GuiApplication.cpp
+++ b/src/frontends/qt4/GuiApplication.cpp
@@ -1390,9 +1390,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).
@@ -1401,10 +1401,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_;
 }
@@ -1861,8 +1857,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, ';');
@@ -1870,9 +1866,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 002e2ab..3e4fa06 100644
--- a/src/frontends/qt4/GuiDocument.cpp
+++ b/src/frontends/qt4/GuiDocument.cpp
@@ -4299,7 +4299,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();
@@ -4366,10 +4367,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 9c63ade..eac58ff 100644
--- 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 65e3991..4dd0713 100644
--- 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();
 }
 
 

Reply via email to