Enhance slugification process and deduplication logic

- Updated the `slugify_name` function to strip trailing spaces and dots before checking for Windows reserved names, ensuring correct handling of inputs like "CON.".
- Added tests to validate the new slugification behavior, particularly for Windows reserved names with trailing characters.
- Refactored `syncback_dir_no_meta` to pre-seed `taken_names` from old children's slugified instance names, improving deduplication against existing siblings regardless of iteration order.
This commit is contained in:
Joe 2026-02-10 04:10:00 -06:00
parent 41264a5ac0
commit 9688224cb4
2 changed files with 44 additions and 10 deletions

View file

@ -8,7 +8,10 @@ use memofs::{DirEntry, Vfs};
use crate::{
snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot, InstigatingSource},
syncback::{hash_instance, FsSnapshot, SyncbackReturn, SyncbackSnapshot},
syncback::{
hash_instance, name_needs_slugify, slugify_name, FsSnapshot, SyncbackReturn,
SyncbackSnapshot,
},
};
use super::{meta_file::DirectoryMetadata, snapshot_from_vfs};
@ -139,10 +142,9 @@ pub fn syncback_dir_no_meta<'sync>(
let mut removed_children = Vec::new();
// taken_names tracks claimed bare slugs (without extensions) for dedup.
// It is NOT seeded from disk filenames — those contain extensions that
// would cause mismatches with bare-slug comparisons. Instead, old
// children's dedup_keys are accumulated as they are processed in the
// loop below (via name_for_inst's third return value).
// Pre-seeded from old tree children's slugified instance names so that
// new-only children correctly dedup against existing siblings, regardless
// of iteration order (see plan: fix_stem-level_dedup §2).
let mut taken_names: HashSet<String> = HashSet::new();
// Detect duplicate child names (case-insensitive for file system safety).
@ -184,6 +186,21 @@ pub fn syncback_dir_no_meta<'sync>(
old_child_map.insert(inst.name(), inst);
}
// Pre-seed taken_names from old children's slugified instance names.
// This ensures new-only children that happen to slugify to the same
// bare slug as an existing sibling will be deduplicated (e.g., new
// "A/B" → slug "A_B" won't collide with existing "A_B").
// Matches the approach used in api.rs (lines 902-920).
for (_, inst) in &old_child_map {
let name = inst.name();
let slug = if name_needs_slugify(name) {
slugify_name(name).to_lowercase()
} else {
name.to_lowercase()
};
taken_names.insert(slug);
}
for new_child_ref in new_inst.children() {
let new_child = snapshot.get_new_instance(*new_child_ref).unwrap();

View file

@ -243,6 +243,13 @@ pub fn slugify_name(name: &str) -> String {
}
}
// Strip trailing spaces and dots (invalid on Windows) BEFORE the
// reserved-name check so that inputs like "CON." or "CON " are
// reduced to "CON" and then correctly caught below.
while result.ends_with(' ') || result.ends_with('.') {
result.pop();
}
// Handle Windows reserved names by appending an underscore
let result_lower = result.to_lowercase();
for forbidden in INVALID_WINDOWS_NAMES {
@ -252,11 +259,6 @@ pub fn slugify_name(name: &str) -> String {
}
}
// Strip trailing spaces and dots (invalid on Windows)
while result.ends_with(' ') || result.ends_with('.') {
result.pop();
}
// If the result is empty or all underscores, use a fallback
if result.is_empty() || result.chars().all(|c| c == '_') {
result = "instance".to_string();
@ -381,6 +383,21 @@ mod tests {
assert_eq!(slugify_name("nul"), "nul_");
}
#[test]
fn slugify_windows_reserved_with_trailing_dots_and_spaces() {
// Trailing dots/spaces must be stripped BEFORE the reserved name
// check, otherwise "CON." bypasses the check and becomes "CON".
assert_eq!(slugify_name("CON."), "CON_");
assert_eq!(slugify_name("CON "), "CON_");
assert_eq!(slugify_name("CON.."), "CON_");
assert_eq!(slugify_name("CON. "), "CON_");
assert_eq!(slugify_name("PRN."), "PRN_");
assert_eq!(slugify_name("AUX "), "AUX_");
assert_eq!(slugify_name("nul."), "nul_");
assert_eq!(slugify_name("com1."), "com1_");
assert_eq!(slugify_name("LPT1 "), "LPT1_");
}
#[test]
fn slugify_trailing_dot() {
// Trailing dots are stripped (invalid on Windows)