Implement new oneshot Vfs for CLI commands and enhance JSON5 serialization

- Added `new_oneshot` method to the Vfs struct, providing a backend suitable for one-time operations without file watching.
- Updated CLI commands (`fmt_project` and `syncback`) to utilize the new oneshot Vfs, improving error handling during file operations.
- Enhanced JSON5 serialization by introducing a custom serializer that directly builds a tree structure, improving performance and maintaining sorted keys.
- Improved error logging in JSON serialization to provide more context on failures.
This commit is contained in:
Joe 2026-01-31 13:06:37 -06:00
parent fca802a4e5
commit 5f91a0777a
7 changed files with 3753 additions and 44 deletions

View file

@ -259,6 +259,28 @@ impl Vfs {
Self::new(StdBackend::new()) Self::new(StdBackend::new())
} }
/// Creates a new `Vfs` suitable for one-shot operations like syncback.
///
/// Unlike `new_default()`, this creates a backend that:
/// - Has file watching disabled by default
/// - Uses a non-fatal error handler for watcher issues (logs instead of exiting)
///
/// This is ideal for CLI commands that don't need real-time file watching
/// and shouldn't be terminated if the watcher thread encounters issues.
pub fn new_oneshot() -> Self {
let backend = StdBackend::new_with_error_handler(Box::new(|err| {
// Log the error but don't exit - one-shot operations don't need file watching
log::debug!(
"File watcher issue (non-fatal for one-shot operation): {}",
err
);
true // Stop the watcher thread, but don't exit the process
}));
let vfs = Self::new(backend);
vfs.set_watch_enabled(false);
vfs
}
/// Creates a new `Vfs` with the given backend. /// Creates a new `Vfs` with the given backend.
pub fn new<B: VfsBackend>(backend: B) -> Self { pub fn new<B: VfsBackend>(backend: B) -> Self {
let lock = VfsInner { let lock = VfsInner {

View file

@ -18,8 +18,8 @@ pub struct FmtProjectCommand {
impl FmtProjectCommand { impl FmtProjectCommand {
pub fn run(self) -> anyhow::Result<()> { pub fn run(self) -> anyhow::Result<()> {
let vfs = Vfs::new_default(); // Use oneshot Vfs - file watching isn't needed for formatting
vfs.set_watch_enabled(false); let vfs = Vfs::new_oneshot();
let base_path = resolve_path(&self.project); let base_path = resolve_path(&self.project);
let project = Project::load_fuzzy(&vfs, &base_path)? let project = Project::load_fuzzy(&vfs, &base_path)?

View file

@ -69,8 +69,9 @@ impl SyncbackCommand {
dom_start_timer.elapsed().as_secs_f32() dom_start_timer.elapsed().as_secs_f32()
); );
let vfs = Vfs::new_default(); // Use oneshot Vfs for syncback - file watching isn't needed and
vfs.set_watch_enabled(false); // watcher errors shouldn't terminate the process
let vfs = Vfs::new_oneshot();
let project_start_timer = Instant::now(); let project_start_timer = Instant::now();
let session_old = ServeSession::new(vfs, path_old.clone())?; let session_old = ServeSession::new(vfs, path_old.clone())?;

File diff suppressed because it is too large Load diff

View file

@ -83,12 +83,32 @@ pub fn syncback_json_model<'sync>(
model.schema = old_inst.metadata().schema.clone(); model.schema = old_inst.metadata().schema.clone();
} }
let serialized = match crate::json::to_vec_pretty_sorted(&model) {
Ok(bytes) => bytes,
Err(e) => {
// Provide more detail about what's in the model
let prop_details: Vec<_> = model
.properties
.iter()
.map(|(k, v)| format!("{}={:?}", k.as_str(), v))
.collect();
let attr_names: Vec<_> = model.attributes.keys().map(|k| k.as_str()).collect();
log::error!(
"Failed to serialize JSON model for class '{}': properties=[{}], attributes={:?}, error: {}",
model.class_name,
prop_details.join(", "),
attr_names,
e
);
return Err(e.context(format!(
"failed to serialize new JSON Model for class '{}'",
model.class_name
)));
}
};
Ok(SyncbackReturn { Ok(SyncbackReturn {
fs_snapshot: FsSnapshot::new().with_added_file( fs_snapshot: FsSnapshot::new().with_added_file(&snapshot.path, serialized),
&snapshot.path,
crate::json::to_vec_pretty_sorted(&model)
.context("failed to serialize new JSON Model")?,
),
children: Vec::new(), children: Vec::new(),
removed_children: Vec::new(), removed_children: Vec::new(),
}) })

View file

@ -1823,14 +1823,14 @@ mod tests {
#[test] #[test]
fn test_float32() { fn test_float32() {
let variant = Variant::Float32(3.14); let variant = Variant::Float32(3.125); // Use a non-constant value
let result = variant_to_json(&variant); let result = variant_to_json(&variant);
assert!(result.is_some()); assert!(result.is_some());
} }
#[test] #[test]
fn test_float64() { fn test_float64() {
let variant = Variant::Float64(2.71828); let variant = Variant::Float64(2.5); // Use a non-constant value
let result = variant_to_json(&variant); let result = variant_to_json(&variant);
assert!(result.is_some()); assert!(result.is_some());
} }
@ -1959,7 +1959,7 @@ mod tests {
// Configuration creates a directory instead // Configuration creates a directory instead
let json5_types = vec!["Part", "Frame", "TextLabel", "Model"]; let json5_types = vec!["Part", "Frame", "TextLabel", "Model"];
for class_name in json5_types { for class_name in json5_types {
let file_name = format!("Test.model.json5"); let file_name = "Test.model.json5";
assert!( assert!(
file_name.ends_with(".model.json5"), file_name.ends_with(".model.json5"),
"{} should create .model.json5 file", "{} should create .model.json5 file",
@ -2062,10 +2062,8 @@ mod tests {
.map(|e| &e.items); .map(|e| &e.items);
if let Some(enums) = run_context_enums { if let Some(enums) = run_context_enums {
if let Some(&client_value) = enums.get("Client") { // Verify the enum value exists - just getting it is the test
// Verify the enum value exists assert!(enums.get("Client").is_some());
assert!(client_value > 0 || client_value == 0);
}
} }
} }
@ -2078,9 +2076,7 @@ mod tests {
.map(|e| &e.items); .map(|e| &e.items);
if let Some(enums) = run_context_enums { if let Some(enums) = run_context_enums {
if let Some(&server_value) = enums.get("Server") { assert!(enums.get("Server").is_some());
assert!(server_value > 0 || server_value == 0);
}
} }
} }
@ -2093,9 +2089,7 @@ mod tests {
.map(|e| &e.items); .map(|e| &e.items);
if let Some(enums) = run_context_enums { if let Some(enums) = run_context_enums {
if let Some(&legacy_value) = enums.get("Legacy") { assert!(enums.get("Legacy").is_some());
assert!(legacy_value > 0 || legacy_value == 0);
}
} }
} }
@ -2108,9 +2102,7 @@ mod tests {
.map(|e| &e.items); .map(|e| &e.items);
if let Some(enums) = run_context_enums { if let Some(enums) = run_context_enums {
if let Some(&plugin_value) = enums.get("Plugin") { assert!(enums.get("Plugin").is_some());
assert!(plugin_value > 0 || plugin_value == 0);
}
} }
} }
@ -3675,7 +3667,7 @@ mod tests {
// When syncing back children, duplicates should be skipped // When syncing back children, duplicates should be skipped
// This test verifies the filter_duplicate_children logic // This test verifies the filter_duplicate_children logic
let children_names = vec!["ChildA", "ChildB", "ChildA", "ChildC"]; let children_names = ["ChildA", "ChildB", "ChildA", "ChildC"];
let unique: Vec<_> = children_names let unique: Vec<_> = children_names
.iter() .iter()
.filter(|&&name| children_names.iter().filter(|&&n| n == name).count() == 1) .filter(|&&name| children_names.iter().filter(|&&n| n == name).count() == 1)
@ -3697,8 +3689,8 @@ mod tests {
// └─ Child2/ // └─ Child2/
// └─ GrandChild (unique within its parent) // └─ GrandChild (unique within its parent)
let child1_grandchildren = vec!["GrandChild", "GrandChild"]; let child1_grandchildren = ["GrandChild", "GrandChild"];
let child2_grandchildren = vec!["GrandChild"]; let child2_grandchildren = ["GrandChild"];
// Child1's grandchildren have duplicates // Child1's grandchildren have duplicates
let child1_filtered: Vec<_> = child1_grandchildren let child1_filtered: Vec<_> = child1_grandchildren
@ -3741,7 +3733,7 @@ mod tests {
// This simulates the is_tree_path_unique check // This simulates the is_tree_path_unique check
// Scenario: Trying to add a script under a folder that has duplicate siblings // Scenario: Trying to add a script under a folder that has duplicate siblings
let parent_path_levels = vec![ let parent_path_levels = [
("Root", vec!["FolderA", "FolderA"]), // Duplicate at this level! ("Root", vec!["FolderA", "FolderA"]), // Duplicate at this level!
]; ];
@ -3758,7 +3750,7 @@ mod tests {
#[test] #[test]
fn test_valid_parent_path() { fn test_valid_parent_path() {
// Valid parent path with no duplicates at any level // Valid parent path with no duplicates at any level
let parent_path_levels = vec![ let parent_path_levels = [
( (
"ReplicatedStorage", "ReplicatedStorage",
vec!["Workspace", "ReplicatedStorage", "ServerStorage"], vec!["Workspace", "ReplicatedStorage", "ServerStorage"],

View file

@ -58,7 +58,7 @@ impl<'a> Internable<&'a HashMap<Ref, Instance<'_>>> for Instance<'a> {
impl Internable<()> for SocketPacket<'_> { impl Internable<()> for SocketPacket<'_> {
fn intern(&self, redactions: &mut RedactionMap, extra: ()) { fn intern(&self, redactions: &mut RedactionMap, extra: ()) {
redactions.intern(&self.session_id); redactions.intern(self.session_id);
match &self.body { match &self.body {
SocketPacketBody::Messages(packet) => packet.intern(redactions, extra), SocketPacketBody::Messages(packet) => packet.intern(redactions, extra),
} }