From 34a9e9109989ba655a0d093d8d5d9309b5afcf42 Mon Sep 17 00:00:00 2001 From: omigamedev Date: Fri, 12 Jun 2026 19:16:24 +0200 Subject: [PATCH] Make document session history effects explicit --- src/app_core/document_session.cpp | 61 ++++++++++++++++++ src/app_core/document_session.h | 9 +++ src/legacy_document_open_services.cpp | 31 ++++++++- src/legacy_document_session_services.cpp | 76 ++++++++++++++++++++++- tests/app_core/document_session_tests.cpp | 74 ++++++++++++++++++++++ 5 files changed, 247 insertions(+), 4 deletions(-) diff --git a/src/app_core/document_session.cpp b/src/app_core/document_session.cpp index fa71c74..488437c 100644 --- a/src/app_core/document_session.cpp +++ b/src/app_core/document_session.cpp @@ -1 +1,62 @@ #include "app_core/document_session.h" + +namespace pp::app { +namespace { + +[[nodiscard]] constexpr HistoryUiPlan make_history_clear_effect() noexcept +{ + HistoryUiPlan plan; + plan.operation = HistoryUiOperation::clear; + plan.clears_history = true; + plan.updates_memory_label = true; + return plan; +} + +[[nodiscard]] constexpr HistoryUiPlan make_history_no_op_effect() noexcept +{ + HistoryUiPlan plan; + plan.operation = HistoryUiOperation::clear; + plan.no_op = true; + return plan; +} + +} // namespace + +HistoryUiPlan plan_document_open_history(const DocumentOpenRoute& route) noexcept +{ + return route.kind == DocumentOpenKind::open_project + ? make_history_clear_effect() + : make_history_no_op_effect(); +} + +HistoryUiPlan plan_close_request_history(CloseRequestDecision) noexcept +{ + return make_history_no_op_effect(); +} + +HistoryUiPlan plan_document_save_history(DocumentSaveDecision) noexcept +{ + return make_history_no_op_effect(); +} + +HistoryUiPlan plan_document_workflow_history(DocumentWorkflowDecision) noexcept +{ + return make_history_no_op_effect(); +} + +HistoryUiPlan plan_document_file_save_history(const DocumentFileSavePlan&) noexcept +{ + return make_history_no_op_effect(); +} + +HistoryUiPlan plan_document_version_save_history(const DocumentVersionTarget&) noexcept +{ + return make_history_no_op_effect(); +} + +HistoryUiPlan plan_new_document_history(const NewDocumentPlan&) noexcept +{ + return make_history_clear_effect(); +} + +} // namespace pp::app diff --git a/src/app_core/document_session.h b/src/app_core/document_session.h index 91365f2..00c50b2 100644 --- a/src/app_core/document_session.h +++ b/src/app_core/document_session.h @@ -2,6 +2,7 @@ #include "app_core/app_dialog.h" #include "app_core/document_route.h" +#include "app_core/history_ui.h" #include "foundation/result.h" #include @@ -144,6 +145,14 @@ public: virtual void prompt_overwrite_new_document(const NewDocumentPlan& plan) = 0; }; +[[nodiscard]] HistoryUiPlan plan_document_open_history(const DocumentOpenRoute& route) noexcept; +[[nodiscard]] HistoryUiPlan plan_close_request_history(CloseRequestDecision decision) noexcept; +[[nodiscard]] HistoryUiPlan plan_document_save_history(DocumentSaveDecision decision) noexcept; +[[nodiscard]] HistoryUiPlan plan_document_workflow_history(DocumentWorkflowDecision decision) noexcept; +[[nodiscard]] HistoryUiPlan plan_document_file_save_history(const DocumentFileSavePlan& plan) noexcept; +[[nodiscard]] HistoryUiPlan plan_document_version_save_history(const DocumentVersionTarget& target) noexcept; +[[nodiscard]] HistoryUiPlan plan_new_document_history(const NewDocumentPlan& plan) noexcept; + [[nodiscard]] inline AppMessageDialogPlan plan_document_session_prompt( DocumentSessionPromptKind kind, std::string_view document_name = {}) diff --git a/src/legacy_document_open_services.cpp b/src/legacy_document_open_services.cpp index 8d15cf3..d07406f 100644 --- a/src/legacy_document_open_services.cpp +++ b/src/legacy_document_open_services.cpp @@ -2,10 +2,10 @@ #include "legacy_document_open_services.h" +#include "action.h" #include "app.h" #include "legacy_brush_package_import_services.h" #include "legacy_canvas_view_services.h" -#include "legacy_history_services.h" #include "legacy_ui_overlay_services.h" #include "log.h" #include "node_panel_brush.h" @@ -14,6 +14,30 @@ namespace pp::panopainter { namespace { +class LegacyDocumentHistoryServices final : public pp::app::HistoryUiServices { +public: + void invoke_undo() override + { + ActionManager::undo(); + } + + void invoke_redo() override + { + ActionManager::redo(); + } + + void clear_history() override + { + ActionManager::clear(); + } +}; + +pp::foundation::Status apply_document_history(const pp::app::HistoryUiPlan& plan) +{ + LegacyDocumentHistoryServices services; + return pp::app::execute_history_ui_plan(plan, services); +} + void open_legacy_project(App& app, const pp::app::DocumentOpenRoute& route) { app.doc_name = route.name; @@ -41,7 +65,10 @@ void open_legacy_project(App& app, const pp::app::DocumentOpenRoute& route) "It may be inaccessible or corrupted."); } }); - pp::panopainter::clear_legacy_history(); + const auto history_status = apply_document_history(pp::app::plan_document_open_history(route)); + if (!history_status.ok()) { + LOG("Project open history effect failed: %s", history_status.message); + } } class LegacyDocumentOpenServices final : public pp::app::DocumentOpenServices { diff --git a/src/legacy_document_session_services.cpp b/src/legacy_document_session_services.cpp index a1c6926..077b8b0 100644 --- a/src/legacy_document_session_services.cpp +++ b/src/legacy_document_session_services.cpp @@ -2,11 +2,11 @@ #include "legacy_document_session_services.h" +#include "action.h" #include "app.h" #include "legacy_app_dialog_services.h" #include "legacy_document_canvas_services.h" #include "legacy_canvas_view_services.h" -#include "legacy_history_services.h" #include "legacy_ui_overlay_services.h" #include "node_dialog_open.h" @@ -15,6 +15,30 @@ namespace pp::panopainter { namespace { +class LegacyDocumentHistoryServices final : public pp::app::HistoryUiServices { +public: + void invoke_undo() override + { + ActionManager::undo(); + } + + void invoke_redo() override + { + ActionManager::redo(); + } + + void clear_history() override + { + ActionManager::clear(); + } +}; + +pp::foundation::Status apply_document_history(const pp::app::HistoryUiPlan& plan) +{ + LegacyDocumentHistoryServices services; + return pp::app::execute_history_ui_plan(plan, services); +} + void log_legacy_document_save_snapshot( const char* context, const pp::app::DocumentCanvasSaveSnapshotReport& report) @@ -110,7 +134,10 @@ void create_legacy_new_document( const auto reset_status = execute_legacy_canvas_camera_reset(app); if (!reset_status.ok()) LOG("New document camera reset failed: %s", reset_status.message); - pp::panopainter::clear_legacy_history(); + const auto history_status = apply_document_history(pp::app::plan_new_document_history(plan)); + if (!history_status.ok()) { + LOG("New document history effect failed: %s", history_status.message); + } app.layers->add_layer("Default", false, true); @@ -158,6 +185,10 @@ void save_legacy_document_file( const pp::app::DocumentFileSavePlan& plan, const std::shared_ptr& dialog) { + const auto history_status = apply_document_history(pp::app::plan_document_file_save_history(plan)); + if (!history_status.ok()) { + LOG("Document file save history effect failed: %s", history_status.message); + } project_save_after_snapshot(app, plan.target.path); app.doc_name = plan.target.name; app.doc_path = plan.target.path; @@ -208,6 +239,10 @@ public: void save_document_version(const pp::app::DocumentVersionTarget& target) override { + const auto history_status = apply_document_history(pp::app::plan_document_version_save_history(target)); + if (!history_status.ok()) { + LOG("Document version history effect failed: %s", history_status.message); + } app_.doc_name = target.name; app_.doc_path = target.path; app_.canvas->m_canvas->m_unsaved = true; @@ -229,10 +264,20 @@ public: void request_close_now() override { + const auto history_status = apply_document_history( + pp::app::plan_close_request_history(pp::app::CloseRequestDecision::close_now)); + if (!history_status.ok()) { + LOG("Close request history effect failed: %s", history_status.message); + } } void show_unsaved_close_prompt() override { + const auto history_status = apply_document_history( + pp::app::plan_close_request_history(pp::app::CloseRequestDecision::show_unsaved_prompt)); + if (!history_status.ok()) { + LOG("Close prompt history effect failed: %s", history_status.message); + } auto* app = &app_; auto* dialog_already_opened = &dialog_already_opened_; auto m = pp::panopainter::create_legacy_app_message_dialog( @@ -264,16 +309,31 @@ public: void show_save_dialog() override { + const auto history_status = apply_document_history( + pp::app::plan_document_save_history(pp::app::DocumentSaveDecision::show_save_dialog)); + if (!history_status.ok()) { + LOG("Save dialog history effect failed: %s", history_status.message); + } app_.dialog_save(); } void save_existing_document() override { + const auto history_status = apply_document_history( + pp::app::plan_document_save_history(pp::app::DocumentSaveDecision::save_existing)); + if (!history_status.ok()) { + LOG("Save existing history effect failed: %s", history_status.message); + } project_save_after_snapshot(app_); } void save_document_version() override { + const auto history_status = apply_document_history( + pp::app::plan_document_save_history(pp::app::DocumentSaveDecision::save_version)); + if (!history_status.ok()) { + LOG("Save version history effect failed: %s", history_status.message); + } app_.dialog_save_ver(); } @@ -291,11 +351,23 @@ public: void continue_workflow_now() override { + const auto history_status = apply_document_history( + pp::app::plan_document_workflow_history( + pp::app::DocumentWorkflowDecision::continue_now)); + if (!history_status.ok()) { + LOG("Workflow continue history effect failed: %s", history_status.message); + } action_(); } void prompt_save_before_continue() override { + const auto history_status = apply_document_history( + pp::app::plan_document_workflow_history( + pp::app::DocumentWorkflowDecision::prompt_save_before_continue)); + if (!history_status.ok()) { + LOG("Workflow prompt history effect failed: %s", history_status.message); + } auto m = pp::panopainter::create_legacy_app_message_dialog( app_, pp::app::plan_document_session_prompt( diff --git a/tests/app_core/document_session_tests.cpp b/tests/app_core/document_session_tests.cpp index d96be60..8b19dd4 100644 --- a/tests/app_core/document_session_tests.cpp +++ b/tests/app_core/document_session_tests.cpp @@ -203,6 +203,22 @@ public: }; } +void expect_history_clear(pp::tests::Harness& harness, const pp::app::HistoryUiPlan& plan) +{ + PP_EXPECT(harness, plan.operation == pp::app::HistoryUiOperation::clear); + PP_EXPECT(harness, plan.clears_history); + PP_EXPECT(harness, plan.updates_memory_label); + PP_EXPECT(harness, !plan.no_op); +} + +void expect_history_no_op(pp::tests::Harness& harness, const pp::app::HistoryUiPlan& plan) +{ + PP_EXPECT(harness, plan.operation == pp::app::HistoryUiOperation::clear); + PP_EXPECT(harness, !plan.clears_history); + PP_EXPECT(harness, !plan.updates_memory_label); + PP_EXPECT(harness, plan.no_op); +} + void project_open_clean_document_executes_immediately(pp::tests::Harness& harness) { PP_EXPECT(harness, pp::app::plan_project_open(false) == pp::app::ProjectOpenDecision::open_now); @@ -319,6 +335,13 @@ void document_open_executor_rejects_mismatched_routes(pp::tests::Harness& harnes PP_EXPECT(harness, services.call_order.empty()); } +void document_open_history_clears_only_for_projects(pp::tests::Harness& harness) +{ + expect_history_clear(harness, pp::app::plan_document_open_history(project_route())); + expect_history_no_op(harness, pp::app::plan_document_open_history(abr_route())); + expect_history_no_op(harness, pp::app::plan_document_open_history(ppbr_route())); +} + void close_clean_document_executes_immediately(pp::tests::Harness& harness) { PP_EXPECT( @@ -526,6 +549,53 @@ void workflow_executor_dispatches_continue_prompt_and_unavailable(pp::tests::Har PP_EXPECT(harness, services.call_order == "continue;prompt-save;"); } +void document_session_history_effects_stay_explicit_and_stable(pp::tests::Harness& harness) +{ + expect_history_no_op( + harness, + pp::app::plan_close_request_history(pp::app::CloseRequestDecision::close_now)); + expect_history_no_op( + harness, + pp::app::plan_close_request_history(pp::app::CloseRequestDecision::show_unsaved_prompt)); + expect_history_no_op( + harness, + pp::app::plan_document_save_history(pp::app::DocumentSaveDecision::show_save_dialog)); + expect_history_no_op( + harness, + pp::app::plan_document_save_history(pp::app::DocumentSaveDecision::save_existing)); + expect_history_no_op( + harness, + pp::app::plan_document_save_history(pp::app::DocumentSaveDecision::save_version)); + expect_history_no_op( + harness, + pp::app::plan_document_workflow_history(pp::app::DocumentWorkflowDecision::continue_now)); + expect_history_no_op( + harness, + pp::app::plan_document_workflow_history( + pp::app::DocumentWorkflowDecision::prompt_save_before_continue)); + + auto file_plan = pp::app::plan_document_file_save( + "D:/Paint", + "demo", + [](const std::string&) { return false; }); + PP_EXPECT(harness, file_plan); + expect_history_no_op(harness, pp::app::plan_document_file_save_history(file_plan.value())); + + const pp::app::DocumentVersionTarget version_target { + .name = "demo.02", + .path = "D:/Paint/demo.02.ppi", + }; + expect_history_no_op(harness, pp::app::plan_document_version_save_history(version_target)); + + auto new_document = pp::app::plan_new_document( + "D:/Paint", + "demo", + 1, + [](const std::string&) { return false; }); + PP_EXPECT(harness, new_document); + expect_history_clear(harness, pp::app::plan_new_document_history(new_document.value())); +} + void document_file_target_rejects_empty_name(pp::tests::Harness& harness) { const auto target = pp::app::make_document_file_target("D:/Paint", ""); @@ -791,6 +861,7 @@ int main() document_open_brush_imports_prompt_regardless_of_unsaved_state); harness.run("document open executor dispatches all actions", document_open_executor_dispatches_all_actions); harness.run("document open executor rejects mismatched routes", document_open_executor_rejects_mismatched_routes); + harness.run("document open history clears only for projects", document_open_history_clears_only_for_projects); harness.run("close clean document executes immediately", close_clean_document_executes_immediately); harness.run("close dirty document opens one prompt", close_dirty_document_opens_one_prompt); harness.run("close request executor dispatches and preserves wait", close_request_executor_dispatches_and_preserves_wait); @@ -809,6 +880,9 @@ int main() harness.run("workflow with clean canvas continues now", workflow_with_clean_canvas_continues_now); harness.run("workflow with dirty canvas prompts for save", workflow_with_dirty_canvas_prompts_for_save); harness.run("workflow executor dispatches continue prompt and unavailable", workflow_executor_dispatches_continue_prompt_and_unavailable); + harness.run( + "document session history effects stay explicit and stable", + document_session_history_effects_stay_explicit_and_stable); harness.run("document file target rejects empty name", document_file_target_rejects_empty_name); harness.run("document file target builds legacy ppi path", document_file_target_builds_legacy_ppi_path); harness.run("document file write prompts only for existing targets", document_file_write_prompts_only_for_existing_targets);