From 8de9dadf1de9e3113f43551f0a83a8fd60142549 Mon Sep 17 00:00:00 2001 From: omigamedev Date: Tue, 2 Jun 2026 23:18:19 +0200 Subject: [PATCH] Plan save-as file writes in app core --- docs/modernization/build-inventory.md | 7 +++-- docs/modernization/roadmap.md | 2 +- src/app_core/document_session.h | 35 +++++++++++++++++++---- src/app_dialogs.cpp | 24 +++++++++------- tests/app_core/document_session_tests.cpp | 31 ++++++++++++++++++++ tools/pano_cli/main.cpp | 20 +++++++------ 6 files changed, 92 insertions(+), 27 deletions(-) diff --git a/docs/modernization/build-inventory.md b/docs/modernization/build-inventory.md index fdad67e..730a0c1 100644 --- a/docs/modernization/build-inventory.md +++ b/docs/modernization/build-inventory.md @@ -403,7 +403,8 @@ Known local toolchain state: states. - `pano_cli plan-document-file` exposes `pp_app_core` document-name validation, legacy `.ppi` path construction, and overwrite-prompt decisions - as JSON and is covered for save-now and existing-target overwrite states. + as JSON through the same combined save-file plan consumed by the live save-as + dialog; it is covered for save-now and existing-target overwrite states. - `pano_cli plan-document-version` exposes `pp_app_core` save-version suffix parsing, candidate path generation, collision skipping, and no-slot failure behavior as JSON and is covered for first-version and existing-path skip @@ -426,8 +427,8 @@ Known local toolchain state: - `pp_app_core_document_session_tests` covers clean and dirty app session, document-open action planning, save-request, save-before-workflow, new-document target/resolution/overwrite planning, document file target, - overwrite, and save-version target decisions without requiring a window, - canvas, or message box. + combined save-file overwrite planning, and save-version target decisions + without requiring a window, canvas, or message box. - `pp_ui_core` consumes vcpkg tinyxml2 only when `PP_USE_VCPKG_TINYXML2=ON` through the vcpkg preset; default and Android validation still use the retained vendored fallback tracked by DEBT-0012. diff --git a/docs/modernization/roadmap.md b/docs/modernization/roadmap.md index 3610a16..d2658b2 100644 --- a/docs/modernization/roadmap.md +++ b/docs/modernization/roadmap.md @@ -433,7 +433,7 @@ same app-core new-document target, legacy resolution-index mapping, and overwrite decision used by the live new-document dialog, including invalid resolution rejection. `pano_cli plan-document-file` exposes the same app-core document-name validation, legacy `.ppi` path construction, and overwrite -prompt decision used by save-as dialogs. +prompt decision used by save-as dialogs through one combined save-file plan. `pano_cli plan-document-version` exposes the save-version suffix parsing, candidate generation, collision skipping, and no-slot failure behavior used by the live save-version dialog. diff --git a/src/app_core/document_session.h b/src/app_core/document_session.h index a9a2c89..7601269 100644 --- a/src/app_core/document_session.h +++ b/src/app_core/document_session.h @@ -66,6 +66,11 @@ struct DocumentVersionTarget { std::string path; }; +struct DocumentFileSavePlan { + DocumentFileTarget target; + DocumentFileWriteDecision write_decision = DocumentFileWriteDecision::save_now; +}; + struct NewDocumentPlan { DocumentFileTarget target; int resolution = 0; @@ -182,6 +187,23 @@ struct NewDocumentPlan { : DocumentFileWriteDecision::save_now; } +template +[[nodiscard]] pp::foundation::Result plan_document_file_save( + std::string_view work_directory, + std::string_view document_name, + ExistsPredicate&& exists) +{ + auto target = make_document_file_target(work_directory, document_name); + if (!target) { + return pp::foundation::Result::failure(target.status()); + } + + DocumentFileSavePlan plan; + plan.target = std::move(target.value()); + plan.write_decision = plan_document_file_write(exists(plan.target.path)); + return pp::foundation::Result::success(std::move(plan)); +} + [[nodiscard]] constexpr pp::foundation::Result document_resolution_from_index(int index) noexcept { constexpr std::array resolutions{ 512, 1024, 1536, 2048, 4096, 8192 }; @@ -205,15 +227,18 @@ template return pp::foundation::Result::failure(resolution.status()); } - auto target = make_document_file_target(work_directory, document_name); - if (!target) { - return pp::foundation::Result::failure(target.status()); + auto save_plan = plan_document_file_save( + work_directory, + document_name, + std::forward(exists)); + if (!save_plan) { + return pp::foundation::Result::failure(save_plan.status()); } NewDocumentPlan plan; - plan.target = std::move(target.value()); + plan.target = std::move(save_plan.value().target); plan.resolution = resolution.value(); - plan.write_decision = plan_document_file_write(exists(plan.target.path)); + plan.write_decision = save_plan.value().write_decision; return pp::foundation::Result::success(std::move(plan)); } diff --git a/src/app_dialogs.cpp b/src/app_dialogs.cpp index a40c311..7cf2f62 100644 --- a/src/app_dialogs.cpp +++ b/src/app_dialogs.cpp @@ -361,32 +361,36 @@ void App::dialog_save() dialog->btn_ok->on_click = [this, dialog](Node*) { std::string name = dialog->input->m_text; - const auto target = pp::app::make_document_file_target(work_path, name); - if (!target) + const auto plan = pp::app::plan_document_file_save( + work_path, + name, + [](const std::string& path) { + return Asset::exist(path); + }); + if (!plan) { message_box("Warning", "You need to specify a name to file."); return; } - auto action = [this, dialog, target = target.value()] { - canvas->m_canvas->project_save(target.path); - doc_name = target.name; - doc_path = target.path; - doc_dir = target.directory; + auto action = [this, dialog, plan = plan.value()] { + canvas->m_canvas->project_save(plan.target.path); + doc_name = plan.target.name; + doc_path = plan.target.path; + doc_dir = plan.target.directory; title_update(); dialog->destroy(); App::I->hideKeyboard(); }; - const auto write_decision = pp::app::plan_document_file_write(Asset::exist(target.value().path)); - if (write_decision == pp::app::DocumentFileWriteDecision::prompt_overwrite) + if (plan.value().write_decision == pp::app::DocumentFileWriteDecision::prompt_overwrite) { // ask confirm is file already exist auto msgbox = new NodeMessageBox(); msgbox->set_manager(&layout); msgbox->init(); msgbox->m_title->set_text("Warning"); - msgbox->m_message->set_text(("Are you sure you want to overwrite " + target.value().name + "?").c_str()); + msgbox->m_message->set_text(("Are you sure you want to overwrite " + plan.value().target.name + "?").c_str()); msgbox->btn_ok->on_click = [this, msgbox, action](Node*) { action(); msgbox->destroy(); diff --git a/tests/app_core/document_session_tests.cpp b/tests/app_core/document_session_tests.cpp index 18bafeb..9c95e89 100644 --- a/tests/app_core/document_session_tests.cpp +++ b/tests/app_core/document_session_tests.cpp @@ -175,6 +175,33 @@ void document_file_write_prompts_only_for_existing_targets(pp::tests::Harness& h == pp::app::DocumentFileWriteDecision::prompt_overwrite); } +void document_file_save_plan_combines_target_and_write_decision(pp::tests::Harness& harness) +{ + const auto plan = pp::app::plan_document_file_save( + "D:/Paint", + "demo", + [](const std::string& path) { + return path == "D:/Paint/demo.ppi"; + }); + PP_EXPECT(harness, plan); + PP_EXPECT(harness, plan.value().target.name == "demo"); + PP_EXPECT(harness, plan.value().target.directory == "D:/Paint"); + PP_EXPECT(harness, plan.value().target.path == "D:/Paint/demo.ppi"); + PP_EXPECT( + harness, + plan.value().write_decision == pp::app::DocumentFileWriteDecision::prompt_overwrite); +} + +void document_file_save_plan_rejects_empty_name(pp::tests::Harness& harness) +{ + const auto plan = pp::app::plan_document_file_save( + "D:/Paint", + "", + [](const std::string&) { return false; }); + PP_EXPECT(harness, !plan); + PP_EXPECT(harness, plan.status().code == pp::foundation::StatusCode::invalid_argument); +} + void document_resolution_index_maps_legacy_choices(pp::tests::Harness& harness) { const auto first = pp::app::document_resolution_from_index(0); @@ -320,6 +347,10 @@ int main() 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); + harness.run( + "document file save plan combines target and write decision", + document_file_save_plan_combines_target_and_write_decision); + harness.run("document file save plan rejects empty name", document_file_save_plan_rejects_empty_name); harness.run("document resolution index maps legacy choices", document_resolution_index_maps_legacy_choices); harness.run("document resolution index rejects out of range", document_resolution_index_rejects_out_of_range); harness.run( diff --git a/tools/pano_cli/main.cpp b/tools/pano_cli/main.cpp index dc2759e..efdef60 100644 --- a/tools/pano_cli/main.cpp +++ b/tools/pano_cli/main.cpp @@ -1345,20 +1345,24 @@ int plan_document_file(int argc, char** argv) return 2; } - const auto target = pp::app::make_document_file_target(args.work_directory, args.name); - if (!target) { - print_error("plan-document-file", target.status().message); + const auto plan = pp::app::plan_document_file_save( + args.work_directory, + args.name, + [&args](const std::string&) { + return args.target_exists; + }); + if (!plan) { + print_error("plan-document-file", plan.status().message); return 2; } - const auto write_decision = pp::app::plan_document_file_write(args.target_exists); std::cout << "{\"ok\":true,\"command\":\"plan-document-file\"" - << ",\"target\":{\"name\":\"" << json_escape(target.value().name) - << "\",\"directory\":\"" << json_escape(target.value().directory) - << "\",\"path\":\"" << json_escape(target.value().path) + << ",\"target\":{\"name\":\"" << json_escape(plan.value().target.name) + << "\",\"directory\":\"" << json_escape(plan.value().target.directory) + << "\",\"path\":\"" << json_escape(plan.value().target.path) << "\",\"exists\":" << json_bool(args.target_exists) << "},\"decision\":\"" - << document_file_write_decision_name(write_decision) + << document_file_write_decision_name(plan.value().write_decision) << "\"}\n"; return 0; }