Add UndoAction class as internal type for undo actions and make Action a struct

that is used for reporting undo steps to Document.
This will allow further minimization of undo memory use.

6 files changed, 66 insertions, 48 deletions

diff --git a/src/CellBuffer.cxx b/src/CellBuffer.cxx
index 9d988fb29..fd6fe55fb 100644
--- a/src/CellBuffer.cxx
+++ b/src/CellBuffer.cxx
@@ -330,25 +330,6 @@ public:
 	}
 };
 
-Action::Action() noexcept = default;
-
-void Action::Create(ActionType at_, Sci::Position position_, const char *data_, Sci::Position lenData_, bool mayCoalesce_) {
-	position = position_;
-	at = at_;
-	mayCoalesce = mayCoalesce_;
-	lenData = lenData_;
-	data = nullptr;
-	if (lenData_) {
-		data = std::make_unique<char[]>(lenData_);
-		memcpy(&data[0], data_, lenData_);
-	}
-}
-
-void Action::Clear() noexcept {
-	data = nullptr;
-	lenData = 0;
-}
-
 CellBuffer::CellBuffer(bool hasStyles_, bool largeDocument_) :
 	hasStyles(hasStyles_), largeDocument(largeDocument_) {
 	readOnly = false;
@@ -1106,12 +1087,17 @@ int CellBuffer::StartUndo() noexcept {
 	return uh->StartUndo();
 }
 
-const Action &CellBuffer::GetUndoStep() const noexcept {
-	return uh->GetUndoStep();
+Action CellBuffer::GetUndoStep() const noexcept {
+	const UndoAction &actionStep = uh->GetUndoStep();
+	Action acta{ actionStep.at, actionStep.mayCoalesce, actionStep.position, nullptr, actionStep.lenData };
+	if (actionStep.lenData) {
+		acta.data = actionStep.data.get();
+	}
+	return acta;
 }
 
 void CellBuffer::PerformUndoStep() {
-	const Action &actionStep = uh->GetUndoStep();
+	const UndoAction &actionStep = uh->GetUndoStep();
 	if (changeHistory && uh->BeforeSavePoint()) {
 		changeHistory->StartReversion();
 	}
@@ -1142,12 +1128,17 @@ int CellBuffer::StartRedo() noexcept {
 	return uh->StartRedo();
 }
 
-const Action &CellBuffer::GetRedoStep() const noexcept {
-	return uh->GetRedoStep();
+Action CellBuffer::GetRedoStep() const noexcept {
+	const UndoAction &actionStep = uh->GetRedoStep();
+	Action acta {actionStep.at, actionStep.mayCoalesce, actionStep.position, nullptr, actionStep.lenData};
+	if (actionStep.lenData) {
+		acta.data = actionStep.data.get();
+	}
+	return acta;
 }
 
 void CellBuffer::PerformRedoStep() {
-	const Action &actionStep = uh->GetRedoStep();
+	const UndoAction &actionStep = uh->GetRedoStep();
 	if (actionStep.at == ActionType::insert) {
 		BasicInsertString(actionStep.position, actionStep.data.get(), actionStep.lenData);
 		if (changeHistory) {
diff --git a/src/CellBuffer.h b/src/CellBuffer.h
index d71287d1b..ecd646570 100644
--- a/src/CellBuffer.h
+++ b/src/CellBuffer.h
@@ -31,19 +31,14 @@ class ILineVector;
 enum class ActionType : unsigned char { insert, remove, start, container };
 
 /**
- * Actions are used to store all the information required to perform one undo/redo step.
+ * Actions are used to return the information required to report one undo/redo step.
  */
-class Action {
-public:
+struct Action {
 	ActionType at = ActionType::insert;
 	bool mayCoalesce = false;
 	Sci::Position position = 0;
-	std::unique_ptr<char[]> data;
+	const char *data = nullptr;
 	Sci::Position lenData = 0;
-
-	Action() noexcept;
-	void Create(ActionType at_, Sci::Position position_=0, const char *data_=nullptr, Sci::Position lenData_=0, bool mayCoalesce_=true);
-	void Clear() noexcept;
 };
 
 struct SplitView {
@@ -178,11 +173,11 @@ public:
 	/// called that many times. Similarly for redo.
 	bool CanUndo() const noexcept;
 	int StartUndo() noexcept;
-	const Action &GetUndoStep() const noexcept;
+	Action GetUndoStep() const noexcept;
 	void PerformUndoStep();
 	bool CanRedo() const noexcept;
 	int StartRedo() noexcept;
-	const Action &GetRedoStep() const noexcept;
+	Action GetRedoStep() const noexcept;
 	void PerformRedoStep();
 
 	void ChangeHistorySet(bool set);
diff --git a/src/Document.cxx b/src/Document.cxx
index 1a8c4020f..306cdb725 100644
--- a/src/Document.cxx
+++ b/src/Document.cxx
@@ -303,7 +303,7 @@ void Document::TentativeUndo() {
 			//Platform::DebugPrintf("Steps=%d\n", steps);
 			for (int step = 0; step < steps; step++) {
 				const Sci::Line prevLinesTotal = LinesTotal();
-				const Action &action = cb.GetUndoStep();
+				const Action action = cb.GetUndoStep();
 				if (action.at == ActionType::remove) {
 					NotifyModified(DocModification(
 									ModificationFlags::BeforeInsert | ModificationFlags::Undo, action));
@@ -338,7 +338,7 @@ void Document::TentativeUndo() {
 						modFlags |= ModificationFlags::MultilineUndoRedo;
 				}
 				NotifyModified(DocModification(modFlags, action.position, action.lenData,
-											   linesAdded, action.data.get()));
+											   linesAdded, action.data));
 			}
 
 			const bool endSavePoint = cb.IsSavePoint();
@@ -1366,7 +1366,7 @@ Sci::Position Document::Undo() {
 			Range coalescedRemove;	// Default is empty at 0
 			for (int step = 0; step < steps; step++) {
 				const Sci::Line prevLinesTotal = LinesTotal();
-				const Action &action = cb.GetUndoStep();
+				const Action action = cb.GetUndoStep();
 				if (action.at == ActionType::remove) {
 					NotifyModified(DocModification(
 									ModificationFlags::BeforeInsert | ModificationFlags::Undo, action));
@@ -1410,7 +1410,7 @@ Sci::Position Document::Undo() {
 						modFlags |= ModificationFlags::MultilineUndoRedo;
 				}
 				NotifyModified(DocModification(modFlags, action.position, action.lenData,
-											   linesAdded, action.data.get()));
+											   linesAdded, action.data));
 			}
 
 			const bool endSavePoint = cb.IsSavePoint();
@@ -1433,7 +1433,7 @@ Sci::Position Document::Redo() {
 			const int steps = cb.StartRedo();
 			for (int step = 0; step < steps; step++) {
 				const Sci::Line prevLinesTotal = LinesTotal();
-				const Action &action = cb.GetRedoStep();
+				const Action action = cb.GetRedoStep();
 				if (action.at == ActionType::insert) {
 					NotifyModified(DocModification(
 									ModificationFlags::BeforeInsert | ModificationFlags::Redo, action));
@@ -1470,7 +1470,7 @@ Sci::Position Document::Redo() {
 				}
 				NotifyModified(
 					DocModification(modFlags, action.position, action.lenData,
-									linesAdded, action.data.get()));
+									linesAdded, action.data));
 			}
 
 			const bool endSavePoint = cb.IsSavePoint();
diff --git a/src/Document.h b/src/Document.h
index 9f51fc719..af6bb98fd 100644
--- a/src/Document.h
+++ b/src/Document.h
@@ -626,7 +626,7 @@ public:
 		position(act.position),
 		length(act.lenData),
 		linesAdded(linesAdded_),
-		text(act.data.get()),
+		text(act.data),
 		line(0),
 		foldLevelNow(Scintilla::FoldLevel::None),
 		foldLevelPrev(Scintilla::FoldLevel::None),
diff --git a/src/UndoHistory.cxx b/src/UndoHistory.cxx
index 6b8c65165..d667c9b4d 100644
--- a/src/UndoHistory.cxx
+++ b/src/UndoHistory.cxx
@@ -36,6 +36,25 @@
 
 namespace Scintilla::Internal {
 
+UndoAction::UndoAction() noexcept = default;
+
+void UndoAction::Create(ActionType at_, Sci::Position position_, const char *data_, Sci::Position lenData_, bool mayCoalesce_) {
+	position = position_;
+	at = at_;
+	mayCoalesce = mayCoalesce_;
+	lenData = lenData_;
+	data = nullptr;
+	if (lenData_) {
+		data = std::make_unique<char[]>(lenData_);
+		memcpy(&data[0], data_, lenData_);
+	}
+}
+
+void UndoAction::Clear() noexcept {
+	data = nullptr;
+	lenData = 0;
+}
+
 // The undo history stores a sequence of user operations that represent the user's view of the
 // commands executed on the text.
 // Each user operation contains a sequence of text insertion and text deletion actions.
@@ -94,7 +113,7 @@ const char *UndoHistory::AppendAction(ActionType at, Sci::Position position, con
 		if (0 == undoSequenceDepth) {
 			// Top level actions may not always be coalesced
 			ptrdiff_t targetAct = -1;
-			const Action *actPrevious = &(actions[currentAction + targetAct]);
+			const UndoAction *actPrevious = &(actions[currentAction + targetAct]);
 			// Container actions may forward the coalesce state of Scintilla Actions.
 			while ((actPrevious->at == ActionType::container) && actPrevious->mayCoalesce) {
 				targetAct--;
@@ -259,7 +278,7 @@ int UndoHistory::StartUndo() noexcept {
 	return currentAction - act;
 }
 
-const Action &UndoHistory::GetUndoStep() const noexcept {
+const UndoAction &UndoHistory::GetUndoStep() const noexcept {
 	return actions[currentAction];
 }
 
@@ -284,7 +303,7 @@ int UndoHistory::StartRedo() noexcept {
 	return act - currentAction;
 }
 
-const Action &UndoHistory::GetRedoStep() const noexcept {
+const UndoAction &UndoHistory::GetRedoStep() const noexcept {
 	return actions[currentAction];
 }
 
diff --git a/src/UndoHistory.h b/src/UndoHistory.h
index 02b7bd307..423b50a7f 100644
--- a/src/UndoHistory.h
+++ b/src/UndoHistory.h
@@ -10,11 +10,24 @@
 
 namespace Scintilla::Internal {
 
+class UndoAction {
+public:
+	ActionType at = ActionType::insert;
+	bool mayCoalesce = false;
+	Sci::Position position = 0;
+	std::unique_ptr<char[]> data;
+	Sci::Position lenData = 0;
+
+	UndoAction() noexcept;
+	void Create(ActionType at_, Sci::Position position_ = 0, const char *data_ = nullptr, Sci::Position lenData_ = 0, bool mayCoalesce_ = true);
+	void Clear() noexcept;
+};
+
 /**
  *
  */
 class UndoHistory {
-	std::vector<Action> actions;
+	std::vector<UndoAction> actions;
 	int maxAction;
 	int currentAction;
 	int undoSequenceDepth;
@@ -53,11 +66,11 @@ public:
 	/// called that many times. Similarly for redo.
 	bool CanUndo() const noexcept;
 	int StartUndo() noexcept;
-	const Action &GetUndoStep() const noexcept;
+	const UndoAction &GetUndoStep() const noexcept;
 	void CompletedUndoStep() noexcept;
 	bool CanRedo() const noexcept;
 	int StartRedo() noexcept;
-	const Action &GetRedoStep() const noexcept;
+	const UndoAction &GetRedoStep() const noexcept;
 	void CompletedRedoStep() noexcept;
 };