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 cd9e42dc35)

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();
 }
 
 

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;
 }
 

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;
 	}
 

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();
 }
 
 

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();
 }
 
 

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();
 }
 
 

status.23x

@@ -44,6 +44,7 @@ What's new
 
 * USER INTERFACE
 
+- Improve Undo for operations that act on several buffers (bug 10823).
 
 
 * INTERNALS