diff options
-rw-r--r-- | doc/ScintillaDoc.html | 8 | ||||
-rw-r--r-- | src/CellBuffer.cxx | 133 | ||||
-rw-r--r-- | src/UndoHistory.cxx | 8 | ||||
-rw-r--r-- | src/UndoHistory.h | 8 | ||||
-rw-r--r-- | test/unit/testCellBuffer.cxx | 141 |
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++) { |