commit 1dc134fb60806ee0d27371049392ab72223b6cd0
Author: Jean-Marc Lasgouttes <lasgout...@lyx.org>
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.
    
    (cherry picked from commit cd9e42dc3529980257d2b0fe6fd623fd2b99a1e6)
---
 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 ++--
 status.23x                           |    1 +
 7 files changed, 16 insertions(+), 23 deletions(-)

diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index ffe522e..ca3c049 100644
--- 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 49f636c..01394bc 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"
@@ -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 35b639c..0ce130e 100644
--- 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 0920378..560883e 100644
--- 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 94182a7..a46d3fb 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 789a774..7f3651f 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();
 }
 
 
diff --git a/status.23x b/status.23x
index 30eff6c..7f57277 100644
--- 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

Reply via email to