From 6f8e5a5cf2c064e27c9a5e8175bbef3349ed853d Mon Sep 17 00:00:00 2001 From: Joe Date: Wed, 18 Feb 2026 04:21:50 -0600 Subject: [PATCH] Refactor code for improved readability and consistency - Simplified function calls and removed unnecessary line breaks in `change_processor.rs`, `rojo_ref.rs`, `syncback/meta.rs`, `syncback/mod.rs`, `syncback/ref_properties.rs`, and `web/api.rs` to enhance code clarity. - Updated test assertions in `two_way_sync.rs` for better readability by consolidating multiple lines into single assertions. These changes improve the overall maintainability and readability of the codebase. --- src/change_processor.rs | 13 ++++++------- src/rojo_ref.rs | 34 +++++++--------------------------- src/syncback/meta.rs | 10 +++------- src/syncback/mod.rs | 3 +-- src/syncback/ref_properties.rs | 3 +-- src/web/api.rs | 27 +++++++++------------------ tests/tests/two_way_sync.rs | 30 +++++------------------------- 7 files changed, 32 insertions(+), 88 deletions(-) diff --git a/src/change_processor.rs b/src/change_processor.rs index 1e95180b..2a583f11 100644 --- a/src/change_processor.rs +++ b/src/change_processor.rs @@ -497,7 +497,10 @@ impl JobThreadContext { self.suppress_path(file_path); match crate::syncback::meta::update_ref_paths_in_file( - file_path, old_path, new_path, &source_abs, + file_path, + old_path, + new_path, + &source_abs, ) { Ok(true) => { updated_count += 1; @@ -1200,9 +1203,7 @@ impl JobThreadContext { crate::ref_target_path_from_tree(&tree, parent_ref); let old_prefix = format!("{}/{}", parent_path, old_ref_segment); let new_prefix = format!("{}/{}", parent_path, new_ref_segment); - self.update_ref_paths_after_rename( - &old_prefix, &new_prefix, &tree, - ); + self.update_ref_paths_after_rename(&old_prefix, &new_prefix, &tree); } // Fix 1: Update the renamed survivor's in-memory @@ -1812,9 +1813,7 @@ impl JobThreadContext { segment.clone() }; if *old_ref_path != new_ref_path { - self.update_ref_paths_after_rename( - old_ref_path, &new_ref_path, &tree, - ); + self.update_ref_paths_after_rename(old_ref_path, &new_ref_path, &tree); } } else if update.changed_name.is_some() || update.changed_class_name.is_some() { // Rename or class change was requested but no filesystem diff --git a/src/rojo_ref.rs b/src/rojo_ref.rs index 8802d03f..a529f505 100644 --- a/src/rojo_ref.rs +++ b/src/rojo_ref.rs @@ -271,9 +271,7 @@ pub fn resolve_ref_path_to_absolute(path: &str, source_abs: &str) -> Option = source_abs.split('/').collect(); p.pop(); - if p.pop().is_none() { - return None; - } + p.pop()?; (p, rest) } else { return Some(path.to_string()); @@ -281,9 +279,7 @@ pub fn resolve_ref_path_to_absolute(path: &str, source_abs: &str) -> Option = - links.placeholder_to_source_and_target.keys().collect(); + let placeholders: Vec<&String> = links.placeholder_to_source_and_target.keys().collect(); assert!( placeholders.len() >= 2, diff --git a/src/web/api.rs b/src/web/api.rs index db0bf0a7..bef4d967 100644 --- a/src/web/api.rs +++ b/src/web/api.rs @@ -2693,10 +2693,8 @@ impl ApiService { if target_ref.is_none() { remove_attributes.push(attr_name); } else if tree.get_instance(*target_ref).is_some() { - let source_abs = - crate::ref_target_path_from_tree(tree, update.id); - let target_abs = - crate::ref_target_path_from_tree(tree, *target_ref); + let source_abs = crate::ref_target_path_from_tree(tree, update.id); + let target_abs = crate::ref_target_path_from_tree(tree, *target_ref); if !Self::is_ref_path_unique(tree, *target_ref) { log::warn!( @@ -2709,15 +2707,11 @@ impl ApiService { ); } - let relative = - crate::compute_relative_ref_path(&source_abs, &target_abs); - ref_attributes - .insert(attr_name, serde_json::Value::String(relative)); + let relative = crate::compute_relative_ref_path(&source_abs, &target_abs); + ref_attributes.insert(attr_name, serde_json::Value::String(relative)); } else if let Some(target_abs) = added_paths.get(target_ref) { - let source_abs = - crate::ref_target_path_from_tree(tree, update.id); - let relative = - crate::compute_relative_ref_path(&source_abs, target_abs); + let source_abs = crate::ref_target_path_from_tree(tree, update.id); + let relative = crate::compute_relative_ref_path(&source_abs, target_abs); log::info!( "Ref property '{}' for instance {:?}: target {:?} is a \ just-added instance, computed relative path '{}'", @@ -2726,8 +2720,7 @@ impl ApiService { target_ref, relative ); - ref_attributes - .insert(attr_name, serde_json::Value::String(relative)); + ref_attributes.insert(attr_name, serde_json::Value::String(relative)); } else { log::warn!( "Cannot persist Ref property '{}' for instance {:?}: \ @@ -2895,10 +2888,8 @@ impl ApiService { if key.starts_with(crate::REF_PATH_ATTRIBUTE_PREFIX) { if let Some(path_str) = value.as_str() { let resolved = - crate::resolve_ref_path_to_absolute( - path_str, &source_abs, - ) - .unwrap_or_else(|| path_str.to_string()); + crate::resolve_ref_path_to_absolute(path_str, &source_abs) + .unwrap_or_else(|| path_str.to_string()); index.add(&resolved, &written_meta_path); } } diff --git a/tests/tests/two_way_sync.rs b/tests/tests/two_way_sync.rs index ac6114aa..a44e822d 100644 --- a/tests/tests/two_way_sync.rs +++ b/tests/tests/two_way_sync.rs @@ -6423,11 +6423,7 @@ fn ref_with_name_change() { poll_file_exists(&new_meta, "Meta file should exist at new location"); // The ref path should be updated to reflect the renamed parent - assert_meta_has_ref_attr( - &new_meta, - "Rojo_Ref_PrimaryPart", - "@self/Part1.model.json5", - ); + assert_meta_has_ref_attr(&new_meta, "Rojo_Ref_PrimaryPart", "@self/Part1.model.json5"); }); } @@ -6859,11 +6855,7 @@ fn ref_ambiguous_path_no_crash() { let meta_path = session .path() .join("src/Workspace/DupParent/init.meta.json5"); - poll_meta_has_ref_attr( - &meta_path, - "Rojo_Ref_PrimaryPart", - "./Target.model.json5", - ); + poll_meta_has_ref_attr(&meta_path, "Rojo_Ref_PrimaryPart", "./Target.model.json5"); }); } @@ -7024,11 +7016,7 @@ fn ref_partial_removal_preserves_remaining_index_entry() { "@self/Part1.model.json5", ); // Verify second ref is there too - assert_meta_has_ref_attr( - &meta_path, - "Rojo_Ref_Adornee", - "@self/Part2.model.json5", - ); + assert_meta_has_ref_attr(&meta_path, "Rojo_Ref_Adornee", "@self/Part2.model.json5"); // Step 2: Remove PrimaryPart (set to nil), keep Adornee let mut props2 = UstrMap::default(); @@ -7048,11 +7036,7 @@ fn ref_partial_removal_preserves_remaining_index_entry() { thread::sleep(Duration::from_millis(300)); assert_meta_no_ref_attr(&meta_path, "Rojo_Ref_PrimaryPart"); // Adornee should still be there - assert_meta_has_ref_attr( - &meta_path, - "Rojo_Ref_Adornee", - "@self/Part2.model.json5", - ); + assert_meta_has_ref_attr(&meta_path, "Rojo_Ref_Adornee", "@self/Part2.model.json5"); // Step 3: Rename Part2. The RefPathIndex should still have the entry // for Adornee (not overbroad-removed when PrimaryPart was deleted). @@ -7350,11 +7334,7 @@ fn ref_same_batch_add_module_target_includes_extension() { let meta_path = session .path() .join("src/Workspace/TestModel/init.meta.json5"); - poll_meta_has_ref_attr( - &meta_path, - "Rojo_Ref_PrimaryPart", - "./ModuleTarget.luau", - ); + poll_meta_has_ref_attr(&meta_path, "Rojo_Ref_PrimaryPart", "./ModuleTarget.luau"); }); }