aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
-rw-r--r--doc/ScintillaDoc.html8
-rw-r--r--src/CellBuffer.cxx133
-rw-r--r--src/UndoHistory.cxx8
-rw-r--r--src/UndoHistory.h8
-rw-r--r--test/unit/testCellBuffer.cxx141
5 files changed, 216 insertions, 82 deletions
diff --git a/doc/ScintillaDoc.html b/doc/ScintillaDoc.html
index b866cf291..b65a9c8ef 100644
--- a/doc/ScintillaDoc.html
+++ b/doc/ScintillaDoc.html
@@ -1995,9 +1995,11 @@ struct Sci_TextToFindFull {
<h2 id="UndoSaveRestore">Undo Save and Restore</h2>
<p>This feature is unfinished and has limitations.
- Restoring change history alongside undo state is unfinished so turn change history off before restoral.
+ When change history is active, it may show different changes than the previous session as undo actions
+ performed in that session are discarded in some circumstances such as when detaching from a save point.
+ A future version may add an API for archiving change history alongside undo history.
The operation sequences discussed here are a 'golden path' that has been tested to some extent and calling
- the APIs in other circumstances or with out-of-bounds values may cause failures.</p>
+ the APIs in other circumstances or with out-of-bounds values may fail.</p>
<p>The behaviour of tentative actions in save and restore is uncertain as these are meant to be short-term states in language input
and which need to synchronize with a language IME (input method editor).
@@ -2098,7 +2100,7 @@ struct Sci_TextToFindFull {
Check for failure with <a class="seealso" href="#SCI_GETSTATUS">SCI_GETSTATUS</a>.
</p>
- <p>The current implementation may only work when the current and save point are the same and there is no tentative point.
+ <p>The current implementation may only work when there is no tentative point.
</p>
<h2 id="ChangeHistory">Change history</h2>
diff --git a/src/CellBuffer.cxx b/src/CellBuffer.cxx
index 320a61639..1ffc749bf 100644
--- a/src/CellBuffer.cxx
+++ b/src/CellBuffer.cxx
@@ -483,7 +483,7 @@ const char *CellBuffer::DeleteChars(Sci::Position position, Sci::Position delete
if (changeHistory) {
changeHistory->DeleteRangeSavingHistory(position, deleteLength,
- uh->BeforeReachableSavePoint(), uh->AfterDetachPoint());
+ uh->BeforeReachableSavePoint(), uh->AfterOrAtDetachPoint());
}
BasicDeleteChars(position, deleteLength);
@@ -1092,24 +1092,26 @@ Action CellBuffer::GetUndoStep() const noexcept {
}
void CellBuffer::PerformUndoStep() {
- const Action actionStep = uh->GetUndoStep();
- if (changeHistory && uh->BeforeOrAtSavePoint()) {
+ const Action previousStep = uh->GetUndoStep();
+ // PreviousBeforeSavePoint and AfterDetachPoint are called since acting on the previous action,
+ // that is currentAction-1
+ if (changeHistory && uh->PreviousBeforeSavePoint()) {
changeHistory->StartReversion();
}
- if (actionStep.at == ActionType::insert) {
- if (substance.Length() < actionStep.lenData) {
+ if (previousStep.at == ActionType::insert) {
+ if (substance.Length() < previousStep.lenData) {
throw std::runtime_error(
"CellBuffer::PerformUndoStep: deletion must be less than document length.");
}
if (changeHistory) {
- changeHistory->DeleteRange(actionStep.position, actionStep.lenData,
- uh->BeforeOrAtSavePoint() && !uh->AfterDetachPoint());
+ changeHistory->DeleteRange(previousStep.position, previousStep.lenData,
+ uh->PreviousBeforeSavePoint() && !uh->AfterDetachPoint());
}
- BasicDeleteChars(actionStep.position, actionStep.lenData);
- } else if (actionStep.at == ActionType::remove) {
- BasicInsertString(actionStep.position, actionStep.data, actionStep.lenData);
+ BasicDeleteChars(previousStep.position, previousStep.lenData);
+ } else if (previousStep.at == ActionType::remove) {
+ BasicInsertString(previousStep.position, previousStep.data, previousStep.lenData);
if (changeHistory) {
- changeHistory->UndoDeleteStep(actionStep.position, actionStep.lenData, uh->AfterDetachPoint());
+ changeHistory->UndoDeleteStep(previousStep.position, previousStep.lenData, uh->AfterDetachPoint());
}
}
uh->CompletedUndoStep();
@@ -1138,7 +1140,7 @@ void CellBuffer::PerformRedoStep() {
} else if (actionStep.at == ActionType::remove) {
if (changeHistory) {
changeHistory->DeleteRangeSavingHistory(actionStep.position, actionStep.lenData,
- uh->BeforeReachableSavePoint(), uh->AfterDetachPoint());
+ uh->BeforeReachableSavePoint(), uh->AfterOrAtDetachPoint());
}
BasicDeleteChars(actionStep.position, actionStep.lenData);
}
@@ -1176,60 +1178,73 @@ int CellBuffer::UndoTentative() const noexcept {
return uh->TentativePoint();
}
+namespace {
+
+void RestoreChangeHistory(const UndoHistory *uh, ChangeHistory *changeHistory) {
+ // Replay all undo actions into changeHistory
+ const int savePoint = uh->SavePoint();
+ const int detachPoint = uh->DetachPoint();
+ const int currentPoint = uh->Current();
+ for (int act = 0; act < uh->Actions(); act++) {
+ const ActionType type = static_cast<ActionType>(uh->Type(act) & ~coalesceFlag);
+ const Sci::Position position = uh->Position(act);
+ const Sci::Position length = uh->Length(act);
+ const bool beforeSave = act < savePoint || ((detachPoint >= 0) && (detachPoint > act));
+ const bool afterDetach = (detachPoint >= 0) && (detachPoint < act);
+ switch (type) {
+ case ActionType::insert:
+ changeHistory->Insert(position, length, true, beforeSave);
+ break;
+ case ActionType::remove:
+ changeHistory->DeleteRangeSavingHistory(position, length, beforeSave, afterDetach);
+ break;
+ default:
+ // Only insertions and deletions go into change history
+ break;
+ }
+ changeHistory->Check();
+ }
+ // Undo back to currentPoint, updating change history
+ for (int act = uh->Actions() - 1; act >= currentPoint; act--) {
+ const ActionType type = static_cast<ActionType>(uh->Type(act) & ~coalesceFlag);
+ const Sci::Position position = uh->Position(act);
+ const Sci::Position length = uh->Length(act);
+ const bool beforeSave = act < savePoint;
+ const bool afterDetach = (detachPoint >= 0) && (detachPoint < act);
+ if (beforeSave) {
+ changeHistory->StartReversion();
+ }
+ switch (type) {
+ case ActionType::insert:
+ changeHistory->DeleteRange(position, length, beforeSave && !afterDetach);
+ break;
+ case ActionType::remove:
+ changeHistory->UndoDeleteStep(position, length, afterDetach);
+ break;
+ default:
+ // Only insertions and deletions go into change history
+ break;
+ }
+ changeHistory->Check();
+ }
+}
+
+}
+
void CellBuffer::SetUndoCurrent(int action) {
uh->SetCurrent(action, Length());
if (changeHistory) {
+ if ((uh->DetachPoint() >= 0) && (uh->SavePoint() >= 0)) {
+ // Can't have a valid save point and a valid detach point at same time
+ uh->DeleteUndoHistory();
+ changeHistory.reset();
+ throw std::runtime_error("UndoHistory::SetCurrent: invalid undo history.");
+ }
const intptr_t sizeChange = uh->Delta(action);
const intptr_t lengthOriginal = Length() - sizeChange;
// Recreate empty change history
changeHistory = std::make_unique<ChangeHistory>(lengthOriginal);
-
- // Replay all undo undo actions into changeHistory
- const int savePoint = uh->SavePoint();
- const int detachPoint = uh->DetachPoint();
- const int currentPoint = uh->Current();
- for (int act = 0; act < uh->Actions(); act++) {
- const ActionType type = static_cast<ActionType>(uh->Type(act) & ~coalesceFlag);
- const Sci::Position position = uh->Position(act);
- const Sci::Position length = uh->Length(act);
- const bool beforeSave = act < savePoint;
- const bool afterDetach = (detachPoint >= 0) && (detachPoint < act);
- switch (type) {
- case ActionType::insert:
- changeHistory->Insert(position, length, true, beforeSave);
- break;
- case ActionType::remove:
- changeHistory->DeleteRangeSavingHistory(position, length, beforeSave, afterDetach);
- break;
- default:
- // Only insertions and deletions go into change history
- break;
- }
- changeHistory->Check();
- }
- // Undo back to currentPoint, updating change history
- for (int act = uh->Actions()-1; act >= currentPoint; act--) {
- const ActionType type = static_cast<ActionType>(uh->Type(act) & ~coalesceFlag);
- const Sci::Position position = uh->Position(act);
- const Sci::Position length = uh->Length(act);
- const bool beforeOrAtSavePoint = (savePoint < 0) || (savePoint >= act);
- const bool afterDetach = (detachPoint >= 0) && (detachPoint < act);
- if (beforeOrAtSavePoint) {
- changeHistory->StartReversion();
- }
- switch (type) {
- case ActionType::insert:
- changeHistory->DeleteRange(position, length, beforeOrAtSavePoint && !afterDetach);
- break;
- case ActionType::remove:
- changeHistory->UndoDeleteStep(position, length, afterDetach);
- break;
- default:
- // Only insertions and deletions go into change history
- break;
- }
- changeHistory->Check();
- }
+ RestoreChangeHistory(uh.get(), changeHistory.get());
if (Length() != changeHistory->Length()) {
uh->DeleteUndoHistory();
changeHistory.reset();
diff --git a/src/UndoHistory.cxx b/src/UndoHistory.cxx
index aac802dfd..c71551e60 100644
--- a/src/UndoHistory.cxx
+++ b/src/UndoHistory.cxx
@@ -383,12 +383,12 @@ bool UndoHistory::BeforeSavePoint() const noexcept {
return (savePoint < 0) || (savePoint > currentAction);
}
-bool UndoHistory::BeforeOrAtSavePoint() const noexcept {
+bool UndoHistory::PreviousBeforeSavePoint() const noexcept {
return (savePoint < 0) || (savePoint >= currentAction);
}
bool UndoHistory::BeforeReachableSavePoint() const noexcept {
- return (savePoint > 0) && !detach && (savePoint > currentAction);
+ return (savePoint > 0) && (savePoint > currentAction);
}
bool UndoHistory::AfterSavePoint() const noexcept {
@@ -544,7 +544,7 @@ bool UndoHistory::CanUndo() const noexcept {
return (currentAction > 0) && (actions.SSize() != 0);
}
-int UndoHistory::StartUndo() noexcept {
+int UndoHistory::StartUndo() const noexcept {
assert(currentAction >= 0);
// Count the steps in this action
@@ -584,7 +584,7 @@ bool UndoHistory::CanRedo() const noexcept {
return actions.SSize() > currentAction;
}
-int UndoHistory::StartRedo() noexcept {
+int UndoHistory::StartRedo() const noexcept {
// Count the steps in this action
if (currentAction >= actions.SSize()) {
diff --git a/src/UndoHistory.h b/src/UndoHistory.h
index 5ddc03593..90b2860a7 100644
--- a/src/UndoHistory.h
+++ b/src/UndoHistory.h
@@ -80,7 +80,7 @@ class UndoHistory {
int undoSequenceDepth = 0;
int savePoint = 0;
int tentativePoint = -1;
- std::optional<int> detach;
+ std::optional<int> detach; // Never set if savePoint set (>= 0)
std::unique_ptr<ScrapStack> scraps;
struct actPos { int act; size_t position; };
std::optional<actPos> memory;
@@ -107,7 +107,7 @@ public:
void SetSavePoint() noexcept;
bool IsSavePoint() const noexcept;
bool BeforeSavePoint() const noexcept;
- bool BeforeOrAtSavePoint() const noexcept;
+ bool PreviousBeforeSavePoint() const noexcept;
bool BeforeReachableSavePoint() const noexcept;
bool AfterSavePoint() const noexcept;
@@ -139,11 +139,11 @@ public:
/// To perform an undo, StartUndo is called to retrieve the number of steps, then UndoStep is
/// called that many times. Similarly for redo.
bool CanUndo() const noexcept;
- int StartUndo() noexcept;
+ int StartUndo() const noexcept;
Action GetUndoStep() const noexcept;
void CompletedUndoStep() noexcept;
bool CanRedo() const noexcept;
- int StartRedo() noexcept;
+ int StartRedo() const noexcept;
Action GetRedoStep() const noexcept;
void CompletedRedoStep() noexcept;
};
diff --git a/test/unit/testCellBuffer.cxx b/test/unit/testCellBuffer.cxx
index 163dbed00..a6bd5dba1 100644
--- a/test/unit/testCellBuffer.cxx
+++ b/test/unit/testCellBuffer.cxx
@@ -41,7 +41,9 @@ TEST_CASE("ScrapStack") {
ScrapStack ss;
SECTION("Push") {
- ss.Push("abc", 3);
+ const char *t = ss.Push("abc", 3);
+ REQUIRE(memcmp(t, "abc", 3) == 0);
+
ss.MoveBack(3);
const char *text = ss.CurrentText();
REQUIRE(memcmp(text, "abc", 3) == 0);
@@ -50,8 +52,16 @@ TEST_CASE("ScrapStack") {
const char *text2 = ss.CurrentText();
REQUIRE(memcmp(text2, "bc", 2) == 0);
- const char *text3 = ss.TextAt(2);
- REQUIRE(memcmp(text3, "c", 1) == 0);
+ ss.SetCurrent(1);
+ const char *text3 = ss.CurrentText();
+ REQUIRE(memcmp(text3, "bc", 2) == 0);
+
+ const char *text4 = ss.TextAt(2);
+ REQUIRE(memcmp(text4, "c", 1) == 0);
+
+ ss.Clear();
+ const char *text5 = ss.Push("1", 1);
+ REQUIRE(memcmp(text5, "1", 1) == 0);
}
}
@@ -258,7 +268,7 @@ bool EqualContainerAction(const Action &a, Sci::Position token) noexcept {
return true;
}
-void TentativeUndo(UndoHistory &uh) {
+void TentativeUndo(UndoHistory &uh) noexcept {
const int steps = uh.TentativeSteps();
for (int step = 0; step < steps; step++) {
/* const Action &actionStep = */ uh.GetUndoStep();
@@ -270,7 +280,7 @@ void TentativeUndo(UndoHistory &uh) {
TEST_CASE("ScaledVector") {
ScaledVector sv;
-
+
SECTION("ScalingUp") {
sv.ReSize(1);
REQUIRE(sv.SizeInBytes() == 1);
@@ -311,9 +321,18 @@ TEST_CASE("ScaledVector") {
sv.ReSize(2);
REQUIRE(sv.SizeInBytes() == 6);
// Truncate
- sv.ReSize(1);
+ sv.Truncate(1);
REQUIRE(sv.SizeInBytes() == 3);
REQUIRE(sv.ValueAt(0) == 0xd4381);
+
+ sv.Clear();
+ REQUIRE(sv.Size() == 0);
+ sv.PushBack();
+ REQUIRE(sv.Size() == 1);
+ REQUIRE(sv.SizeInBytes() == 3);
+ sv.SetValueAt(0, 0x1fd4381);
+ REQUIRE(sv.SizeInBytes() == 4);
+ REQUIRE(sv.ValueAt(0) == 0x1fd4381);
}
}
@@ -600,6 +619,30 @@ TEST_CASE("UndoHistory") {
}
}
+TEST_CASE("UndoActions") {
+
+ UndoActions ua;
+
+ SECTION("Basics") {
+ ua.PushBack();
+ REQUIRE(ua.SSize() == 1);
+ ua.Create(0, ActionType::insert, 0, 2, false);
+ REQUIRE(ua.AtStart(0));
+ REQUIRE(ua.LengthTo(0) == 0);
+ REQUIRE(ua.AtStart(1));
+ REQUIRE(ua.LengthTo(1) == 2);
+ ua.PushBack();
+ REQUIRE(ua.SSize() == 2);
+ ua.Create(0, ActionType::insert, 0, 2, false);
+ REQUIRE(ua.SSize() == 2);
+ ua.Truncate(1);
+ REQUIRE(ua.SSize() == 1);
+ ua.Clear();
+ REQUIRE(ua.SSize() == 0);
+ }
+}
+
+
TEST_CASE("CharacterIndex") {
CellBuffer cb(true, false);
@@ -1083,20 +1126,20 @@ TEST_CASE("ChangeHistory") {
SECTION("Delete Contiguous Backward") {
// Deletes that touch
- constexpr size_t length = 20;
- constexpr size_t rounds = 8;
+ constexpr Sci::Position length = 20;
+ constexpr Sci::Position rounds = 8;
il.Insert(0, length, false, true);
REQUIRE(il.Length() == length);
il.SetSavePoint();
- for (size_t i = 0; i < rounds; i++) {
+ for (Sci::Position i = 0; i < rounds; i++) {
il.DeleteRangeSavingHistory(9-i, 1, false, false);
}
- constexpr size_t lengthAfterDeletions = length - rounds;
+ constexpr Sci::Position lengthAfterDeletions = length - rounds;
REQUIRE(il.Length() == lengthAfterDeletions);
REQUIRE(il.DeletionCount(0, lengthAfterDeletions) == rounds);
- for (size_t j = 0; j < rounds; j++) {
+ for (Sci::Position j = 0; j < rounds; j++) {
il.UndoDeleteStep(2+j, 1, false);
}
@@ -1450,6 +1493,80 @@ TEST_CASE("CellBufferWithChangeHistory") {
namespace {
+void PushUndoAction(CellBuffer &cb, int type, Sci::Position pos, std::string_view sv) {
+ cb.PushUndoActionType(type, pos);
+ cb.ChangeLastUndoActionText(sv.length(), sv.data());
+}
+
+}
+
+TEST_CASE("CellBufferLoadUndoHistory") {
+
+ CellBuffer cb(false, false);
+ constexpr int remove = 1;
+ constexpr int insert = 0;
+
+ SECTION("Basics") {
+ cb.SetUndoCollection(false);
+ constexpr std::string_view sInsert = "abcdef";
+ bool startSequence = false;
+ cb.InsertString(0, sInsert.data(), sInsert.length(), startSequence);
+ cb.SetUndoCollection(true);
+ cb.ChangeHistorySet(true);
+
+ // Create an undo history that matches the contents at current point 2
+ // So, 2 actions; current point; 2 actions
+ // a_cdef
+ PushUndoAction(cb, remove, 1, "_");
+ // acdef
+ PushUndoAction(cb, insert, 1, "b");
+ // abcdef -> current
+ PushUndoAction(cb, remove, 3, "d");
+ // abcef -> save
+ PushUndoAction(cb, insert, 3, "*");
+ // abc*ef
+ cb.SetUndoSavePoint(3);
+ cb.SetUndoDetach(-1);
+ cb.SetUndoTentative(-1);
+ cb.SetUndoCurrent(2);
+
+ // 2nd insertion is removed from change history as it isn't visible and isn't saved
+ // 2nd deletion is visible (as insertion) as it was saved but then reverted to original
+ // 1st insertion and 1st deletion are both visible as saved
+ const History hist{ {{1, 1, changeSaved}, {3, 1, changeRevertedOriginal}}, {{2, changeSaved}} };
+ REQUIRE(HistoryOf(cb) == hist);
+ }
+
+ SECTION("Detached") {
+ cb.SetUndoCollection(false);
+ constexpr std::string_view sInsert = "a-b=cdef";
+ bool startSequence = false;
+ cb.InsertString(0, sInsert.data(), sInsert.length(), startSequence);
+ cb.SetUndoCollection(true);
+ cb.ChangeHistorySet(true);
+
+ // Create an undo history that matches the contents at current point 2 which detached at 1
+ // So, insert saved; insert detached; current point
+ // abcdef
+ PushUndoAction(cb, insert, 1, "-");
+ // a-bcdef
+ PushUndoAction(cb, insert, 3, "=");
+ // a-b=cdef
+ cb.SetUndoSavePoint(-1);
+ cb.SetUndoDetach(1);
+ cb.SetUndoTentative(-1);
+ cb.SetUndoCurrent(2);
+
+ // This doesn't show elements due to undo.
+ // There was also a modified delete (reverting the insert) at 3 in the original but that is missing.
+ const History hist{ {{1, 1, changeSaved}, {3, 1, changeModified}}, {} };
+ REQUIRE(HistoryOf(cb) == hist);
+ }
+
+}
+
+namespace {
+
// Implement low quality reproducible pseudo-random numbers.
// Pseudo-random algorithm based on R. G. Dromey "How to Solve it by Computer" page 122.
@@ -1480,7 +1597,7 @@ TEST_CASE("CellBufferLong") {
const int r = rseq.Next() % 10;
if (r <= 2) { // 30%
// Insert text
- const int pos = rseq.Next() % (cb.Length() + 1);
+ const Sci::Position pos = rseq.Next() % (cb.Length() + 1);
const int len = rseq.Next() % 10 + 1;
std::string sInsert;
for (int j = 0; j < len; j++) {