From 658e538f1565b7e4b3396f8d07888c42c5a31780 Mon Sep 17 00:00:00 2001 From: Micah Date: Thu, 26 Oct 2023 09:50:58 -0700 Subject: [PATCH 01/42] Implement hashing for Glob --- src/glob.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/glob.rs b/src/glob.rs index 7b17286af..cc8563fd7 100644 --- a/src/glob.rs +++ b/src/glob.rs @@ -1,7 +1,7 @@ //! Wrapper around globset's Glob type that has better serialization //! characteristics by coupling Glob and GlobMatcher into a single type. -use std::path::Path; +use std::{hash::Hash, path::Path}; use globset::{Glob as InnerGlob, GlobMatcher}; use serde::{de::Error as _, Deserialize, Deserializer, Serialize, Serializer}; @@ -35,6 +35,12 @@ impl PartialEq for Glob { impl Eq for Glob {} +impl Hash for Glob { + fn hash(&self, state: &mut H) { + self.inner.hash(state) + } +} + impl Serialize for Glob { fn serialize(&self, serializer: S) -> Result { serializer.serialize_str(self.inner.glob()) From ee53ffbd5963a7577b043365b40196eb4bf8b732 Mon Sep 17 00:00:00 2001 From: Micah Date: Thu, 26 Oct 2023 13:28:25 -0700 Subject: [PATCH 02/42] Pass overloads to lua middleware --- src/snapshot_middleware/lua.rs | 50 +++++++++++++++++-- ...ddleware__lua__test__double_name_trim.snap | 24 +++++++++ src/snapshot_middleware/util.rs | 8 +++ 3 files changed, 77 insertions(+), 5 deletions(-) create mode 100644 src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__double_name_trim.snap diff --git a/src/snapshot_middleware/lua.rs b/src/snapshot_middleware/lua.rs index 1307b0666..de6f369b4 100644 --- a/src/snapshot_middleware/lua.rs +++ b/src/snapshot_middleware/lua.rs @@ -9,11 +9,11 @@ use crate::snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot}; use super::{ dir::{dir_meta, snapshot_dir_no_meta}, meta_file::AdjacentMetadata, - util::match_trailing, + util::{match_trailing, PathExt as _}, }; #[derive(Debug)] -enum ScriptType { +pub enum ScriptType { Server, Client, Module, @@ -24,6 +24,7 @@ pub fn snapshot_lua( context: &InstanceContext, vfs: &Vfs, path: &Path, + script_type_overload: Option, ) -> anyhow::Result> { let file_name = path.file_name().unwrap().to_string_lossy(); @@ -33,8 +34,9 @@ pub fn snapshot_lua( .expect("Unable to get RunContext enums!") .items; - let (script_type, instance_name) = if let Some(name) = match_trailing(&file_name, ".server.lua") - { + let (script_type, instance_name) = if let Some(script_type) = script_type_overload { + (script_type, path.file_name_trim_extension()?) + } else if let Some(name) = match_trailing(&file_name, ".server.lua") { (ScriptType::Server, name) } else if let Some(name) = match_trailing(&file_name, ".client.lua") { (ScriptType::Client, name) @@ -119,7 +121,7 @@ pub fn snapshot_lua_init( ); } - let mut init_snapshot = snapshot_lua(context, vfs, init_path)?.unwrap(); + let mut init_snapshot = snapshot_lua(context, vfs, init_path, None)?.unwrap(); init_snapshot.name = dir_snapshot.name; init_snapshot.children = dir_snapshot.children; @@ -151,6 +153,7 @@ mod test { &InstanceContext::with_emit_legacy_scripts(Some(true)), &mut vfs, Path::new("/foo.lua"), + None, ) .unwrap() .unwrap(); @@ -172,6 +175,7 @@ mod test { &InstanceContext::with_emit_legacy_scripts(Some(false)), &mut vfs, Path::new("/foo.lua"), + None, ) .unwrap() .unwrap(); @@ -193,6 +197,7 @@ mod test { &InstanceContext::with_emit_legacy_scripts(Some(true)), &mut vfs, Path::new("/foo.server.lua"), + None, ) .unwrap() .unwrap(); @@ -214,6 +219,7 @@ mod test { &InstanceContext::with_emit_legacy_scripts(Some(false)), &mut vfs, Path::new("/foo.server.lua"), + None, ) .unwrap() .unwrap(); @@ -235,6 +241,7 @@ mod test { &InstanceContext::with_emit_legacy_scripts(Some(true)), &mut vfs, Path::new("/foo.client.lua"), + None, ) .unwrap() .unwrap(); @@ -256,6 +263,7 @@ mod test { &InstanceContext::with_emit_legacy_scripts(Some(false)), &mut vfs, Path::new("/foo.client.lua"), + None, ) .unwrap() .unwrap(); @@ -283,6 +291,7 @@ mod test { &InstanceContext::with_emit_legacy_scripts(Some(true)), &mut vfs, Path::new("/root"), + None, ) .unwrap() .unwrap(); @@ -315,6 +324,7 @@ mod test { &InstanceContext::with_emit_legacy_scripts(Some(true)), &mut vfs, Path::new("/foo.lua"), + None, ) .unwrap() .unwrap(); @@ -347,6 +357,7 @@ mod test { &InstanceContext::with_emit_legacy_scripts(Some(false)), &mut vfs, Path::new("/foo.lua"), + None, ) .unwrap() .unwrap(); @@ -379,6 +390,7 @@ mod test { &InstanceContext::with_emit_legacy_scripts(Some(true)), &mut vfs, Path::new("/foo.server.lua"), + None, ) .unwrap() .unwrap(); @@ -411,6 +423,7 @@ mod test { &InstanceContext::with_emit_legacy_scripts(Some(false)), &mut vfs, Path::new("/foo.server.lua"), + None, ) .unwrap() .unwrap(); @@ -445,6 +458,7 @@ mod test { &InstanceContext::with_emit_legacy_scripts(Some(true)), &mut vfs, Path::new("/bar.server.lua"), + None, ) .unwrap() .unwrap(); @@ -479,6 +493,32 @@ mod test { &InstanceContext::with_emit_legacy_scripts(Some(false)), &mut vfs, Path::new("/bar.server.lua"), + None, + ) + .unwrap() + .unwrap(); + + insta::with_settings!({ sort_maps => true }, { + insta::assert_yaml_snapshot!(instance_snapshot); + }); + } + + #[test] + fn double_name_trim() { + // Due to the existence of transformers, the way file extensions + // get trimmed is different now. A regression of this nature is + // possible, so we test against double-trimming. + let mut imfs = InMemoryFs::new(); + imfs.load_snapshot("/.server.server.lua", VfsSnapshot::file("Hello there!")) + .unwrap(); + + let mut vfs = Vfs::new(imfs); + + let instance_snapshot = snapshot_lua( + &InstanceContext::with_emit_legacy_scripts(Some(true)), + &mut vfs, + Path::new("/.server.server.lua"), + None, ) .unwrap() .unwrap(); diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__double_name_trim.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__double_name_trim.snap new file mode 100644 index 000000000..2784d8275 --- /dev/null +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__double_name_trim.snap @@ -0,0 +1,24 @@ +--- +source: src/snapshot_middleware/lua.rs +assertion_line: 524 +expression: instance_snapshot +--- +snapshot_id: "00000000000000000000000000000000" +metadata: + ignore_unknown_instances: false + instigating_source: + Path: /.server.server.lua + relevant_paths: + - /.server.server.lua + - /.server.meta.json + context: + emit_legacy_scripts: true +name: ".server" +class_name: Script +properties: + RunContext: + Enum: 0 + Source: + String: Hello there! +children: [] + diff --git a/src/snapshot_middleware/util.rs b/src/snapshot_middleware/util.rs index 625910b77..a072ec983 100644 --- a/src/snapshot_middleware/util.rs +++ b/src/snapshot_middleware/util.rs @@ -16,6 +16,7 @@ pub fn match_trailing<'a>(input: &'a str, suffix: &str) -> Option<&'a str> { pub trait PathExt { fn file_name_ends_with(&self, suffix: &str) -> bool; fn file_name_trim_end<'a>(&'a self, suffix: &str) -> anyhow::Result<&'a str>; + fn file_name_trim_extension(&self) -> anyhow::Result<&str>; } impl

PathExt for P @@ -40,6 +41,13 @@ where match_trailing(file_name, suffix) .with_context(|| format!("Path did not end in {}: {}", suffix, path.display())) } + + fn file_name_trim_extension(&self) -> anyhow::Result<&str> { + self.as_ref() + .file_stem() + .and_then(|n| n.to_str()) + .with_context(|| format!("file name of {} is invalid", self.as_ref().display())) + } } // TEMP function until rojo 8.0, when it can be replaced with bool::default (aka false) From 77bcfdf3fa87ba70bdf52ffea2a792ce3927e6e2 Mon Sep 17 00:00:00 2001 From: Micah Date: Thu, 26 Oct 2023 13:28:42 -0700 Subject: [PATCH 03/42] Basics of type transformers --- src/project.rs | 10 +- src/snapshot/metadata.rs | 27 +++- src/snapshot_middleware/mod.rs | 238 ++++++++++++++++++++--------- src/snapshot_middleware/project.rs | 1 + 4 files changed, 200 insertions(+), 76 deletions(-) diff --git a/src/project.rs b/src/project.rs index 1a645b76a..2682fb0b4 100644 --- a/src/project.rs +++ b/src/project.rs @@ -9,7 +9,9 @@ use serde::{Deserialize, Serialize}; use thiserror::Error; use crate::{ - glob::Glob, resolution::UnresolvedValue, snapshot_middleware::emit_legacy_scripts_default, + glob::Glob, + resolution::UnresolvedValue, + snapshot_middleware::{emit_legacy_scripts_default, SyncRule}, }; static PROJECT_FILENAME: &str = "default.project.json"; @@ -88,6 +90,12 @@ pub struct Project { #[serde(default, skip_serializing_if = "Vec::is_empty")] pub glob_ignore_paths: Vec, + /// A list of mappings of globs to syncing rules. If a file matches a glob, + /// it will be 'transformed' into an Instance following the rule provided. + /// Globs are relative to the folder the project file is in. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub sync_rules: Vec, + /// The path to the file that this project came from. Relative paths in the /// project should be considered relative to the parent of this field, also /// given by `Project::folder_location`. diff --git a/src/snapshot/metadata.rs b/src/snapshot/metadata.rs index 1a3273ed7..1f75260cd 100644 --- a/src/snapshot/metadata.rs +++ b/src/snapshot/metadata.rs @@ -7,8 +7,10 @@ use std::{ use serde::{Deserialize, Serialize}; use crate::{ - glob::Glob, path_serializer, project::ProjectNode, - snapshot_middleware::emit_legacy_scripts_default, + glob::Glob, + path_serializer, + project::ProjectNode, + snapshot_middleware::{emit_legacy_scripts_default, Middleware, SyncRule}, }; /// Rojo-specific metadata that can be associated with an instance or a snapshot @@ -107,6 +109,8 @@ pub struct InstanceContext { #[serde(skip_serializing_if = "Vec::is_empty")] pub path_ignore_rules: Arc>, pub emit_legacy_scripts: bool, + #[serde(skip_serializing_if = "Vec::is_empty")] + pub sync_rules: Vec, } impl InstanceContext { @@ -114,6 +118,7 @@ impl InstanceContext { Self { path_ignore_rules: Arc::new(Vec::new()), emit_legacy_scripts: emit_legacy_scripts_default().unwrap(), + sync_rules: Vec::new(), } } @@ -144,9 +149,27 @@ impl InstanceContext { rules.extend(new_rules); } + /// Extend the list of syncing rules in the context with the given new rules. + pub fn add_sync_rules(&mut self, new_rules: I) + where + I: IntoIterator, + { + self.sync_rules.extend(new_rules); + } + pub fn set_emit_legacy_scripts(&mut self, emit_legacy_scripts: bool) { self.emit_legacy_scripts = emit_legacy_scripts; } + + pub fn get_sync_rule(&self, path: &Path) -> Option { + for rule in &self.sync_rules { + if rule.matches(path) { + return Some(rule.middleware()); + } + } + + None + } } impl Default for InstanceContext { diff --git a/src/snapshot_middleware/mod.rs b/src/snapshot_middleware/mod.rs index d8cd39409..8db7b11b9 100644 --- a/src/snapshot_middleware/mod.rs +++ b/src/snapshot_middleware/mod.rs @@ -18,18 +18,23 @@ mod toml; mod txt; mod util; -use std::path::Path; +use std::path::{Path, PathBuf}; +use anyhow::Context; use memofs::{IoResultExt, Vfs}; +use serde::{Deserialize, Serialize}; -use crate::snapshot::{InstanceContext, InstanceSnapshot}; +use crate::{ + glob::Glob, + snapshot::{InstanceContext, InstanceSnapshot}, +}; use self::{ csv::{snapshot_csv, snapshot_csv_init}, dir::snapshot_dir, json::snapshot_json, json_model::snapshot_json_model, - lua::{snapshot_lua, snapshot_lua_init}, + lua::{snapshot_lua, snapshot_lua_init, ScriptType}, project::snapshot_project, rbxm::snapshot_rbxm, rbxmx::snapshot_rbxmx, @@ -54,89 +59,176 @@ pub fn snapshot_from_vfs( }; if meta.is_dir() { - let project_path = path.join("default.project.json"); - if vfs.metadata(&project_path).with_not_found()?.is_some() { - return snapshot_project(context, vfs, &project_path); - } + if let Some(init_path) = get_init_path(vfs, path)? { + match get_middleware(context, &init_path) { + Some(Middleware::Project) => snapshot_project(context, vfs, &init_path), - let init_path = path.join("init.luau"); - if vfs.metadata(&init_path).with_not_found()?.is_some() { - return snapshot_lua_init(context, vfs, &init_path); - } + Some(Middleware::ModuleScript) => snapshot_lua_init(context, vfs, &init_path), + Some(Middleware::ServerScript) => snapshot_lua_init(context, vfs, &init_path), + Some(Middleware::ClientScript) => snapshot_lua_init(context, vfs, &init_path), - let init_path = path.join("init.lua"); - if vfs.metadata(&init_path).with_not_found()?.is_some() { - return snapshot_lua_init(context, vfs, &init_path); - } + Some(Middleware::Csv) => snapshot_csv_init(context, vfs, &init_path), - let init_path = path.join("init.server.luau"); - if vfs.metadata(&init_path).with_not_found()?.is_some() { - return snapshot_lua_init(context, vfs, &init_path); + Some(_) | None => snapshot_dir(context, vfs, path), + } + } else { + snapshot_dir(context, vfs, path) } - - let init_path = path.join("init.server.lua"); - if vfs.metadata(&init_path).with_not_found()?.is_some() { - return snapshot_lua_init(context, vfs, &init_path); + } else { + let file_name = path + .file_name() + .and_then(|n| n.to_str()) + .with_context(|| format!("file name of {} is invalid", path.display()))?; + + match file_name { + "init.server.luau" | "init.server.lua" | "init.client.luau" | "init.client.lua" + | "init.luau" | "init.lua" | "init.csv" => return Ok(None), + _ => {} } - let init_path = path.join("init.client.luau"); - if vfs.metadata(&init_path).with_not_found()?.is_some() { - return snapshot_lua_init(context, vfs, &init_path); + if let Some(middleware) = get_middleware(context, path) { + return middleware.snapshot(context, vfs, path); } + Ok(None) + } +} - let init_path = path.join("init.client.lua"); - if vfs.metadata(&init_path).with_not_found()?.is_some() { - return snapshot_lua_init(context, vfs, &init_path); - } +pub fn get_middleware>(context: &InstanceContext, path: P) -> Option { + let path = path.as_ref(); + + if let Some(middleware) = context.get_sync_rule(path) { + Some(middleware) + } else if path.file_name_ends_with(".server.lua") || path.file_name_ends_with(".server.luau") { + Some(Middleware::ServerScript) + } else if path.file_name_ends_with(".client.lua") || path.file_name_ends_with(".client.luau") { + Some(Middleware::ClientScript) + } else if path.file_name_ends_with(".lua") || path.file_name_ends_with(".luau") { + Some(Middleware::ModuleScript) + } else if path.file_name_ends_with(".project.json") { + Some(Middleware::Project) + } else if path.file_name_ends_with(".model.json") { + Some(Middleware::JsonModel) + } else if path.file_name_ends_with(".meta.json") { + // .meta.json files do not turn into their own instances. + None + } else if path.file_name_ends_with(".json") { + Some(Middleware::Json) + } else if path.file_name_ends_with(".toml") { + Some(Middleware::Toml) + } else if path.file_name_ends_with(".csv") { + Some(Middleware::Csv) + } else if path.file_name_ends_with(".txt") { + Some(Middleware::Text) + } else if path.file_name_ends_with(".rbxmx") { + Some(Middleware::Rbxmx) + } else if path.file_name_ends_with(".rbxm") { + Some(Middleware::Rbxm) + } else { + None + } +} - let init_path = path.join("init.csv"); - if vfs.metadata(&init_path).with_not_found()?.is_some() { - return snapshot_csv_init(context, vfs, &init_path); - } +fn get_init_path>(vfs: &Vfs, dir: P) -> anyhow::Result> { + let path = dir.as_ref(); - snapshot_dir(context, vfs, path) - } else { - let script_name = path - .file_name_trim_end(".lua") - .or_else(|_| path.file_name_trim_end(".luau")); + let project_path = path.join("default.project.json"); + if vfs.metadata(&project_path).with_not_found()?.is_some() { + return Ok(Some(project_path)); + } - let csv_name = path.file_name_trim_end(".csv"); + let init_path = path.join("init.luau"); + if vfs.metadata(&init_path).with_not_found()?.is_some() { + return Ok(Some(init_path)); + } - if let Ok(name) = script_name { - match name { - // init scripts are handled elsewhere and should not turn into - // their own children. - "init" | "init.client" | "init.server" => return Ok(None), + let init_path = path.join("init.lua"); + if vfs.metadata(&init_path).with_not_found()?.is_some() { + return Ok(Some(init_path)); + } - _ => return snapshot_lua(context, vfs, path), - } - } else if path.file_name_ends_with(".project.json") { - return snapshot_project(context, vfs, path); - } else if path.file_name_ends_with(".model.json") { - return snapshot_json_model(context, vfs, path); - } else if path.file_name_ends_with(".meta.json") { - // .meta.json files do not turn into their own instances. - return Ok(None); - } else if path.file_name_ends_with(".json") { - return snapshot_json(context, vfs, path); - } else if path.file_name_ends_with(".toml") { - return snapshot_toml(context, vfs, path); - } else if let Ok(name) = csv_name { - match name { - // init csv are handled elsewhere and should not turn into - // their own children. - "init" => return Ok(None), - - _ => return snapshot_csv(context, vfs, path), - } - } else if path.file_name_ends_with(".txt") { - return snapshot_txt(context, vfs, path); - } else if path.file_name_ends_with(".rbxmx") { - return snapshot_rbxmx(context, vfs, path); - } else if path.file_name_ends_with(".rbxm") { - return snapshot_rbxm(context, vfs, path); - } + let init_path = path.join("init.server.luau"); + if vfs.metadata(&init_path).with_not_found()?.is_some() { + return Ok(Some(init_path)); + } - Ok(None) + let init_path = path.join("init.server.lua"); + if vfs.metadata(&init_path).with_not_found()?.is_some() { + return Ok(Some(init_path)); + } + + let init_path = path.join("init.client.luau"); + if vfs.metadata(&init_path).with_not_found()?.is_some() { + return Ok(Some(init_path)); + } + + let init_path = path.join("init.client.lua"); + if vfs.metadata(&init_path).with_not_found()?.is_some() { + return Ok(Some(init_path)); + } + + let init_path = path.join("init.csv"); + if vfs.metadata(&init_path).with_not_found()?.is_some() { + return Ok(Some(init_path)); + } + + Ok(None) +} + +#[derive(Debug, Deserialize, Serialize, Clone, PartialEq)] +pub struct SyncRule { + #[serde(rename = "pattern")] + glob: Glob, + #[serde(rename = "use")] + middleware: Middleware, +} + +impl SyncRule { + pub fn matches(&self, path: &Path) -> bool { + self.glob.is_match(path) + } + + pub fn middleware(&self) -> Middleware { + self.middleware + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Deserialize, Serialize)] +#[serde(rename_all = "lowercase")] +pub enum Middleware { + Csv, + JsonModel, + Json, + ServerScript, + ClientScript, + ModuleScript, + Project, + Rbxm, + Rbxmx, + Toml, + Text, + Ignore, +} + +impl Middleware { + pub fn snapshot( + &self, + context: &InstanceContext, + vfs: &Vfs, + path: &Path, + ) -> anyhow::Result> { + match self { + Self::Csv => snapshot_csv(context, vfs, path), + Self::JsonModel => snapshot_json_model(context, vfs, path), + Self::Json => snapshot_json(context, vfs, path), + Self::ServerScript => snapshot_lua(context, vfs, path, Some(ScriptType::Server)), + Self::ClientScript => snapshot_lua(context, vfs, path, Some(ScriptType::Client)), + Self::ModuleScript => snapshot_lua(context, vfs, path, Some(ScriptType::Module)), + Self::Project => snapshot_project(context, vfs, path), + Self::Rbxm => snapshot_rbxm(context, vfs, path), + Self::Rbxmx => snapshot_rbxmx(context, vfs, path), + Self::Toml => snapshot_toml(context, vfs, path), + Self::Text => snapshot_txt(context, vfs, path), + Self::Ignore => Ok(None), + } } } diff --git a/src/snapshot_middleware/project.rs b/src/snapshot_middleware/project.rs index d1b4a2e48..17f7db30c 100644 --- a/src/snapshot_middleware/project.rs +++ b/src/snapshot_middleware/project.rs @@ -29,6 +29,7 @@ pub fn snapshot_project( base_path: project.folder_location().to_path_buf(), }); + context.add_sync_rules(project.sync_rules.clone()); context.add_path_ignore_rules(rules); context.set_emit_legacy_scripts( project From cb6cd3b4a7d4ecdb5efcf12ec479bd07c73323e0 Mon Sep 17 00:00:00 2001 From: Micah Date: Thu, 26 Oct 2023 14:43:43 -0700 Subject: [PATCH 04/42] Fix script paths not being trimmed --- src/snapshot_middleware/mod.rs | 103 ++++++++++++++++++++------------- 1 file changed, 63 insertions(+), 40 deletions(-) diff --git a/src/snapshot_middleware/mod.rs b/src/snapshot_middleware/mod.rs index 8db7b11b9..38a23c846 100644 --- a/src/snapshot_middleware/mod.rs +++ b/src/snapshot_middleware/mod.rs @@ -60,7 +60,7 @@ pub fn snapshot_from_vfs( if meta.is_dir() { if let Some(init_path) = get_init_path(vfs, path)? { - match get_middleware(context, &init_path) { + match Middleware::from_path(context, &init_path) { Some(Middleware::Project) => snapshot_project(context, vfs, &init_path), Some(Middleware::ModuleScript) => snapshot_lua_init(context, vfs, &init_path), @@ -86,45 +86,7 @@ pub fn snapshot_from_vfs( _ => {} } - if let Some(middleware) = get_middleware(context, path) { - return middleware.snapshot(context, vfs, path); - } - Ok(None) - } -} - -pub fn get_middleware>(context: &InstanceContext, path: P) -> Option { - let path = path.as_ref(); - - if let Some(middleware) = context.get_sync_rule(path) { - Some(middleware) - } else if path.file_name_ends_with(".server.lua") || path.file_name_ends_with(".server.luau") { - Some(Middleware::ServerScript) - } else if path.file_name_ends_with(".client.lua") || path.file_name_ends_with(".client.luau") { - Some(Middleware::ClientScript) - } else if path.file_name_ends_with(".lua") || path.file_name_ends_with(".luau") { - Some(Middleware::ModuleScript) - } else if path.file_name_ends_with(".project.json") { - Some(Middleware::Project) - } else if path.file_name_ends_with(".model.json") { - Some(Middleware::JsonModel) - } else if path.file_name_ends_with(".meta.json") { - // .meta.json files do not turn into their own instances. - None - } else if path.file_name_ends_with(".json") { - Some(Middleware::Json) - } else if path.file_name_ends_with(".toml") { - Some(Middleware::Toml) - } else if path.file_name_ends_with(".csv") { - Some(Middleware::Csv) - } else if path.file_name_ends_with(".txt") { - Some(Middleware::Text) - } else if path.file_name_ends_with(".rbxmx") { - Some(Middleware::Rbxmx) - } else if path.file_name_ends_with(".rbxm") { - Some(Middleware::Rbxm) - } else { - None + snapshot_from_path(context, vfs, path) } } @@ -174,6 +136,28 @@ fn get_init_path>(vfs: &Vfs, dir: P) -> anyhow::Result>( + context: &InstanceContext, + vfs: &Vfs, + path: P, +) -> anyhow::Result> { + let path = path.as_ref(); + + if let Some(middleware) = context.get_sync_rule(path) { + middleware.snapshot(context, vfs, path) + } else if path.file_name_ends_with(".server.lua") || path.file_name_ends_with(".server.luau") { + snapshot_lua(context, vfs, path, None) + } else if path.file_name_ends_with(".client.lua") || path.file_name_ends_with(".client.luau") { + snapshot_lua(context, vfs, path, None) + } else if path.file_name_ends_with(".lua") || path.file_name_ends_with(".luau") { + snapshot_lua(context, vfs, path, None) + } else if let Some(middleware) = Middleware::from_path(context, path) { + middleware.snapshot(context, vfs, path) + } else { + Ok(None) + } +} + #[derive(Debug, Deserialize, Serialize, Clone, PartialEq)] pub struct SyncRule { #[serde(rename = "pattern")] @@ -210,6 +194,45 @@ pub enum Middleware { } impl Middleware { + pub fn from_path>(context: &InstanceContext, path: P) -> Option { + let path = path.as_ref(); + + if let Some(middleware) = context.get_sync_rule(path) { + Some(middleware) + } else if path.file_name_ends_with(".server.lua") + || path.file_name_ends_with(".server.luau") + { + Some(Middleware::ServerScript) + } else if path.file_name_ends_with(".client.lua") + || path.file_name_ends_with(".client.luau") + { + Some(Middleware::ClientScript) + } else if path.file_name_ends_with(".lua") || path.file_name_ends_with(".luau") { + Some(Middleware::ModuleScript) + } else if path.file_name_ends_with(".project.json") { + Some(Middleware::Project) + } else if path.file_name_ends_with(".model.json") { + Some(Middleware::JsonModel) + } else if path.file_name_ends_with(".meta.json") { + // .meta.json files do not turn into their own instances. + None + } else if path.file_name_ends_with(".json") { + Some(Middleware::Json) + } else if path.file_name_ends_with(".toml") { + Some(Middleware::Toml) + } else if path.file_name_ends_with(".csv") { + Some(Middleware::Csv) + } else if path.file_name_ends_with(".txt") { + Some(Middleware::Text) + } else if path.file_name_ends_with(".rbxmx") { + Some(Middleware::Rbxmx) + } else if path.file_name_ends_with(".rbxm") { + Some(Middleware::Rbxm) + } else { + None + } + } + pub fn snapshot( &self, context: &InstanceContext, From 9830ef7c260b55623a10314c955cd3f6b77fe6a7 Mon Sep 17 00:00:00 2001 From: Micah Date: Thu, 26 Oct 2023 14:47:06 -0700 Subject: [PATCH 05/42] Document items in middleware and make them private --- src/snapshot_middleware/mod.rs | 38 +++++++++++++++++++++++++++++----- 1 file changed, 33 insertions(+), 5 deletions(-) diff --git a/src/snapshot_middleware/mod.rs b/src/snapshot_middleware/mod.rs index 38a23c846..51f90493d 100644 --- a/src/snapshot_middleware/mod.rs +++ b/src/snapshot_middleware/mod.rs @@ -45,8 +45,10 @@ use self::{ pub use self::{project::snapshot_project_node, util::emit_legacy_scripts_default}; -/// The main entrypoint to the snapshot function. This function can be pointed -/// at any path and will return something if Rojo knows how to deal with it. +/// Returns an `InstanceSnapshot` for the provided path. +/// This will inspect the path and find the appropriate middleware for it, +/// taking user-written rules into account. Then, it will attempt to convert +/// the path into an InstanceSnapshot using that middleware. #[profiling::function] pub fn snapshot_from_vfs( context: &InstanceContext, @@ -90,6 +92,9 @@ pub fn snapshot_from_vfs( } } +/// Gets an `init` path for the given directory. +/// This uses an intrinsic priority list and for compatibility, +/// it should not be changed. fn get_init_path>(vfs: &Vfs, dir: P) -> anyhow::Result> { let path = dir.as_ref(); @@ -136,7 +141,13 @@ fn get_init_path>(vfs: &Vfs, dir: P) -> anyhow::Result>( +/// Gets a snapshot for a path given an InstanceContext and Vfs. +/// +/// This is independent of the actual middleware and this function +/// should be used when possible. The middleware enum assumes that it is being +/// used as an override, and as a result Scripts do not have their paths +/// trimmed properly if it's used directly. +fn snapshot_from_path>( context: &InstanceContext, vfs: &Vfs, path: P, @@ -158,6 +169,8 @@ pub fn snapshot_from_path>( } } +/// Represents an override or extension for Rojo's 'syncing rules'. +/// That is, for a specified pattern, Rojo will use the provided middleware. #[derive(Debug, Deserialize, Serialize, Clone, PartialEq)] pub struct SyncRule { #[serde(rename = "pattern")] @@ -167,15 +180,21 @@ pub struct SyncRule { } impl SyncRule { + /// Returns whether the given path matches this rule. pub fn matches(&self, path: &Path) -> bool { self.glob.is_match(path) } + /// Returns the middleware this rule represents. pub fn middleware(&self) -> Middleware { self.middleware } } +/// Represents a possible 'transformer' used by Rojo to turn a file system +/// item into a Roblox Instance. Missing from this list are directories and +/// metadata. This is deliberate, as metadata is not a snapshot middleware +/// and directories do not make sense to turn into files. #[derive(Debug, Clone, Copy, PartialEq, Deserialize, Serialize)] #[serde(rename_all = "lowercase")] pub enum Middleware { @@ -194,7 +213,10 @@ pub enum Middleware { } impl Middleware { - pub fn from_path>(context: &InstanceContext, path: P) -> Option { + /// Returns a `Middleware` from the provided path, taking user-specified + /// syncing rules into account. If one exists, it is returned. Otherwise, + /// `None` is returned. + fn from_path>(context: &InstanceContext, path: P) -> Option { let path = path.as_ref(); if let Some(middleware) = context.get_sync_rule(path) { @@ -233,7 +255,13 @@ impl Middleware { } } - pub fn snapshot( + /// Creates a snapshot for the given path from the Middleware. + /// + /// This function assumes that the snapshot is being used as an override + /// and thus no processing is done on the the path. This means that paths + /// with multiple extensions (i.e. scripts) are not handled well by it. + /// You should prefer `snapshot_from_path` when possible as a result. + fn snapshot( &self, context: &InstanceContext, vfs: &Vfs, From ebfee154671b52d488c35c8705c5b2769cb3053f Mon Sep 17 00:00:00 2001 From: Micah Date: Thu, 26 Oct 2023 14:52:14 -0700 Subject: [PATCH 06/42] Make snapshot middleware file extension agnostic --- src/snapshot_middleware/csv.rs | 2 +- src/snapshot_middleware/json.rs | 2 +- src/snapshot_middleware/json_model.rs | 2 +- src/snapshot_middleware/rbxm.rs | 2 +- src/snapshot_middleware/rbxmx.rs | 2 +- src/snapshot_middleware/toml.rs | 2 +- src/snapshot_middleware/txt.rs | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/snapshot_middleware/csv.rs b/src/snapshot_middleware/csv.rs index 708f1bb8c..e9634e4c0 100644 --- a/src/snapshot_middleware/csv.rs +++ b/src/snapshot_middleware/csv.rs @@ -18,7 +18,7 @@ pub fn snapshot_csv( vfs: &Vfs, path: &Path, ) -> anyhow::Result> { - let name = path.file_name_trim_end(".csv")?; + let name = path.file_name_trim_extension()?; let meta_path = path.with_file_name(format!("{}.meta.json", name)); let contents = vfs.read(path)?; diff --git a/src/snapshot_middleware/json.rs b/src/snapshot_middleware/json.rs index 8c7f369e3..cdd4cffba 100644 --- a/src/snapshot_middleware/json.rs +++ b/src/snapshot_middleware/json.rs @@ -16,7 +16,7 @@ pub fn snapshot_json( vfs: &Vfs, path: &Path, ) -> anyhow::Result> { - let name = path.file_name_trim_end(".json")?; + let name = path.file_name_trim_extension()?; let contents = vfs.read(path)?; let value: serde_json::Value = serde_json::from_slice(&contents) diff --git a/src/snapshot_middleware/json_model.rs b/src/snapshot_middleware/json_model.rs index b38c47a78..20b1f22c6 100644 --- a/src/snapshot_middleware/json_model.rs +++ b/src/snapshot_middleware/json_model.rs @@ -17,7 +17,7 @@ pub fn snapshot_json_model( vfs: &Vfs, path: &Path, ) -> anyhow::Result> { - let name = path.file_name_trim_end(".model.json")?; + let name = path.file_name_trim_extension()?; let contents = vfs.read(path)?; let contents_str = str::from_utf8(&contents) diff --git a/src/snapshot_middleware/rbxm.rs b/src/snapshot_middleware/rbxm.rs index 969f022dc..5151e5072 100644 --- a/src/snapshot_middleware/rbxm.rs +++ b/src/snapshot_middleware/rbxm.rs @@ -13,7 +13,7 @@ pub fn snapshot_rbxm( vfs: &Vfs, path: &Path, ) -> anyhow::Result> { - let name = path.file_name_trim_end(".rbxm")?; + let name = path.file_name_trim_extension()?; let temp_tree = rbx_binary::from_reader(vfs.read(path)?.as_slice()) .with_context(|| format!("Malformed rbxm file: {}", path.display()))?; diff --git a/src/snapshot_middleware/rbxmx.rs b/src/snapshot_middleware/rbxmx.rs index 4f128ff6e..a10c0e944 100644 --- a/src/snapshot_middleware/rbxmx.rs +++ b/src/snapshot_middleware/rbxmx.rs @@ -12,7 +12,7 @@ pub fn snapshot_rbxmx( vfs: &Vfs, path: &Path, ) -> anyhow::Result> { - let name = path.file_name_trim_end(".rbxmx")?; + let name = path.file_name_trim_extension()?; let options = rbx_xml::DecodeOptions::new() .property_behavior(rbx_xml::DecodePropertyBehavior::ReadUnknown); diff --git a/src/snapshot_middleware/toml.rs b/src/snapshot_middleware/toml.rs index 6ee9be47e..d92db4b3b 100644 --- a/src/snapshot_middleware/toml.rs +++ b/src/snapshot_middleware/toml.rs @@ -16,7 +16,7 @@ pub fn snapshot_toml( vfs: &Vfs, path: &Path, ) -> anyhow::Result> { - let name = path.file_name_trim_end(".toml")?; + let name = path.file_name_trim_extension()?; let contents = vfs.read(path)?; let value: toml::Value = toml::from_slice(&contents) diff --git a/src/snapshot_middleware/txt.rs b/src/snapshot_middleware/txt.rs index 13d5b9907..8f2126feb 100644 --- a/src/snapshot_middleware/txt.rs +++ b/src/snapshot_middleware/txt.rs @@ -13,7 +13,7 @@ pub fn snapshot_txt( vfs: &Vfs, path: &Path, ) -> anyhow::Result> { - let name = path.file_name_trim_end(".txt")?; + let name = path.file_name_trim_extension()?; let contents = vfs.read(path)?; let contents_str = str::from_utf8(&contents) From 486c18967f0a7c110eb112562234efede2b073b6 Mon Sep 17 00:00:00 2001 From: Micah Date: Thu, 26 Oct 2023 14:58:02 -0700 Subject: [PATCH 07/42] Test basic syncing rules --- ...to_end__tests__build__sync_rule_alone.snap | 18 +++++++++++ .../sync_rule_alone/default.project.json | 12 ++++++++ .../sync_rule_alone/src/foo.nothing | 1 + ...nd__tests__serve__sync_rule_alone_all.snap | 30 +++++++++++++++++++ ...d__tests__serve__sync_rule_alone_info.snap | 14 +++++++++ .../sync_rule_alone/default.project.json | 12 ++++++++ .../sync_rule_alone/src/foo.nothing | 1 + tests/tests/build.rs | 1 + tests/tests/serve.rs | 16 ++++++++++ 9 files changed, 105 insertions(+) create mode 100644 rojo-test/build-test-snapshots/end_to_end__tests__build__sync_rule_alone.snap create mode 100644 rojo-test/build-tests/sync_rule_alone/default.project.json create mode 100644 rojo-test/build-tests/sync_rule_alone/src/foo.nothing create mode 100644 rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_alone_all.snap create mode 100644 rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_alone_info.snap create mode 100644 rojo-test/serve-tests/sync_rule_alone/default.project.json create mode 100644 rojo-test/serve-tests/sync_rule_alone/src/foo.nothing diff --git a/rojo-test/build-test-snapshots/end_to_end__tests__build__sync_rule_alone.snap b/rojo-test/build-test-snapshots/end_to_end__tests__build__sync_rule_alone.snap new file mode 100644 index 000000000..53e329cc7 --- /dev/null +++ b/rojo-test/build-test-snapshots/end_to_end__tests__build__sync_rule_alone.snap @@ -0,0 +1,18 @@ +--- +source: tests/tests/build.rs +assertion_line: 102 +expression: contents +--- + + + + sync_rule_alone + + + + foo + Hello, world! + + + + diff --git a/rojo-test/build-tests/sync_rule_alone/default.project.json b/rojo-test/build-tests/sync_rule_alone/default.project.json new file mode 100644 index 000000000..f3a5aa342 --- /dev/null +++ b/rojo-test/build-tests/sync_rule_alone/default.project.json @@ -0,0 +1,12 @@ +{ + "name": "sync_rule_alone", + "tree": { + "$path": "src" + }, + "syncRules": [ + { + "pattern": "*.nothing", + "use": "text" + } + ] +} \ No newline at end of file diff --git a/rojo-test/build-tests/sync_rule_alone/src/foo.nothing b/rojo-test/build-tests/sync_rule_alone/src/foo.nothing new file mode 100644 index 000000000..5dd01c177 --- /dev/null +++ b/rojo-test/build-tests/sync_rule_alone/src/foo.nothing @@ -0,0 +1 @@ +Hello, world! \ No newline at end of file diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_alone_all.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_alone_all.snap new file mode 100644 index 000000000..e4fa4adfc --- /dev/null +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_alone_all.snap @@ -0,0 +1,30 @@ +--- +source: tests/tests/serve.rs +assertion_line: 268 +expression: "read_response.intern_and_redact(&mut redactions, root_id)" +--- +instances: + id-2: + Children: + - id-3 + ClassName: Folder + Id: id-2 + Metadata: + ignoreUnknownInstances: false + Name: sync_rule_alone + Parent: "00000000000000000000000000000000" + Properties: {} + id-3: + Children: [] + ClassName: StringValue + Id: id-3 + Metadata: + ignoreUnknownInstances: false + Name: foo + Parent: id-2 + Properties: + Value: + String: "Hello, world!" +messageCursor: 0 +sessionId: id-1 + diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_alone_info.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_alone_info.snap new file mode 100644 index 000000000..514b88580 --- /dev/null +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_alone_info.snap @@ -0,0 +1,14 @@ +--- +source: tests/tests/serve.rs +assertion_line: 265 +expression: redactions.redacted_yaml(info) +--- +expectedPlaceIds: ~ +gameId: ~ +placeId: ~ +projectName: sync_rule_alone +protocolVersion: 4 +rootInstanceId: id-2 +serverVersion: "[server-version]" +sessionId: id-1 + diff --git a/rojo-test/serve-tests/sync_rule_alone/default.project.json b/rojo-test/serve-tests/sync_rule_alone/default.project.json new file mode 100644 index 000000000..f3a5aa342 --- /dev/null +++ b/rojo-test/serve-tests/sync_rule_alone/default.project.json @@ -0,0 +1,12 @@ +{ + "name": "sync_rule_alone", + "tree": { + "$path": "src" + }, + "syncRules": [ + { + "pattern": "*.nothing", + "use": "text" + } + ] +} \ No newline at end of file diff --git a/rojo-test/serve-tests/sync_rule_alone/src/foo.nothing b/rojo-test/serve-tests/sync_rule_alone/src/foo.nothing new file mode 100644 index 000000000..5dd01c177 --- /dev/null +++ b/rojo-test/serve-tests/sync_rule_alone/src/foo.nothing @@ -0,0 +1 @@ +Hello, world! \ No newline at end of file diff --git a/tests/tests/build.rs b/tests/tests/build.rs index 01e87da73..e20cfdcb9 100644 --- a/tests/tests/build.rs +++ b/tests/tests/build.rs @@ -59,6 +59,7 @@ gen_build_tests! { txt_in_folder, unresolved_values, weldconstraint, + sync_rule_alone, } fn run_build_test(test_name: &str) { diff --git a/tests/tests/serve.rs b/tests/tests/serve.rs index d55d339bb..d5f6fb07e 100644 --- a/tests/tests/serve.rs +++ b/tests/tests/serve.rs @@ -255,3 +255,19 @@ fn add_optional_folder() { ); }); } + +#[test] +fn sync_rule_alone() { + run_serve_test("sync_rule_alone", |session, mut redactions| { + let info = session.get_api_rojo().unwrap(); + let root_id = info.root_instance_id; + + assert_yaml_snapshot!("sync_rule_alone_info", redactions.redacted_yaml(info)); + + let read_response = session.get_api_read(root_id).unwrap(); + assert_yaml_snapshot!( + "sync_rule_alone_all", + read_response.intern_and_redact(&mut redactions, root_id) + ); + }); +} From 9dfb020573396a5df7a66d13bea6a984ab7a4ed8 Mon Sep 17 00:00:00 2001 From: Micah Date: Fri, 27 Oct 2023 12:54:00 -0700 Subject: [PATCH 08/42] Unimplement Hash for Glob --- src/glob.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/glob.rs b/src/glob.rs index cc8563fd7..7b17286af 100644 --- a/src/glob.rs +++ b/src/glob.rs @@ -1,7 +1,7 @@ //! Wrapper around globset's Glob type that has better serialization //! characteristics by coupling Glob and GlobMatcher into a single type. -use std::{hash::Hash, path::Path}; +use std::path::Path; use globset::{Glob as InnerGlob, GlobMatcher}; use serde::{de::Error as _, Deserialize, Deserializer, Serialize, Serializer}; @@ -35,12 +35,6 @@ impl PartialEq for Glob { impl Eq for Glob {} -impl Hash for Glob { - fn hash(&self, state: &mut H) { - self.inner.hash(state) - } -} - impl Serialize for Glob { fn serialize(&self, serializer: S) -> Result { serializer.serialize_str(self.inner.glob()) From 5a0c24d2d1384c5c229dfaff34bf1a85bbfafd51 Mon Sep 17 00:00:00 2001 From: Micah Date: Fri, 27 Oct 2023 13:48:00 -0700 Subject: [PATCH 09/42] Trim extensions instead of just one --- src/snapshot_middleware/util.rs | 39 +++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/src/snapshot_middleware/util.rs b/src/snapshot_middleware/util.rs index a072ec983..bc7112088 100644 --- a/src/snapshot_middleware/util.rs +++ b/src/snapshot_middleware/util.rs @@ -16,7 +16,7 @@ pub fn match_trailing<'a>(input: &'a str, suffix: &str) -> Option<&'a str> { pub trait PathExt { fn file_name_ends_with(&self, suffix: &str) -> bool; fn file_name_trim_end<'a>(&'a self, suffix: &str) -> anyhow::Result<&'a str>; - fn file_name_trim_extension(&self) -> anyhow::Result<&str>; + fn file_name_trim_extensions(&self) -> anyhow::Result<&str>; } impl

PathExt for P @@ -42,11 +42,23 @@ where .with_context(|| format!("Path did not end in {}: {}", suffix, path.display())) } - fn file_name_trim_extension(&self) -> anyhow::Result<&str> { - self.as_ref() + /// Returns the name of a file after all extensions have been removed. + fn file_name_trim_extensions(&self) -> anyhow::Result<&str> { + // I would love for this to be less verbose, but I'm not sure it's + // really possible this. It doesn't allocate, so it's no huge loss + // either way though. + let mut file_name = self + .as_ref() .file_stem() .and_then(|n| n.to_str()) - .with_context(|| format!("file name of {} is invalid", self.as_ref().display())) + .with_context(|| format!("file name of {} is invalid", self.as_ref().display()))?; + while Path::new(file_name).extension().is_some() { + file_name = Path::new(file_name) + .file_stem() + .and_then(|n| n.to_str()) + .with_context(|| format!("file name of {} is invalid", self.as_ref().display()))?; + } + Ok(file_name) } } @@ -54,3 +66,22 @@ where pub fn emit_legacy_scripts_default() -> Option { Some(true) } + +#[test] +fn file_name_trim_extensions_invalid() { + // Basic test to make sure that the loop for this function + // isn't infinite and that it works right + assert!(Path::new("").file_name_trim_extensions().is_err()); + + assert_eq!(Path::new("foo").file_name_trim_extensions().unwrap(), "foo"); + assert_eq!( + Path::new("foo.bar").file_name_trim_extensions().unwrap(), + "foo" + ); + assert_eq!( + Path::new("foo.bar.baz") + .file_name_trim_extensions() + .unwrap(), + "foo" + ); +} From ee850c45e2e6ec22f6a58d38f9e974ffea76831f Mon Sep 17 00:00:00 2001 From: Micah Date: Fri, 27 Oct 2023 13:52:53 -0700 Subject: [PATCH 10/42] Move middleware to trim multiple extensions --- src/snapshot_middleware/csv.rs | 2 +- src/snapshot_middleware/json.rs | 2 +- src/snapshot_middleware/json_model.rs | 2 +- src/snapshot_middleware/lua.rs | 2 +- src/snapshot_middleware/rbxm.rs | 2 +- src/snapshot_middleware/rbxmx.rs | 2 +- src/snapshot_middleware/toml.rs | 2 +- src/snapshot_middleware/txt.rs | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/snapshot_middleware/csv.rs b/src/snapshot_middleware/csv.rs index e9634e4c0..dbaeab1c8 100644 --- a/src/snapshot_middleware/csv.rs +++ b/src/snapshot_middleware/csv.rs @@ -18,7 +18,7 @@ pub fn snapshot_csv( vfs: &Vfs, path: &Path, ) -> anyhow::Result> { - let name = path.file_name_trim_extension()?; + let name = path.file_name_trim_extensions()?; let meta_path = path.with_file_name(format!("{}.meta.json", name)); let contents = vfs.read(path)?; diff --git a/src/snapshot_middleware/json.rs b/src/snapshot_middleware/json.rs index cdd4cffba..a674480d9 100644 --- a/src/snapshot_middleware/json.rs +++ b/src/snapshot_middleware/json.rs @@ -16,7 +16,7 @@ pub fn snapshot_json( vfs: &Vfs, path: &Path, ) -> anyhow::Result> { - let name = path.file_name_trim_extension()?; + let name = path.file_name_trim_extensions()?; let contents = vfs.read(path)?; let value: serde_json::Value = serde_json::from_slice(&contents) diff --git a/src/snapshot_middleware/json_model.rs b/src/snapshot_middleware/json_model.rs index 20b1f22c6..97ace092f 100644 --- a/src/snapshot_middleware/json_model.rs +++ b/src/snapshot_middleware/json_model.rs @@ -17,7 +17,7 @@ pub fn snapshot_json_model( vfs: &Vfs, path: &Path, ) -> anyhow::Result> { - let name = path.file_name_trim_extension()?; + let name = path.file_name_trim_extensions()?; let contents = vfs.read(path)?; let contents_str = str::from_utf8(&contents) diff --git a/src/snapshot_middleware/lua.rs b/src/snapshot_middleware/lua.rs index de6f369b4..dada491a3 100644 --- a/src/snapshot_middleware/lua.rs +++ b/src/snapshot_middleware/lua.rs @@ -35,7 +35,7 @@ pub fn snapshot_lua( .items; let (script_type, instance_name) = if let Some(script_type) = script_type_overload { - (script_type, path.file_name_trim_extension()?) + (script_type, path.file_name_trim_extensions()?) } else if let Some(name) = match_trailing(&file_name, ".server.lua") { (ScriptType::Server, name) } else if let Some(name) = match_trailing(&file_name, ".client.lua") { diff --git a/src/snapshot_middleware/rbxm.rs b/src/snapshot_middleware/rbxm.rs index 5151e5072..db9cdab3a 100644 --- a/src/snapshot_middleware/rbxm.rs +++ b/src/snapshot_middleware/rbxm.rs @@ -13,7 +13,7 @@ pub fn snapshot_rbxm( vfs: &Vfs, path: &Path, ) -> anyhow::Result> { - let name = path.file_name_trim_extension()?; + let name = path.file_name_trim_extensions()?; let temp_tree = rbx_binary::from_reader(vfs.read(path)?.as_slice()) .with_context(|| format!("Malformed rbxm file: {}", path.display()))?; diff --git a/src/snapshot_middleware/rbxmx.rs b/src/snapshot_middleware/rbxmx.rs index a10c0e944..139a8b390 100644 --- a/src/snapshot_middleware/rbxmx.rs +++ b/src/snapshot_middleware/rbxmx.rs @@ -12,7 +12,7 @@ pub fn snapshot_rbxmx( vfs: &Vfs, path: &Path, ) -> anyhow::Result> { - let name = path.file_name_trim_extension()?; + let name = path.file_name_trim_extensions()?; let options = rbx_xml::DecodeOptions::new() .property_behavior(rbx_xml::DecodePropertyBehavior::ReadUnknown); diff --git a/src/snapshot_middleware/toml.rs b/src/snapshot_middleware/toml.rs index d92db4b3b..9572c3de8 100644 --- a/src/snapshot_middleware/toml.rs +++ b/src/snapshot_middleware/toml.rs @@ -16,7 +16,7 @@ pub fn snapshot_toml( vfs: &Vfs, path: &Path, ) -> anyhow::Result> { - let name = path.file_name_trim_extension()?; + let name = path.file_name_trim_extensions()?; let contents = vfs.read(path)?; let value: toml::Value = toml::from_slice(&contents) diff --git a/src/snapshot_middleware/txt.rs b/src/snapshot_middleware/txt.rs index 8f2126feb..99df69dc4 100644 --- a/src/snapshot_middleware/txt.rs +++ b/src/snapshot_middleware/txt.rs @@ -13,7 +13,7 @@ pub fn snapshot_txt( vfs: &Vfs, path: &Path, ) -> anyhow::Result> { - let name = path.file_name_trim_extension()?; + let name = path.file_name_trim_extensions()?; let contents = vfs.read(path)?; let contents_str = str::from_utf8(&contents) From 7f3dfda65d8446d47bdb772f2b344b59ba41a632 Mon Sep 17 00:00:00 2001 From: Micah Date: Fri, 27 Oct 2023 13:56:19 -0700 Subject: [PATCH 11/42] Add a more complex test case --- ..._end__tests__build__sync_rule_complex.snap | 37 +++++++++++++++++++ .../sync_rule_complex/default.project.json | 24 ++++++++++++ .../sync_rule_complex/src/bar.server | 1 + .../sync_rule_complex/src/baz.client | 1 + .../sync_rule_complex/src/foo.module | 1 + .../sync_rule_complex/src/qux.rojo | 9 +++++ tests/tests/build.rs | 1 + 7 files changed, 74 insertions(+) create mode 100644 rojo-test/build-test-snapshots/end_to_end__tests__build__sync_rule_complex.snap create mode 100644 rojo-test/build-tests/sync_rule_complex/default.project.json create mode 100644 rojo-test/build-tests/sync_rule_complex/src/bar.server create mode 100644 rojo-test/build-tests/sync_rule_complex/src/baz.client create mode 100644 rojo-test/build-tests/sync_rule_complex/src/foo.module create mode 100644 rojo-test/build-tests/sync_rule_complex/src/qux.rojo diff --git a/rojo-test/build-test-snapshots/end_to_end__tests__build__sync_rule_complex.snap b/rojo-test/build-test-snapshots/end_to_end__tests__build__sync_rule_complex.snap new file mode 100644 index 000000000..0fa62b63b --- /dev/null +++ b/rojo-test/build-test-snapshots/end_to_end__tests__build__sync_rule_complex.snap @@ -0,0 +1,37 @@ +--- +source: tests/tests/build.rs +assertion_line: 103 +expression: contents +--- + + + + sync_rule_complex + + + + bar + 0 + -- Hello, from bar (a Script)! + + + + + baz + -- Hello, from baz (a LocalScript)! + + + + + foo + -- Hello, from foo (a ModuleScript)! + + + + + qux + Hello, from qux (a .rojo file that's turned into a StringValue)! + + + + diff --git a/rojo-test/build-tests/sync_rule_complex/default.project.json b/rojo-test/build-tests/sync_rule_complex/default.project.json new file mode 100644 index 000000000..7235395e6 --- /dev/null +++ b/rojo-test/build-tests/sync_rule_complex/default.project.json @@ -0,0 +1,24 @@ +{ + "name": "sync_rule_complex", + "tree": { + "$path": "src" + }, + "syncRules": [ + { + "pattern": "*.module", + "use": "modulescript" + }, + { + "pattern": "*.server", + "use": "serverscript" + }, + { + "pattern": "*.client", + "use": "clientscript" + }, + { + "pattern": "*.rojo", + "use": "project" + } + ] +} \ No newline at end of file diff --git a/rojo-test/build-tests/sync_rule_complex/src/bar.server b/rojo-test/build-tests/sync_rule_complex/src/bar.server new file mode 100644 index 000000000..e860bd77f --- /dev/null +++ b/rojo-test/build-tests/sync_rule_complex/src/bar.server @@ -0,0 +1 @@ +-- Hello, from bar (a Script)! \ No newline at end of file diff --git a/rojo-test/build-tests/sync_rule_complex/src/baz.client b/rojo-test/build-tests/sync_rule_complex/src/baz.client new file mode 100644 index 000000000..4326a2a43 --- /dev/null +++ b/rojo-test/build-tests/sync_rule_complex/src/baz.client @@ -0,0 +1 @@ +-- Hello, from baz (a LocalScript)! \ No newline at end of file diff --git a/rojo-test/build-tests/sync_rule_complex/src/foo.module b/rojo-test/build-tests/sync_rule_complex/src/foo.module new file mode 100644 index 000000000..3a55f9b2b --- /dev/null +++ b/rojo-test/build-tests/sync_rule_complex/src/foo.module @@ -0,0 +1 @@ +-- Hello, from foo (a ModuleScript)! \ No newline at end of file diff --git a/rojo-test/build-tests/sync_rule_complex/src/qux.rojo b/rojo-test/build-tests/sync_rule_complex/src/qux.rojo new file mode 100644 index 000000000..7153477dc --- /dev/null +++ b/rojo-test/build-tests/sync_rule_complex/src/qux.rojo @@ -0,0 +1,9 @@ +{ + "name": "qux", + "tree": { + "$className": "StringValue", + "$properties": { + "Value": "Hello, from qux (a .rojo file that's turned into a StringValue)!" + } + } +} \ No newline at end of file diff --git a/tests/tests/build.rs b/tests/tests/build.rs index e20cfdcb9..3235d6c3c 100644 --- a/tests/tests/build.rs +++ b/tests/tests/build.rs @@ -60,6 +60,7 @@ gen_build_tests! { unresolved_values, weldconstraint, sync_rule_alone, + sync_rule_complex, } fn run_build_test(test_name: &str) { From 0f79b0a8ce13233ce6b3ff08fe0d6019fc7fa4d4 Mon Sep 17 00:00:00 2001 From: Micah Date: Fri, 27 Oct 2023 14:06:32 -0700 Subject: [PATCH 12/42] Add complex sync rules serve test --- ...__tests__serve__sync_rule_complex_all.snap | 68 +++++++++++++++++++ ..._tests__serve__sync_rule_complex_info.snap | 14 ++++ .../sync_rule_complex/default.project.json | 24 +++++++ .../sync_rule_complex/src/bar.server | 1 + .../sync_rule_complex/src/baz.client | 1 + .../sync_rule_complex/src/foo.module | 1 + .../sync_rule_complex/src/qux.rojo | 9 +++ tests/tests/serve.rs | 16 +++++ 8 files changed, 134 insertions(+) create mode 100644 rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_complex_all.snap create mode 100644 rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_complex_info.snap create mode 100644 rojo-test/serve-tests/sync_rule_complex/default.project.json create mode 100644 rojo-test/serve-tests/sync_rule_complex/src/bar.server create mode 100644 rojo-test/serve-tests/sync_rule_complex/src/baz.client create mode 100644 rojo-test/serve-tests/sync_rule_complex/src/foo.module create mode 100644 rojo-test/serve-tests/sync_rule_complex/src/qux.rojo diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_complex_all.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_complex_all.snap new file mode 100644 index 000000000..8b1183560 --- /dev/null +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_complex_all.snap @@ -0,0 +1,68 @@ +--- +source: tests/tests/serve.rs +assertion_line: 284 +expression: "read_response.intern_and_redact(&mut redactions, root_id)" +--- +instances: + id-2: + Children: + - id-3 + - id-4 + - id-5 + - id-6 + ClassName: Folder + Id: id-2 + Metadata: + ignoreUnknownInstances: false + Name: sync_rule_complex + Parent: "00000000000000000000000000000000" + Properties: {} + id-3: + Children: [] + ClassName: Script + Id: id-3 + Metadata: + ignoreUnknownInstances: false + Name: bar + Parent: id-2 + Properties: + RunContext: + Enum: 0 + Source: + String: "-- Hello, from bar (a Script)!" + id-4: + Children: [] + ClassName: LocalScript + Id: id-4 + Metadata: + ignoreUnknownInstances: false + Name: baz + Parent: id-2 + Properties: + Source: + String: "-- Hello, from baz (a LocalScript)!" + id-5: + Children: [] + ClassName: ModuleScript + Id: id-5 + Metadata: + ignoreUnknownInstances: false + Name: foo + Parent: id-2 + Properties: + Source: + String: "-- Hello, from foo (a ModuleScript)!" + id-6: + Children: [] + ClassName: StringValue + Id: id-6 + Metadata: + ignoreUnknownInstances: true + Name: qux + Parent: id-2 + Properties: + Value: + String: "Hello, from qux (a .rojo file that's turned into a StringValue)!" +messageCursor: 0 +sessionId: id-1 + diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_complex_info.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_complex_info.snap new file mode 100644 index 000000000..3ed2b805a --- /dev/null +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_complex_info.snap @@ -0,0 +1,14 @@ +--- +source: tests/tests/serve.rs +assertion_line: 281 +expression: redactions.redacted_yaml(info) +--- +expectedPlaceIds: ~ +gameId: ~ +placeId: ~ +projectName: sync_rule_complex +protocolVersion: 4 +rootInstanceId: id-2 +serverVersion: "[server-version]" +sessionId: id-1 + diff --git a/rojo-test/serve-tests/sync_rule_complex/default.project.json b/rojo-test/serve-tests/sync_rule_complex/default.project.json new file mode 100644 index 000000000..7235395e6 --- /dev/null +++ b/rojo-test/serve-tests/sync_rule_complex/default.project.json @@ -0,0 +1,24 @@ +{ + "name": "sync_rule_complex", + "tree": { + "$path": "src" + }, + "syncRules": [ + { + "pattern": "*.module", + "use": "modulescript" + }, + { + "pattern": "*.server", + "use": "serverscript" + }, + { + "pattern": "*.client", + "use": "clientscript" + }, + { + "pattern": "*.rojo", + "use": "project" + } + ] +} \ No newline at end of file diff --git a/rojo-test/serve-tests/sync_rule_complex/src/bar.server b/rojo-test/serve-tests/sync_rule_complex/src/bar.server new file mode 100644 index 000000000..e860bd77f --- /dev/null +++ b/rojo-test/serve-tests/sync_rule_complex/src/bar.server @@ -0,0 +1 @@ +-- Hello, from bar (a Script)! \ No newline at end of file diff --git a/rojo-test/serve-tests/sync_rule_complex/src/baz.client b/rojo-test/serve-tests/sync_rule_complex/src/baz.client new file mode 100644 index 000000000..4326a2a43 --- /dev/null +++ b/rojo-test/serve-tests/sync_rule_complex/src/baz.client @@ -0,0 +1 @@ +-- Hello, from baz (a LocalScript)! \ No newline at end of file diff --git a/rojo-test/serve-tests/sync_rule_complex/src/foo.module b/rojo-test/serve-tests/sync_rule_complex/src/foo.module new file mode 100644 index 000000000..3a55f9b2b --- /dev/null +++ b/rojo-test/serve-tests/sync_rule_complex/src/foo.module @@ -0,0 +1 @@ +-- Hello, from foo (a ModuleScript)! \ No newline at end of file diff --git a/rojo-test/serve-tests/sync_rule_complex/src/qux.rojo b/rojo-test/serve-tests/sync_rule_complex/src/qux.rojo new file mode 100644 index 000000000..7153477dc --- /dev/null +++ b/rojo-test/serve-tests/sync_rule_complex/src/qux.rojo @@ -0,0 +1,9 @@ +{ + "name": "qux", + "tree": { + "$className": "StringValue", + "$properties": { + "Value": "Hello, from qux (a .rojo file that's turned into a StringValue)!" + } + } +} \ No newline at end of file diff --git a/tests/tests/serve.rs b/tests/tests/serve.rs index d5f6fb07e..c8748d462 100644 --- a/tests/tests/serve.rs +++ b/tests/tests/serve.rs @@ -271,3 +271,19 @@ fn sync_rule_alone() { ); }); } + +#[test] +fn sync_rule_complex() { + run_serve_test("sync_rule_complex", |session, mut redactions| { + let info = session.get_api_rojo().unwrap(); + let root_id = info.root_instance_id; + + assert_yaml_snapshot!("sync_rule_complex_info", redactions.redacted_yaml(info)); + + let read_response = session.get_api_read(root_id).unwrap(); + assert_yaml_snapshot!( + "sync_rule_complex_all", + read_response.intern_and_redact(&mut redactions, root_id) + ); + }); +} From 188168f2e23021a5823cb6eacc311efbb7358ee3 Mon Sep 17 00:00:00 2001 From: Micah Date: Fri, 27 Oct 2023 14:28:52 -0700 Subject: [PATCH 13/42] Add test case for nested projects using sync rules --- .../sync_rule_nested_projects/default.project.json | 6 ++++++ .../src/default.project.json | 12 ++++++++++++ .../sync_rule_nested_projects/src/ignored.rojo | 1 + .../sync_rule_nested_projects/src2/second.txt | 12 ++++++++++++ .../sync_rule_nested_projects/src3/third.txt | 1 + tests/tests/build.rs | 1 + 6 files changed, 33 insertions(+) create mode 100644 rojo-test/build-tests/sync_rule_nested_projects/default.project.json create mode 100644 rojo-test/build-tests/sync_rule_nested_projects/src/default.project.json create mode 100644 rojo-test/build-tests/sync_rule_nested_projects/src/ignored.rojo create mode 100644 rojo-test/build-tests/sync_rule_nested_projects/src2/second.txt create mode 100644 rojo-test/build-tests/sync_rule_nested_projects/src3/third.txt diff --git a/rojo-test/build-tests/sync_rule_nested_projects/default.project.json b/rojo-test/build-tests/sync_rule_nested_projects/default.project.json new file mode 100644 index 000000000..ae18ccf98 --- /dev/null +++ b/rojo-test/build-tests/sync_rule_nested_projects/default.project.json @@ -0,0 +1,6 @@ +{ + "name": "sync_rule_nested_projects", + "tree": { + "$path": "src" + } +} \ No newline at end of file diff --git a/rojo-test/build-tests/sync_rule_nested_projects/src/default.project.json b/rojo-test/build-tests/sync_rule_nested_projects/src/default.project.json new file mode 100644 index 000000000..cb66c118b --- /dev/null +++ b/rojo-test/build-tests/sync_rule_nested_projects/src/default.project.json @@ -0,0 +1,12 @@ +{ + "name": "first", + "tree": { + "$path": "../src2" + }, + "syncRules": [ + { + "pattern": "*.txt", + "use": "project" + } + ] +} \ No newline at end of file diff --git a/rojo-test/build-tests/sync_rule_nested_projects/src/ignored.rojo b/rojo-test/build-tests/sync_rule_nested_projects/src/ignored.rojo new file mode 100644 index 000000000..d7ed8b4fd --- /dev/null +++ b/rojo-test/build-tests/sync_rule_nested_projects/src/ignored.rojo @@ -0,0 +1 @@ +This shouldn't be in the built file. If it is, something is wrong. \ No newline at end of file diff --git a/rojo-test/build-tests/sync_rule_nested_projects/src2/second.txt b/rojo-test/build-tests/sync_rule_nested_projects/src2/second.txt new file mode 100644 index 000000000..9a1eb21b7 --- /dev/null +++ b/rojo-test/build-tests/sync_rule_nested_projects/src2/second.txt @@ -0,0 +1,12 @@ +{ + "name": "second", + "tree": { + "$path": "../src3" + }, + "syncRules": [ + { + "pattern": "*.rojo", + "use": "text" + } + ] +} \ No newline at end of file diff --git a/rojo-test/build-tests/sync_rule_nested_projects/src3/third.txt b/rojo-test/build-tests/sync_rule_nested_projects/src3/third.txt new file mode 100644 index 000000000..62bb0fb5a --- /dev/null +++ b/rojo-test/build-tests/sync_rule_nested_projects/src3/third.txt @@ -0,0 +1 @@ +This should be a StringValue. If it isn't, something is wrong. \ No newline at end of file diff --git a/tests/tests/build.rs b/tests/tests/build.rs index 3235d6c3c..f23d4a4e4 100644 --- a/tests/tests/build.rs +++ b/tests/tests/build.rs @@ -61,6 +61,7 @@ gen_build_tests! { weldconstraint, sync_rule_alone, sync_rule_complex, + sync_rule_nested_projects, } fn run_build_test(test_name: &str) { From 35ff6326d6f5e01f7d7f375e1a20945d9af41fda Mon Sep 17 00:00:00 2001 From: Micah Date: Fri, 27 Oct 2023 15:21:18 -0700 Subject: [PATCH 14/42] Move SyncRule to a system similar to ignore globs --- src/project.rs | 7 +++---- src/snapshot/metadata.rs | 31 ++++++++++++++++++++++++++++-- src/snapshot_middleware/mod.rs | 27 +------------------------- src/snapshot_middleware/project.rs | 9 ++++++++- 4 files changed, 41 insertions(+), 33 deletions(-) diff --git a/src/project.rs b/src/project.rs index 2682fb0b4..82b3d98c4 100644 --- a/src/project.rs +++ b/src/project.rs @@ -9,9 +9,8 @@ use serde::{Deserialize, Serialize}; use thiserror::Error; use crate::{ - glob::Glob, - resolution::UnresolvedValue, - snapshot_middleware::{emit_legacy_scripts_default, SyncRule}, + glob::Glob, resolution::UnresolvedValue, snapshot::SyncRuleOuter, + snapshot_middleware::emit_legacy_scripts_default, }; static PROJECT_FILENAME: &str = "default.project.json"; @@ -94,7 +93,7 @@ pub struct Project { /// it will be 'transformed' into an Instance following the rule provided. /// Globs are relative to the folder the project file is in. #[serde(default, skip_serializing_if = "Vec::is_empty")] - pub sync_rules: Vec, + pub sync_rules: Vec, /// The path to the file that this project came from. Relative paths in the /// project should be considered relative to the parent of this field, also diff --git a/src/snapshot/metadata.rs b/src/snapshot/metadata.rs index 1f75260cd..a1676b56c 100644 --- a/src/snapshot/metadata.rs +++ b/src/snapshot/metadata.rs @@ -10,7 +10,7 @@ use crate::{ glob::Glob, path_serializer, project::ProjectNode, - snapshot_middleware::{emit_legacy_scripts_default, Middleware, SyncRule}, + snapshot_middleware::{emit_legacy_scripts_default, Middleware}, }; /// Rojo-specific metadata that can be associated with an instance or a snapshot @@ -164,7 +164,7 @@ impl InstanceContext { pub fn get_sync_rule(&self, path: &Path) -> Option { for rule in &self.sync_rules { if rule.matches(path) { - return Some(rule.middleware()); + return Some(rule.middleware); } } @@ -239,3 +239,30 @@ impl From<&Path> for InstigatingSource { InstigatingSource::Path(path.to_path_buf()) } } + +/// Represents an user-specified rule for transforming files +/// into Instances using a given middleware. +#[derive(Debug, Deserialize, Serialize, Clone, PartialEq)] +pub struct SyncRuleOuter { + #[serde(rename = "pattern")] + pub glob: Glob, + #[serde(rename = "use")] + pub middleware: Middleware, +} + +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct SyncRule { + pub glob: Glob, + pub base_path: PathBuf, + pub middleware: Middleware, +} + +impl SyncRule { + /// Returns whether the given path matches this rule. + pub fn matches(&self, path: &Path) -> bool { + match path.strip_prefix(&self.base_path) { + Ok(suffix) => self.glob.is_match(suffix), + Err(_) => false, + } + } +} diff --git a/src/snapshot_middleware/mod.rs b/src/snapshot_middleware/mod.rs index 51f90493d..193635fbc 100644 --- a/src/snapshot_middleware/mod.rs +++ b/src/snapshot_middleware/mod.rs @@ -24,10 +24,7 @@ use anyhow::Context; use memofs::{IoResultExt, Vfs}; use serde::{Deserialize, Serialize}; -use crate::{ - glob::Glob, - snapshot::{InstanceContext, InstanceSnapshot}, -}; +use crate::snapshot::{InstanceContext, InstanceSnapshot}; use self::{ csv::{snapshot_csv, snapshot_csv_init}, @@ -169,28 +166,6 @@ fn snapshot_from_path>( } } -/// Represents an override or extension for Rojo's 'syncing rules'. -/// That is, for a specified pattern, Rojo will use the provided middleware. -#[derive(Debug, Deserialize, Serialize, Clone, PartialEq)] -pub struct SyncRule { - #[serde(rename = "pattern")] - glob: Glob, - #[serde(rename = "use")] - middleware: Middleware, -} - -impl SyncRule { - /// Returns whether the given path matches this rule. - pub fn matches(&self, path: &Path) -> bool { - self.glob.is_match(path) - } - - /// Returns the middleware this rule represents. - pub fn middleware(&self) -> Middleware { - self.middleware - } -} - /// Represents a possible 'transformer' used by Rojo to turn a file system /// item into a Roblox Instance. Missing from this list are directories and /// metadata. This is deliberate, as metadata is not a snapshot middleware diff --git a/src/snapshot_middleware/project.rs b/src/snapshot_middleware/project.rs index 17f7db30c..85c007339 100644 --- a/src/snapshot_middleware/project.rs +++ b/src/snapshot_middleware/project.rs @@ -9,6 +9,7 @@ use crate::{ project::{PathNode, Project, ProjectNode}, snapshot::{ InstanceContext, InstanceMetadata, InstanceSnapshot, InstigatingSource, PathIgnoreRule, + SyncRule, }, }; @@ -29,7 +30,13 @@ pub fn snapshot_project( base_path: project.folder_location().to_path_buf(), }); - context.add_sync_rules(project.sync_rules.clone()); + let sync_rules = project.sync_rules.iter().map(|outer| SyncRule { + glob: outer.glob.clone(), + base_path: project.folder_location().to_path_buf(), + middleware: outer.middleware, + }); + + context.add_sync_rules(sync_rules); context.add_path_ignore_rules(rules); context.set_emit_legacy_scripts( project From 27a98c63346eebd8893d1dadb8f2c1bf719a0e58 Mon Sep 17 00:00:00 2001 From: Micah Date: Fri, 27 Oct 2023 15:48:37 -0700 Subject: [PATCH 15/42] Use a better test for nested project files --- .../default.project.json | 10 ++++++++-- .../nested.project.json | 16 ++++++++++++++++ .../src/default.project.json | 12 ------------ .../sync_rule_nested_projects/src/ignored.txt | 1 + .../sync_rule_nested_projects/src2/second.txt | 12 ------------ .../sync_rule_nested_projects/src3/third.txt | 1 - 6 files changed, 25 insertions(+), 27 deletions(-) create mode 100644 rojo-test/build-tests/sync_rule_nested_projects/nested.project.json delete mode 100644 rojo-test/build-tests/sync_rule_nested_projects/src/default.project.json create mode 100644 rojo-test/build-tests/sync_rule_nested_projects/src/ignored.txt delete mode 100644 rojo-test/build-tests/sync_rule_nested_projects/src2/second.txt delete mode 100644 rojo-test/build-tests/sync_rule_nested_projects/src3/third.txt diff --git a/rojo-test/build-tests/sync_rule_nested_projects/default.project.json b/rojo-test/build-tests/sync_rule_nested_projects/default.project.json index ae18ccf98..1b6193bbf 100644 --- a/rojo-test/build-tests/sync_rule_nested_projects/default.project.json +++ b/rojo-test/build-tests/sync_rule_nested_projects/default.project.json @@ -1,6 +1,12 @@ { "name": "sync_rule_nested_projects", "tree": { - "$path": "src" - } + "$path": "nested.project.json" + }, + "syncRules": [ + { + "pattern": "*.rojo", + "use": "text" + } + ] } \ No newline at end of file diff --git a/rojo-test/build-tests/sync_rule_nested_projects/nested.project.json b/rojo-test/build-tests/sync_rule_nested_projects/nested.project.json new file mode 100644 index 000000000..929074f2d --- /dev/null +++ b/rojo-test/build-tests/sync_rule_nested_projects/nested.project.json @@ -0,0 +1,16 @@ +{ + "name": "nested", + "tree": { + "$path": "src" + }, + "syncRules": [ + { + "pattern": "*.txt", + "use": "ignore" + }, + { + "pattern": "*.rojo", + "use": "ignore" + } + ] +} \ No newline at end of file diff --git a/rojo-test/build-tests/sync_rule_nested_projects/src/default.project.json b/rojo-test/build-tests/sync_rule_nested_projects/src/default.project.json deleted file mode 100644 index cb66c118b..000000000 --- a/rojo-test/build-tests/sync_rule_nested_projects/src/default.project.json +++ /dev/null @@ -1,12 +0,0 @@ -{ - "name": "first", - "tree": { - "$path": "../src2" - }, - "syncRules": [ - { - "pattern": "*.txt", - "use": "project" - } - ] -} \ No newline at end of file diff --git a/rojo-test/build-tests/sync_rule_nested_projects/src/ignored.txt b/rojo-test/build-tests/sync_rule_nested_projects/src/ignored.txt new file mode 100644 index 000000000..d7ed8b4fd --- /dev/null +++ b/rojo-test/build-tests/sync_rule_nested_projects/src/ignored.txt @@ -0,0 +1 @@ +This shouldn't be in the built file. If it is, something is wrong. \ No newline at end of file diff --git a/rojo-test/build-tests/sync_rule_nested_projects/src2/second.txt b/rojo-test/build-tests/sync_rule_nested_projects/src2/second.txt deleted file mode 100644 index 9a1eb21b7..000000000 --- a/rojo-test/build-tests/sync_rule_nested_projects/src2/second.txt +++ /dev/null @@ -1,12 +0,0 @@ -{ - "name": "second", - "tree": { - "$path": "../src3" - }, - "syncRules": [ - { - "pattern": "*.rojo", - "use": "text" - } - ] -} \ No newline at end of file diff --git a/rojo-test/build-tests/sync_rule_nested_projects/src3/third.txt b/rojo-test/build-tests/sync_rule_nested_projects/src3/third.txt deleted file mode 100644 index 62bb0fb5a..000000000 --- a/rojo-test/build-tests/sync_rule_nested_projects/src3/third.txt +++ /dev/null @@ -1 +0,0 @@ -This should be a StringValue. If it isn't, something is wrong. \ No newline at end of file From 918e42f920a4e394462b3d30f7886a4e5f1227e1 Mon Sep 17 00:00:00 2001 From: Micah Date: Fri, 27 Oct 2023 15:48:56 -0700 Subject: [PATCH 16/42] Clear sync rules between projects --- src/snapshot/metadata.rs | 4 ++++ src/snapshot_middleware/project.rs | 1 + 2 files changed, 5 insertions(+) diff --git a/src/snapshot/metadata.rs b/src/snapshot/metadata.rs index a1676b56c..4056a8b7e 100644 --- a/src/snapshot/metadata.rs +++ b/src/snapshot/metadata.rs @@ -157,6 +157,10 @@ impl InstanceContext { self.sync_rules.extend(new_rules); } + pub fn clear_sync_rules(&mut self) { + self.sync_rules.clear(); + } + pub fn set_emit_legacy_scripts(&mut self, emit_legacy_scripts: bool) { self.emit_legacy_scripts = emit_legacy_scripts; } diff --git a/src/snapshot_middleware/project.rs b/src/snapshot_middleware/project.rs index 85c007339..1823efbcb 100644 --- a/src/snapshot_middleware/project.rs +++ b/src/snapshot_middleware/project.rs @@ -24,6 +24,7 @@ pub fn snapshot_project( .with_context(|| format!("File was not a valid Rojo project: {}", path.display()))?; let mut context = context.clone(); + context.clear_sync_rules(); let rules = project.glob_ignore_paths.iter().map(|glob| PathIgnoreRule { glob: glob.clone(), From fb12a9222fcba52834419931bb271cc4a7cf126c Mon Sep 17 00:00:00 2001 From: Micah Date: Fri, 27 Oct 2023 15:49:59 -0700 Subject: [PATCH 17/42] Add snapshot for nested project test --- ...end__tests__build__sync_rule_nested_projects.snap | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 rojo-test/build-test-snapshots/end_to_end__tests__build__sync_rule_nested_projects.snap diff --git a/rojo-test/build-test-snapshots/end_to_end__tests__build__sync_rule_nested_projects.snap b/rojo-test/build-test-snapshots/end_to_end__tests__build__sync_rule_nested_projects.snap new file mode 100644 index 000000000..6867ac375 --- /dev/null +++ b/rojo-test/build-test-snapshots/end_to_end__tests__build__sync_rule_nested_projects.snap @@ -0,0 +1,12 @@ +--- +source: tests/tests/build.rs +assertion_line: 104 +expression: contents +--- + + + + sync_rule_nested_projects + + + From 2bcf374304e29d033261bf1f4e03efaa096e7e2a Mon Sep 17 00:00:00 2001 From: Micah Date: Fri, 27 Oct 2023 15:56:36 -0700 Subject: [PATCH 18/42] Add documentation to new sync rule functions --- src/snapshot/metadata.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/snapshot/metadata.rs b/src/snapshot/metadata.rs index 4056a8b7e..04ac9b5db 100644 --- a/src/snapshot/metadata.rs +++ b/src/snapshot/metadata.rs @@ -157,6 +157,7 @@ impl InstanceContext { self.sync_rules.extend(new_rules); } + /// Clears all sync rules for this InstanceContext pub fn clear_sync_rules(&mut self) { self.sync_rules.clear(); } @@ -165,6 +166,8 @@ impl InstanceContext { self.emit_legacy_scripts = emit_legacy_scripts; } + /// Returns the middleware specified by the first syncrule that that + /// matches the provided path. This does not handle default syncing rules. pub fn get_sync_rule(&self, path: &Path) -> Option { for rule in &self.sync_rules { if rule.matches(path) { @@ -248,12 +251,17 @@ impl From<&Path> for InstigatingSource { /// into Instances using a given middleware. #[derive(Debug, Deserialize, Serialize, Clone, PartialEq)] pub struct SyncRuleOuter { + /// The pattern specified by the user #[serde(rename = "pattern")] pub glob: Glob, + /// The middleware specified by the user #[serde(rename = "use")] pub middleware: Middleware, } +/// An internal representation of the user-specified sync rules. This +/// allows the base path of the pattern to be set by Rojo but not be handled +/// by Serde. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct SyncRule { pub glob: Glob, From 49d8bd94c8f91b5df11eeb6a700a40b036457f16 Mon Sep 17 00:00:00 2001 From: Micah Date: Fri, 27 Oct 2023 16:11:05 -0700 Subject: [PATCH 19/42] Update changelog --- CHANGELOG.md | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d2673f564..7ce124b65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,44 @@ ## Unreleased Changes +* Projects may now specify rules for syncing files as if they had a different file extension. ([#DDD]) + This is specified via a new field on project files, `syncRules`: + + ```json + { + "syncRules": [ + { + "pattern": "include_this.extension", + "use": "text", + } + ], + "name": "SyncRulesAreCool", + "tree": { + "$path": "src" + } + } + ``` + + The `use` field corresponds to one of the potential file type that Rojo will currently include in a project. Files that match the provided pattern will be treated as if they had the file extension for that file type. A full list is below: + + | `use` value | file extension | + |:---------------|:----------------| + | `serverscript` | `.server.lua` | + | `clientscript` | `.client.lua` | + | `modulescript` | `.lua` | + | `json` | `.json` | + | `toml` | `.toml` | + | `csv` | `.csv` | + | `text` | `.txt` | + | `jsonmodel` | `.model.json` | + | `rbxm` | `.rbxm` | + | `rbxmx` | `.rbxmx` | + | `project` | `.project.json` | + | `ignore` | None! | + + +[#DDD]: https://github.com/rojo-rbx/rojo/pull/800 + ## [7.4.0-rc3] - October 25, 2023 * Changed `sourcemap --watch` to only generate the sourcemap when it's necessary ([#800]) * Switched script source property getter and setter to `ScriptEditorService` methods ([#801]) From 82cc4d44a35a2d66b582a720d92565448ae8e7b2 Mon Sep 17 00:00:00 2001 From: Micah Date: Mon, 30 Oct 2023 12:59:44 -0700 Subject: [PATCH 20/42] Add suffix field for sync rules --- src/snapshot/metadata.rs | 28 ++++++++++++++++++++++++++++ src/snapshot_middleware/project.rs | 1 + 2 files changed, 29 insertions(+) diff --git a/src/snapshot/metadata.rs b/src/snapshot/metadata.rs index 04ac9b5db..9abc25e46 100644 --- a/src/snapshot/metadata.rs +++ b/src/snapshot/metadata.rs @@ -4,6 +4,7 @@ use std::{ sync::Arc, }; +use anyhow::Context; use serde::{Deserialize, Serialize}; use crate::{ @@ -257,6 +258,10 @@ pub struct SyncRuleOuter { /// The middleware specified by the user #[serde(rename = "use")] pub middleware: Middleware, + /// A suffix to trim off of file names, including the file extension. + /// If not specified, the file extension is the only thing cut off. + #[serde(skip_serializing_if = "Option::is_none")] + pub suffix: Option, } /// An internal representation of the user-specified sync rules. This @@ -267,6 +272,7 @@ pub struct SyncRule { pub glob: Glob, pub base_path: PathBuf, pub middleware: Middleware, + pub suffix: Option, } impl SyncRule { @@ -277,4 +283,26 @@ impl SyncRule { Err(_) => false, } } + + pub fn file_name_for_path<'a>(&self, path: &'a Path) -> anyhow::Result<&'a str> { + let suffix = if let Some(suffix) = &self.suffix { + suffix + } else { + // If the user doesn't specify a suffix, we assume the extension + // should be trimmed (or "" if there is no extension) + path.extension() + .map_or_else(|| Some(""), |s| s.to_str()) + .with_context(|| format!("file name of {} is invalid", path.display()))? + }; + let file_name = path + .file_name() + .and_then(|s| s.to_str()) + .with_context(|| format!("file name of {} is invalid", path.display()))?; + if file_name.ends_with(suffix) { + let end = file_name.len().saturating_sub(suffix.len()); + Ok(&file_name[..end]) + } else { + Ok(file_name) + } + } } diff --git a/src/snapshot_middleware/project.rs b/src/snapshot_middleware/project.rs index 1823efbcb..728fca735 100644 --- a/src/snapshot_middleware/project.rs +++ b/src/snapshot_middleware/project.rs @@ -35,6 +35,7 @@ pub fn snapshot_project( glob: outer.glob.clone(), base_path: project.folder_location().to_path_buf(), middleware: outer.middleware, + suffix: outer.suffix.clone(), }); context.add_sync_rules(sync_rules); From 8f64ccafa4af81184dcdea9e29f9d262a525395c Mon Sep 17 00:00:00 2001 From: Micah Date: Mon, 30 Oct 2023 14:15:31 -0700 Subject: [PATCH 21/42] Support suffix for snapshot_from_path --- src/snapshot/metadata.rs | 4 +- src/snapshot_middleware/mod.rs | 86 ++++++++++++++++++++++++---------- 2 files changed, 64 insertions(+), 26 deletions(-) diff --git a/src/snapshot/metadata.rs b/src/snapshot/metadata.rs index 9abc25e46..6499bf898 100644 --- a/src/snapshot/metadata.rs +++ b/src/snapshot/metadata.rs @@ -169,10 +169,10 @@ impl InstanceContext { /// Returns the middleware specified by the first syncrule that that /// matches the provided path. This does not handle default syncing rules. - pub fn get_sync_rule(&self, path: &Path) -> Option { + pub fn get_sync_rule(&self, path: &Path) -> Option<&SyncRule> { for rule in &self.sync_rules { if rule.matches(path) { - return Some(rule.middleware); + return Some(rule); } } diff --git a/src/snapshot_middleware/mod.rs b/src/snapshot_middleware/mod.rs index 193635fbc..a6600825c 100644 --- a/src/snapshot_middleware/mod.rs +++ b/src/snapshot_middleware/mod.rs @@ -144,26 +144,67 @@ fn get_init_path>(vfs: &Vfs, dir: P) -> anyhow::Result>( +fn snapshot_from_path( context: &InstanceContext, vfs: &Vfs, - path: P, + path: &Path, ) -> anyhow::Result> { - let path = path.as_ref(); - - if let Some(middleware) = context.get_sync_rule(path) { - middleware.snapshot(context, vfs, path) - } else if path.file_name_ends_with(".server.lua") || path.file_name_ends_with(".server.luau") { - snapshot_lua(context, vfs, path, None) - } else if path.file_name_ends_with(".client.lua") || path.file_name_ends_with(".client.luau") { - snapshot_lua(context, vfs, path, None) - } else if path.file_name_ends_with(".lua") || path.file_name_ends_with(".luau") { - snapshot_lua(context, vfs, path, None) - } else if let Some(middleware) = Middleware::from_path(context, path) { - middleware.snapshot(context, vfs, path) + let (middleware, name) = if let Some(rule) = context.get_sync_rule(path) { + (rule.middleware, rule.file_name_for_path(path)?) + } else if path.file_name_ends_with(".server.lua") { + ( + Middleware::ServerScript, + path.file_name_trim_end(".server.lua")?, + ) + } else if path.file_name_ends_with(".server.luau") { + ( + Middleware::ServerScript, + path.file_name_trim_end(".server.luau")?, + ) + } else if path.file_name_ends_with(".client.lua") { + ( + Middleware::ClientScript, + path.file_name_trim_end(".client.lua")?, + ) + } else if path.file_name_ends_with(".client.luau") { + ( + Middleware::ClientScript, + path.file_name_trim_end(".client.luau")?, + ) + } else if path.file_name_ends_with(".lua") { + (Middleware::ModuleScript, path.file_name_trim_end(".lua")?) + } else if path.file_name_ends_with(".luau") { + (Middleware::ModuleScript, path.file_name_trim_end(".luau")?) + } else if path.file_name_ends_with(".project.json") { + ( + Middleware::Project, + path.file_name_trim_end(".project.json")?, + ) + } else if path.file_name_ends_with(".model.json") { + ( + Middleware::JsonModel, + path.file_name_trim_end(".model.json")?, + ) + } else if path.file_name_ends_with(".meta.json") { + // .meta.json files do not turn into InstanceSnapshots + return Ok(None); + } else if path.file_name_ends_with(".json") { + (Middleware::Json, path.file_name_trim_end(".json")?) + } else if path.file_name_ends_with(".toml") { + (Middleware::Toml, path.file_name_trim_end(".toml")?) + } else if path.file_name_ends_with(".csv") { + (Middleware::Csv, path.file_name_trim_end(".csv")?) + } else if path.file_name_ends_with(".txt") { + (Middleware::Text, path.file_name_trim_end(".txt")?) + } else if path.file_name_ends_with(".rbxmx") { + (Middleware::Rbxmx, path.file_name_trim_end(".rbxmx")?) + } else if path.file_name_ends_with(".rbxm") { + (Middleware::Rbxm, path.file_name_trim_end(".rbxm")?) } else { - Ok(None) - } + return Ok(None); + }; + + middleware.snapshot(context, vfs, path, name) } /// Represents a possible 'transformer' used by Rojo to turn a file system @@ -194,8 +235,8 @@ impl Middleware { fn from_path>(context: &InstanceContext, path: P) -> Option { let path = path.as_ref(); - if let Some(middleware) = context.get_sync_rule(path) { - Some(middleware) + if let Some(rule) = context.get_sync_rule(path) { + Some(rule.middleware) } else if path.file_name_ends_with(".server.lua") || path.file_name_ends_with(".server.luau") { @@ -230,17 +271,14 @@ impl Middleware { } } - /// Creates a snapshot for the given path from the Middleware. - /// - /// This function assumes that the snapshot is being used as an override - /// and thus no processing is done on the the path. This means that paths - /// with multiple extensions (i.e. scripts) are not handled well by it. - /// You should prefer `snapshot_from_path` when possible as a result. + /// Creates a snapshot for the given path from the Middleware with + /// the provided name. fn snapshot( &self, context: &InstanceContext, vfs: &Vfs, path: &Path, + name: &str, ) -> anyhow::Result> { match self { Self::Csv => snapshot_csv(context, vfs, path), From e7dbd1d8e180f33c9de04601c975e2ea564051e1 Mon Sep 17 00:00:00 2001 From: Micah Date: Mon, 30 Oct 2023 14:44:35 -0700 Subject: [PATCH 22/42] Pass name from snapshot origin to middleware --- src/snapshot_middleware/csv.rs | 31 ++++++++------- src/snapshot_middleware/json.rs | 5 ++- src/snapshot_middleware/json_model.rs | 7 ++-- src/snapshot_middleware/lua.rs | 54 +++++++++++++++++---------- src/snapshot_middleware/mod.rs | 22 ++++++----- src/snapshot_middleware/rbxm.rs | 6 +-- src/snapshot_middleware/rbxmx.rs | 6 +-- src/snapshot_middleware/toml.rs | 5 ++- src/snapshot_middleware/txt.rs | 17 +++++---- 9 files changed, 88 insertions(+), 65 deletions(-) diff --git a/src/snapshot_middleware/csv.rs b/src/snapshot_middleware/csv.rs index dbaeab1c8..52e8ab0fd 100644 --- a/src/snapshot_middleware/csv.rs +++ b/src/snapshot_middleware/csv.rs @@ -10,16 +10,14 @@ use crate::snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot}; use super::{ dir::{dir_meta, snapshot_dir_no_meta}, meta_file::AdjacentMetadata, - util::PathExt, }; pub fn snapshot_csv( _context: &InstanceContext, vfs: &Vfs, path: &Path, + name: &str, ) -> anyhow::Result> { - let name = path.file_name_trim_extensions()?; - let meta_path = path.with_file_name(format!("{}.meta.json", name)); let contents = vfs.read(path)?; @@ -74,9 +72,8 @@ pub fn snapshot_csv_init( ); } - let mut init_snapshot = snapshot_csv(context, vfs, init_path)?.unwrap(); + let mut init_snapshot = snapshot_csv(context, vfs, init_path, &dir_snapshot.name)?.unwrap(); - init_snapshot.name = dir_snapshot.name; init_snapshot.children = dir_snapshot.children; init_snapshot.metadata = dir_snapshot.metadata; @@ -185,10 +182,14 @@ Ack,Ack!,,An exclamation of despair,¡Ay!"#, let mut vfs = Vfs::new(imfs); - let instance_snapshot = - snapshot_csv(&InstanceContext::default(), &mut vfs, Path::new("/foo.csv")) - .unwrap() - .unwrap(); + let instance_snapshot = snapshot_csv( + &InstanceContext::default(), + &mut vfs, + Path::new("/foo.csv"), + "foo", + ) + .unwrap() + .unwrap(); insta::assert_yaml_snapshot!(instance_snapshot); } @@ -213,10 +214,14 @@ Ack,Ack!,,An exclamation of despair,¡Ay!"#, let mut vfs = Vfs::new(imfs); - let instance_snapshot = - snapshot_csv(&InstanceContext::default(), &mut vfs, Path::new("/foo.csv")) - .unwrap() - .unwrap(); + let instance_snapshot = snapshot_csv( + &InstanceContext::default(), + &mut vfs, + Path::new("/foo.csv"), + "foo", + ) + .unwrap() + .unwrap(); insta::assert_yaml_snapshot!(instance_snapshot); } diff --git a/src/snapshot_middleware/json.rs b/src/snapshot_middleware/json.rs index a674480d9..7594d96c2 100644 --- a/src/snapshot_middleware/json.rs +++ b/src/snapshot_middleware/json.rs @@ -9,14 +9,14 @@ use crate::{ snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot}, }; -use super::{meta_file::AdjacentMetadata, util::PathExt}; +use super::meta_file::AdjacentMetadata; pub fn snapshot_json( context: &InstanceContext, vfs: &Vfs, path: &Path, + name: &str, ) -> anyhow::Result> { - let name = path.file_name_trim_extensions()?; let contents = vfs.read(path)?; let value: serde_json::Value = serde_json::from_slice(&contents) @@ -107,6 +107,7 @@ mod test { &InstanceContext::default(), &mut vfs, Path::new("/foo.json"), + "foo", ) .unwrap() .unwrap(); diff --git a/src/snapshot_middleware/json_model.rs b/src/snapshot_middleware/json_model.rs index 97ace092f..93d566a2e 100644 --- a/src/snapshot_middleware/json_model.rs +++ b/src/snapshot_middleware/json_model.rs @@ -10,15 +10,12 @@ use crate::{ snapshot::{InstanceContext, InstanceSnapshot}, }; -use super::util::PathExt; - pub fn snapshot_json_model( context: &InstanceContext, vfs: &Vfs, path: &Path, + name: &str, ) -> anyhow::Result> { - let name = path.file_name_trim_extensions()?; - let contents = vfs.read(path)?; let contents_str = str::from_utf8(&contents) .with_context(|| format!("File was not valid UTF-8: {}", path.display()))?; @@ -158,6 +155,7 @@ mod test { &InstanceContext::default(), &vfs, Path::new("/foo.model.json"), + "foo", ) .unwrap() .unwrap(); @@ -195,6 +193,7 @@ mod test { &InstanceContext::default(), &vfs, Path::new("/foo.model.json"), + "foo", ) .unwrap() .unwrap(); diff --git a/src/snapshot_middleware/lua.rs b/src/snapshot_middleware/lua.rs index dada491a3..0390de43a 100644 --- a/src/snapshot_middleware/lua.rs +++ b/src/snapshot_middleware/lua.rs @@ -9,7 +9,7 @@ use crate::snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot}; use super::{ dir::{dir_meta, snapshot_dir_no_meta}, meta_file::AdjacentMetadata, - util::{match_trailing, PathExt as _}, + util::match_trailing, }; #[derive(Debug)] @@ -24,6 +24,7 @@ pub fn snapshot_lua( context: &InstanceContext, vfs: &Vfs, path: &Path, + name: &str, script_type_overload: Option, ) -> anyhow::Result> { let file_name = path.file_name().unwrap().to_string_lossy(); @@ -34,20 +35,20 @@ pub fn snapshot_lua( .expect("Unable to get RunContext enums!") .items; - let (script_type, instance_name) = if let Some(script_type) = script_type_overload { - (script_type, path.file_name_trim_extensions()?) - } else if let Some(name) = match_trailing(&file_name, ".server.lua") { - (ScriptType::Server, name) - } else if let Some(name) = match_trailing(&file_name, ".client.lua") { - (ScriptType::Client, name) - } else if let Some(name) = match_trailing(&file_name, ".lua") { - (ScriptType::Module, name) - } else if let Some(name) = match_trailing(&file_name, ".server.luau") { - (ScriptType::Server, name) - } else if let Some(name) = match_trailing(&file_name, ".client.luau") { - (ScriptType::Client, name) - } else if let Some(name) = match_trailing(&file_name, ".luau") { - (ScriptType::Module, name) + let script_type = if let Some(script_type) = script_type_overload { + script_type + } else if match_trailing(&file_name, ".server.lua").is_some() { + ScriptType::Server + } else if match_trailing(&file_name, ".client.lua").is_some() { + ScriptType::Client + } else if match_trailing(&file_name, ".lua").is_some() { + ScriptType::Module + } else if match_trailing(&file_name, ".server.luau").is_some() { + ScriptType::Server + } else if match_trailing(&file_name, ".client.luau").is_some() { + ScriptType::Client + } else if match_trailing(&file_name, ".luau").is_some() { + ScriptType::Module } else { return Ok(None); }; @@ -75,10 +76,10 @@ pub fn snapshot_lua( ); } - let meta_path = path.with_file_name(format!("{}.meta.json", instance_name)); + let meta_path = path.with_file_name(format!("{}.meta.json", name)); let mut snapshot = InstanceSnapshot::new() - .name(instance_name) + .name(name) .class_name(class_name) .properties(properties) .metadata( @@ -121,9 +122,9 @@ pub fn snapshot_lua_init( ); } - let mut init_snapshot = snapshot_lua(context, vfs, init_path, None)?.unwrap(); + let mut init_snapshot = + snapshot_lua(context, vfs, init_path, &dir_snapshot.name, None)?.unwrap(); - init_snapshot.name = dir_snapshot.name; init_snapshot.children = dir_snapshot.children; init_snapshot.metadata = dir_snapshot.metadata; @@ -153,6 +154,7 @@ mod test { &InstanceContext::with_emit_legacy_scripts(Some(true)), &mut vfs, Path::new("/foo.lua"), + "foo", None, ) .unwrap() @@ -175,6 +177,7 @@ mod test { &InstanceContext::with_emit_legacy_scripts(Some(false)), &mut vfs, Path::new("/foo.lua"), + "foo", None, ) .unwrap() @@ -197,6 +200,7 @@ mod test { &InstanceContext::with_emit_legacy_scripts(Some(true)), &mut vfs, Path::new("/foo.server.lua"), + "foo", None, ) .unwrap() @@ -219,6 +223,7 @@ mod test { &InstanceContext::with_emit_legacy_scripts(Some(false)), &mut vfs, Path::new("/foo.server.lua"), + "foo", None, ) .unwrap() @@ -241,6 +246,7 @@ mod test { &InstanceContext::with_emit_legacy_scripts(Some(true)), &mut vfs, Path::new("/foo.client.lua"), + "foo", None, ) .unwrap() @@ -263,6 +269,7 @@ mod test { &InstanceContext::with_emit_legacy_scripts(Some(false)), &mut vfs, Path::new("/foo.client.lua"), + "foo", None, ) .unwrap() @@ -291,6 +298,7 @@ mod test { &InstanceContext::with_emit_legacy_scripts(Some(true)), &mut vfs, Path::new("/root"), + "root", None, ) .unwrap() @@ -324,6 +332,7 @@ mod test { &InstanceContext::with_emit_legacy_scripts(Some(true)), &mut vfs, Path::new("/foo.lua"), + "foo", None, ) .unwrap() @@ -357,6 +366,7 @@ mod test { &InstanceContext::with_emit_legacy_scripts(Some(false)), &mut vfs, Path::new("/foo.lua"), + "foo", None, ) .unwrap() @@ -390,6 +400,7 @@ mod test { &InstanceContext::with_emit_legacy_scripts(Some(true)), &mut vfs, Path::new("/foo.server.lua"), + "foo", None, ) .unwrap() @@ -423,6 +434,7 @@ mod test { &InstanceContext::with_emit_legacy_scripts(Some(false)), &mut vfs, Path::new("/foo.server.lua"), + "foo", None, ) .unwrap() @@ -458,6 +470,7 @@ mod test { &InstanceContext::with_emit_legacy_scripts(Some(true)), &mut vfs, Path::new("/bar.server.lua"), + "bar", None, ) .unwrap() @@ -493,6 +506,7 @@ mod test { &InstanceContext::with_emit_legacy_scripts(Some(false)), &mut vfs, Path::new("/bar.server.lua"), + "bar", None, ) .unwrap() @@ -503,6 +517,7 @@ mod test { }); } + #[ignore = "name trimming is no longer handled by individual snapshots"] #[test] fn double_name_trim() { // Due to the existence of transformers, the way file extensions @@ -518,6 +533,7 @@ mod test { &InstanceContext::with_emit_legacy_scripts(Some(true)), &mut vfs, Path::new("/.server.server.lua"), + "", None, ) .unwrap() diff --git a/src/snapshot_middleware/mod.rs b/src/snapshot_middleware/mod.rs index a6600825c..9d65c6816 100644 --- a/src/snapshot_middleware/mod.rs +++ b/src/snapshot_middleware/mod.rs @@ -281,17 +281,19 @@ impl Middleware { name: &str, ) -> anyhow::Result> { match self { - Self::Csv => snapshot_csv(context, vfs, path), - Self::JsonModel => snapshot_json_model(context, vfs, path), - Self::Json => snapshot_json(context, vfs, path), - Self::ServerScript => snapshot_lua(context, vfs, path, Some(ScriptType::Server)), - Self::ClientScript => snapshot_lua(context, vfs, path, Some(ScriptType::Client)), - Self::ModuleScript => snapshot_lua(context, vfs, path, Some(ScriptType::Module)), + Self::Csv => snapshot_csv(context, vfs, path, name), + Self::JsonModel => snapshot_json_model(context, vfs, path, name), + Self::Json => snapshot_json(context, vfs, path, name), + Self::ServerScript => snapshot_lua(context, vfs, path, name, Some(ScriptType::Server)), + Self::ClientScript => snapshot_lua(context, vfs, path, name, Some(ScriptType::Client)), + Self::ModuleScript => snapshot_lua(context, vfs, path, name, Some(ScriptType::Module)), + // At the moment, snapshot_project does not use `name` so we + // don't provide it. Self::Project => snapshot_project(context, vfs, path), - Self::Rbxm => snapshot_rbxm(context, vfs, path), - Self::Rbxmx => snapshot_rbxmx(context, vfs, path), - Self::Toml => snapshot_toml(context, vfs, path), - Self::Text => snapshot_txt(context, vfs, path), + Self::Rbxm => snapshot_rbxm(context, vfs, path, name), + Self::Rbxmx => snapshot_rbxmx(context, vfs, path, name), + Self::Toml => snapshot_toml(context, vfs, path, name), + Self::Text => snapshot_txt(context, vfs, path, name), Self::Ignore => Ok(None), } } diff --git a/src/snapshot_middleware/rbxm.rs b/src/snapshot_middleware/rbxm.rs index db9cdab3a..7983d0e71 100644 --- a/src/snapshot_middleware/rbxm.rs +++ b/src/snapshot_middleware/rbxm.rs @@ -5,16 +5,13 @@ use memofs::Vfs; use crate::snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot}; -use super::util::PathExt; - #[profiling::function] pub fn snapshot_rbxm( context: &InstanceContext, vfs: &Vfs, path: &Path, + name: &str, ) -> anyhow::Result> { - let name = path.file_name_trim_extensions()?; - let temp_tree = rbx_binary::from_reader(vfs.read(path)?.as_slice()) .with_context(|| format!("Malformed rbxm file: {}", path.display()))?; @@ -63,6 +60,7 @@ mod test { &InstanceContext::default(), &mut vfs, Path::new("/foo.rbxm"), + "foo", ) .unwrap() .unwrap(); diff --git a/src/snapshot_middleware/rbxmx.rs b/src/snapshot_middleware/rbxmx.rs index 139a8b390..4266dc0d7 100644 --- a/src/snapshot_middleware/rbxmx.rs +++ b/src/snapshot_middleware/rbxmx.rs @@ -5,15 +5,12 @@ use memofs::Vfs; use crate::snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot}; -use super::util::PathExt; - pub fn snapshot_rbxmx( context: &InstanceContext, vfs: &Vfs, path: &Path, + name: &str, ) -> anyhow::Result> { - let name = path.file_name_trim_extensions()?; - let options = rbx_xml::DecodeOptions::new() .property_behavior(rbx_xml::DecodePropertyBehavior::ReadUnknown); @@ -75,6 +72,7 @@ mod test { &InstanceContext::default(), &mut vfs, Path::new("/foo.rbxmx"), + "foo", ) .unwrap() .unwrap(); diff --git a/src/snapshot_middleware/toml.rs b/src/snapshot_middleware/toml.rs index 9572c3de8..086c539d5 100644 --- a/src/snapshot_middleware/toml.rs +++ b/src/snapshot_middleware/toml.rs @@ -9,14 +9,14 @@ use crate::{ snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot}, }; -use super::{meta_file::AdjacentMetadata, util::PathExt}; +use super::meta_file::AdjacentMetadata; pub fn snapshot_toml( context: &InstanceContext, vfs: &Vfs, path: &Path, + name: &str, ) -> anyhow::Result> { - let name = path.file_name_trim_extensions()?; let contents = vfs.read(path)?; let value: toml::Value = toml::from_slice(&contents) @@ -114,6 +114,7 @@ mod test { &InstanceContext::default(), &mut vfs, Path::new("/foo.toml"), + "foo", ) .unwrap() .unwrap(); diff --git a/src/snapshot_middleware/txt.rs b/src/snapshot_middleware/txt.rs index 99df69dc4..207243f9f 100644 --- a/src/snapshot_middleware/txt.rs +++ b/src/snapshot_middleware/txt.rs @@ -6,15 +6,14 @@ use memofs::{IoResultExt, Vfs}; use crate::snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot}; -use super::{meta_file::AdjacentMetadata, util::PathExt}; +use super::meta_file::AdjacentMetadata; pub fn snapshot_txt( context: &InstanceContext, vfs: &Vfs, path: &Path, + name: &str, ) -> anyhow::Result> { - let name = path.file_name_trim_extensions()?; - let contents = vfs.read(path)?; let contents_str = str::from_utf8(&contents) .with_context(|| format!("File was not valid UTF-8: {}", path.display()))? @@ -59,10 +58,14 @@ mod test { let mut vfs = Vfs::new(imfs.clone()); - let instance_snapshot = - snapshot_txt(&InstanceContext::default(), &mut vfs, Path::new("/foo.txt")) - .unwrap() - .unwrap(); + let instance_snapshot = snapshot_txt( + &InstanceContext::default(), + &mut vfs, + Path::new("/foo.txt"), + "foo", + ) + .unwrap() + .unwrap(); insta::assert_yaml_snapshot!(instance_snapshot); } From fd8f8081829f2cdfb1817de2b70048d15963a3ff Mon Sep 17 00:00:00 2001 From: Micah Date: Mon, 30 Oct 2023 14:45:25 -0700 Subject: [PATCH 23/42] Remove file_name_trim_extensions --- src/snapshot_middleware/util.rs | 39 --------------------------------- 1 file changed, 39 deletions(-) diff --git a/src/snapshot_middleware/util.rs b/src/snapshot_middleware/util.rs index bc7112088..625910b77 100644 --- a/src/snapshot_middleware/util.rs +++ b/src/snapshot_middleware/util.rs @@ -16,7 +16,6 @@ pub fn match_trailing<'a>(input: &'a str, suffix: &str) -> Option<&'a str> { pub trait PathExt { fn file_name_ends_with(&self, suffix: &str) -> bool; fn file_name_trim_end<'a>(&'a self, suffix: &str) -> anyhow::Result<&'a str>; - fn file_name_trim_extensions(&self) -> anyhow::Result<&str>; } impl

PathExt for P @@ -41,47 +40,9 @@ where match_trailing(file_name, suffix) .with_context(|| format!("Path did not end in {}: {}", suffix, path.display())) } - - /// Returns the name of a file after all extensions have been removed. - fn file_name_trim_extensions(&self) -> anyhow::Result<&str> { - // I would love for this to be less verbose, but I'm not sure it's - // really possible this. It doesn't allocate, so it's no huge loss - // either way though. - let mut file_name = self - .as_ref() - .file_stem() - .and_then(|n| n.to_str()) - .with_context(|| format!("file name of {} is invalid", self.as_ref().display()))?; - while Path::new(file_name).extension().is_some() { - file_name = Path::new(file_name) - .file_stem() - .and_then(|n| n.to_str()) - .with_context(|| format!("file name of {} is invalid", self.as_ref().display()))?; - } - Ok(file_name) - } } // TEMP function until rojo 8.0, when it can be replaced with bool::default (aka false) pub fn emit_legacy_scripts_default() -> Option { Some(true) } - -#[test] -fn file_name_trim_extensions_invalid() { - // Basic test to make sure that the loop for this function - // isn't infinite and that it works right - assert!(Path::new("").file_name_trim_extensions().is_err()); - - assert_eq!(Path::new("foo").file_name_trim_extensions().unwrap(), "foo"); - assert_eq!( - Path::new("foo.bar").file_name_trim_extensions().unwrap(), - "foo" - ); - assert_eq!( - Path::new("foo.bar.baz") - .file_name_trim_extensions() - .unwrap(), - "foo" - ); -} From 8cab62855727a7bc023fb7833bf25b45217f4171 Mon Sep 17 00:00:00 2001 From: Micah Date: Mon, 30 Oct 2023 14:51:20 -0700 Subject: [PATCH 24/42] Adjust how file_name_for_path works --- src/snapshot/metadata.rs | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/src/snapshot/metadata.rs b/src/snapshot/metadata.rs index 6499bf898..cc9f3e0f0 100644 --- a/src/snapshot/metadata.rs +++ b/src/snapshot/metadata.rs @@ -285,24 +285,23 @@ impl SyncRule { } pub fn file_name_for_path<'a>(&self, path: &'a Path) -> anyhow::Result<&'a str> { - let suffix = if let Some(suffix) = &self.suffix { - suffix - } else { - // If the user doesn't specify a suffix, we assume the extension - // should be trimmed (or "" if there is no extension) - path.extension() - .map_or_else(|| Some(""), |s| s.to_str()) - .with_context(|| format!("file name of {} is invalid", path.display()))? - }; - let file_name = path - .file_name() - .and_then(|s| s.to_str()) - .with_context(|| format!("file name of {} is invalid", path.display()))?; - if file_name.ends_with(suffix) { - let end = file_name.len().saturating_sub(suffix.len()); - Ok(&file_name[..end]) + if let Some(suffix) = &self.suffix { + let file_name = path + .file_name() + .and_then(|s| s.to_str()) + .with_context(|| format!("file name of {} is invalid", path.display()))?; + if file_name.ends_with(suffix) { + let end = file_name.len().saturating_sub(suffix.len()); + Ok(&file_name[..end]) + } else { + Ok(file_name) + } } else { - Ok(file_name) + // If the user doesn't specify a suffix, we assume they just want + // the name of the file (the file_stem) + path.file_stem() + .and_then(|s| s.to_str()) + .with_context(|| format!("file name of {} is invalid", path.display())) } } } From 53a273fe562dcdfac0a7303fa0f065ab9a630c33 Mon Sep 17 00:00:00 2001 From: Micah Date: Mon, 30 Oct 2023 14:57:52 -0700 Subject: [PATCH 25/42] Add test for suffix trimming --- ...o_end__tests__build__sync_rule_complex.snap | 12 +++++++++--- .../sync_rule_complex/default.project.json | 5 +++++ .../sync_rule_complex/src/cat.dog.rojo2 | 1 + ...d__tests__serve__sync_rule_complex_all.snap | 18 +++++++++++++++--- .../sync_rule_complex/default.project.json | 5 +++++ .../sync_rule_complex/src/cat.dog.rojo2 | 1 + 6 files changed, 36 insertions(+), 6 deletions(-) create mode 100644 rojo-test/build-tests/sync_rule_complex/src/cat.dog.rojo2 create mode 100644 rojo-test/serve-tests/sync_rule_complex/src/cat.dog.rojo2 diff --git a/rojo-test/build-test-snapshots/end_to_end__tests__build__sync_rule_complex.snap b/rojo-test/build-test-snapshots/end_to_end__tests__build__sync_rule_complex.snap index 0fa62b63b..e38050d81 100644 --- a/rojo-test/build-test-snapshots/end_to_end__tests__build__sync_rule_complex.snap +++ b/rojo-test/build-test-snapshots/end_to_end__tests__build__sync_rule_complex.snap @@ -1,6 +1,6 @@ --- source: tests/tests/build.rs -assertion_line: 103 +assertion_line: 104 expression: contents --- @@ -21,13 +21,19 @@ expression: contents -- Hello, from baz (a LocalScript)! - + + + cat + Hello, from cat (a StringValue)! + + + foo -- Hello, from foo (a ModuleScript)! - + qux Hello, from qux (a .rojo file that's turned into a StringValue)! diff --git a/rojo-test/build-tests/sync_rule_complex/default.project.json b/rojo-test/build-tests/sync_rule_complex/default.project.json index 7235395e6..2e39964f7 100644 --- a/rojo-test/build-tests/sync_rule_complex/default.project.json +++ b/rojo-test/build-tests/sync_rule_complex/default.project.json @@ -19,6 +19,11 @@ { "pattern": "*.rojo", "use": "project" + }, + { + "pattern": "*.dog.rojo2", + "use": "text", + "suffix": ".dog.rojo2" } ] } \ No newline at end of file diff --git a/rojo-test/build-tests/sync_rule_complex/src/cat.dog.rojo2 b/rojo-test/build-tests/sync_rule_complex/src/cat.dog.rojo2 new file mode 100644 index 000000000..e185da86c --- /dev/null +++ b/rojo-test/build-tests/sync_rule_complex/src/cat.dog.rojo2 @@ -0,0 +1 @@ +Hello, from cat (a StringValue)! \ No newline at end of file diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_complex_all.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_complex_all.snap index 8b1183560..80cd76f66 100644 --- a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_complex_all.snap +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_complex_all.snap @@ -10,6 +10,7 @@ instances: - id-4 - id-5 - id-6 + - id-7 ClassName: Folder Id: id-2 Metadata: @@ -43,8 +44,19 @@ instances: String: "-- Hello, from baz (a LocalScript)!" id-5: Children: [] - ClassName: ModuleScript + ClassName: StringValue Id: id-5 + Metadata: + ignoreUnknownInstances: false + Name: cat + Parent: id-2 + Properties: + Value: + String: "Hello, from cat (a StringValue)!" + id-6: + Children: [] + ClassName: ModuleScript + Id: id-6 Metadata: ignoreUnknownInstances: false Name: foo @@ -52,10 +64,10 @@ instances: Properties: Source: String: "-- Hello, from foo (a ModuleScript)!" - id-6: + id-7: Children: [] ClassName: StringValue - Id: id-6 + Id: id-7 Metadata: ignoreUnknownInstances: true Name: qux diff --git a/rojo-test/serve-tests/sync_rule_complex/default.project.json b/rojo-test/serve-tests/sync_rule_complex/default.project.json index 7235395e6..2e39964f7 100644 --- a/rojo-test/serve-tests/sync_rule_complex/default.project.json +++ b/rojo-test/serve-tests/sync_rule_complex/default.project.json @@ -19,6 +19,11 @@ { "pattern": "*.rojo", "use": "project" + }, + { + "pattern": "*.dog.rojo2", + "use": "text", + "suffix": ".dog.rojo2" } ] } \ No newline at end of file diff --git a/rojo-test/serve-tests/sync_rule_complex/src/cat.dog.rojo2 b/rojo-test/serve-tests/sync_rule_complex/src/cat.dog.rojo2 new file mode 100644 index 000000000..e185da86c --- /dev/null +++ b/rojo-test/serve-tests/sync_rule_complex/src/cat.dog.rojo2 @@ -0,0 +1 @@ +Hello, from cat (a StringValue)! \ No newline at end of file From ca2b2d855ae3bbbb4841baa5c9d9b65e6fa3a61f Mon Sep 17 00:00:00 2001 From: Micah Date: Mon, 30 Oct 2023 15:15:07 -0700 Subject: [PATCH 26/42] Add test for sync rules with no file extension --- ...ts__serve__sync_rule_no_extension_all.snap | 30 +++++++++++++++++++ ...s__serve__sync_rule_no_extension_info.snap | 14 +++++++++ .../default.project.json | 12 ++++++++ .../sync_rule_no_extension/src/no_extension | 1 + tests/tests/serve.rs | 19 ++++++++++++ 5 files changed, 76 insertions(+) create mode 100644 rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_no_extension_all.snap create mode 100644 rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_no_extension_info.snap create mode 100644 rojo-test/serve-tests/sync_rule_no_extension/default.project.json create mode 100644 rojo-test/serve-tests/sync_rule_no_extension/src/no_extension diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_no_extension_all.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_no_extension_all.snap new file mode 100644 index 000000000..f3210d3c3 --- /dev/null +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_no_extension_all.snap @@ -0,0 +1,30 @@ +--- +source: tests/tests/serve.rs +assertion_line: 303 +expression: "read_response.intern_and_redact(&mut redactions, root_id)" +--- +instances: + id-2: + Children: + - id-3 + ClassName: Folder + Id: id-2 + Metadata: + ignoreUnknownInstances: false + Name: sync_rule_no_extension + Parent: "00000000000000000000000000000000" + Properties: {} + id-3: + Children: [] + ClassName: ModuleScript + Id: id-3 + Metadata: + ignoreUnknownInstances: false + Name: no_extension + Parent: id-2 + Properties: + Source: + String: "return {\"This file has no extension but should be a ModuleScript named `no_extension`\"}" +messageCursor: 0 +sessionId: id-1 + diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_no_extension_info.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_no_extension_info.snap new file mode 100644 index 000000000..3503e595e --- /dev/null +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_no_extension_info.snap @@ -0,0 +1,14 @@ +--- +source: tests/tests/serve.rs +assertion_line: 297 +expression: redactions.redacted_yaml(info) +--- +expectedPlaceIds: ~ +gameId: ~ +placeId: ~ +projectName: sync_rule_no_extension +protocolVersion: 4 +rootInstanceId: id-2 +serverVersion: "[server-version]" +sessionId: id-1 + diff --git a/rojo-test/serve-tests/sync_rule_no_extension/default.project.json b/rojo-test/serve-tests/sync_rule_no_extension/default.project.json new file mode 100644 index 000000000..185c66334 --- /dev/null +++ b/rojo-test/serve-tests/sync_rule_no_extension/default.project.json @@ -0,0 +1,12 @@ +{ + "name": "sync_rule_no_extension", + "tree": { + "$path": "src" + }, + "syncRules": [ + { + "pattern": "src/**", + "use": "json" + } + ] +} \ No newline at end of file diff --git a/rojo-test/serve-tests/sync_rule_no_extension/src/no_extension b/rojo-test/serve-tests/sync_rule_no_extension/src/no_extension new file mode 100644 index 000000000..41c705c4a --- /dev/null +++ b/rojo-test/serve-tests/sync_rule_no_extension/src/no_extension @@ -0,0 +1 @@ +["This file has no extension but should be a ModuleScript named `no_extension`"] \ No newline at end of file diff --git a/tests/tests/serve.rs b/tests/tests/serve.rs index c8748d462..5d716b397 100644 --- a/tests/tests/serve.rs +++ b/tests/tests/serve.rs @@ -287,3 +287,22 @@ fn sync_rule_complex() { ); }); } + +#[test] +fn sync_rule_no_extension() { + run_serve_test("sync_rule_no_extension", |session, mut redactions| { + let info = session.get_api_rojo().unwrap(); + let root_id = info.root_instance_id; + + assert_yaml_snapshot!( + "sync_rule_no_extension_info", + redactions.redacted_yaml(info) + ); + + let read_response = session.get_api_read(root_id).unwrap(); + assert_yaml_snapshot!( + "sync_rule_no_extension_all", + read_response.intern_and_redact(&mut redactions, root_id) + ); + }); +} From 36d33f71d2d0b3dc5363fc75418cd0c68228b102 Mon Sep 17 00:00:00 2001 From: Micah Date: Tue, 31 Oct 2023 11:34:29 -0700 Subject: [PATCH 27/42] Don't process file name for Lua snapshots --- src/snapshot_middleware/lua.rs | 54 +++++++++++----------------------- src/snapshot_middleware/mod.rs | 18 ++++++++---- 2 files changed, 29 insertions(+), 43 deletions(-) diff --git a/src/snapshot_middleware/lua.rs b/src/snapshot_middleware/lua.rs index 0390de43a..fc2c22b36 100644 --- a/src/snapshot_middleware/lua.rs +++ b/src/snapshot_middleware/lua.rs @@ -9,7 +9,6 @@ use crate::snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot}; use super::{ dir::{dir_meta, snapshot_dir_no_meta}, meta_file::AdjacentMetadata, - util::match_trailing, }; #[derive(Debug)] @@ -25,34 +24,14 @@ pub fn snapshot_lua( vfs: &Vfs, path: &Path, name: &str, - script_type_overload: Option, + script_type: ScriptType, ) -> anyhow::Result> { - let file_name = path.file_name().unwrap().to_string_lossy(); - let run_context_enums = &rbx_reflection_database::get() .enums .get("RunContext") .expect("Unable to get RunContext enums!") .items; - let script_type = if let Some(script_type) = script_type_overload { - script_type - } else if match_trailing(&file_name, ".server.lua").is_some() { - ScriptType::Server - } else if match_trailing(&file_name, ".client.lua").is_some() { - ScriptType::Client - } else if match_trailing(&file_name, ".lua").is_some() { - ScriptType::Module - } else if match_trailing(&file_name, ".server.luau").is_some() { - ScriptType::Server - } else if match_trailing(&file_name, ".client.luau").is_some() { - ScriptType::Client - } else if match_trailing(&file_name, ".luau").is_some() { - ScriptType::Module - } else { - return Ok(None); - }; - let (class_name, run_context) = match (context.emit_legacy_scripts, script_type) { (false, ScriptType::Server) => ("Script", run_context_enums.get("Server")), (false, ScriptType::Client) => ("Script", run_context_enums.get("Client")), @@ -106,6 +85,7 @@ pub fn snapshot_lua_init( context: &InstanceContext, vfs: &Vfs, init_path: &Path, + script_type: ScriptType, ) -> anyhow::Result> { let folder_path = init_path.parent().unwrap(); let dir_snapshot = snapshot_dir_no_meta(context, vfs, folder_path)?.unwrap(); @@ -123,7 +103,7 @@ pub fn snapshot_lua_init( } let mut init_snapshot = - snapshot_lua(context, vfs, init_path, &dir_snapshot.name, None)?.unwrap(); + snapshot_lua(context, vfs, init_path, &dir_snapshot.name, script_type)?.unwrap(); init_snapshot.children = dir_snapshot.children; init_snapshot.metadata = dir_snapshot.metadata; @@ -155,7 +135,7 @@ mod test { &mut vfs, Path::new("/foo.lua"), "foo", - None, + ScriptType::Module, ) .unwrap() .unwrap(); @@ -178,7 +158,7 @@ mod test { &mut vfs, Path::new("/foo.lua"), "foo", - None, + ScriptType::Module, ) .unwrap() .unwrap(); @@ -201,7 +181,7 @@ mod test { &mut vfs, Path::new("/foo.server.lua"), "foo", - None, + ScriptType::Server, ) .unwrap() .unwrap(); @@ -224,7 +204,7 @@ mod test { &mut vfs, Path::new("/foo.server.lua"), "foo", - None, + ScriptType::Server, ) .unwrap() .unwrap(); @@ -247,7 +227,7 @@ mod test { &mut vfs, Path::new("/foo.client.lua"), "foo", - None, + ScriptType::Client, ) .unwrap() .unwrap(); @@ -270,7 +250,7 @@ mod test { &mut vfs, Path::new("/foo.client.lua"), "foo", - None, + ScriptType::Client, ) .unwrap() .unwrap(); @@ -299,7 +279,7 @@ mod test { &mut vfs, Path::new("/root"), "root", - None, + ScriptType::Module, ) .unwrap() .unwrap(); @@ -333,7 +313,7 @@ mod test { &mut vfs, Path::new("/foo.lua"), "foo", - None, + ScriptType::Module, ) .unwrap() .unwrap(); @@ -367,7 +347,7 @@ mod test { &mut vfs, Path::new("/foo.lua"), "foo", - None, + ScriptType::Module, ) .unwrap() .unwrap(); @@ -401,7 +381,7 @@ mod test { &mut vfs, Path::new("/foo.server.lua"), "foo", - None, + ScriptType::Server, ) .unwrap() .unwrap(); @@ -435,7 +415,7 @@ mod test { &mut vfs, Path::new("/foo.server.lua"), "foo", - None, + ScriptType::Server, ) .unwrap() .unwrap(); @@ -471,7 +451,7 @@ mod test { &mut vfs, Path::new("/bar.server.lua"), "bar", - None, + ScriptType::Server, ) .unwrap() .unwrap(); @@ -507,7 +487,7 @@ mod test { &mut vfs, Path::new("/bar.server.lua"), "bar", - None, + ScriptType::Server, ) .unwrap() .unwrap(); @@ -534,7 +514,7 @@ mod test { &mut vfs, Path::new("/.server.server.lua"), "", - None, + ScriptType::Server, ) .unwrap() .unwrap(); diff --git a/src/snapshot_middleware/mod.rs b/src/snapshot_middleware/mod.rs index 9d65c6816..c70ac1ddb 100644 --- a/src/snapshot_middleware/mod.rs +++ b/src/snapshot_middleware/mod.rs @@ -62,9 +62,15 @@ pub fn snapshot_from_vfs( match Middleware::from_path(context, &init_path) { Some(Middleware::Project) => snapshot_project(context, vfs, &init_path), - Some(Middleware::ModuleScript) => snapshot_lua_init(context, vfs, &init_path), - Some(Middleware::ServerScript) => snapshot_lua_init(context, vfs, &init_path), - Some(Middleware::ClientScript) => snapshot_lua_init(context, vfs, &init_path), + Some(Middleware::ModuleScript) => { + snapshot_lua_init(context, vfs, &init_path, ScriptType::Module) + } + Some(Middleware::ServerScript) => { + snapshot_lua_init(context, vfs, &init_path, ScriptType::Server) + } + Some(Middleware::ClientScript) => { + snapshot_lua_init(context, vfs, &init_path, ScriptType::Client) + } Some(Middleware::Csv) => snapshot_csv_init(context, vfs, &init_path), @@ -284,9 +290,9 @@ impl Middleware { Self::Csv => snapshot_csv(context, vfs, path, name), Self::JsonModel => snapshot_json_model(context, vfs, path, name), Self::Json => snapshot_json(context, vfs, path, name), - Self::ServerScript => snapshot_lua(context, vfs, path, name, Some(ScriptType::Server)), - Self::ClientScript => snapshot_lua(context, vfs, path, name, Some(ScriptType::Client)), - Self::ModuleScript => snapshot_lua(context, vfs, path, name, Some(ScriptType::Module)), + Self::ServerScript => snapshot_lua(context, vfs, path, name, ScriptType::Server), + Self::ClientScript => snapshot_lua(context, vfs, path, name, ScriptType::Client), + Self::ModuleScript => snapshot_lua(context, vfs, path, name, ScriptType::Module), // At the moment, snapshot_project does not use `name` so we // don't provide it. Self::Project => snapshot_project(context, vfs, path), From 0c62a6e2f697c130b45dd54ccda7546d2ec35d67 Mon Sep 17 00:00:00 2001 From: Micah Date: Tue, 31 Oct 2023 11:38:43 -0700 Subject: [PATCH 28/42] Fix comments --- src/snapshot/metadata.rs | 2 +- src/snapshot_middleware/mod.rs | 8 ++------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/snapshot/metadata.rs b/src/snapshot/metadata.rs index cc9f3e0f0..a820e61c0 100644 --- a/src/snapshot/metadata.rs +++ b/src/snapshot/metadata.rs @@ -167,7 +167,7 @@ impl InstanceContext { self.emit_legacy_scripts = emit_legacy_scripts; } - /// Returns the middleware specified by the first syncrule that that + /// Returns the middleware specified by the first sync rule that /// matches the provided path. This does not handle default syncing rules. pub fn get_sync_rule(&self, path: &Path) -> Option<&SyncRule> { for rule in &self.sync_rules { diff --git a/src/snapshot_middleware/mod.rs b/src/snapshot_middleware/mod.rs index c70ac1ddb..a5b5a749c 100644 --- a/src/snapshot_middleware/mod.rs +++ b/src/snapshot_middleware/mod.rs @@ -144,12 +144,8 @@ fn get_init_path>(vfs: &Vfs, dir: P) -> anyhow::Result Date: Tue, 31 Oct 2023 11:40:44 -0700 Subject: [PATCH 29/42] Remove outdated test --- src/snapshot_middleware/lua.rs | 27 ------------------- ...ddleware__lua__test__double_name_trim.snap | 24 ----------------- 2 files changed, 51 deletions(-) delete mode 100644 src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__double_name_trim.snap diff --git a/src/snapshot_middleware/lua.rs b/src/snapshot_middleware/lua.rs index fc2c22b36..ab7f15a1f 100644 --- a/src/snapshot_middleware/lua.rs +++ b/src/snapshot_middleware/lua.rs @@ -496,31 +496,4 @@ mod test { insta::assert_yaml_snapshot!(instance_snapshot); }); } - - #[ignore = "name trimming is no longer handled by individual snapshots"] - #[test] - fn double_name_trim() { - // Due to the existence of transformers, the way file extensions - // get trimmed is different now. A regression of this nature is - // possible, so we test against double-trimming. - let mut imfs = InMemoryFs::new(); - imfs.load_snapshot("/.server.server.lua", VfsSnapshot::file("Hello there!")) - .unwrap(); - - let mut vfs = Vfs::new(imfs); - - let instance_snapshot = snapshot_lua( - &InstanceContext::with_emit_legacy_scripts(Some(true)), - &mut vfs, - Path::new("/.server.server.lua"), - "", - ScriptType::Server, - ) - .unwrap() - .unwrap(); - - insta::with_settings!({ sort_maps => true }, { - insta::assert_yaml_snapshot!(instance_snapshot); - }); - } } diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__double_name_trim.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__double_name_trim.snap deleted file mode 100644 index 2784d8275..000000000 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__double_name_trim.snap +++ /dev/null @@ -1,24 +0,0 @@ ---- -source: src/snapshot_middleware/lua.rs -assertion_line: 524 -expression: instance_snapshot ---- -snapshot_id: "00000000000000000000000000000000" -metadata: - ignore_unknown_instances: false - instigating_source: - Path: /.server.server.lua - relevant_paths: - - /.server.server.lua - - /.server.meta.json - context: - emit_legacy_scripts: true -name: ".server" -class_name: Script -properties: - RunContext: - Enum: 0 - Source: - String: Hello there! -children: [] - From d8dca682ee79bab510bf467ffaec3067a1b4d669 Mon Sep 17 00:00:00 2001 From: Micah Date: Tue, 31 Oct 2023 12:17:36 -0700 Subject: [PATCH 30/42] Define fields more clearly in changelog --- CHANGELOG.md | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ce124b65..bd734f388 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,19 +7,26 @@ ```json { - "syncRules": [ - { - "pattern": "include_this.extension", - "use": "text", - } - ], - "name": "SyncRulesAreCool", - "tree": { - "$path": "src" - } + "syncRules": [ + { + "pattern": "*.foo", + "use": "text", + }, + { + "pattern": "*.bar.baz", + "use": "json", + "suffix": ".bar.baz", + }, + ], + "name": "SyncRulesAreCool", + "tree": { + "$path": "src" + } } ``` + The `pattern` field is a glob used to match the sync rule to files. If present, the `suffix` field allows you to specify parts of a file's name get cut off by Rojo to name the Instance, including the file extension. If it isn't specified, Rojo will only cut off the first part of the file extension, up to the first dot. + The `use` field corresponds to one of the potential file type that Rojo will currently include in a project. Files that match the provided pattern will be treated as if they had the file extension for that file type. A full list is below: | `use` value | file extension | @@ -38,7 +45,7 @@ | `ignore` | None! | -[#DDD]: https://github.com/rojo-rbx/rojo/pull/800 +[#DDD]: https://github.com/rojo-rbx/rojo/pull/DDD ## [7.4.0-rc3] - October 25, 2023 * Changed `sourcemap --watch` to only generate the sourcemap when it's necessary ([#800]) From ee105dada2fd45db992bebe3b5a9597c5fe12a66 Mon Sep 17 00:00:00 2001 From: Micah Date: Tue, 31 Oct 2023 12:18:56 -0700 Subject: [PATCH 31/42] Add PR number to changelog --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bd734f388..6429b908f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## Unreleased Changes -* Projects may now specify rules for syncing files as if they had a different file extension. ([#DDD]) +* Projects may now specify rules for syncing files as if they had a different file extension. ([#813]) This is specified via a new field on project files, `syncRules`: ```json @@ -45,7 +45,7 @@ | `ignore` | None! | -[#DDD]: https://github.com/rojo-rbx/rojo/pull/DDD +[#813]: https://github.com/rojo-rbx/rojo/pull/813 ## [7.4.0-rc3] - October 25, 2023 * Changed `sourcemap --watch` to only generate the sourcemap when it's necessary ([#800]) From 1dbfb9122853a7c7fd3b6f01f73dab60c4ae5e64 Mon Sep 17 00:00:00 2001 From: Dekkonot Date: Tue, 31 Oct 2023 16:23:07 -0700 Subject: [PATCH 32/42] Delete the SyncRuleOuter struct --- src/project.rs | 4 ++-- src/snapshot/metadata.rs | 15 ++++----------- 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/src/project.rs b/src/project.rs index 82b3d98c4..68cd18fa8 100644 --- a/src/project.rs +++ b/src/project.rs @@ -9,7 +9,7 @@ use serde::{Deserialize, Serialize}; use thiserror::Error; use crate::{ - glob::Glob, resolution::UnresolvedValue, snapshot::SyncRuleOuter, + glob::Glob, resolution::UnresolvedValue, snapshot::SyncRule, snapshot_middleware::emit_legacy_scripts_default, }; @@ -93,7 +93,7 @@ pub struct Project { /// it will be 'transformed' into an Instance following the rule provided. /// Globs are relative to the folder the project file is in. #[serde(default, skip_serializing_if = "Vec::is_empty")] - pub sync_rules: Vec, + pub sync_rules: Vec, /// The path to the file that this project came from. Relative paths in the /// project should be considered relative to the parent of this field, also diff --git a/src/snapshot/metadata.rs b/src/snapshot/metadata.rs index a820e61c0..74b08f462 100644 --- a/src/snapshot/metadata.rs +++ b/src/snapshot/metadata.rs @@ -251,7 +251,7 @@ impl From<&Path> for InstigatingSource { /// Represents an user-specified rule for transforming files /// into Instances using a given middleware. #[derive(Debug, Deserialize, Serialize, Clone, PartialEq)] -pub struct SyncRuleOuter { +pub struct SyncRule { /// The pattern specified by the user #[serde(rename = "pattern")] pub glob: Glob, @@ -262,17 +262,10 @@ pub struct SyncRuleOuter { /// If not specified, the file extension is the only thing cut off. #[serde(skip_serializing_if = "Option::is_none")] pub suffix: Option, -} - -/// An internal representation of the user-specified sync rules. This -/// allows the base path of the pattern to be set by Rojo but not be handled -/// by Serde. -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] -pub struct SyncRule { - pub glob: Glob, + /// The 'base' of the glob above, allowing it to be used + /// relative to a path instead of absolute. + #[serde(skip)] pub base_path: PathBuf, - pub middleware: Middleware, - pub suffix: Option, } impl SyncRule { From 540d7d61ff63316a5572191a349467028da15625 Mon Sep 17 00:00:00 2001 From: Dekkonot Date: Tue, 31 Oct 2023 16:35:48 -0700 Subject: [PATCH 33/42] Use struct update syntax in project snapshots --- src/snapshot_middleware/project.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/snapshot_middleware/project.rs b/src/snapshot_middleware/project.rs index 728fca735..c92f42d8a 100644 --- a/src/snapshot_middleware/project.rs +++ b/src/snapshot_middleware/project.rs @@ -31,11 +31,9 @@ pub fn snapshot_project( base_path: project.folder_location().to_path_buf(), }); - let sync_rules = project.sync_rules.iter().map(|outer| SyncRule { - glob: outer.glob.clone(), + let sync_rules = project.sync_rules.iter().map(|rule| SyncRule { base_path: project.folder_location().to_path_buf(), - middleware: outer.middleware, - suffix: outer.suffix.clone(), + ..rule.clone() }); context.add_sync_rules(sync_rules); From c36ad2ee6566bf958b50a0e140327e6cfa2dade9 Mon Sep 17 00:00:00 2001 From: Dekkonot Date: Tue, 31 Oct 2023 16:39:20 -0700 Subject: [PATCH 34/42] Use camelCase for Middleware enum in Serde --- CHANGELOG.md | 8 ++++---- .../build-tests/sync_rule_complex/default.project.json | 6 +++--- .../serve-tests/sync_rule_complex/default.project.json | 6 +++--- src/snapshot_middleware/mod.rs | 2 +- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6429b908f..b23a06a69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,14 +31,14 @@ | `use` value | file extension | |:---------------|:----------------| - | `serverscript` | `.server.lua` | - | `clientscript` | `.client.lua` | - | `modulescript` | `.lua` | + | `serverScript` | `.server.lua` | + | `clientScript` | `.client.lua` | + | `moduleScript` | `.lua` | | `json` | `.json` | | `toml` | `.toml` | | `csv` | `.csv` | | `text` | `.txt` | - | `jsonmodel` | `.model.json` | + | `jsonModel` | `.model.json` | | `rbxm` | `.rbxm` | | `rbxmx` | `.rbxmx` | | `project` | `.project.json` | diff --git a/rojo-test/build-tests/sync_rule_complex/default.project.json b/rojo-test/build-tests/sync_rule_complex/default.project.json index 2e39964f7..21922a568 100644 --- a/rojo-test/build-tests/sync_rule_complex/default.project.json +++ b/rojo-test/build-tests/sync_rule_complex/default.project.json @@ -6,15 +6,15 @@ "syncRules": [ { "pattern": "*.module", - "use": "modulescript" + "use": "moduleScript" }, { "pattern": "*.server", - "use": "serverscript" + "use": "serverScript" }, { "pattern": "*.client", - "use": "clientscript" + "use": "clientScript" }, { "pattern": "*.rojo", diff --git a/rojo-test/serve-tests/sync_rule_complex/default.project.json b/rojo-test/serve-tests/sync_rule_complex/default.project.json index 2e39964f7..21922a568 100644 --- a/rojo-test/serve-tests/sync_rule_complex/default.project.json +++ b/rojo-test/serve-tests/sync_rule_complex/default.project.json @@ -6,15 +6,15 @@ "syncRules": [ { "pattern": "*.module", - "use": "modulescript" + "use": "moduleScript" }, { "pattern": "*.server", - "use": "serverscript" + "use": "serverScript" }, { "pattern": "*.client", - "use": "clientscript" + "use": "clientScript" }, { "pattern": "*.rojo", diff --git a/src/snapshot_middleware/mod.rs b/src/snapshot_middleware/mod.rs index a5b5a749c..ff11a6e6b 100644 --- a/src/snapshot_middleware/mod.rs +++ b/src/snapshot_middleware/mod.rs @@ -214,7 +214,7 @@ fn snapshot_from_path( /// metadata. This is deliberate, as metadata is not a snapshot middleware /// and directories do not make sense to turn into files. #[derive(Debug, Clone, Copy, PartialEq, Deserialize, Serialize)] -#[serde(rename_all = "lowercase")] +#[serde(rename_all = "camelCase")] pub enum Middleware { Csv, JsonModel, From 499f79ac548f2537454968b4e8a58124549c5341 Mon Sep 17 00:00:00 2001 From: Micah Date: Tue, 7 Nov 2023 08:39:28 -0800 Subject: [PATCH 35/42] Change nested project test to not overwrite parent syncrule --- .../build-tests/sync_rule_nested_projects/nested.project.json | 4 ---- 1 file changed, 4 deletions(-) diff --git a/rojo-test/build-tests/sync_rule_nested_projects/nested.project.json b/rojo-test/build-tests/sync_rule_nested_projects/nested.project.json index 929074f2d..3c5fea2c0 100644 --- a/rojo-test/build-tests/sync_rule_nested_projects/nested.project.json +++ b/rojo-test/build-tests/sync_rule_nested_projects/nested.project.json @@ -7,10 +7,6 @@ { "pattern": "*.txt", "use": "ignore" - }, - { - "pattern": "*.rojo", - "use": "ignore" } ] } \ No newline at end of file From 30e38af71c11455b4f33ab83058bb92995205bad Mon Sep 17 00:00:00 2001 From: Micah Date: Tue, 7 Nov 2023 08:50:02 -0800 Subject: [PATCH 36/42] Rename `get_sync_rule` to `get_user_sync_rule` --- src/snapshot/metadata.rs | 2 +- src/snapshot_middleware/mod.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/snapshot/metadata.rs b/src/snapshot/metadata.rs index 74b08f462..d1a6c19cf 100644 --- a/src/snapshot/metadata.rs +++ b/src/snapshot/metadata.rs @@ -169,7 +169,7 @@ impl InstanceContext { /// Returns the middleware specified by the first sync rule that /// matches the provided path. This does not handle default syncing rules. - pub fn get_sync_rule(&self, path: &Path) -> Option<&SyncRule> { + pub fn get_user_sync_rule(&self, path: &Path) -> Option<&SyncRule> { for rule in &self.sync_rules { if rule.matches(path) { return Some(rule); diff --git a/src/snapshot_middleware/mod.rs b/src/snapshot_middleware/mod.rs index ff11a6e6b..710436bf5 100644 --- a/src/snapshot_middleware/mod.rs +++ b/src/snapshot_middleware/mod.rs @@ -151,7 +151,7 @@ fn snapshot_from_path( vfs: &Vfs, path: &Path, ) -> anyhow::Result> { - let (middleware, name) = if let Some(rule) = context.get_sync_rule(path) { + let (middleware, name) = if let Some(rule) = context.get_user_sync_rule(path) { (rule.middleware, rule.file_name_for_path(path)?) } else if path.file_name_ends_with(".server.lua") { ( @@ -237,7 +237,7 @@ impl Middleware { fn from_path>(context: &InstanceContext, path: P) -> Option { let path = path.as_ref(); - if let Some(rule) = context.get_sync_rule(path) { + if let Some(rule) = context.get_user_sync_rule(path) { Some(rule.middleware) } else if path.file_name_ends_with(".server.lua") || path.file_name_ends_with(".server.luau") From 282a6f26f4ab518462ac141c0e4090de12c08a30 Mon Sep 17 00:00:00 2001 From: Micah Date: Tue, 7 Nov 2023 09:05:35 -0800 Subject: [PATCH 37/42] Support exclusions in sync rules --- .../sync_rule_complex/default.project.json | 1 + .../sync_rule_complex/src/rat.ignore.rojo | 1 + .../sync_rule_complex/default.project.json | 1 + .../sync_rule_complex/src/rat.ignore.rojo | 1 + src/snapshot/metadata.rs | 18 ++++++++++++++---- 5 files changed, 18 insertions(+), 4 deletions(-) create mode 100644 rojo-test/build-tests/sync_rule_complex/src/rat.ignore.rojo create mode 100644 rojo-test/serve-tests/sync_rule_complex/src/rat.ignore.rojo diff --git a/rojo-test/build-tests/sync_rule_complex/default.project.json b/rojo-test/build-tests/sync_rule_complex/default.project.json index 21922a568..62764bb4c 100644 --- a/rojo-test/build-tests/sync_rule_complex/default.project.json +++ b/rojo-test/build-tests/sync_rule_complex/default.project.json @@ -18,6 +18,7 @@ }, { "pattern": "*.rojo", + "exclude": "*.ignore.rojo", "use": "project" }, { diff --git a/rojo-test/build-tests/sync_rule_complex/src/rat.ignore.rojo b/rojo-test/build-tests/sync_rule_complex/src/rat.ignore.rojo new file mode 100644 index 000000000..d80a8cc88 --- /dev/null +++ b/rojo-test/build-tests/sync_rule_complex/src/rat.ignore.rojo @@ -0,0 +1 @@ +This file should be ignored! \ No newline at end of file diff --git a/rojo-test/serve-tests/sync_rule_complex/default.project.json b/rojo-test/serve-tests/sync_rule_complex/default.project.json index 21922a568..62764bb4c 100644 --- a/rojo-test/serve-tests/sync_rule_complex/default.project.json +++ b/rojo-test/serve-tests/sync_rule_complex/default.project.json @@ -18,6 +18,7 @@ }, { "pattern": "*.rojo", + "exclude": "*.ignore.rojo", "use": "project" }, { diff --git a/rojo-test/serve-tests/sync_rule_complex/src/rat.ignore.rojo b/rojo-test/serve-tests/sync_rule_complex/src/rat.ignore.rojo new file mode 100644 index 000000000..d80a8cc88 --- /dev/null +++ b/rojo-test/serve-tests/sync_rule_complex/src/rat.ignore.rojo @@ -0,0 +1 @@ +This file should be ignored! \ No newline at end of file diff --git a/src/snapshot/metadata.rs b/src/snapshot/metadata.rs index d1a6c19cf..c76ec775f 100644 --- a/src/snapshot/metadata.rs +++ b/src/snapshot/metadata.rs @@ -252,10 +252,13 @@ impl From<&Path> for InstigatingSource { /// into Instances using a given middleware. #[derive(Debug, Deserialize, Serialize, Clone, PartialEq)] pub struct SyncRule { - /// The pattern specified by the user + /// A pattern used to determine if a file is included in this SyncRule #[serde(rename = "pattern")] - pub glob: Glob, - /// The middleware specified by the user + pub include: Glob, + /// A pattern used to determine if a file is excluded from this SyncRule. + #[serde(skip_serializing_if = "Option::is_none")] + pub exclude: Option, + /// The middleware specified by the user for this SyncRule #[serde(rename = "use")] pub middleware: Middleware, /// A suffix to trim off of file names, including the file extension. @@ -272,7 +275,14 @@ impl SyncRule { /// Returns whether the given path matches this rule. pub fn matches(&self, path: &Path) -> bool { match path.strip_prefix(&self.base_path) { - Ok(suffix) => self.glob.is_match(suffix), + Ok(suffix) => { + if let Some(pattern) = &self.exclude { + if pattern.is_match(suffix) { + return false; + } + } + self.include.is_match(suffix) + } Err(_) => false, } } From c61d8eeb50190b9ad40b7045b5242c9478f8005f Mon Sep 17 00:00:00 2001 From: Micah Date: Tue, 7 Nov 2023 10:06:02 -0800 Subject: [PATCH 38/42] Refactor to use syncing rules for default behavior --- src/snapshot_middleware/mod.rs | 218 ++++++++++++++++----------------- 1 file changed, 105 insertions(+), 113 deletions(-) diff --git a/src/snapshot_middleware/mod.rs b/src/snapshot_middleware/mod.rs index 710436bf5..3083e0508 100644 --- a/src/snapshot_middleware/mod.rs +++ b/src/snapshot_middleware/mod.rs @@ -18,13 +18,17 @@ mod toml; mod txt; mod util; -use std::path::{Path, PathBuf}; +use std::{ + path::{Path, PathBuf}, + sync::OnceLock, +}; use anyhow::Context; use memofs::{IoResultExt, Vfs}; use serde::{Deserialize, Serialize}; -use crate::snapshot::{InstanceContext, InstanceSnapshot}; +use crate::glob::Glob; +use crate::snapshot::{InstanceContext, InstanceSnapshot, SyncRule}; use self::{ csv::{snapshot_csv, snapshot_csv_init}, @@ -37,11 +41,12 @@ use self::{ rbxmx::snapshot_rbxmx, toml::snapshot_toml, txt::snapshot_txt, - util::PathExt, }; pub use self::{project::snapshot_project_node, util::emit_legacy_scripts_default}; +static DEFAULT_SYNC_RULES: OnceLock> = OnceLock::new(); + /// Returns an `InstanceSnapshot` for the provided path. /// This will inspect the path and find the appropriate middleware for it, /// taking user-written rules into account. Then, it will attempt to convert @@ -59,23 +64,29 @@ pub fn snapshot_from_vfs( if meta.is_dir() { if let Some(init_path) = get_init_path(vfs, path)? { - match Middleware::from_path(context, &init_path) { - Some(Middleware::Project) => snapshot_project(context, vfs, &init_path), + // TODO: support user-defined init paths + for rule in default_sync_rules() { + if rule.matches(&init_path) { + return match rule.middleware { + Middleware::Project => snapshot_project(context, vfs, &init_path), - Some(Middleware::ModuleScript) => { - snapshot_lua_init(context, vfs, &init_path, ScriptType::Module) - } - Some(Middleware::ServerScript) => { - snapshot_lua_init(context, vfs, &init_path, ScriptType::Server) - } - Some(Middleware::ClientScript) => { - snapshot_lua_init(context, vfs, &init_path, ScriptType::Client) - } + Middleware::ModuleScript => { + snapshot_lua_init(context, vfs, &init_path, ScriptType::Module) + } + Middleware::ServerScript => { + snapshot_lua_init(context, vfs, &init_path, ScriptType::Server) + } + Middleware::ClientScript => { + snapshot_lua_init(context, vfs, &init_path, ScriptType::Client) + } - Some(Middleware::Csv) => snapshot_csv_init(context, vfs, &init_path), + Middleware::Csv => snapshot_csv_init(context, vfs, &init_path), - Some(_) | None => snapshot_dir(context, vfs, path), + _ => snapshot_dir(context, vfs, path), + }; + } } + snapshot_dir(context, vfs, path) } else { snapshot_dir(context, vfs, path) } @@ -85,6 +96,7 @@ pub fn snapshot_from_vfs( .and_then(|n| n.to_str()) .with_context(|| format!("file name of {} is invalid", path.display()))?; + // TODO: Is this even necessary anymore? match file_name { "init.server.luau" | "init.server.lua" | "init.client.luau" | "init.client.lua" | "init.luau" | "init.lua" | "init.csv" => return Ok(None), @@ -151,62 +163,23 @@ fn snapshot_from_path( vfs: &Vfs, path: &Path, ) -> anyhow::Result> { - let (middleware, name) = if let Some(rule) = context.get_user_sync_rule(path) { - (rule.middleware, rule.file_name_for_path(path)?) - } else if path.file_name_ends_with(".server.lua") { - ( - Middleware::ServerScript, - path.file_name_trim_end(".server.lua")?, - ) - } else if path.file_name_ends_with(".server.luau") { - ( - Middleware::ServerScript, - path.file_name_trim_end(".server.luau")?, - ) - } else if path.file_name_ends_with(".client.lua") { - ( - Middleware::ClientScript, - path.file_name_trim_end(".client.lua")?, - ) - } else if path.file_name_ends_with(".client.luau") { - ( - Middleware::ClientScript, - path.file_name_trim_end(".client.luau")?, - ) - } else if path.file_name_ends_with(".lua") { - (Middleware::ModuleScript, path.file_name_trim_end(".lua")?) - } else if path.file_name_ends_with(".luau") { - (Middleware::ModuleScript, path.file_name_trim_end(".luau")?) - } else if path.file_name_ends_with(".project.json") { - ( - Middleware::Project, - path.file_name_trim_end(".project.json")?, - ) - } else if path.file_name_ends_with(".model.json") { - ( - Middleware::JsonModel, - path.file_name_trim_end(".model.json")?, - ) - } else if path.file_name_ends_with(".meta.json") { - // .meta.json files do not turn into InstanceSnapshots - return Ok(None); - } else if path.file_name_ends_with(".json") { - (Middleware::Json, path.file_name_trim_end(".json")?) - } else if path.file_name_ends_with(".toml") { - (Middleware::Toml, path.file_name_trim_end(".toml")?) - } else if path.file_name_ends_with(".csv") { - (Middleware::Csv, path.file_name_trim_end(".csv")?) - } else if path.file_name_ends_with(".txt") { - (Middleware::Text, path.file_name_trim_end(".txt")?) - } else if path.file_name_ends_with(".rbxmx") { - (Middleware::Rbxmx, path.file_name_trim_end(".rbxmx")?) - } else if path.file_name_ends_with(".rbxm") { - (Middleware::Rbxm, path.file_name_trim_end(".rbxm")?) + if let Some(rule) = context.get_user_sync_rule(path) { + return rule + .middleware + .snapshot(context, vfs, path, rule.file_name_for_path(path)?); } else { - return Ok(None); - }; - - middleware.snapshot(context, vfs, path, name) + for rule in default_sync_rules() { + if rule.matches(path) { + return rule.middleware.snapshot( + context, + vfs, + path, + rule.file_name_for_path(path)?, + ); + } + } + } + Ok(None) } /// Represents a possible 'transformer' used by Rojo to turn a file system @@ -231,48 +204,6 @@ pub enum Middleware { } impl Middleware { - /// Returns a `Middleware` from the provided path, taking user-specified - /// syncing rules into account. If one exists, it is returned. Otherwise, - /// `None` is returned. - fn from_path>(context: &InstanceContext, path: P) -> Option { - let path = path.as_ref(); - - if let Some(rule) = context.get_user_sync_rule(path) { - Some(rule.middleware) - } else if path.file_name_ends_with(".server.lua") - || path.file_name_ends_with(".server.luau") - { - Some(Middleware::ServerScript) - } else if path.file_name_ends_with(".client.lua") - || path.file_name_ends_with(".client.luau") - { - Some(Middleware::ClientScript) - } else if path.file_name_ends_with(".lua") || path.file_name_ends_with(".luau") { - Some(Middleware::ModuleScript) - } else if path.file_name_ends_with(".project.json") { - Some(Middleware::Project) - } else if path.file_name_ends_with(".model.json") { - Some(Middleware::JsonModel) - } else if path.file_name_ends_with(".meta.json") { - // .meta.json files do not turn into their own instances. - None - } else if path.file_name_ends_with(".json") { - Some(Middleware::Json) - } else if path.file_name_ends_with(".toml") { - Some(Middleware::Toml) - } else if path.file_name_ends_with(".csv") { - Some(Middleware::Csv) - } else if path.file_name_ends_with(".txt") { - Some(Middleware::Text) - } else if path.file_name_ends_with(".rbxmx") { - Some(Middleware::Rbxmx) - } else if path.file_name_ends_with(".rbxm") { - Some(Middleware::Rbxm) - } else { - None - } - } - /// Creates a snapshot for the given path from the Middleware with /// the provided name. fn snapshot( @@ -300,3 +231,64 @@ impl Middleware { } } } + +/// A helper for easily defining a SyncRule. Arguments are passed literally +/// to this macro in the order `include`, `middleware`, `suffix`, +/// and `exclude`. Both `suffix` and `exclude` are optional. +/// +/// All arguments except `middleware` are expected to be strings. +/// The `middleware` parameter is expected to be a variant of `Middleware`, +/// not including the enum name itself. +macro_rules! sync_rule { + ($pattern:expr, $middleware:ident) => { + SyncRule { + middleware: Middleware::$middleware, + include: Glob::new($pattern).unwrap(), + exclude: None, + suffix: None, + base_path: PathBuf::new(), + } + }; + ($pattern:expr, $middleware:ident, $suffix:expr) => { + SyncRule { + middleware: Middleware::$middleware, + include: Glob::new($pattern).unwrap(), + exclude: None, + suffix: Some($suffix.into()), + base_path: PathBuf::new(), + } + }; + ($pattern:expr, $middleware:ident, $suffix:expr, $exclude:expr) => { + SyncRule { + middleware: Middleware::$middleware, + include: Glob::new($pattern).unwrap(), + exclude: Some(Glob::new($exclude).unwrap()), + suffix: Some($suffix.into()), + base_path: PathBuf::new(), + } + }; +} + +/// Defines the 'default' syncing rules that Rojo uses. +/// These do not broadly overlap, but the order matters for some in the case of +/// e.g. JSON models. +fn default_sync_rules() -> &'static [SyncRule] { + DEFAULT_SYNC_RULES.get_or_init(|| { + vec![ + sync_rule!("*.server.lua", ServerScript, ".server.lua"), + sync_rule!("*.server.luau", ServerScript, ".server.luau"), + sync_rule!("*.client.lua", ClientScript, ".client.lua"), + sync_rule!("*.client.luau", ClientScript, ".client.luau"), + sync_rule!("*.{lua,luau}", ModuleScript), + // Project middleware doesn't use the file name. + sync_rule!("*.project.json", Project), + sync_rule!("*.model.json", JsonModel, ".model.json"), + sync_rule!("*.json", Json, ".json", "*.meta.json"), + sync_rule!("*.toml", Toml), + sync_rule!("*.csv", Csv), + sync_rule!("*.txt", Text), + sync_rule!("*.rbxmx", Rbxmx), + sync_rule!("*.rbxm", Rbxm), + ] + }) +} From b980fb26157ddd21f534672540549b2f161a165a Mon Sep 17 00:00:00 2001 From: Micah Date: Tue, 7 Nov 2023 10:15:05 -0800 Subject: [PATCH 39/42] Mention `exclude` in changelog + mention nested behavior --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b23a06a69..d083322c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ { "pattern": "*.foo", "use": "text", + "exclude": "*.exclude.foo", }, { "pattern": "*.bar.baz", @@ -27,6 +28,8 @@ The `pattern` field is a glob used to match the sync rule to files. If present, the `suffix` field allows you to specify parts of a file's name get cut off by Rojo to name the Instance, including the file extension. If it isn't specified, Rojo will only cut off the first part of the file extension, up to the first dot. + Additionally, the `exclude` field allows files to be excluded from the sync rule if they match a pattern specified by it. If it's not present, all files that match `pattern` will be modified using the sync rule. + The `use` field corresponds to one of the potential file type that Rojo will currently include in a project. Files that match the provided pattern will be treated as if they had the file extension for that file type. A full list is below: | `use` value | file extension | @@ -44,6 +47,8 @@ | `project` | `.project.json` | | `ignore` | None! | + **All** sync rules are reset between project files, so they must be specified in each one when nesting them. This is to ensure that nothing can break other projects by changing how files are synced! + [#813]: https://github.com/rojo-rbx/rojo/pull/813 From 8aa9011a0e72037926a7799376798f6f0d86912f Mon Sep 17 00:00:00 2001 From: Micah Date: Wed, 8 Nov 2023 13:46:39 -0800 Subject: [PATCH 40/42] Place default sync rule static into function --- src/snapshot_middleware/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/snapshot_middleware/mod.rs b/src/snapshot_middleware/mod.rs index 3083e0508..30ea57061 100644 --- a/src/snapshot_middleware/mod.rs +++ b/src/snapshot_middleware/mod.rs @@ -45,8 +45,6 @@ use self::{ pub use self::{project::snapshot_project_node, util::emit_legacy_scripts_default}; -static DEFAULT_SYNC_RULES: OnceLock> = OnceLock::new(); - /// Returns an `InstanceSnapshot` for the provided path. /// This will inspect the path and find the appropriate middleware for it, /// taking user-written rules into account. Then, it will attempt to convert @@ -273,6 +271,8 @@ macro_rules! sync_rule { /// These do not broadly overlap, but the order matters for some in the case of /// e.g. JSON models. fn default_sync_rules() -> &'static [SyncRule] { + static DEFAULT_SYNC_RULES: OnceLock> = OnceLock::new(); + DEFAULT_SYNC_RULES.get_or_init(|| { vec![ sync_rule!("*.server.lua", ServerScript, ".server.lua"), From 96ab5cd589a0ba924cb330f50c36a8299fcf876a Mon Sep 17 00:00:00 2001 From: Micah Date: Mon, 13 Nov 2023 07:55:40 -0800 Subject: [PATCH 41/42] Use an iterator for `get_user_sync_rule` --- src/snapshot/metadata.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/snapshot/metadata.rs b/src/snapshot/metadata.rs index c76ec775f..1578af230 100644 --- a/src/snapshot/metadata.rs +++ b/src/snapshot/metadata.rs @@ -170,13 +170,7 @@ impl InstanceContext { /// Returns the middleware specified by the first sync rule that /// matches the provided path. This does not handle default syncing rules. pub fn get_user_sync_rule(&self, path: &Path) -> Option<&SyncRule> { - for rule in &self.sync_rules { - if rule.matches(path) { - return Some(rule); - } - } - - None + self.sync_rules.iter().find(|&rule| rule.matches(path)) } } From 61d34e91857bf951d0a7fd58f8b1f3c8f4d01c98 Mon Sep 17 00:00:00 2001 From: Micah Date: Tue, 16 Jan 2024 12:50:23 -0800 Subject: [PATCH 42/42] Fix typo made in github web view --- src/project.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/project.rs b/src/project.rs index a20f01eb0..1095fba20 100644 --- a/src/project.rs +++ b/src/project.rs @@ -8,7 +8,7 @@ use std::{ use serde::{Deserialize, Serialize}; use thiserror::Error; -use crate::{glob::Glob, resolution::UnresolvedValue. snapshot::SyncRule}; +use crate::{glob::Glob, resolution::UnresolvedValue, snapshot::SyncRule}; static PROJECT_FILENAME: &str = "default.project.json";