diff options
author | Sybren A. Stüvel <sybren@blender.org> | 2021-10-25 13:36:47 +0300 |
---|---|---|
committer | Sybren A. Stüvel <sybren@blender.org> | 2021-10-25 13:37:35 +0300 |
commit | 892e5f4a9f8a3f4e67a6ff00e4195b628decc127 (patch) | |
tree | 737e29b1991e8a0919f1cc0a4b5ae98d6130b9f2 | |
parent | 550cbec5c4dcf785d382950e2651a423f55b2f6a (diff) |
Asset Catalogs: be more accepting of backslashes as separators
Asset Catalog Paths should only contain forward slashes as separators, but
now the UI is more resilient to people using blackslashes instead.
Manifest Task: T90553
4 files changed, 59 insertions, 6 deletions
diff --git a/source/blender/blenkernel/intern/asset_catalog_path.cc b/source/blender/blenkernel/intern/asset_catalog_path.cc index fec2b76e7a1..20cac76b40b 100644 --- a/source/blender/blenkernel/intern/asset_catalog_path.cc +++ b/source/blender/blenkernel/intern/asset_catalog_path.cc @@ -197,6 +197,8 @@ void AssetCatalogPath::iterate_components(ComponentIteratorFn callback) const for (const char *path_component = this->path_.data(); path_component && path_component[0]; /* Jump to one after the next slash if there is any. */ path_component = next_slash_ptr ? next_slash_ptr + 1 : nullptr) { + /* Note that this also treats backslashes as component separators, which + * helps in cleaning up backslash-separated paths. */ next_slash_ptr = BLI_path_slash_find(path_component); const bool is_last_component = next_slash_ptr == nullptr; diff --git a/source/blender/blenkernel/intern/asset_catalog_path_test.cc b/source/blender/blenkernel/intern/asset_catalog_path_test.cc index 997f57a161c..f248863ce77 100644 --- a/source/blender/blenkernel/intern/asset_catalog_path_test.cc +++ b/source/blender/blenkernel/intern/asset_catalog_path_test.cc @@ -208,6 +208,18 @@ TEST(AssetCatalogPathTest, cleanup) AssetCatalogPath with_colons("some/key:subkey=value/path"); EXPECT_EQ(AssetCatalogPath("some/key-subkey=value/path"), with_colons.cleanup()); } + { + const AssetCatalogPath with_backslashes("windows\\for\\life"); + EXPECT_EQ(AssetCatalogPath("windows/for/life"), with_backslashes.cleanup()); + } + { + const AssetCatalogPath with_mixed("windows\\for/life"); + EXPECT_EQ(AssetCatalogPath("windows/for/life"), with_mixed.cleanup()); + } + { + const AssetCatalogPath with_punctuation("is!/this?/¿valid?"); + EXPECT_EQ(AssetCatalogPath("is!/this?/¿valid?"), with_punctuation.cleanup()); + } } TEST(AssetCatalogPathTest, iterate_components) diff --git a/source/blender/blenkernel/intern/asset_catalog_test.cc b/source/blender/blenkernel/intern/asset_catalog_test.cc index abe6d384247..2cef34966f8 100644 --- a/source/blender/blenkernel/intern/asset_catalog_test.cc +++ b/source/blender/blenkernel/intern/asset_catalog_test.cc @@ -902,8 +902,6 @@ TEST_F(AssetCatalogTest, update_catalog_path) const AssetCatalog *renamed_cat = service.find_catalog(UUID_POSES_RUZENA); ASSERT_NE(nullptr, renamed_cat); ASSERT_EQ(orig_cat, renamed_cat) << "Changing the path should not reallocate the catalog."; - EXPECT_EQ(orig_cat->simple_name, renamed_cat->simple_name) - << "Changing the path should not change the simple name."; EXPECT_EQ(orig_cat->catalog_id, renamed_cat->catalog_id) << "Changing the path should not change the catalog ID."; @@ -932,6 +930,47 @@ TEST_F(AssetCatalogTest, update_catalog_path_simple_name) << "Changing the path should update the simplename of children."; } +TEST_F(AssetCatalogTest, update_catalog_path_add_slashes) +{ + AssetCatalogService service(asset_library_root_); + service.load_from_disk(asset_library_root_ + "/" + + AssetCatalogService::DEFAULT_CATALOG_FILENAME); + + const AssetCatalog *orig_cat = service.find_catalog(UUID_POSES_RUZENA); + const AssetCatalogPath orig_path = orig_cat->path; + + /* Original path is `character/Ružena/poselib`. + * This rename will also create a new catalog for `character/Ružena/poses`. */ + service.update_catalog_path(UUID_POSES_RUZENA, "character/Ružena/poses/general"); + + EXPECT_EQ(nullptr, service.find_catalog_by_path(orig_path)) + << "The original (pre-rename) path should not be associated with a catalog any more."; + + const AssetCatalog *renamed_cat = service.find_catalog(UUID_POSES_RUZENA); + ASSERT_NE(nullptr, renamed_cat); + EXPECT_EQ(orig_cat->catalog_id, renamed_cat->catalog_id) + << "Changing the path should not change the catalog ID."; + + EXPECT_EQ("character/Ružena/poses/general", renamed_cat->path.str()) + << "When creating a new catalog by renaming + adding a slash, the renamed catalog should be " + "assigned the path passed to update_catalog_path()"; + + /* Test the newly created catalog. */ + const AssetCatalog *new_cat = service.find_catalog_by_path("character/Ružena/poses"); + ASSERT_NE(nullptr, new_cat) << "Renaming to .../X/Y should cause .../X to exist as well."; + EXPECT_EQ("character/Ružena/poses", new_cat->path.str()); + EXPECT_EQ("character-Ružena-poses", new_cat->simple_name); + EXPECT_TRUE(new_cat->flags.has_unsaved_changes); + + /* Test the children. */ + EXPECT_EQ("character/Ružena/poses/general/hand", + service.find_catalog(UUID_POSES_RUZENA_HAND)->path.str()) + << "Changing the path should update children."; + EXPECT_EQ("character/Ružena/poses/general/face", + service.find_catalog(UUID_POSES_RUZENA_FACE)->path.str()) + << "Changing the path should update children."; +} + TEST_F(AssetCatalogTest, merge_catalog_files) { const CatalogFilePath cdf_dir = create_temp_path(); diff --git a/source/blender/editors/asset/intern/asset_catalog.cc b/source/blender/editors/asset/intern/asset_catalog.cc index dae960cbb0a..f3ba12a6324 100644 --- a/source/blender/editors/asset/intern/asset_catalog.cc +++ b/source/blender/editors/asset/intern/asset_catalog.cc @@ -107,17 +107,17 @@ void ED_asset_catalog_rename(::AssetLibrary *library, AssetCatalog *catalog = catalog_service->find_catalog(catalog_id); - AssetCatalogPath new_path = catalog->path.parent(); - new_path = new_path / StringRef(new_name); + const AssetCatalogPath new_path = catalog->path.parent() / StringRef(new_name); + const AssetCatalogPath clean_new_path = new_path.cleanup(); - if (new_path == catalog->path) { + if (new_path == catalog->path || clean_new_path == catalog->path) { /* Nothing changed, so don't bother renaming for nothing. */ return; } catalog_service->undo_push(); catalog_service->tag_has_unsaved_changes(catalog); - catalog_service->update_catalog_path(catalog_id, new_path); + catalog_service->update_catalog_path(catalog_id, clean_new_path); WM_main_add_notifier(NC_SPACE | ND_SPACE_ASSET_PARAMS, nullptr); } |