include/svl/undo.hxx | 3 --- svl/source/undo/undo.cxx | 25 +++++++++---------------- 2 files changed, 9 insertions(+), 19 deletions(-)
New commits: commit 09df48f87f04b0176593cb5f649ac3cd2244891e Author: Justin Luth <[email protected]> AuthorDate: Fri Aug 16 21:26:56 2024 -0400 Commit: Miklos Vajna <[email protected]> CommitDate: Fri Sep 6 08:42:31 2024 +0200 tdf#161741 undo nits: NeedsClearRedo is private + prioritize TopLevel moNeedsClearRedo is an implementation detail, so there should not be public functions for it. Plus, NeedsClearRedo can be called either against the CurrentLevel stack or the TopLevel stack (whatever that implies - there is no documentation...) and since it is a delayed call that means that multiple NeedsClearRedo attempts could be made, theoretically against both stacks. My initial implementation was "last one wins", (which may very well prove to be the best one) but whatever happens, it should be clearly intentional. Based on my limited skill at code reading, it sounds like CurrentLevel might be more of an implemention detail or a temporary expansion of a ListUndoAction, so I am guessing that any request for TopLevel clearing should be given priority. For Writer, it is always clearing the TopLevel stack sw/source/core/undo/docundo.cxx: ClearRedo() return SdrUndoManager::ImplClearRedo_NoLock(TopLevel); Change-Id: I195aefb696599f018712135a2e015549d534791f Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171984 Tested-by: Jenkins Reviewed-by: Miklos Vajna <[email protected]> Reviewed-by: Justin Luth <[email protected]> diff --git a/include/svl/undo.hxx b/include/svl/undo.hxx index 707a6d8b025b..2ca0f3b0f8a4 100644 --- a/include/svl/undo.hxx +++ b/include/svl/undo.hxx @@ -222,9 +222,6 @@ public: */ virtual void ClearRedo(); - const std::optional<bool>& GetNeedsClearRedo() const; - void SetNeedsClearRedo(const std::optional<bool>& oSet); - /** leaves any possible open list action (<member>IsInListAction</member>), and clears both the Undo and the Redo stack. diff --git a/svl/source/undo/undo.cxx b/svl/source/undo/undo.cxx index 51849ebc0744..ab33374507fc 100644 --- a/svl/source/undo/undo.cxx +++ b/svl/source/undo/undo.cxx @@ -185,7 +185,7 @@ struct SfxUndoManager_Data bool mbDoing; bool mbClearUntilTopLevel; bool mbEmptyActions; - std::optional<bool> moNeedsClearRedo; + std::optional<bool> moNeedsClearRedo; // holds a requested ClearRedo until safe to clear stack UndoListeners aListeners; @@ -477,7 +477,10 @@ void SfxUndoManager::ImplClearRedo_NoLock( bool const i_currentLevel ) { if (IsDoing()) { - SetNeedsClearRedo(i_currentLevel); + // cannot clear redo while undo/redo is in process. Delay ClearRedo until safe to clear. + // (assuming if TopLevel requests a clear, it should have priority over CurrentLevel) + if (!m_xData->moNeedsClearRedo.has_value() || i_currentLevel == TopLevel) + m_xData->moNeedsClearRedo = i_currentLevel; return; } UndoManagerGuard aGuard( *m_xData ); @@ -492,16 +495,6 @@ void SfxUndoManager::ClearRedo() ImplClearRedo_NoLock( CurrentLevel ); } -const std::optional<bool>& SfxUndoManager::GetNeedsClearRedo() const -{ - return m_xData->moNeedsClearRedo; -} - -void SfxUndoManager::SetNeedsClearRedo(const std::optional<bool>& oSet) -{ - m_xData->moNeedsClearRedo = oSet; -} - void SfxUndoManager::Reset() { UndoManagerGuard aGuard( *m_xData ); @@ -764,10 +757,10 @@ bool SfxUndoManager::ImplUndo( SfxUndoContext* i_contextOrNull ) } m_xData->mbDoing = false; - if (GetNeedsClearRedo().has_value()) + if (m_xData->moNeedsClearRedo.has_value()) { - ImplClearRedo_NoLock(*GetNeedsClearRedo()); - SetNeedsClearRedo(std::optional<bool>()); + ImplClearRedo_NoLock(*m_xData->moNeedsClearRedo); + m_xData->moNeedsClearRedo.reset(); } aGuard.scheduleNotification( &SfxUndoListener::actionUndone, sActionComment ); @@ -883,7 +876,7 @@ bool SfxUndoManager::ImplRedo( SfxUndoContext* i_contextOrNull ) } m_xData->mbDoing = false; - assert(!GetNeedsClearRedo().has_value() && "Assuming I don't need to handle it here. What about if thrown?"); + assert(!m_xData->moNeedsClearRedo.has_value() && "Assuming I don't need to handle it here. What about if thrown?"); ImplCheckEmptyActions(); aGuard.scheduleNotification( &SfxUndoListener::actionRedone, sActionComment );
