Clean-up the undo API

* Remove the UndoKind parameter in the general interface
* move recordUndoInset to Cursor
* remove one variant of Undo::recordUndo.
* get rid of Text::recUndo.

src/Cursor.cpp

@@ -2476,15 +2476,15 @@ void Cursor::endUndoGroup() const
 }
 
 
-void Cursor::recordUndo(UndoKind kind, pit_type from, pit_type to) const
+void Cursor::recordUndo(pit_type from, pit_type to) const
 {
-	buffer()->undo().recordUndo(*this, kind, from, to);
+	buffer()->undo().recordUndo(*this, from, to);
 }
 
 
-void Cursor::recordUndo(UndoKind kind, pit_type from) const
+void Cursor::recordUndo(pit_type from) const
 {
-	buffer()->undo().recordUndo(*this, kind, from);
+	buffer()->undo().recordUndo(*this, from, pit());
 }
 
 
@@ -2494,9 +2494,16 @@ void Cursor::recordUndo(UndoKind kind) const
 }
 
 
-void Cursor::recordUndoInset(UndoKind kind, Inset const * inset) const
+void Cursor::recordUndoInset(Inset const * in) const
 {
-	buffer()->undo().recordUndoInset(*this, kind, inset);
+	if (!in || in == &inset()) {
+		CursorData c = *this;
+		c.pop_back();
+		buffer()->undo().recordUndo(c, c.pit(), c.pit());
+	} else if (in == nextInset())
+		recordUndo();
+	else
+		LYXERR0("Inset not found, no undo element added.");
 }
 
 
@@ -2520,7 +2527,7 @@ void Cursor::recordUndoSelection() const
 		else
 			recordUndo();
 	} else {
-		buffer()->undo().recordUndo(*this, ATOMIC_UNDO,
+		buffer()->undo().recordUndo(*this,
 			selBegin().pit(), selEnd().pit());
 	}
 }

src/Cursor.h

@@ -364,18 +364,17 @@ public:
 	void endUndoGroup() const;
 
 	/// The general case: prepare undo for an arbitrary range.
-	void recordUndo(UndoKind kind, pit_type from, pit_type to) const;
+	void recordUndo(pit_type from, pit_type to) const;
 
 	/// Convenience: prepare undo for the range between 'from' and cursor.
-	void recordUndo(UndoKind kind, pit_type from) const;
+	void recordUndo(pit_type from) const;
 
 	/// Convenience: prepare undo for the single paragraph or cell
 	/// containing the cursor
 	void recordUndo(UndoKind kind = ATOMIC_UNDO) const;
 
 	/// Convenience: prepare undo for the inset containing the cursor
-	void recordUndoInset(UndoKind kind = ATOMIC_UNDO,
+	void recordUndoInset(Inset const * inset = 0) const;
-			     Inset const * inset = 0) const;
 
 	/// Convenience: prepare undo for the whole buffer
 	void recordUndoFullBuffer() const;

src/Text.cpp

@@ -1558,7 +1558,7 @@ bool Text::handleBibitems(Cursor & cur)
 	// if a bibitem is deleted, merge with previous paragraph
 	// if this is a bibliography item as well
 	if (cur.pit() > 0 && par.layout() == prevpar.layout()) {
-		cur.recordUndo(ATOMIC_UNDO, prevcur.pit());
+		cur.recordUndo(prevcur.pit());
 		mergeParagraph(bufparams, cur.text()->paragraphs(),
 							prevcur.pit());
 		cur.forceBufferUpdate();
@@ -1640,14 +1640,14 @@ bool Text::backspacePos0(Cursor & cur)
 	// is it an empty paragraph?
 	if (cur.lastpos() == 0
 	    || (cur.lastpos() == 1 && par.isSeparator(0))) {
-		cur.recordUndo(ATOMIC_UNDO, prevcur.pit(), cur.pit());
+		cur.recordUndo(prevcur.pit());
 		plist.erase(boost::next(plist.begin(), cur.pit()));
 		needsUpdate = true;
 	}
 	// is previous par empty?
 	else if (prevcur.lastpos() == 0
 		 || (prevcur.lastpos() == 1 && prevpar.isSeparator(0))) {
-		cur.recordUndo(ATOMIC_UNDO, prevcur.pit(), cur.pit());
+		cur.recordUndo(prevcur.pit());
 		plist.erase(boost::next(plist.begin(), prevcur.pit()));
 		needsUpdate = true;
 	}
@@ -1659,7 +1659,7 @@ bool Text::backspacePos0(Cursor & cur)
 	else if (par.layout() == prevpar.layout()
 		 || tclass.isDefaultLayout(par.layout())
 		 || tclass.isPlainLayout(par.layout())) {
-		cur.recordUndo(ATOMIC_UNDO, prevcur.pit());
+		cur.recordUndo(prevcur.pit());
 		mergeParagraph(bufparams, plist, prevcur.pit());
 		needsUpdate = true;
 	}
@@ -1685,7 +1685,7 @@ bool Text::backspace(Cursor & cur)
 		--prev_cur.pit();
 
 		if (!prev_cur.paragraph().isMergedOnEndOfParDeletion(cur.buffer()->params().track_changes)) {
-			cur.recordUndo(ATOMIC_UNDO, prev_cur.pit(), prev_cur.pit());
+			cur.recordUndo(prev_cur.pit(), prev_cur.pit());
 			prev_cur.paragraph().setChange(prev_cur.lastpos(), Change(Change::DELETED));
 			setCursorIntern(cur, prev_cur.pit(), prev_cur.lastpos());
 			return true;

src/Text.h

@@ -188,11 +188,6 @@ public:
 	void setCursorIntern(Cursor & cur, pit_type par,
 		 pos_type pos, bool setfont = true, bool boundary = false);
 
-	///
-	void recUndo(Cursor & cur, pit_type first, pit_type last) const;
-	///
-	void recUndo(Cursor & cur, pit_type first) const;
-
 	/// Move cursor one position backwards
 	/**
 	 * Returns true if an update is needed after the move.

src/Text2.cpp

@@ -897,9 +897,8 @@ bool Text::deleteEmptyParagraphMechanism(Cursor & cur,
 
 	if (oldpar.empty() || (oldpar.size() == 1 && oldpar.isLineSeparator(0))) {
 		// Delete old par.
-		old.recordUndo(ATOMIC_UNDO,
+		old.recordUndo(max(old.pit() - 1, pit_type(0)),
-			   max(old.pit() - 1, pit_type(0)),
+		               min(old.pit() + 1, old.lastpit()));
-			   min(old.pit() + 1, old.lastpit()));
 		ParagraphList & plist = old.text()->paragraphs();
 		bool const soa = oldpar.params().startOfAppendix();
 		plist.erase(boost::next(plist.begin(), old.pit()));
@@ -977,15 +976,4 @@ void Text::deleteEmptyParagraphMechanism(pit_type first, pit_type last, bool tra
 }
 
 
-void Text::recUndo(Cursor & cur, pit_type first, pit_type last) const
-{
-	cur.recordUndo(ATOMIC_UNDO, first, last);
-}
-
-
-void Text::recUndo(Cursor & cur, pit_type par) const
-{
-	cur.recordUndo(ATOMIC_UNDO, par, par);
-}
-
 } // namespace lyx

src/Text3.cpp

@@ -381,7 +381,7 @@ static void outline(OutlineOp mode, Cursor & cur)
 			pit_type const newpit = distance(bgn, dest);
 			pit_type const len = distance(start, finish);
 			pit_type const deletepit = pit + len;
-			buf.undo().recordUndo(cur, ATOMIC_UNDO, newpit, deletepit - 1);
+			buf.undo().recordUndo(cur, newpit, deletepit - 1);
 			pars.splice(dest, start, finish);
 			cur.pit() = newpit;
 			break;
@@ -401,7 +401,7 @@ static void outline(OutlineOp mode, Cursor & cur)
 			}
 			// One such was found:
 			pit_type newpit = distance(bgn, dest);
-			buf.undo().recordUndo(cur, ATOMIC_UNDO, pit, newpit - 1);
+			buf.undo().recordUndo(cur, pit, newpit - 1);
 			pit_type const len = distance(start, finish);
 			pars.splice(dest, start, finish);
 			cur.pit() = newpit - len;
@@ -410,7 +410,7 @@ static void outline(OutlineOp mode, Cursor & cur)
 		case OutlineIn:
 		case OutlineOut: {
 			pit_type const len = distance(start, finish);
-			buf.undo().recordUndo(cur, ATOMIC_UNDO, pit, pit + len - 1);
+			buf.undo().recordUndo(cur, pit, pit + len - 1);
 			for (; start != finish; ++start) {
 				toclevel = buf.text().getTocLevel(distance(bgn, start));
 				if (toclevel == Layout::NOT_IN_TOC)
@@ -496,7 +496,7 @@ void Text::dispatch(Cursor & cur, FuncRequest & cmd)
 
 	case LFUN_PARAGRAPH_MOVE_DOWN: {
 		pit_type const pit = cur.pit();
-		recUndo(cur, pit, pit + 1);
+		cur.recordUndo(pit, pit + 1);
 		cur.finishUndo();
 		pars_.swap(pit, pit + 1);
 		needsUpdate = true;
@@ -507,7 +507,7 @@ void Text::dispatch(Cursor & cur, FuncRequest & cmd)
 
 	case LFUN_PARAGRAPH_MOVE_UP: {
 		pit_type const pit = cur.pit();
-		recUndo(cur, pit - 1, pit);
+		cur.recordUndo(pit - 1, pit);
 		cur.finishUndo();
 		pars_.swap(pit, pit - 1);
 		--cur.pit();
@@ -526,7 +526,7 @@ void Text::dispatch(Cursor & cur, FuncRequest & cmd)
 		// FIXME: this don't work for multipart document!
 		for (pit_type tmp = 0, end = pars_.size(); tmp != end; ++tmp) {
 			if (pars_[tmp].params().startOfAppendix()) {
-				recUndo(cur, tmp);
+				cur.recordUndo(tmp, tmp);
 				pars_[tmp].params().startOfAppendix(false);
 				break;
 			}

src/TextMetrics.cpp

@@ -385,7 +385,7 @@ bool TextMetrics::redoParagraph(pit_type const pit)
 		Cursor & cur = const_cast<Cursor &>(bv_->cursor());
 		// In some cases, we do not know how to record undo
 		if (&cur.inset() == &text_->inset())
-			cur.recordUndo(ATOMIC_UNDO, pit, pit);
+			cur.recordUndo(pit, pit);
 
 		int const moveCursor = par.fixBiblio(buffer);
 

src/Undo.cpp

@@ -575,40 +575,15 @@ void Undo::endUndoGroup(CursorData const & cur)
 }
 
 
-// FIXME: remove these convenience functions and make
-// Private::recordUndo public as sole interface. The code in the
-// convenience functions can move to Cursor.cpp.
-
 void Undo::recordUndo(CursorData const & cur, UndoKind kind)
 {
 	d->recordUndo(kind, cur, cur.pit(), cur.pit(), cur);
 }
 
 
-void Undo::recordUndoInset(CursorData const & cur, UndoKind kind,
+void Undo::recordUndo(CursorData const & cur, pit_type from, pit_type to)
-			   Inset const * inset)
 {
-	if (!inset || inset == &cur.inset()) {
+	d->recordUndo(ATOMIC_UNDO, cur, from, to, cur);
-		DocIterator c = cur;
-		c.pop_back();
-		d->recordUndo(kind, c, c.pit(), c.pit(), cur);
-	} else if (inset == cur.nextInset())
-		recordUndo(cur, kind);
-	else
-		LYXERR0("Inset not found, no undo stack added.");
-}
-
-
-void Undo::recordUndo(CursorData const & cur, UndoKind kind, pit_type from)
-{
-	d->recordUndo(kind, cur, cur.pit(), from, cur);
-}
-
-
-void Undo::recordUndo(CursorData const & cur, UndoKind kind,
-	pit_type from, pit_type to)
-{
-	d->recordUndo(kind, cur, from, to, cur);
 }
 
 

src/Undo.h

@@ -94,24 +94,14 @@ public:
 	 * give an inclusive range. This is called before you make the
 	 * changes to the paragraph, and it will record the original
 	 * information of the paragraphs in the undo stack.
+	 * Kind of undo is always ATOMIC_UNDO.
 	 */
-	void recordUndo(CursorData const & cur, UndoKind kind,
+	void recordUndo(CursorData const & cur, pit_type from, pit_type to);
-		pit_type from, pit_type to);
-
-	/// Convenience: record undo information for the range between
-	/// 'from' and cursor.
-	void recordUndo(CursorData const & cur, UndoKind kind, pit_type from);
 
 	/// Convenience: record undo information for the single
 	/// paragraph or cell containing the cursor.
 	void recordUndo(CursorData const & cur, UndoKind kind = ATOMIC_UNDO);
 
-	/// Convenience: record undo information for the inset
-	/// containing the cursor.
-	void recordUndoInset(CursorData const & cur,
-			     UndoKind kind = ATOMIC_UNDO,
-			     Inset const * inset = 0);
-
 	/// Convenience: record undo for buffer parameters
 	void recordUndoBufferParams(CursorData const & cur);
 

src/insets/InsetArgument.cpp

@@ -148,7 +148,7 @@ void InsetArgument::doDispatch(Cursor & cur, FuncRequest & cmd)
 			cur.undispatched();
 			return;
 		}
-		cur.recordUndoInset(ATOMIC_UNDO, this);
+		cur.recordUndoInset(this);
 		name_ = cmd.getArg(1);
 		cur.forceBufferUpdate();
 		break;

src/insets/InsetBox.cpp

@@ -221,7 +221,7 @@ void InsetBox::doDispatch(Cursor & cur, FuncRequest & cmd)
 			cur.undispatched();
 			return;
 		}
-		cur.recordUndoInset(ATOMIC_UNDO, this);
+		cur.recordUndoInset(this);
 		if (change_type)
 			params_.type = cmd.getArg(1);
 		else // if (for_box)

src/insets/InsetBranch.cpp

@@ -132,7 +132,7 @@ void InsetBranch::doDispatch(Cursor & cur, FuncRequest & cmd)
 		InsetBranchParams params;
 		InsetBranch::string2params(to_utf8(cmd.argument()), params);
 
-		cur.recordUndoInset(ATOMIC_UNDO, this);
+		cur.recordUndoInset(this);
 		params_.branch = params.branch;
 		// what we really want here is a TOC update, but that means
 		// a full buffer update

src/insets/InsetCaption.cpp

@@ -197,7 +197,7 @@ void InsetCaption::doDispatch(Cursor & cur, FuncRequest & cmd)
 
 	case LFUN_INSET_MODIFY: {
 		if (cmd.getArg(0) == "changetype") {
-			cur.recordUndoInset(ATOMIC_UNDO, this);
+			cur.recordUndoInset(this);
 			type_ = cmd.getArg(1);
 			cur.forceBufferUpdate();
 			break;

src/insets/InsetERT.cpp

@@ -111,7 +111,7 @@ void InsetERT::doDispatch(Cursor & cur, FuncRequest & cmd)
 	switch (cmd.action()) {
 	case LFUN_INSET_MODIFY:
 		if (cmd.getArg(0) == "ert") {
-			cur.recordUndoInset(ATOMIC_UNDO, this);
+			cur.recordUndoInset(this);
 			setStatus(cur, string2params(to_utf8(cmd.argument())));
 			break;
 		}

src/insets/InsetFloat.cpp

@@ -142,7 +142,7 @@ void InsetFloat::doDispatch(Cursor & cur, FuncRequest & cmd)
 	case LFUN_INSET_MODIFY: {
 		InsetFloatParams params;
 		string2params(to_utf8(cmd.argument()), params);
-		cur.recordUndoInset(ATOMIC_UNDO, this);
+		cur.recordUndoInset(this);
 
 		// placement, wide and sideways are not used for subfloats
 		if (!params_.subfloat) {

src/insets/InsetIndex.cpp

@@ -200,13 +200,13 @@ void InsetIndex::doDispatch(Cursor & cur, FuncRequest & cmd)
 
 	case LFUN_INSET_MODIFY: {
 		if (cmd.getArg(0) == "changetype") {
-			cur.recordUndoInset(ATOMIC_UNDO, this);
+			cur.recordUndoInset(this);
 			params_.index = from_utf8(cmd.getArg(1));
 			break;
 		}
 		InsetIndexParams params;
 		InsetIndex::string2params(to_utf8(cmd.argument()), params);
-		cur.recordUndoInset(ATOMIC_UNDO, this);
+		cur.recordUndoInset(this);
 		params_.index = params.index;
 		// what we really want here is a TOC update, but that means
 		// a full buffer update

src/insets/InsetListings.cpp

@@ -324,7 +324,7 @@ void InsetListings::doDispatch(Cursor & cur, FuncRequest & cmd)
 	switch (cmd.action()) {
 
 	case LFUN_INSET_MODIFY: {
-		cur.recordUndoInset(ATOMIC_UNDO, this);
+		cur.recordUndoInset(this);
 		InsetListings::string2params(to_utf8(cmd.argument()), params());
 		break;
 	}

src/insets/InsetNote.cpp

@@ -161,7 +161,7 @@ void InsetNote::doDispatch(Cursor & cur, FuncRequest & cmd)
 		if (params_.type == params.type)
 		  break;
 
-		cur.recordUndoInset(ATOMIC_UNDO, this);
+		cur.recordUndoInset(this);
 		string2params(to_utf8(cmd.argument()), params_);
 		setButtonLabel();
 		// what we really want here is a TOC update, but that means

src/insets/InsetPhantom.cpp

@@ -261,7 +261,7 @@ void InsetPhantom::doDispatch(Cursor & cur, FuncRequest & cmd)
 	switch (cmd.action()) {
 
 	case LFUN_INSET_MODIFY:
-		cur.recordUndoInset(ATOMIC_UNDO, this);
+		cur.recordUndoInset(this);
 		string2params(to_utf8(cmd.argument()), params_);
 		setButtonLabel();
 		cur.forceBufferUpdate();

src/insets/InsetScript.cpp

@@ -216,7 +216,7 @@ void InsetScript::doDispatch(Cursor & cur, FuncRequest & cmd)
 {
 	switch (cmd.action()) {
 	case LFUN_INSET_MODIFY:
-		cur.recordUndoInset(ATOMIC_UNDO, this);
+		cur.recordUndoInset(this);
 		string2params(to_utf8(cmd.argument()), params_);
 		break;
 	default:

src/insets/InsetTabular.cpp

@@ -5421,7 +5421,7 @@ void InsetTabular::tabularFeatures(Cursor & cur,
 		break;
 	}
 
-	cur.recordUndoInset(ATOMIC_UNDO, this);
+	cur.recordUndoInset(this);
 
 	getSelection(cur, sel_row_start, sel_row_end, sel_col_start, sel_col_end);
 	row_type const row = tabular.cellRow(cur.idx());

src/insets/InsetWrap.cpp

@@ -82,7 +82,7 @@ void InsetWrap::doDispatch(Cursor & cur, FuncRequest & cmd)
 {
 	switch (cmd.action()) {
 	case LFUN_INSET_MODIFY: {
-		cur.recordUndoInset(ATOMIC_UNDO, this);
+		cur.recordUndoInset(this);
 		InsetWrapParams params;
 		InsetWrap::string2params(to_utf8(cmd.argument()), params);
 		params_.lines = params.lines;