Completion: handle undo in insets' insertCompletion methods

I mathed, undo should record the underlying inset on completion,
whereas in test recording the paragraph is enough.

Therefore the recordUndo() in GuiCompleter::tab is removed, and the
relevant recordUndo/recordUndoInset are used at the right places.

As a further cleanup, the parameter `finished' of
Text::insertCompletion is removed as it is useless.

Fixes bug #12581.

src/Text.cpp

@@ -2395,9 +2395,10 @@ CompletionList const * Text::createCompletionList(Cursor const & cur) const
 }
 
 
-bool Text::insertCompletion(Cursor & cur, docstring const & s, bool /*finished*/)
+bool Text::insertCompletion(Cursor & cur, docstring const & s)
 {
 	LBUFERR(cur.bv().cursor() == cur);
+	cur.recordUndo();
 	cur.insert(s);
 	cur.bv().cursor() = cur;
 	if (!(cur.result().screenUpdate() & Update::Force))

src/Text.h

@@ -331,8 +331,9 @@ public:
 	bool completionSupported(Cursor const & cur) const;
 	///
 	CompletionList const * createCompletionList(Cursor const & cur) const;
-	///
-	bool insertCompletion(Cursor & cur, docstring const & s, bool /*finished*/);
+	/// Do a completion at the cursor position. Return true on success.
+	/// The completion does not contain the prefix. Handles undo.
+	bool insertCompletion(Cursor & cur, docstring const & s);
 	///
 	docstring completionPrefix(Cursor const & cur) const;
 	/// find a paragraph before \p par with the given \p depth, if such

src/frontends/qt/GuiCompleter.cpp

@@ -682,9 +682,8 @@ void GuiCompleter::tab()
 		return;
 	}
 
-	// Make undo possible
+	// Prepare for undo (recordUndo is invoked in the insets' insertCompletion methods)
 	UndoGroupHelper ugh(cur.buffer());
-	cur.recordUndo();
 
 	// If completion is active, at least complete by one character
 	docstring prefix = cur.inset().completionPrefix(cur);

src/insets/Inset.h

@@ -402,10 +402,10 @@ public:
 	/// Returns the completion prefix to filter the suggestions for completion.
 	/// This is only called if completionList returned a non-null list.
 	virtual docstring completionPrefix(Cursor const &) const;
-	/// Do a completion at the cursor position. Return true on success.
-	/// The completion does not contain the prefix. If finished is true, the
-	/// completion is final. If finished is false, completion might only be
-	/// a partial completion.
+	/// Do a completion at the cursor position. Return true on success. Handles undo.
+	/// The completion does not contain the prefix.
+	/// If finished is true, the completion is final, otherwise it
+	/// might be only partial. (only useful for mathed)
 	virtual bool insertCompletion(Cursor & /*cur*/,
 		docstring const & /*completion*/, bool /*finished*/)
 		{ return false; }

src/insets/InsetTabular.cpp

@@ -7747,12 +7747,12 @@ docstring InsetTabular::completionPrefix(Cursor const & cur) const
 }
 
 
-bool InsetTabular::insertCompletion(Cursor & cur, docstring const & s, bool finished)
+bool InsetTabular::insertCompletion(Cursor & cur, docstring const & s, bool /*finished*/)
 {
 	if (!completionSupported(cur))
 		return false;
 
-	return cur.text()->insertCompletion(cur, s, finished);
+	return cur.text()->insertCompletion(cur, s);
 }
 
 

src/insets/InsetTabular.h

@@ -1124,7 +1124,7 @@ public:
 	///
 	docstring completionPrefix(Cursor const & cur) const override;
 	///
-	bool insertCompletion(Cursor & cur, docstring const & s, bool finished) override;
+	bool insertCompletion(Cursor & cur, docstring const & s, bool /*finished*/) override;
 	///
 	void completionPosAndDim(Cursor const &, int & x, int & y, Dimension & dim) const override;
 	///

src/insets/InsetText.cpp

@@ -1322,13 +1322,12 @@ docstring InsetText::completionPrefix(Cursor const & cur) const
 }
 
 
-bool InsetText::insertCompletion(Cursor & cur, docstring const & s,
-	bool finished)
+bool InsetText::insertCompletion(Cursor & cur, docstring const & s, bool /*finished*/)
 {
 	if (!completionSupported(cur))
 		return false;
 
-	return text_.insertCompletion(cur, s, finished);
+	return text_.insertCompletion(cur, s);
 }
 
 

src/insets/InsetText.h

@@ -203,7 +203,7 @@ public:
 	///
 	docstring completionPrefix(Cursor const & cur) const override;
 	///
-	bool insertCompletion(Cursor & cur, docstring const & s, bool finished) override;
+	bool insertCompletion(Cursor & cur, docstring const & s, bool /*finished*/) override;
 	///
 	void completionPosAndDim(Cursor const &, int & x, int & y, Dimension & dim) const override;
 	/// returns the text to be used as tooltip

src/mathed/InsetMathMacro.cpp

@@ -1370,8 +1370,7 @@ docstring InsetMathMacro::completionPrefix(Cursor const & cur) const
 }
 
 
-bool InsetMathMacro::insertCompletion(Cursor & cur, docstring const & s,
-					bool finished)
+bool InsetMathMacro::insertCompletion(Cursor & cur, docstring const & s, bool finished)
 {
 	if (displayMode() != DISPLAY_UNFOLDED)
 		return InsetMathNest::insertCompletion(cur, s, finished);
@@ -1379,6 +1378,9 @@ bool InsetMathMacro::insertCompletion(Cursor & cur, docstring const & s,
 	if (!completionSupported(cur))
 		return false;
 
+	// Contrary to Text, the whole inset should be recorded (#12581).
+	cur.recordUndoInset();
+
 	// append completion
 	docstring newName = name() + s;
 	asArray(newName, cell(0));

src/mathed/InsetMathNest.cpp

@@ -2086,12 +2086,14 @@ docstring InsetMathNest::completionPrefix(Cursor const & cur) const
 }
 
 
-bool InsetMathNest::insertCompletion(Cursor & cur, docstring const & s,
-				     bool finished)
+bool InsetMathNest::insertCompletion(Cursor & cur, docstring const & s, bool finished)
 {
 	if (!cur.inMacroMode())
 		return false;
 
+	// Contrary to Text, the whole inset should be recorded (#12581).
+	cur.recordUndoInset();
+
 	// append completion to active macro
 	InsetMathUnknown * inset = cur.activeMacro();
 	inset->setName(inset->name() + s);