From 1df506a176911950fe0b854d30e6e7ec84d81ded Mon Sep 17 00:00:00 2001 From: omigamedev Date: Tue, 2 Jun 2026 22:58:28 +0200 Subject: [PATCH] Plan save-version targets in app core --- docs/modernization/build-inventory.md | 9 ++- docs/modernization/capability-map.md | 2 +- docs/modernization/debt.md | 2 +- docs/modernization/roadmap.md | 3 + src/app_core/document_session.h | 85 +++++++++++++++++++++++ src/app_dialogs.cpp | 33 +++------ tests/CMakeLists.txt | 12 ++++ tests/app_core/document_session_tests.cpp | 63 +++++++++++++++++ tools/pano_cli/main.cpp | 80 +++++++++++++++++++++ 9 files changed, 263 insertions(+), 26 deletions(-) diff --git a/docs/modernization/build-inventory.md b/docs/modernization/build-inventory.md index e2f68c6..d58f619 100644 --- a/docs/modernization/build-inventory.md +++ b/docs/modernization/build-inventory.md @@ -397,6 +397,10 @@ Known local toolchain state: - `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. +- `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 + states. - `pano_cli plan-export-target` exposes `pp_app_core` export target planning for image file exports, layer/frame collection directories, picked-directory stems, and MP4 suggested names as JSON and is covered for file, collection, @@ -413,8 +417,9 @@ Known local toolchain state: directory/stem targets, picked-directory stems, MP4 suggested names, and invalid export naming inputs. - `pp_app_core_document_session_tests` covers clean and dirty app session, - save-request, save-before-workflow, document file target, and overwrite - decisions without requiring a window, canvas, or message box. + save-request, save-before-workflow, document file target, overwrite, 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/capability-map.md b/docs/modernization/capability-map.md index 20256a8..d1e0c93 100644 --- a/docs/modernization/capability-map.md +++ b/docs/modernization/capability-map.md @@ -13,7 +13,7 @@ and validation command. | --- | --- | --- | --- | | PPI open/save | `Canvas`, serializer, dialogs | `pp_document`, `pp_assets`, `pano_cli` | Round-trip tiny project, old-version fixture, corrupt/truncated fixture | | Open-document routing | `App::open_document` | `pp_app_core`, `pano_cli`, `pp_panopainter_ui`, `pp_document`, `pp_assets` | Project/ABR/PPBR route tests, malformed path tests, CLI route smoke, app open smoke | -| Document session decisions | `App::open_document`, `App::request_close`, save hotkeys, file menu, dialogs | `pp_app_core`, `pano_cli`, `pp_panopainter_ui` | Clean/dirty/prompt-open/save/save-as/save-version/save-before-workflow/name/overwrite decision tests, CLI session and document-file smoke, app close/open/save/new/browse smoke | +| Document session decisions | `App::open_document`, `App::request_close`, save hotkeys, file menu, dialogs | `pp_app_core`, `pano_cli`, `pp_panopainter_ui` | Clean/dirty/prompt-open/save/save-as/save-version/save-before-workflow/name/overwrite/version-target decision tests, CLI session, document-file, and document-version smoke, app close/open/save/new/browse smoke | | Version metadata | `scripts/pre-build.py`, `version.*` | build system, `pp_foundation` | Generated header smoke test, missing-tag behavior | | Thumbnail generation/read | `Canvas`, `Image` | `pp_assets`, `pp_paint_renderer` | Golden thumbnail, corrupt input | | Save-as, overwrite prompts | App/dialogs | `pp_app_core`, `pp_panopainter_ui`, `pp_platform_*` | Decision tests, UI automation, and platform smoke | diff --git a/docs/modernization/debt.md b/docs/modernization/debt.md index 30ece1d..e8dc688 100644 --- a/docs/modernization/debt.md +++ b/docs/modernization/debt.md @@ -22,7 +22,7 @@ agent or engineer to remove them without reconstructing context from chat. | --- | --- | --- | --- | --- | --- | --- | | DEBT-0001 | Open | Modernization | Existing platform build files remain alongside new CMake | Required for incremental migration without losing platform coverage | Existing platform builds plus new CMake configure | Remove after all platform builds consume shared CMake targets | | DEBT-0002 | Open | Modernization | Vendored SDK and patched libraries retained initially | Some dependencies are SDK-only, patched, or have platform-specific binaries | Dependency inventory and platform build smoke tests | Replace with vcpkg packages or document permanent vendored status after triplet evaluation | -| DEBT-0003 | Open | Modernization | Existing singletons remain during initial split; `App::open_document`, `App::request_close`, file-menu save actions, `NodeCanvas` save hotkeys, new/open/browse dirty-document workflow prompts, new/save-as document file naming and overwrite decisions, export target naming/path decisions, `pano_cli classify-open`, `pano_cli plan-document-file`, `pano_cli plan-export-target`, and `pano_cli simulate-app-session` now consume pure `pp_app_core` route/session/export contracts, but document loading, saving, and export execution still reach legacy `Canvas::I` and UI singletons | Avoid behavior changes while introducing component boundaries | App launch and component tests; `pp_app_core_document_route_tests`; `pp_app_core_document_export_tests`; `pp_app_core_document_session_tests`; `pano_cli classify-open --path D:/Paint/demo.ppi`; `pano_cli plan-document-file --work-dir D:/Paint --name demo --target-exists`; `pano_cli plan-export-target --kind file --work-dir D:/Paint --doc-name demo --extension .png`; `pano_cli simulate-app-session --unsaved --save-intent save-dirty-version`; `pano_cli simulate-app-session --no-canvas`; `ctest --preset desktop-fast --build-config Debug` | Replace singleton reaches with context/service injection at component boundaries | +| DEBT-0003 | Open | Modernization | Existing singletons remain during initial split; `App::open_document`, `App::request_close`, file-menu save actions, `NodeCanvas` save hotkeys, new/open/browse dirty-document workflow prompts, new/save-as document file naming and overwrite decisions, save-version target decisions, export target naming/path decisions, `pano_cli classify-open`, `pano_cli plan-document-file`, `pano_cli plan-document-version`, `pano_cli plan-export-target`, and `pano_cli simulate-app-session` now consume pure `pp_app_core` route/session/export contracts, but document loading, saving, and export execution still reach legacy `Canvas::I` and UI singletons | Avoid behavior changes while introducing component boundaries | App launch and component tests; `pp_app_core_document_route_tests`; `pp_app_core_document_export_tests`; `pp_app_core_document_session_tests`; `pano_cli classify-open --path D:/Paint/demo.ppi`; `pano_cli plan-document-file --work-dir D:/Paint --name demo --target-exists`; `pano_cli plan-document-version --directory D:/Paint --doc-name demo.01 --existing-path D:/Paint/demo.02.ppi`; `pano_cli plan-export-target --kind file --work-dir D:/Paint --doc-name demo --extension .png`; `pano_cli simulate-app-session --unsaved --save-intent save-dirty-version`; `pano_cli simulate-app-session --no-canvas`; `ctest --preset desktop-fast --build-config Debug` | Replace singleton reaches with context/service injection at component boundaries | | DEBT-0004 | Open | Modernization | Android, Linux, WebGL, Apple, and AppX build files remain platform-specific until root CMake alignment reaches them | Prevent platform regressions during incremental migration; raw Windows `.sln/.vcxproj` files were removed on 2026-05-31 by user decision | `cmake --preset windows-msvc-default`; platform-specific configure/build smoke checks as each platform is migrated | Root CMake owns every platform source list and package path | | DEBT-0005 | Open | Modernization | Temporary local CTest harness is used before Catch2 is wired through vcpkg | `vcpkg` is not currently on PATH, but headless tests need to run now | `ctest --preset desktop-fast --build-config Debug` | Replace `tests/test_harness.h` tests with Catch2 tests once vcpkg toolchain/presets are validated | | DEBT-0007 | Open | Modernization | `vcpkg.json` and `windows-msvc-vcpkg-headless` are validated for the headless Windows component matrix, but app targets still use vendored libraries and Android/Apple triplets are not proven | Dependency migration must stay incremental while SDK/patched/vendor dependencies remain in use | `$env:VCPKG_ROOT="C:\Program Files\Microsoft Visual Studio\2022\Community\VC\vcpkg"; cmake --preset windows-msvc-vcpkg-headless`; `ctest --preset desktop-fast-vcpkg --build-config Debug` | Component targets consume vcpkg packages where reliable and desktop app, Android, and Apple triplets are validated or explicitly documented as permanent vendor exceptions | diff --git a/docs/modernization/roadmap.md b/docs/modernization/roadmap.md index b8682f7..07fddd2 100644 --- a/docs/modernization/roadmap.md +++ b/docs/modernization/roadmap.md @@ -429,6 +429,9 @@ save-version flows, plus the save-before-continue workflow gate used by new-document/open/browse dialogs. `pano_cli plan-document-file` exposes the same app-core document-name validation, legacy `.ppi` path construction, and overwrite prompt decision used by new-document and save-as dialogs. +`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. `pano_cli plan-export-target` exposes app-core export target planning for equirectangular image files, layer/frame collection stems, picked-directory stems, and MP4 suggested names used by the live export dialogs. diff --git a/src/app_core/document_session.h b/src/app_core/document_session.h index 93ae102..31544b5 100644 --- a/src/app_core/document_session.h +++ b/src/app_core/document_session.h @@ -2,6 +2,8 @@ #include "foundation/result.h" +#include +#include #include #include #include @@ -50,6 +52,11 @@ struct DocumentFileTarget { std::string path; }; +struct DocumentVersionTarget { + std::string name; + std::string path; +}; + [[nodiscard]] constexpr ProjectOpenDecision plan_project_open(bool has_unsaved_changes) noexcept { return has_unsaved_changes @@ -142,4 +149,82 @@ struct DocumentFileTarget { : DocumentFileWriteDecision::save_now; } +[[nodiscard]] inline bool has_legacy_two_character_version_suffix(std::string_view document_name) noexcept +{ + const auto dot = document_name.rfind('.'); + if (dot == std::string_view::npos || dot + 3 != document_name.size()) { + return false; + } + + const auto is_word = [](char ch) noexcept { + return std::isalnum(static_cast(ch)) != 0 || ch == '_'; + }; + return is_word(document_name[dot + 1]) && is_word(document_name[dot + 2]); +} + +[[nodiscard]] inline int legacy_version_number(std::string_view suffix) noexcept +{ + int value = 0; + for (const char ch : suffix) { + if (ch < '0' || ch > '9') { + break; + } + + value = value * 10 + (ch - '0'); + } + return value; +} + +[[nodiscard]] inline std::string make_legacy_version_name(std::string_view base_name, int version) +{ + char suffix[4] {}; + std::snprintf(suffix, sizeof(suffix), ".%02d", version); + std::string name; + name.reserve(base_name.size() + 3); + name += base_name; + name += suffix; + return name; +} + +template +[[nodiscard]] pp::foundation::Result find_next_document_version_target( + std::string_view directory, + std::string_view document_name, + ExistsPredicate&& exists) +{ + if (directory.empty()) { + return pp::foundation::Result::failure( + pp::foundation::Status::invalid_argument("directory must not be empty")); + } + + if (document_name.empty()) { + return pp::foundation::Result::failure( + pp::foundation::Status::invalid_argument("document name must not be empty")); + } + + int current = 0; + std::string_view base = document_name; + if (has_legacy_two_character_version_suffix(document_name)) { + const auto dot = document_name.rfind('.'); + base = document_name.substr(0, dot); + current = legacy_version_number(document_name.substr(dot + 1)); + } + + for (int version = current + 1; version < 99; ++version) { + DocumentVersionTarget target; + target.name = make_legacy_version_name(base, version); + target.path.reserve(directory.size() + target.name.size() + 5); + target.path += directory; + target.path += "/"; + target.path += target.name; + target.path += ".ppi"; + if (!exists(target.path)) { + return pp::foundation::Result::success(std::move(target)); + } + } + + return pp::foundation::Result::failure( + pp::foundation::Status::out_of_range("no available document version target")); +} + } diff --git a/src/app_dialogs.cpp b/src/app_dialogs.cpp index f43f324..398bbfe 100644 --- a/src/app_dialogs.cpp +++ b/src/app_dialogs.cpp @@ -292,30 +292,19 @@ void App::dialog_save_ver() return; } - int current = 0; - std::string next = doc_name + ".01"; - std::string base = doc_name; - - std::regex r(R"((.*)\.(\w{2})$)"); - std::smatch m; - if (std::regex_search(doc_name, m, r)) - { - base = m[1].str(); - current = atoi(m[2].str().c_str()); + const auto target = pp::app::find_next_document_version_target( + doc_dir, + doc_name, + [](const std::string& path) { + return Asset::exist(path); + }); + if (!target) { + message_box("Saving Error", target.status().message); + return; } - for (int i = current + 1; i < 99; i++) - { - static char tmp_name[256]; - sprintf(tmp_name, "%s.%02d", base.c_str(), i); - next = tmp_name; - if (Asset::exist(doc_dir + "/" + next + ".ppi")) - continue; - break; - } - - doc_name = next; - doc_path = doc_dir + "/" + next + ".ppi"; + doc_name = target.value().name; + doc_path = target.value().path; canvas->m_canvas->m_unsaved = true; title_update(); canvas->m_canvas->project_save(doc_path); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 3265e23..6095567 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -393,6 +393,18 @@ if(TARGET pano_cli) LABELS "app;integration;desktop-fast" PASS_REGULAR_EXPRESSION "\"command\":\"plan-document-file\".*\"path\":\"D:/Paint/demo.ppi\".*\"exists\":true.*\"decision\":\"prompt-overwrite\"") + add_test(NAME pano_cli_plan_document_version_first_smoke + COMMAND pano_cli plan-document-version --directory D:/Paint --doc-name demo) + set_tests_properties(pano_cli_plan_document_version_first_smoke PROPERTIES + LABELS "app;integration;desktop-fast" + PASS_REGULAR_EXPRESSION "\"command\":\"plan-document-version\".*\"documentName\":\"demo\".*\"name\":\"demo.01\".*\"path\":\"D:/Paint/demo.01.ppi\"") + + add_test(NAME pano_cli_plan_document_version_skip_existing_smoke + COMMAND pano_cli plan-document-version --directory D:/Paint --doc-name demo.01 --existing-path D:/Paint/demo.02.ppi) + set_tests_properties(pano_cli_plan_document_version_skip_existing_smoke PROPERTIES + LABELS "app;integration;desktop-fast" + PASS_REGULAR_EXPRESSION "\"command\":\"plan-document-version\".*\"documentName\":\"demo.01\".*\"existingPaths\":1.*\"name\":\"demo.03\".*\"path\":\"D:/Paint/demo.03.ppi\"") + add_test(NAME pano_cli_plan_export_target_file_smoke COMMAND pano_cli plan-export-target --kind file --work-dir D:/Paint --doc-name demo --extension .png) set_tests_properties(pano_cli_plan_export_target_file_smoke PROPERTIES diff --git a/tests/app_core/document_session_tests.cpp b/tests/app_core/document_session_tests.cpp index e62f6a2..14c46e7 100644 --- a/tests/app_core/document_session_tests.cpp +++ b/tests/app_core/document_session_tests.cpp @@ -143,6 +143,62 @@ void document_file_write_prompts_only_for_existing_targets(pp::tests::Harness& h == pp::app::DocumentFileWriteDecision::prompt_overwrite); } +void document_version_target_starts_at_first_version(pp::tests::Harness& harness) +{ + const auto target = pp::app::find_next_document_version_target( + "D:/Paint", + "demo", + [](const std::string&) { return false; }); + PP_EXPECT(harness, target); + PP_EXPECT(harness, target.value().name == "demo.01"); + PP_EXPECT(harness, target.value().path == "D:/Paint/demo.01.ppi"); +} + +void document_version_target_increments_existing_suffix(pp::tests::Harness& harness) +{ + const auto target = pp::app::find_next_document_version_target( + "D:/Paint", + "demo.07", + [](const std::string&) { return false; }); + PP_EXPECT(harness, target); + PP_EXPECT(harness, target.value().name == "demo.08"); + PP_EXPECT(harness, target.value().path == "D:/Paint/demo.08.ppi"); +} + +void document_version_target_skips_existing_paths(pp::tests::Harness& harness) +{ + const auto target = pp::app::find_next_document_version_target( + "D:/Paint", + "demo", + [](const std::string& path) { + return path == "D:/Paint/demo.01.ppi" || path == "D:/Paint/demo.02.ppi"; + }); + PP_EXPECT(harness, target); + PP_EXPECT(harness, target.value().name == "demo.03"); + PP_EXPECT(harness, target.value().path == "D:/Paint/demo.03.ppi"); +} + +void document_version_target_preserves_legacy_nonnumeric_suffix_handling(pp::tests::Harness& harness) +{ + const auto target = pp::app::find_next_document_version_target( + "D:/Paint", + "demo.ab", + [](const std::string&) { return false; }); + PP_EXPECT(harness, target); + PP_EXPECT(harness, target.value().name == "demo.01"); + PP_EXPECT(harness, target.value().path == "D:/Paint/demo.01.ppi"); +} + +void document_version_target_reports_when_no_slots_remain(pp::tests::Harness& harness) +{ + const auto target = pp::app::find_next_document_version_target( + "D:/Paint", + "demo.98", + [](const std::string&) { return false; }); + PP_EXPECT(harness, !target); + PP_EXPECT(harness, target.status().code == pp::foundation::StatusCode::out_of_range); +} + } int main() @@ -162,5 +218,12 @@ 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 version target starts at first version", document_version_target_starts_at_first_version); + harness.run("document version target increments existing suffix", document_version_target_increments_existing_suffix); + harness.run("document version target skips existing paths", document_version_target_skips_existing_paths); + harness.run( + "document version target preserves legacy nonnumeric suffix handling", + document_version_target_preserves_legacy_nonnumeric_suffix_handling); + harness.run("document version target reports when no slots remain", document_version_target_reports_when_no_slots_remain); return harness.finish(); } diff --git a/tools/pano_cli/main.cpp b/tools/pano_cli/main.cpp index c6649bb..f0f4c9f 100644 --- a/tools/pano_cli/main.cpp +++ b/tools/pano_cli/main.cpp @@ -103,6 +103,12 @@ struct PlanDocumentFileArgs { bool target_exists = false; }; +struct PlanDocumentVersionArgs { + std::string directory; + std::string document_name; + std::vector existing_paths; +}; + struct PlanExportTargetArgs { std::string kind; std::string work_directory; @@ -372,6 +378,7 @@ void print_help() << " inspect-project --path FILE\n" << " classify-open --path FILE\n" << " plan-document-file --work-dir DIR --name NAME [--target-exists]\n" + << " plan-document-version --directory DIR --doc-name NAME [--existing-path FILE]\n" << " plan-export-target --kind file|collection|stem|name --doc-name NAME [--work-dir DIR] [--directory DIR] [--extension EXT] [--suffix SUFFIX]\n" << " load-project --path FILE\n" << " parse-layout --path FILE\n" @@ -1272,6 +1279,75 @@ int plan_document_file(int argc, char** argv) return 0; } +pp::foundation::Status parse_plan_document_version_args( + int argc, + char** argv, + PlanDocumentVersionArgs& args) +{ + for (int i = 2; i < argc; ++i) { + const std::string_view key(argv[i]); + if (key == "--directory") { + if (i + 1 >= argc) { + return pp::foundation::Status::invalid_argument("missing value for option"); + } + args.directory = argv[++i]; + } else if (key == "--doc-name") { + if (i + 1 >= argc) { + return pp::foundation::Status::invalid_argument("missing value for option"); + } + args.document_name = argv[++i]; + } else if (key == "--existing-path") { + if (i + 1 >= argc) { + return pp::foundation::Status::invalid_argument("missing value for option"); + } + args.existing_paths.push_back(argv[++i]); + } else { + return pp::foundation::Status::invalid_argument("unknown option"); + } + } + + if (args.directory.empty()) { + return pp::foundation::Status::invalid_argument("directory must not be empty"); + } + + if (args.document_name.empty()) { + return pp::foundation::Status::invalid_argument("document name must not be empty"); + } + + return pp::foundation::Status::success(); +} + +int plan_document_version(int argc, char** argv) +{ + PlanDocumentVersionArgs args; + const auto status = parse_plan_document_version_args(argc, argv, args); + if (!status.ok()) { + print_error("plan-document-version", status.message); + return 2; + } + + const auto target = pp::app::find_next_document_version_target( + args.directory, + args.document_name, + [&args](const std::string& path) { + return std::find(args.existing_paths.begin(), args.existing_paths.end(), path) + != args.existing_paths.end(); + }); + if (!target) { + print_error("plan-document-version", target.status().message); + return 2; + } + + std::cout << "{\"ok\":true,\"command\":\"plan-document-version\"" + << ",\"input\":{\"directory\":\"" << json_escape(args.directory) + << "\",\"documentName\":\"" << json_escape(args.document_name) + << "\",\"existingPaths\":" << args.existing_paths.size() + << "},\"target\":{\"name\":\"" << json_escape(target.value().name) + << "\",\"path\":\"" << json_escape(target.value().path) + << "\"}}\n"; + return 0; +} + pp::foundation::Status parse_plan_export_target_args( int argc, char** argv, @@ -3403,6 +3479,10 @@ int main(int argc, char** argv) return plan_document_file(argc, argv); } + if (command == "plan-document-version") { + return plan_document_version(argc, argv); + } + if (command == "plan-export-target") { return plan_export_target(argc, argv); }