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
This commit is contained in:
Sybren A. Stüvel
2021-10-25 12:36:47 +02:00
parent 550cbec5c4
commit 892e5f4a9f
4 changed files with 59 additions and 6 deletions

View File

@@ -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;

View File

@@ -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)

View File

@@ -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();

View File

@@ -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);
}