From 0b243b4ca9d42eee38be68cb79c7c4941a7957e6 Mon Sep 17 00:00:00 2001 From: Joe Date: Fri, 30 Jan 2026 10:42:38 -0600 Subject: [PATCH] Add CSV input files and enhance syncback warning logic - Introduced a new CSV input file with translation data for Spanish. - Updated the `init.csv` file to include a new entry for "Rojo" with its context and example. - Enhanced the syncback logic to conditionally emit warnings for duplicate child names based on the new `warn_duplicate_names` configuration in `SyncbackRules`. - Added documentation for the new warning behavior in the `SyncbackRules` and `SyncbackSnapshot` implementations. --- .../csv/input-project/src/csv.csv | 2 ++ .../csv/input-project/src/csv_init/init.csv | 2 +- src/snapshot_middleware/dir.rs | 2 +- src/syncback/mod.rs | 33 ++++++++++++++++++- src/syncback/snapshot.rs | 12 +++++++ 5 files changed, 48 insertions(+), 3 deletions(-) create mode 100644 rojo-test/syncback-tests/csv/input-project/src/csv.csv diff --git a/rojo-test/syncback-tests/csv/input-project/src/csv.csv b/rojo-test/syncback-tests/csv/input-project/src/csv.csv new file mode 100644 index 00000000..b7c42222 --- /dev/null +++ b/rojo-test/syncback-tests/csv/input-project/src/csv.csv @@ -0,0 +1,2 @@ +Key,Source,Context,Example,es +Ack,Ack!,,An exclamation of despair,¡Ay! diff --git a/rojo-test/syncback-tests/csv/input-project/src/csv_init/init.csv b/rojo-test/syncback-tests/csv/input-project/src/csv_init/init.csv index 24244d6c..61a49274 100644 --- a/rojo-test/syncback-tests/csv/input-project/src/csv_init/init.csv +++ b/rojo-test/syncback-tests/csv/input-project/src/csv_init/init.csv @@ -1,2 +1,2 @@ Key,Source,Context,Example,en -,,,, +Rojo,Rojo,,Rojo is a really cool program,Red diff --git a/src/snapshot_middleware/dir.rs b/src/snapshot_middleware/dir.rs index ab541014..3527fdad 100644 --- a/src/snapshot_middleware/dir.rs +++ b/src/snapshot_middleware/dir.rs @@ -153,7 +153,7 @@ pub fn syncback_dir_no_meta<'sync>( .map(|(name, _)| name) .collect(); - if !duplicate_names.is_empty() { + if !duplicate_names.is_empty() && snapshot.warn_duplicate_names() { let inst_path = crate::syncback::inst_path(snapshot.new_tree(), snapshot.new); let duplicate_list: Vec<&str> = duplicate_names.iter().map(|s| s.as_str()).collect(); log::warn!( diff --git a/src/syncback/mod.rs b/src/syncback/mod.rs index eced6b16..2f5e6c9a 100644 --- a/src/syncback/mod.rs +++ b/src/syncback/mod.rs @@ -349,7 +349,10 @@ pub fn get_best_middleware(snapshot: &SyncbackSnapshot) -> Middleware { if let Some(override_middleware) = snapshot.middleware { return override_middleware; } else if let Some(old_middleware) = old_middleware { - return old_middleware; + // Use old middleware, but upgrade to *Dir variant if new instance has children + // This handles cases where the old file was a single file (e.g., Csv) + // but the new instance has children (needs CsvDir) + middleware = old_middleware; } else { // Specific classes that need special middleware, everything else defaults to JsonModel middleware = match inst.class.as_str() { @@ -445,6 +448,12 @@ pub struct SyncbackRules { /// Defaults to `true`. #[serde(skip_serializing_if = "Option::is_none")] ignore_hidden_services: Option, + /// Whether to emit warnings when duplicate child names are encountered during + /// syncback. Duplicate names (case-insensitive) cannot be reliably synced to + /// the file system, so those children are skipped. + /// Defaults to `false` (warnings are suppressed). + #[serde(skip_serializing_if = "Option::is_none")] + warn_duplicate_names: Option, } impl SyncbackRules { @@ -498,6 +507,13 @@ impl SyncbackRules { pub fn ignore_hidden_services(&self) -> bool { self.ignore_hidden_services.unwrap_or(true) } + + /// Returns whether to emit warnings when duplicate child names are + /// encountered during syncback. Defaults to `false` (suppressed). + #[inline] + pub fn warn_duplicate_names(&self) -> bool { + self.warn_duplicate_names.unwrap_or(false) + } } fn is_valid_path(globs: &Option>, base_path: &Path, path: &Path) -> bool { @@ -572,7 +588,22 @@ fn descendants(dom: &WeakDom, root_ref: Ref) -> Vec { /// Removes root children (services) that are not in the `VISIBLE_SERVICES` list. /// This is used when `ignoreHiddenServices` is enabled to filter out internal /// services like Chat, HttpService, etc. +/// +/// This function only applies when the root is a DataModel (i.e., for place files). +/// For model files, the root children are regular instances, not services, so +/// filtering would incorrectly remove user content. fn strip_hidden_services(dom: &mut WeakDom) { + // Only apply service filtering when the root is a DataModel (place file) + // For model files (rbxm), the root is typically not a DataModel and + // children are regular instances, not services + if dom.root().class != "DataModel" { + log::trace!( + "Skipping hidden services filter: root class is '{}', not 'DataModel'", + dom.root().class + ); + return; + } + let root_children = dom.root().children().to_vec(); for child_ref in root_children { diff --git a/src/syncback/snapshot.rs b/src/syncback/snapshot.rs index 60256dc2..95f3f547 100644 --- a/src/syncback/snapshot.rs +++ b/src/syncback/snapshot.rs @@ -302,6 +302,18 @@ impl<'sync> SyncbackSnapshot<'sync> { .map(|rules| rules.encode_windows_invalid_chars()) .unwrap_or(false) } + + /// Returns whether to emit warnings when duplicate child names are + /// encountered during syncback. Defaults to `false` (suppressed). + #[inline] + pub fn warn_duplicate_names(&self) -> bool { + self.data + .project + .syncback_rules + .as_ref() + .map(|rules| rules.warn_duplicate_names()) + .unwrap_or(false) + } } pub fn filter_out_property(inst: &Instance, prop_name: &str) -> bool {