Update vscode-rojo submodule reference and complete migration plan for path encoding

- Updated the vscode-rojo submodule to the latest commit.
- Marked all phases of the path encoding migration plan as completed, transitioning to a slugify and deduplication approach.
- Updated documentation to reflect changes in filename handling and metadata management.
This commit is contained in:
Joe 2026-02-10 03:14:18 -06:00
parent 0955e38b45
commit 126c8e66a4
77 changed files with 3512 additions and 799 deletions

View file

@ -4,25 +4,25 @@ overview: "Remove the `%NAME%` path encoding system entirely and replace it with
todos: todos:
- id: phase1-infrastructure - id: phase1-infrastructure
content: "Manual port of PR #1187 infrastructure: specified_name on InstanceMetadata, slugify_name(), deduplicate_name(), name field on meta file structs, apply_name() methods, json_model name field support" content: "Manual port of PR #1187 infrastructure: specified_name on InstanceMetadata, slugify_name(), deduplicate_name(), name field on meta file structs, apply_name() methods, json_model name field support"
status: pending status: completed
- id: phase2-replace-encode-decode - id: phase2-replace-encode-decode
content: Replace all encode_path_name/decode_path_name callsites with slugify + dedup + name property approach in snapshot middleware, syncback/file_names.rs, syncback/snapshot.rs content: Replace all encode_path_name/decode_path_name callsites with slugify + dedup + name property approach in snapshot middleware, syncback/file_names.rs, syncback/snapshot.rs
status: pending status: completed
- id: phase3-two-way-sync - id: phase3-two-way-sync
content: "Replace encoding in two-way sync: change_processor.rs renames and web/api.rs added instances, with dedup checks" content: "Replace encoding in two-way sync: change_processor.rs renames and web/api.rs added instances, with dedup checks"
status: pending status: completed
- id: phase4-remove-config - id: phase4-remove-config
content: Remove decode_windows_invalid_chars from InstanceContext, encode_windows_invalid_chars from SyncbackRules, serve_session propagation, JSON schema content: Remove decode_windows_invalid_chars from InstanceContext, encode_windows_invalid_chars from SyncbackRules, serve_session propagation, JSON schema
status: pending status: completed
- id: phase5-delete-module - id: phase5-delete-module
content: Delete src/path_encoding.rs and remove pub mod declaration from lib.rs content: Delete src/path_encoding.rs and remove pub mod declaration from lib.rs
status: pending status: completed
- id: phase6-hardcore-tests - id: phase6-hardcore-tests
content: "Hardcore test suite: unit tests for slugify/dedup, integration tests for build/syncback/two-way-sync roundtrips, collision tests, edge case tests; update existing encoded pattern tests; regenerate snapshots" content: "Hardcore test suite: unit tests for slugify/dedup, integration tests for build/syncback/two-way-sync roundtrips, collision tests, edge case tests; update existing encoded pattern tests; regenerate snapshots"
status: pending status: completed
- id: phase7-update-docs - id: phase7-update-docs
content: Update cursor rules (.cursor/rules/rojo.mdc, src.mdc) to reflect new approach content: Update cursor rules (.cursor/rules/rojo.mdc, src.mdc) to reflect new approach
status: pending status: completed
isProject: false isProject: false
--- ---

View file

@ -210,10 +210,12 @@ snapshot.with_new_path(path, new_ref, old_ref)
``` ```
**Filename generation** (`file_names.rs`): **Filename generation** (`file_names.rs`):
- Uses instance name - Slugifies instance name (`slugify_name()`: forbidden chars → `_`)
- Deduplicates against taken names (`deduplicate_name()`: appends `~1`, `~2`, etc.)
- Adds appropriate extension based on middleware - Adds appropriate extension based on middleware
- Encodes Windows-invalid chars if enabled - Stores real name in `.meta.json5` `name` field when slug differs
- Example: `Script` named "My Script" → `My Script.server.luau` - Example: `Script` named "My Script" → `My Script.server.luau`
- Example: `Script` named "Hey/Bro" → `Hey_Bro.server.luau` + meta with `name: "Hey/Bro"`
### Property Filtering ### Property Filtering
@ -378,7 +380,7 @@ Filesystem:
- Solution: Use clean mode - Solution: Use clean mode
2. **Invalid filename** - Instance name contains invalid chars 2. **Invalid filename** - Instance name contains invalid chars
- Solution: Enable `decodeWindowsInvalidChars` or rename in Studio - Solution: Handled automatically by `slugify_name()` + metadata `name` field
3. **Ref to pruned instance** - Property points to instance not in project 3. **Ref to pruned instance** - Property points to instance not in project
- Logged as warning, property not synced - Logged as warning, property not synced

View file

@ -185,10 +185,11 @@ fn do_something() -> Result<()> {
pub struct InstanceContext { pub struct InstanceContext {
pub path_ignore_rules: Arc<Vec<PathIgnoreRule>>, pub path_ignore_rules: Arc<Vec<PathIgnoreRule>>,
pub sync_rules: Vec<SyncRule>, pub sync_rules: Vec<SyncRule>,
pub decode_windows_invalid_chars: bool,
} }
``` ```
**Filename handling:** Instance names with forbidden filesystem chars are slugified (forbidden chars → `_`), deduplicated (`~1`, `~2`, etc. on collision), and the real name is stored in `.meta.json5`/`.model.json5` `name` fields. See `src/syncback/file_names.rs`.
**VFS passed separately to snapshot functions:** **VFS passed separately to snapshot functions:**
```rust ```rust
snapshot_from_vfs(&context, &vfs, &path)? snapshot_from_vfs(&context, &vfs, &path)?

View file

@ -68,10 +68,16 @@ Transforms files → `InstanceSnapshot`. Main entry: `snapshot_from_vfs()`
pub struct InstanceContext { pub struct InstanceContext {
pub path_ignore_rules: Arc<Vec<PathIgnoreRule>>, pub path_ignore_rules: Arc<Vec<PathIgnoreRule>>,
pub sync_rules: Vec<SyncRule>, pub sync_rules: Vec<SyncRule>,
pub decode_windows_invalid_chars: bool, // Default: true
} }
``` ```
**Filename handling (syncback/two-way sync):**
Instance names with forbidden filesystem chars (`<>:"/\|?*`) are handled by:
1. `slugify_name()` -- replaces forbidden chars with `_` (pure, stateless)
2. `deduplicate_name()` -- appends `~1`, `~2`, etc. on collision (stateful)
3. Metadata `name` field -- stores the real instance name in `.meta.json5`/`.model.json5` when the slug differs
### 3. Commands (`src/cli/`) ### 3. Commands (`src/cli/`)
- `serve.rs` - Live sync with WebSocket - `serve.rs` - Live sync with WebSocket

View file

@ -1,5 +1,6 @@
--- ---
source: tests/tests/build.rs source: tests/tests/build.rs
assertion_line: 106
expression: contents expression: contents
--- ---
<roblox version="4"> <roblox version="4">
@ -9,7 +10,7 @@ expression: contents
</Properties> </Properties>
<Item class="Folder" referent="1"> <Item class="Folder" referent="1">
<Properties> <Properties>
<string name="Name">Expected Name</string> <string name="Name">Overridden Name</string>
</Properties> </Properties>
</Item> </Item>
</Item> </Item>

View file

@ -0,0 +1,3 @@
{
"name": "Key:Script"
}

View file

@ -1,6 +0,0 @@
{
"className": "Configuration",
"properties": {
"Name": "What?Model"
}
}

View file

@ -0,0 +1,4 @@
{
"name": "What?Model",
"className": "Configuration"
}

View file

@ -0,0 +1,3 @@
{
"name": "What?Module"
}

View file

@ -11,11 +11,11 @@ use std::{
use crate::{ use crate::{
message_queue::MessageQueue, message_queue::MessageQueue,
path_encoding::encode_path_name,
snapshot::{ snapshot::{
apply_patch_set, compute_patch_set, AppliedPatchSet, InstigatingSource, PatchSet, RojoTree, apply_patch_set, compute_patch_set, AppliedPatchSet, InstigatingSource, PatchSet, RojoTree,
}, },
snapshot_middleware::{snapshot_from_vfs, snapshot_project_node}, snapshot_middleware::{snapshot_from_vfs, snapshot_project_node},
syncback::{name_needs_slugify, slugify_name},
}; };
/// Processes file change events, updates the DOM, and sends those updates /// Processes file change events, updates the DOM, and sends those updates
@ -226,6 +226,42 @@ impl JobThreadContext {
} }
} }
/// Upsert the `name` field in a `.meta.json5` file. If the file exists,
/// parse it and add/update the `name` key. If it doesn't exist, create it
/// with just the `name` key.
fn upsert_meta_name_field(&self, meta_path: &Path, real_name: &str) {
self.suppress_path(meta_path);
let mut obj = if meta_path.exists() {
match fs::read(meta_path) {
Ok(bytes) => match crate::json::from_slice::<serde_json::Value>(&bytes) {
Ok(serde_json::Value::Object(map)) => map,
_ => serde_json::Map::new(),
},
Err(_) => serde_json::Map::new(),
}
} else {
serde_json::Map::new()
};
obj.insert(
"name".to_string(),
serde_json::Value::String(real_name.to_string()),
);
match crate::json::to_vec_pretty_sorted(&serde_json::Value::Object(obj)) {
Ok(content) => {
if let Err(err) = fs::write(meta_path, &content) {
log::error!(
"Failed to write name to meta file {}: {}",
meta_path.display(),
err
);
}
}
Err(err) => {
log::error!("Failed to serialize meta for name field: {}", err);
}
}
}
/// Computes and applies patches to the DOM for a given file path. /// Computes and applies patches to the DOM for a given file path.
/// ///
/// This function finds the nearest ancestor to the given path that has associated instances /// This function finds the nearest ancestor to the given path that has associated instances
@ -561,47 +597,27 @@ impl JobThreadContext {
let applied_patch = { let applied_patch = {
let mut tree = self.tree.lock().unwrap(); let mut tree = self.tree.lock().unwrap();
// NOTE: We do NOT delete files from disk here. The API handler
// (handle_api_write → syncback_removed_instance) already deleted
// the files before sending this PatchSet. Re-deleting would
// destroy any new file created at the same path — e.g., when an
// instance is renamed from "joe_test" to "joe/test", the new slug
// is also "joe_test", so the API creates a new file at the same
// path that the removal just deleted. If we deleted again here,
// the new file would be destroyed.
//
// The PatchSet removal only needs to update the DOM tree (handled
// by apply_patch_set below).
for &id in &patch_set.removed_instances { for &id in &patch_set.removed_instances {
if let Some(instance) = tree.get_instance(id) { if let Some(instance) = tree.get_instance(id) {
if let Some(instigating_source) = &instance.metadata().instigating_source { if let Some(instigating_source) = &instance.metadata().instigating_source {
match instigating_source { match instigating_source {
InstigatingSource::Path(path) => { InstigatingSource::Path(path) => {
// Guard: file may already be deleted by the API's log::info!(
// syncback_removed_instance before this PatchSet arrives. "Two-way sync: Removing instance {:?} from tree (path: {})",
if path.exists() { id,
if path.is_dir() { path.display()
log::info!( );
"Two-way sync: Removing directory {}",
path.display()
);
if let Err(err) = fs::remove_dir_all(path) {
log::error!(
"Failed to remove directory {:?} for instance {:?}: {}",
path,
id,
err
);
}
} else {
log::info!(
"Two-way sync: Removing file {}",
path.display()
);
if let Err(err) = fs::remove_file(path) {
log::error!(
"Failed to remove file {:?} for instance {:?}: {}",
path,
id,
err
);
}
}
} else {
log::info!(
"Two-way sync: File already removed: {}",
path.display()
);
}
} }
InstigatingSource::ProjectNode { .. } => { InstigatingSource::ProjectNode { .. } => {
log::warn!( log::warn!(
@ -653,17 +669,19 @@ impl JobThreadContext {
if is_init_file { if is_init_file {
let dir_path = path.parent().unwrap(); let dir_path = path.parent().unwrap();
if let Some(grandparent) = dir_path.parent() { if let Some(grandparent) = dir_path.parent() {
// Use the encoded dir name from the filesystem
// for the old meta lookup, and encode the new
// name for the new path. Using instance.name()
// (decoded) would fail for encoded names.
let dir_name = dir_path let dir_name = dir_path
.file_name() .file_name()
.and_then(|f| f.to_str()) .and_then(|f| f.to_str())
.unwrap_or(""); .unwrap_or("");
let encoded_new_name = encode_path_name(new_name); // Slugify the new name for filesystem safety
let slugified_new_name =
if name_needs_slugify(new_name) {
slugify_name(new_name)
} else {
new_name.to_string()
};
let new_dir_path = let new_dir_path =
grandparent.join(&encoded_new_name); grandparent.join(&slugified_new_name);
if new_dir_path != dir_path { if new_dir_path != dir_path {
log::info!( log::info!(
@ -696,7 +714,7 @@ impl JobThreadContext {
let new_meta = let new_meta =
grandparent.join(format!( grandparent.join(format!(
"{}.meta.json5", "{}.meta.json5",
encoded_new_name slugified_new_name
)); ));
self.suppress_path_any(&old_meta); self.suppress_path_any(&old_meta);
self.suppress_path(&new_meta); self.suppress_path(&new_meta);
@ -707,14 +725,22 @@ impl JobThreadContext {
self.unsuppress_path(&new_meta); self.unsuppress_path(&new_meta);
} }
} }
// Write name to init.meta.json5 if slug differs
if slugified_new_name != *new_name {
let init_meta = new_dir_path
.join("init.meta.json5");
self.upsert_meta_name_field(
&init_meta, new_name,
);
}
} }
} }
} }
} else if let Some(parent) = path.parent() { } else if let Some(parent) = path.parent() {
// Derive the suffix from the existing filename // Derive the suffix from the existing filename
// rather than using replacen with the decoded // rather than using replacen with the instance
// instance name, which fails for encoded names // name, which may differ from the filesystem
// (e.g., What%QUESTION%.server.luau). // name (e.g., slugified names).
let extension = path let extension = path
.extension() .extension()
.and_then(|e| e.to_str()) .and_then(|e| e.to_str())
@ -738,13 +764,18 @@ impl JobThreadContext {
&stem[..stem.len() - script_suffix.len()] &stem[..stem.len() - script_suffix.len()]
}; };
let encoded_new_name = encode_path_name(new_name); let slugified_new_name = if name_needs_slugify(new_name)
{
slugify_name(new_name)
} else {
new_name.to_string()
};
let new_file_name = if extension.is_empty() { let new_file_name = if extension.is_empty() {
format!("{}{}", encoded_new_name, script_suffix) format!("{}{}", slugified_new_name, script_suffix)
} else { } else {
format!( format!(
"{}{}.{}", "{}{}.{}",
encoded_new_name, script_suffix, extension slugified_new_name, script_suffix, extension
) )
}; };
let new_path = parent.join(&new_file_name); let new_path = parent.join(&new_file_name);
@ -770,11 +801,11 @@ impl JobThreadContext {
overridden_source_path = Some(new_path.clone()); overridden_source_path = Some(new_path.clone());
let old_meta = parent let old_meta = parent
.join(format!("{}.meta.json5", old_base)); .join(format!("{}.meta.json5", old_base));
let new_meta = parent.join(format!(
"{}.meta.json5",
slugified_new_name
));
if old_meta.exists() { if old_meta.exists() {
let new_meta = parent.join(format!(
"{}.meta.json5",
encoded_new_name
));
self.suppress_path_any(&old_meta); self.suppress_path_any(&old_meta);
self.suppress_path(&new_meta); self.suppress_path(&new_meta);
if fs::rename(&old_meta, &new_meta).is_err() if fs::rename(&old_meta, &new_meta).is_err()
@ -783,6 +814,12 @@ impl JobThreadContext {
self.unsuppress_path(&new_meta); self.unsuppress_path(&new_meta);
} }
} }
// Write name to adjacent meta if slug differs
if slugified_new_name != *new_name {
self.upsert_meta_name_field(
&new_meta, new_name,
);
}
} }
} }
} }
@ -861,10 +898,9 @@ impl JobThreadContext {
format!("init{}.luau", new_suffix) format!("init{}.luau", new_suffix)
} }
} else { } else {
// Derive the encoded base name from the // Derive the base name from the filesystem
// filesystem path, not instance.name() // path, not instance.name() (which may
// (which is decoded). This preserves // differ from the slugified filename).
// encoding like What%QUESTION% on disk.
let stem = actual_file let stem = actual_file
.file_stem() .file_stem()
.and_then(|s| s.to_str()) .and_then(|s| s.to_str())

View file

@ -13,7 +13,6 @@ mod json;
mod lua_ast; mod lua_ast;
mod message_queue; mod message_queue;
mod multimap; mod multimap;
pub mod path_encoding;
mod path_serializer; mod path_serializer;
mod project; mod project;
mod resolution; mod resolution;

View file

@ -1,342 +0,0 @@
//! Encoding and decoding utilities for special filename characters.
//!
//! This module encodes characters that are problematic in file names:
//! - Windows-invalid characters: < > : " / \ | ? *
//! - Periods (.) to prevent conflicts with file extension parsing
//! - Leading/trailing spaces
//! - Literal percent signs (%) to allow round-tripping
//!
//! Characters are encoded using a %NAME% format (e.g., `<` becomes `%LT%`,
//! `.` becomes `%DOT%`). Literal `%` is escaped as `%%` (like printf).
use std::collections::HashMap;
use std::sync::OnceLock;
/// Mapping of special characters to their encoded representations.
const CHAR_ENCODINGS: &[(&str, &str)] = &[
(".", "%DOT%"),
("<", "%LT%"),
(">", "%GT%"),
(":", "%COLON%"),
("\"", "%QUOTE%"),
("/", "%SLASH%"),
("\\", "%BACKSLASH%"),
("|", "%PIPE%"),
("?", "%QUESTION%"),
("*", "%STAR%"),
];
/// Returns a map from encoded pattern (e.g., "DOT") to decoded char (e.g., ".").
fn get_decode_map() -> &'static HashMap<&'static str, &'static str> {
static DECODE_MAP: OnceLock<HashMap<&'static str, &'static str>> = OnceLock::new();
DECODE_MAP.get_or_init(|| {
let mut map = HashMap::new();
for (char, encoded) in CHAR_ENCODINGS {
// Strip the leading and trailing % from the pattern
let pattern = &encoded[1..encoded.len() - 1];
map.insert(pattern, *char);
}
map
})
}
const SPACE_ENCODING: &str = "%SPACE%";
/// Encodes special characters in a file name.
///
/// This replaces characters like `.`, `<`, `>`, `:`, `"`, `/`, `\`, `|`, `?`, `*`
/// with their encoded representations like `%DOT%`, `%LT%`, `%GT%`, etc.
/// Literal `%` is escaped as `%%`. Leading and trailing spaces are encoded as `%SPACE%`.
pub fn encode_path_name(name: &str) -> String {
let trimmed = name.trim_matches(' ');
// Handle the case where the string is entirely spaces (or empty)
if trimmed.is_empty() {
return SPACE_ENCODING.repeat(name.len());
}
// Count leading and trailing spaces
let leading_spaces = name.len() - name.trim_start_matches(' ').len();
let trailing_spaces = name.len() - name.trim_end_matches(' ').len();
// Get the middle part (without leading/trailing spaces)
let middle = &name[leading_spaces..name.len() - trailing_spaces];
// First: escape % as %% (before adding new % signs from encodings)
let mut encoded_middle = middle.replace('%', "%%");
// Then: encode special characters
for (char, encoded) in CHAR_ENCODINGS {
encoded_middle = encoded_middle.replace(char, encoded);
}
// Build final result with encoded spaces
let encoded_prefix = SPACE_ENCODING.repeat(leading_spaces);
let encoded_suffix = SPACE_ENCODING.repeat(trailing_spaces);
format!("{}{}{}", encoded_prefix, encoded_middle, encoded_suffix)
}
/// Decodes encoded special characters back to their original form.
///
/// This replaces encoded representations like `%DOT%`, `%LT%`, `%GT%`, etc.
/// back to their original characters `.`, `<`, `>`, etc.
/// `%%` is decoded to literal `%`. `%SPACE%` at the start/end is decoded back to spaces.
///
/// Uses a proper left-to-right parser to correctly handle `%%` escaping.
pub fn decode_path_name(name: &str) -> String {
let mut result = name.to_string();
// Count and strip leading %SPACE%
let mut leading_spaces = 0;
while result.starts_with(SPACE_ENCODING) {
leading_spaces += 1;
result = result[SPACE_ENCODING.len()..].to_string();
}
// Count and strip trailing %SPACE%
let mut trailing_spaces = 0;
while result.ends_with(SPACE_ENCODING) {
trailing_spaces += 1;
result = result[..result.len() - SPACE_ENCODING.len()].to_string();
}
// Parse and decode the middle part
let decoded_middle = decode_patterns(&result);
// Rebuild with actual spaces
let prefix = " ".repeat(leading_spaces);
let suffix = " ".repeat(trailing_spaces);
format!("{}{}{}", prefix, decoded_middle, suffix)
}
/// Parses an encoded string left-to-right, properly handling `%%` escapes and `%NAME%` patterns.
fn decode_patterns(input: &str) -> String {
let decode_map = get_decode_map();
let chars: Vec<char> = input.chars().collect();
let mut output = String::with_capacity(input.len());
let mut i = 0;
while i < chars.len() {
if chars[i] == '%' {
// Check for %% (escaped percent)
if i + 1 < chars.len() && chars[i + 1] == '%' {
output.push('%');
i += 2;
continue;
}
// Try to match a %NAME% pattern
if let Some((decoded, consumed)) = try_decode_pattern(&chars, i, decode_map) {
output.push_str(&decoded);
i += consumed;
continue;
}
}
// Regular character, just copy it
output.push(chars[i]);
i += 1;
}
output
}
/// Tries to decode a %NAME% pattern starting at position `start`.
/// Returns Some((decoded_string, chars_consumed)) if successful, None otherwise.
fn try_decode_pattern(
chars: &[char],
start: usize,
decode_map: &HashMap<&str, &str>,
) -> Option<(String, usize)> {
// Must start with %
if chars.get(start) != Some(&'%') {
return None;
}
// Find the closing %
let mut end = start + 1;
while end < chars.len() && chars[end] != '%' {
// Pattern names are uppercase letters only
if !chars[end].is_ascii_uppercase() {
return None;
}
end += 1;
}
// Must have found a closing %
if end >= chars.len() || chars[end] != '%' {
return None;
}
// Extract the pattern name (without the % delimiters)
let pattern_name: String = chars[start + 1..end].iter().collect();
// Look up the pattern
if let Some(&decoded) = decode_map.get(pattern_name.as_str()) {
Some((decoded.to_string(), end - start + 1))
} else {
None
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn test_encode_all_chars() {
let input = r#".<>:"/\|?*"#;
let expected = "%DOT%%LT%%GT%%COLON%%QUOTE%%SLASH%%BACKSLASH%%PIPE%%QUESTION%%STAR%";
assert_eq!(encode_path_name(input), expected);
}
#[test]
fn test_decode_all_chars() {
let input = "%DOT%%LT%%GT%%COLON%%QUOTE%%SLASH%%BACKSLASH%%PIPE%%QUESTION%%STAR%";
let expected = r#".<>:"/\|?*"#;
assert_eq!(decode_path_name(input), expected);
}
#[test]
fn test_roundtrip() {
let original = "Test<File>With:Special\"Chars/And\\More|Stuff?Here*End";
let encoded = encode_path_name(original);
let decoded = decode_path_name(&encoded);
assert_eq!(decoded, original);
}
#[test]
fn test_no_special_chars() {
let input = "NormalFileName";
assert_eq!(encode_path_name(input), input);
assert_eq!(decode_path_name(input), input);
}
#[test]
fn test_leading_spaces() {
let input = " LeadingSpaces";
let encoded = encode_path_name(input);
assert_eq!(encoded, "%SPACE%%SPACE%LeadingSpaces");
assert_eq!(decode_path_name(&encoded), input);
}
#[test]
fn test_trailing_spaces() {
let input = "TrailingSpaces ";
let encoded = encode_path_name(input);
assert_eq!(encoded, "TrailingSpaces%SPACE%%SPACE%");
assert_eq!(decode_path_name(&encoded), input);
}
#[test]
fn test_both_spaces() {
let input = " Both Spaces ";
let encoded = encode_path_name(input);
assert_eq!(encoded, "%SPACE%Both Spaces%SPACE%");
assert_eq!(decode_path_name(&encoded), input);
}
#[test]
fn test_middle_spaces_unchanged() {
let input = "Middle Spaces Here";
let encoded = encode_path_name(input);
assert_eq!(encoded, "Middle Spaces Here");
assert_eq!(decode_path_name(&encoded), input);
}
#[test]
fn test_roundtrip_with_spaces() {
let original = " <Test> ";
let encoded = encode_path_name(original);
let decoded = decode_path_name(&encoded);
assert_eq!(decoded, original);
}
#[test]
fn test_encode_period() {
let input = "My.Script";
let expected = "My%DOT%Script";
assert_eq!(encode_path_name(input), expected);
}
#[test]
fn test_decode_period() {
let input = "My%DOT%Script";
let expected = "My.Script";
assert_eq!(decode_path_name(input), expected);
}
#[test]
fn test_roundtrip_with_period() {
let original = "My.Module.Name";
let encoded = encode_path_name(original);
let decoded = decode_path_name(&encoded);
assert_eq!(decoded, original);
}
#[test]
fn test_encode_percent() {
let input = "My%Thing";
let expected = "My%%Thing";
assert_eq!(encode_path_name(input), expected);
}
#[test]
fn test_decode_percent() {
let input = "My%%Thing";
let expected = "My%Thing";
assert_eq!(decode_path_name(input), expected);
}
#[test]
fn test_roundtrip_with_percent() {
let original = "My%Thing";
let encoded = encode_path_name(original);
let decoded = decode_path_name(&encoded);
assert_eq!(decoded, original);
}
#[test]
fn test_percent_encoding_pattern_roundtrip() {
// This tests that a name containing what looks like an encoding pattern
// round-trips correctly and doesn't get incorrectly decoded
let original = "My%DOT%Thing";
let encoded = encode_path_name(original);
assert_eq!(encoded, "My%%DOT%%Thing");
let decoded = decode_path_name(&encoded);
assert_eq!(decoded, original);
}
#[test]
fn test_combined_period_and_percent() {
let original = "My.%Thing.Other";
let encoded = encode_path_name(original);
let decoded = decode_path_name(&encoded);
assert_eq!(decoded, original);
}
#[test]
fn test_single_space() {
let input = " ";
let encoded = encode_path_name(input);
assert_eq!(encoded, "%SPACE%");
assert_eq!(decode_path_name(&encoded), input);
}
#[test]
fn test_multiple_spaces_only() {
let input = " ";
let encoded = encode_path_name(input);
assert_eq!(encoded, "%SPACE%%SPACE%%SPACE%");
assert_eq!(decode_path_name(&encoded), input);
}
#[test]
fn test_empty_string() {
let input = "";
let encoded = encode_path_name(input);
assert_eq!(encoded, "");
assert_eq!(decode_path_name(&encoded), input);
}
}

View file

@ -114,14 +114,7 @@ impl ServeSession {
let root_id = tree.get_root_id(); let root_id = tree.get_root_id();
let decode_windows_invalid_chars = root_project let instance_context = InstanceContext::new();
.syncback_rules
.as_ref()
.map(|rules| rules.encode_windows_invalid_chars())
.unwrap_or(true);
let mut instance_context = InstanceContext::new();
instance_context.set_decode_windows_invalid_chars(decode_windows_invalid_chars);
log::trace!("Generating snapshot of instances from VFS"); log::trace!("Generating snapshot of instances from VFS");
let snapshot = snapshot_from_vfs(&instance_context, &vfs, start_path)?; let snapshot = snapshot_from_vfs(&instance_context, &vfs, start_path)?;

View file

@ -66,6 +66,12 @@ pub struct InstanceMetadata {
/// A schema provided via a JSON file, if one exists. Will be `None` for /// A schema provided via a JSON file, if one exists. Will be `None` for
/// all non-JSON middleware. /// all non-JSON middleware.
pub schema: Option<String>, pub schema: Option<String>,
/// A custom name specified via meta.json or model.json files. If present,
/// this name will be used for the instance while the filesystem name will
/// be slugified to remove illegal characters.
#[serde(skip_serializing_if = "Option::is_none")]
pub specified_name: Option<String>,
} }
impl InstanceMetadata { impl InstanceMetadata {
@ -78,6 +84,7 @@ impl InstanceMetadata {
specified_id: None, specified_id: None,
middleware: None, middleware: None,
schema: None, schema: None,
specified_name: None,
} }
} }
@ -126,6 +133,13 @@ impl InstanceMetadata {
pub fn schema(self, schema: Option<String>) -> Self { pub fn schema(self, schema: Option<String>) -> Self {
Self { schema, ..self } Self { schema, ..self }
} }
pub fn specified_name(self, specified_name: Option<String>) -> Self {
Self {
specified_name,
..self
}
}
} }
impl Default for InstanceMetadata { impl Default for InstanceMetadata {
@ -140,16 +154,6 @@ pub struct InstanceContext {
pub path_ignore_rules: Arc<Vec<PathIgnoreRule>>, pub path_ignore_rules: Arc<Vec<PathIgnoreRule>>,
#[serde(skip_serializing_if = "Vec::is_empty")] #[serde(skip_serializing_if = "Vec::is_empty")]
pub sync_rules: Vec<SyncRule>, pub sync_rules: Vec<SyncRule>,
/// Whether to decode special characters in file names.
/// Characters like `%DOT%`, `%LT%`, `%GT%`, `%COLON%`, etc. will be decoded
/// back to `.`, `<`, `>`, `:`, etc. in instance names sent to the plugin.
/// Defaults to true.
#[serde(default = "decode_windows_invalid_chars_default")]
pub decode_windows_invalid_chars: bool,
}
fn decode_windows_invalid_chars_default() -> bool {
true
} }
impl InstanceContext { impl InstanceContext {
@ -157,14 +161,9 @@ impl InstanceContext {
Self { Self {
path_ignore_rules: Arc::new(Vec::new()), path_ignore_rules: Arc::new(Vec::new()),
sync_rules: Vec::new(), sync_rules: Vec::new(),
decode_windows_invalid_chars: true,
} }
} }
pub fn set_decode_windows_invalid_chars(&mut self, decode: bool) {
self.decode_windows_invalid_chars = decode;
}
/// Extend the list of ignore rules in the context with the given new rules. /// Extend the list of ignore rules in the context with the given new rules.
pub fn add_path_ignore_rules<I>(&mut self, new_rules: I) pub fn add_path_ignore_rules<I>(&mut self, new_rules: I)
where where

View file

@ -11,8 +11,7 @@ properties:
metadata: metadata:
ignore_unknown_instances: false ignore_unknown_instances: false
relevant_paths: [] relevant_paths: []
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -9,8 +9,7 @@ properties: {}
metadata: metadata:
ignore_unknown_instances: false ignore_unknown_instances: false
relevant_paths: [] relevant_paths: []
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -11,8 +11,7 @@ properties:
metadata: metadata:
ignore_unknown_instances: false ignore_unknown_instances: false
relevant_paths: [] relevant_paths: []
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -9,8 +9,7 @@ properties: {}
metadata: metadata:
ignore_unknown_instances: false ignore_unknown_instances: false
relevant_paths: [] relevant_paths: []
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -10,8 +10,7 @@ added_instances:
metadata: metadata:
ignore_unknown_instances: false ignore_unknown_instances: false
relevant_paths: [] relevant_paths: []
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -10,9 +10,8 @@ use rbx_dom_weak::{types::Variant, ustr};
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use crate::{ use crate::{
path_encoding::encode_path_name,
snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot}, snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot},
syncback::{FsSnapshot, SyncbackReturn, SyncbackSnapshot}, syncback::{name_needs_slugify, slugify_name, FsSnapshot, SyncbackReturn, SyncbackSnapshot},
}; };
use super::{ use super::{
@ -110,10 +109,11 @@ pub fn syncback_csv<'sync>(
if !meta.is_empty() { if !meta.is_empty() {
let parent = snapshot.path.parent_err()?; let parent = snapshot.path.parent_err()?;
let meta_name = if snapshot.encode_windows_invalid_chars() { let instance_name = &new_inst.name;
encode_path_name(&new_inst.name) let meta_name = if name_needs_slugify(instance_name) {
slugify_name(instance_name)
} else { } else {
new_inst.name.clone() instance_name.clone()
}; };
fs_snapshot.add_file( fs_snapshot.add_file(
parent.join(format!("{}.meta.json5", meta_name)), parent.join(format!("{}.meta.json5", meta_name)),

View file

@ -138,6 +138,25 @@ pub fn syncback_dir_no_meta<'sync>(
let mut children = Vec::new(); let mut children = Vec::new();
let mut removed_children = Vec::new(); let mut removed_children = Vec::new();
// Seed taken_names with existing files on disk so new children don't
// collide with files already present in the directory.
let mut taken_names: HashSet<String> = snapshot
.vfs()
.read_dir(&snapshot.path)
.ok()
.map(|entries| {
entries
.filter_map(|e| e.ok())
.filter_map(|e| {
e.path()
.file_name()
.and_then(|n| n.to_str())
.map(|n| n.to_lowercase())
})
.collect()
})
.unwrap_or_default();
// Detect duplicate child names (case-insensitive for file system safety). // Detect duplicate child names (case-insensitive for file system safety).
// We skip duplicates instead of failing, tracking them in stats. // We skip duplicates instead of failing, tracking them in stats.
let mut child_name_counts: HashMap<String, usize> = HashMap::new(); let mut child_name_counts: HashMap<String, usize> = HashMap::new();
@ -220,10 +239,23 @@ pub fn syncback_dir_no_meta<'sync>(
continue; continue;
} }
// This child exists in both doms. Pass it on. // This child exists in both doms. Pass it on.
children.push(snapshot.with_joined_path(*new_child_ref, Some(old_child.id()))?); let (child_snap, _needs_meta) = snapshot.with_joined_path(
*new_child_ref,
Some(old_child.id()),
&taken_names,
)?;
if let Some(name) = child_snap.path.file_name().and_then(|n| n.to_str()) {
taken_names.insert(name.to_lowercase());
}
children.push(child_snap);
} else { } else {
// The child only exists in the the new dom // The child only exists in the the new dom
children.push(snapshot.with_joined_path(*new_child_ref, None)?); let (child_snap, _needs_meta) =
snapshot.with_joined_path(*new_child_ref, None, &taken_names)?;
if let Some(name) = child_snap.path.file_name().and_then(|n| n.to_str()) {
taken_names.insert(name.to_lowercase());
}
children.push(child_snap);
} }
} }
// Any children that are in the old dom but not the new one are removed. // Any children that are in the old dom but not the new one are removed.
@ -267,7 +299,12 @@ pub fn syncback_dir_no_meta<'sync>(
if snapshot.should_ignore_tree(*new_child_ref) { if snapshot.should_ignore_tree(*new_child_ref) {
continue; continue;
} }
children.push(snapshot.with_joined_path(*new_child_ref, None)?); let (child_snap, _needs_meta) =
snapshot.with_joined_path(*new_child_ref, None, &taken_names)?;
if let Some(name) = child_snap.path.file_name().and_then(|n| n.to_str()) {
taken_names.insert(name.to_lowercase());
}
children.push(child_snap);
} }
} }
let mut fs_snapshot = FsSnapshot::new(); let mut fs_snapshot = FsSnapshot::new();

View file

@ -35,20 +35,14 @@ pub fn snapshot_json_model(
format!("File is not a valid JSON model: {}", path.display()) format!("File is not a valid JSON model: {}", path.display())
})?; })?;
if let Some(top_level_name) = &instance.name { // If the JSON has a name property, preserve it in metadata for syncback
let new_name = format!("{}.model.json5", top_level_name); let specified_name = instance.name.clone();
log::warn!( // Use the name from JSON if present, otherwise fall back to filename-derived name
"Model at path {} had a top-level Name field. \ if instance.name.is_none() {
This field has been ignored since Rojo 6.0.\n\ instance.name = Some(name.to_owned());
Consider removing this field and renaming the file to {}.",
new_name,
path.display()
);
} }
instance.name = Some(name.to_owned());
let id = instance.id.take().map(RojoRef::new); let id = instance.id.take().map(RojoRef::new);
let schema = instance.schema.take(); let schema = instance.schema.take();
@ -62,7 +56,8 @@ pub fn snapshot_json_model(
.relevant_paths(vec![vfs.canonicalize(path)?]) .relevant_paths(vec![vfs.canonicalize(path)?])
.context(context) .context(context)
.specified_id(id) .specified_id(id)
.schema(schema); .schema(schema)
.specified_name(specified_name);
Ok(Some(snapshot)) Ok(Some(snapshot))
} }
@ -81,6 +76,7 @@ pub fn syncback_json_model<'sync>(
// schemas will ever exist in one project for it to matter, but it // schemas will ever exist in one project for it to matter, but it
// could have a performance cost. // could have a performance cost.
model.schema = old_inst.metadata().schema.clone(); model.schema = old_inst.metadata().schema.clone();
model.name = old_inst.metadata().specified_name.clone();
} }
let serialized = match crate::json::to_vec_pretty_sorted(&model) { let serialized = match crate::json::to_vec_pretty_sorted(&model) {

View file

@ -8,9 +8,8 @@ use rbx_dom_weak::{
}; };
use crate::{ use crate::{
path_encoding::encode_path_name,
snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot}, snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot},
syncback::{FsSnapshot, SyncbackReturn, SyncbackSnapshot}, syncback::{name_needs_slugify, slugify_name, FsSnapshot, SyncbackReturn, SyncbackSnapshot},
}; };
use super::{ use super::{
@ -165,10 +164,11 @@ pub fn syncback_lua<'sync>(
.or_else(|| file_stem.strip_suffix(".legacy")) .or_else(|| file_stem.strip_suffix(".legacy"))
.unwrap_or(file_stem); .unwrap_or(file_stem);
let meta_name = if meta_name.is_empty() { let meta_name = if meta_name.is_empty() {
if snapshot.encode_windows_invalid_chars() { let instance_name = &new_inst.name;
encode_path_name(&new_inst.name) if name_needs_slugify(instance_name) {
slugify_name(instance_name)
} else { } else {
new_inst.name.clone() instance_name.clone()
} }
} else { } else {
meta_name.to_string() meta_name.to_string()

View file

@ -10,7 +10,10 @@ use rbx_dom_weak::{
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use crate::{ use crate::{
json, resolution::UnresolvedValue, snapshot::InstanceSnapshot, syncback::SyncbackSnapshot, json,
resolution::UnresolvedValue,
snapshot::InstanceSnapshot,
syncback::{name_needs_slugify, SyncbackSnapshot},
RojoRef, RojoRef,
}; };
@ -36,6 +39,9 @@ pub struct AdjacentMetadata {
#[serde(default, skip_serializing_if = "IndexMap::is_empty")] #[serde(default, skip_serializing_if = "IndexMap::is_empty")]
pub attributes: IndexMap<String, UnresolvedValue>, pub attributes: IndexMap<String, UnresolvedValue>,
#[serde(skip_serializing_if = "Option::is_none")]
pub name: Option<String>,
#[serde(skip)] #[serde(skip)]
pub path: PathBuf, pub path: PathBuf,
} }
@ -52,9 +58,8 @@ impl AdjacentMetadata {
_name: &str, _name: &str,
snapshot: &mut InstanceSnapshot, snapshot: &mut InstanceSnapshot,
) -> anyhow::Result<()> { ) -> anyhow::Result<()> {
// Use the file stem from the actual path (which has encoded chars) // Use the file stem from the actual path (which may be slugified)
// rather than the decoded name, so meta files are found correctly // rather than the instance name, so meta files are found correctly.
// when encodeWindowsInvalidChars is enabled.
// Strip known script type suffixes to get the base name for the meta file. // Strip known script type suffixes to get the base name for the meta file.
let file_stem = path.file_stem().and_then(|s| s.to_str()).unwrap_or(""); let file_stem = path.file_stem().and_then(|s| s.to_str()).unwrap_or("");
let base_name = file_stem let base_name = file_stem
@ -156,6 +161,24 @@ impl AdjacentMetadata {
} }
} }
let name = snapshot
.old_inst()
.and_then(|inst| inst.metadata().specified_name.clone())
.or_else(|| {
// If this is a new instance and its name is invalid for the filesystem,
// we need to specify the name in meta.json so it can be preserved
if snapshot.old_inst().is_none() {
let instance_name = &snapshot.new_inst().name;
if name_needs_slugify(instance_name) {
Some(instance_name.clone())
} else {
None
}
} else {
None
}
});
Ok(Some(Self { Ok(Some(Self {
ignore_unknown_instances: if ignore_unknown_instances { ignore_unknown_instances: if ignore_unknown_instances {
Some(true) Some(true)
@ -164,6 +187,7 @@ impl AdjacentMetadata {
}, },
properties, properties,
attributes, attributes,
name,
path, path,
id: None, id: None,
schema, schema,
@ -225,11 +249,26 @@ impl AdjacentMetadata {
Ok(()) Ok(())
} }
fn apply_name(&mut self, snapshot: &mut InstanceSnapshot) -> anyhow::Result<()> {
if self.name.is_some() && snapshot.metadata.specified_name.is_some() {
anyhow::bail!(
"cannot specify a name using {} (instance has a name from somewhere else)",
self.path.display()
);
}
if let Some(name) = &self.name {
snapshot.name = name.clone().into();
}
snapshot.metadata.specified_name = self.name.take();
Ok(())
}
pub fn apply_all(&mut self, snapshot: &mut InstanceSnapshot) -> anyhow::Result<()> { pub fn apply_all(&mut self, snapshot: &mut InstanceSnapshot) -> anyhow::Result<()> {
self.apply_ignore_unknown_instances(snapshot); self.apply_ignore_unknown_instances(snapshot);
self.apply_properties(snapshot)?; self.apply_properties(snapshot)?;
self.apply_id(snapshot)?; self.apply_id(snapshot)?;
self.apply_schema(snapshot)?; self.apply_schema(snapshot)?;
self.apply_name(snapshot)?;
Ok(()) Ok(())
} }
@ -238,11 +277,13 @@ impl AdjacentMetadata {
/// ///
/// - The number of properties and attributes is 0 /// - The number of properties and attributes is 0
/// - `ignore_unknown_instances` is None /// - `ignore_unknown_instances` is None
/// - `name` is None
#[inline] #[inline]
pub fn is_empty(&self) -> bool { pub fn is_empty(&self) -> bool {
self.attributes.is_empty() self.attributes.is_empty()
&& self.properties.is_empty() && self.properties.is_empty()
&& self.ignore_unknown_instances.is_none() && self.ignore_unknown_instances.is_none()
&& self.name.is_none()
} }
// TODO: Add method to allow selectively applying parts of metadata and // TODO: Add method to allow selectively applying parts of metadata and
@ -274,6 +315,9 @@ pub struct DirectoryMetadata {
#[serde(skip_serializing_if = "Option::is_none")] #[serde(skip_serializing_if = "Option::is_none")]
pub class_name: Option<Ustr>, pub class_name: Option<Ustr>,
#[serde(skip_serializing_if = "Option::is_none")]
pub name: Option<String>,
#[serde(skip)] #[serde(skip)]
pub path: PathBuf, pub path: PathBuf,
} }
@ -383,6 +427,24 @@ impl DirectoryMetadata {
} }
} }
let name = snapshot
.old_inst()
.and_then(|inst| inst.metadata().specified_name.clone())
.or_else(|| {
// If this is a new instance and its name is invalid for the filesystem,
// we need to specify the name in meta.json so it can be preserved
if snapshot.old_inst().is_none() {
let instance_name = &snapshot.new_inst().name;
if name_needs_slugify(instance_name) {
Some(instance_name.clone())
} else {
None
}
} else {
None
}
});
Ok(Some(Self { Ok(Some(Self {
ignore_unknown_instances: if ignore_unknown_instances { ignore_unknown_instances: if ignore_unknown_instances {
Some(true) Some(true)
@ -392,6 +454,7 @@ impl DirectoryMetadata {
properties, properties,
attributes, attributes,
class_name: None, class_name: None,
name,
path, path,
id: None, id: None,
schema, schema,
@ -404,6 +467,7 @@ impl DirectoryMetadata {
self.apply_properties(snapshot)?; self.apply_properties(snapshot)?;
self.apply_id(snapshot)?; self.apply_id(snapshot)?;
self.apply_schema(snapshot)?; self.apply_schema(snapshot)?;
self.apply_name(snapshot)?;
Ok(()) Ok(())
} }
@ -475,17 +539,34 @@ impl DirectoryMetadata {
snapshot.metadata.schema = self.schema.take(); snapshot.metadata.schema = self.schema.take();
Ok(()) Ok(())
} }
fn apply_name(&mut self, snapshot: &mut InstanceSnapshot) -> anyhow::Result<()> {
if self.name.is_some() && snapshot.metadata.specified_name.is_some() {
anyhow::bail!(
"cannot specify a name using {} (instance has a name from somewhere else)",
self.path.display()
);
}
if let Some(name) = &self.name {
snapshot.name = name.clone().into();
}
snapshot.metadata.specified_name = self.name.take();
Ok(())
}
/// Returns whether the metadata is 'empty', meaning it doesn't have anything /// Returns whether the metadata is 'empty', meaning it doesn't have anything
/// worth persisting in it. Specifically: /// worth persisting in it. Specifically:
/// ///
/// - The number of properties and attributes is 0 /// - The number of properties and attributes is 0
/// - `ignore_unknown_instances` is None /// - `ignore_unknown_instances` is None
/// - `class_name` is either None or not Some("Folder") /// - `class_name` is either None or not Some("Folder")
/// - `name` is None
#[inline] #[inline]
pub fn is_empty(&self) -> bool { pub fn is_empty(&self) -> bool {
self.attributes.is_empty() self.attributes.is_empty()
&& self.properties.is_empty() && self.properties.is_empty()
&& self.ignore_unknown_instances.is_none() && self.ignore_unknown_instances.is_none()
&& self.name.is_none()
&& if let Some(class) = &self.class_name { && if let Some(class) = &self.class_name {
class == "Folder" class == "Folder"
} else { } else {

View file

@ -30,7 +30,6 @@ use serde::{Deserialize, Serialize};
use crate::{ use crate::{
glob::Glob, glob::Glob,
path_encoding::decode_path_name,
project::DEFAULT_PROJECT_NAMES, project::DEFAULT_PROJECT_NAMES,
syncback::{SyncbackReturn, SyncbackSnapshot}, syncback::{SyncbackReturn, SyncbackSnapshot},
}; };
@ -72,19 +71,17 @@ pub fn snapshot_from_vfs(
if meta.is_dir() { if meta.is_dir() {
let (middleware, dir_name, init_path) = get_dir_middleware(vfs, path)?; let (middleware, dir_name, init_path) = get_dir_middleware(vfs, path)?;
// Decode directory name if the flag is set // The directory name is used as-is from the filesystem.
let decoded_name = if context.decode_windows_invalid_chars { // If a different instance name is desired, it comes from the
decode_path_name(dir_name) // `name` field in init.meta.json / init.meta.json5 (applied later
} else { // by DirectoryMetadata::apply_name).
dir_name.to_string()
};
// TODO: Support user defined init paths // TODO: Support user defined init paths
// If and when we do, make sure to go support it in // If and when we do, make sure to go support it in
// `Project::set_file_name`, as right now it special-cases // `Project::set_file_name`, as right now it special-cases
// `default.project.json5` as an `init` path. // `default.project.json5` as an `init` path.
match middleware { match middleware {
Middleware::Dir => middleware.snapshot(context, vfs, path, &decoded_name), Middleware::Dir => middleware.snapshot(context, vfs, path, dir_name),
_ => middleware.snapshot(context, vfs, &init_path, &decoded_name), _ => middleware.snapshot(context, vfs, &init_path, dir_name),
} }
} else { } else {
let file_name = path let file_name = path
@ -166,23 +163,17 @@ fn snapshot_from_path(
vfs: &Vfs, vfs: &Vfs,
path: &Path, path: &Path,
) -> anyhow::Result<Option<InstanceSnapshot>> { ) -> anyhow::Result<Option<InstanceSnapshot>> {
// Helper to decode name if the flag is set // File names are used as-is from the filesystem. If a different instance
let decode_name = |name: &str| -> String { // name is needed (e.g. for names with special chars), it comes from the
if context.decode_windows_invalid_chars { // `name` field in adjacent .meta.json / .model.json files.
decode_path_name(name)
} else {
name.to_string()
}
};
if let Some(rule) = context.get_user_sync_rule(path) { if let Some(rule) = context.get_user_sync_rule(path) {
let name = decode_name(rule.file_name_for_path(path)?); let name = rule.file_name_for_path(path)?;
return rule.middleware.snapshot(context, vfs, path, &name); return rule.middleware.snapshot(context, vfs, path, name);
} else { } else {
for rule in default_sync_rules() { for rule in default_sync_rules() {
if rule.matches(path) { if rule.matches(path) {
let name = decode_name(rule.file_name_for_path(path)?); let name = rule.file_name_for_path(path)?;
return rule.middleware.snapshot(context, vfs, path, &name); return rule.middleware.snapshot(context, vfs, path, name);
} }
} }
} }

View file

@ -647,6 +647,14 @@ pub fn syncback_project<'sync>(
// All of the children in this loop are by their nature not in the // All of the children in this loop are by their nature not in the
// project, so we just need to run syncback on them. // project, so we just need to run syncback on them.
//
// Track taken names per parent directory so siblings under the same
// parent deduplicate against each other (and against existing files).
let mut taken_names_per_dir: std::collections::HashMap<
std::path::PathBuf,
std::collections::HashSet<String>,
> = std::collections::HashMap::new();
for (name, new_child) in new_child_map.drain() { for (name, new_child) in new_child_map.drain() {
// Skip instances of ignored classes // Skip instances of ignored classes
if snapshot.should_ignore_class(&new_child.class) { if snapshot.should_ignore_class(&new_child.class) {
@ -680,11 +688,38 @@ pub fn syncback_project<'sync>(
// concern with directories because they're singular things, // concern with directories because they're singular things,
// files that contain their own children. // files that contain their own children.
if parent_middleware != Middleware::Project { if parent_middleware != Middleware::Project {
descendant_snapshots.push(snapshot.with_base_path( let taken_names = taken_names_per_dir
.entry(parent_path.clone())
.or_insert_with(|| {
// Seed with existing files on disk so new children
// don't collide with files already present.
vfs.read_dir(&parent_path)
.ok()
.map(|entries| {
entries
.filter_map(|e| e.ok())
.filter_map(|e| {
e.path()
.file_name()
.and_then(|n| n.to_str())
.map(|n| n.to_lowercase())
})
.collect()
})
.unwrap_or_default()
});
let (child_snap, _needs_meta) = snapshot.with_base_path(
&parent_path, &parent_path,
new_child.referent(), new_child.referent(),
None, None,
)?); taken_names,
)?;
if let Some(file_name) =
child_snap.path.file_name().and_then(|n| n.to_str())
{
taken_names.insert(file_name.to_lowercase());
}
descendant_snapshots.push(child_snap);
} }
} }
} }

View file

@ -10,8 +10,7 @@ metadata:
relevant_paths: relevant_paths:
- /foo.csv - /foo.csv
- /foo.meta.json5 - /foo.meta.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -19,8 +19,7 @@ metadata:
- /root/init.server.lua - /root/init.server.lua
- /root/init.client.lua - /root/init.client.lua
- /root/init.meta.json5 - /root/init.meta.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -19,8 +19,7 @@ metadata:
- /root/init.server.lua - /root/init.server.lua
- /root/init.client.lua - /root/init.client.lua
- /root/init.meta.json5 - /root/init.meta.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: manually specified specified_id: manually specified
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -10,8 +10,7 @@ metadata:
relevant_paths: relevant_paths:
- /foo.csv - /foo.csv
- /foo.meta.json5 - /foo.meta.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -19,8 +19,7 @@ metadata:
- /foo/init.server.lua - /foo/init.server.lua
- /foo/init.client.lua - /foo/init.client.lua
- /foo/init.meta.json5 - /foo/init.meta.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -19,8 +19,7 @@ metadata:
- /foo/init.server.lua - /foo/init.server.lua
- /foo/init.client.lua - /foo/init.client.lua
- /foo/init.meta.json5 - /foo/init.meta.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~
@ -45,8 +44,7 @@ children:
- /foo/Child/init.server.lua - /foo/Child/init.server.lua
- /foo/Child/init.client.lua - /foo/Child/init.client.lua
- /foo/Child/init.meta.json5 - /foo/Child/init.meta.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: dir middleware: dir
schema: ~ schema: ~

View file

@ -10,8 +10,7 @@ metadata:
relevant_paths: relevant_paths:
- /foo.json5 - /foo.json5
- /foo.meta.json5 - /foo.meta.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -10,8 +10,7 @@ metadata:
relevant_paths: relevant_paths:
- /foo.json5 - /foo.json5
- /foo.meta.json5 - /foo.meta.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: manually specified specified_id: manually specified
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -9,8 +9,7 @@ metadata:
Path: /foo.model.json5 Path: /foo.model.json5
relevant_paths: relevant_paths:
- /foo.model.json5 - /foo.model.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~
@ -24,8 +23,7 @@ children:
metadata: metadata:
ignore_unknown_instances: false ignore_unknown_instances: false
relevant_paths: [] relevant_paths: []
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -9,8 +9,7 @@ metadata:
Path: /foo.model.json5 Path: /foo.model.json5
relevant_paths: relevant_paths:
- /foo.model.json5 - /foo.model.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~
@ -24,8 +23,7 @@ children:
metadata: metadata:
ignore_unknown_instances: false ignore_unknown_instances: false
relevant_paths: [] relevant_paths: []
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -10,8 +10,7 @@ metadata:
relevant_paths: relevant_paths:
- /foo.client.luau - /foo.client.luau
- /foo.meta.json5 - /foo.meta.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -19,8 +19,7 @@ metadata:
- /root/init.server.lua - /root/init.server.lua
- /root/init.client.lua - /root/init.client.lua
- /root/init.meta.json5 - /root/init.meta.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -19,8 +19,7 @@ metadata:
- /root/init.server.lua - /root/init.server.lua
- /root/init.client.lua - /root/init.client.lua
- /root/init.meta.json5 - /root/init.meta.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: manually specified specified_id: manually specified
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -10,8 +10,7 @@ metadata:
relevant_paths: relevant_paths:
- /foo.legacy.luau - /foo.legacy.luau
- /foo.meta.json5 - /foo.meta.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -10,8 +10,7 @@ metadata:
relevant_paths: relevant_paths:
- /foo.local.luau - /foo.local.luau
- /foo.meta.json5 - /foo.meta.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -10,8 +10,7 @@ metadata:
relevant_paths: relevant_paths:
- /foo.luau - /foo.luau
- /foo.meta.json5 - /foo.meta.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -10,8 +10,7 @@ metadata:
relevant_paths: relevant_paths:
- /foo.luau - /foo.luau
- /foo.meta.json5 - /foo.meta.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -10,8 +10,7 @@ metadata:
relevant_paths: relevant_paths:
- /foo.plugin.luau - /foo.plugin.luau
- /foo.meta.json5 - /foo.meta.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -10,8 +10,7 @@ metadata:
relevant_paths: relevant_paths:
- /bar.server.luau - /bar.server.luau
- /bar.meta.json5 - /bar.meta.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -10,8 +10,7 @@ metadata:
relevant_paths: relevant_paths:
- /foo.server.luau - /foo.server.luau
- /foo.meta.json5 - /foo.meta.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -10,8 +10,7 @@ metadata:
relevant_paths: relevant_paths:
- /foo.server.luau - /foo.server.luau
- /foo.meta.json5 - /foo.meta.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -9,8 +9,7 @@ metadata:
- /foo/bar.meta.json - /foo/bar.meta.json
- /foo/bar.meta.jsonc - /foo/bar.meta.jsonc
- /foo/bar.meta.json5 - /foo/bar.meta.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: manually specified specified_id: manually specified
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -7,8 +7,7 @@ metadata:
ignore_unknown_instances: false ignore_unknown_instances: false
relevant_paths: relevant_paths:
- /foo/bar.meta.json5 - /foo/bar.meta.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: manually specified specified_id: manually specified
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -9,8 +9,7 @@ metadata:
- /foo/bar.meta.json - /foo/bar.meta.json
- /foo/bar.meta.jsonc - /foo/bar.meta.jsonc
- /foo/bar.meta.json5 - /foo/bar.meta.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: manually specified specified_id: manually specified
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -9,8 +9,7 @@ metadata:
- /foo/init.meta.json - /foo/init.meta.json
- /foo/init.meta.jsonc - /foo/init.meta.jsonc
- /foo/init.meta.json5 - /foo/init.meta.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: manually specified specified_id: manually specified
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -7,8 +7,7 @@ metadata:
ignore_unknown_instances: false ignore_unknown_instances: false
relevant_paths: relevant_paths:
- /foo/init.meta.json5 - /foo/init.meta.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: manually specified specified_id: manually specified
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -9,8 +9,7 @@ metadata:
- /foo/init.meta.json - /foo/init.meta.json
- /foo/init.meta.jsonc - /foo/init.meta.jsonc
- /foo/init.meta.json5 - /foo/init.meta.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: manually specified specified_id: manually specified
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -9,8 +9,7 @@ metadata:
Path: /foo/default.project.json5 Path: /foo/default.project.json5
relevant_paths: relevant_paths:
- /foo/default.project.json5 - /foo/default.project.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -9,8 +9,7 @@ metadata:
Path: /foo/hello.project.json5 Path: /foo/hello.project.json5
relevant_paths: relevant_paths:
- /foo/hello.project.json5 - /foo/hello.project.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -10,8 +10,7 @@ metadata:
relevant_paths: relevant_paths:
- /foo/other.project.json5 - /foo/other.project.json5
- /foo/default.project.json5 - /foo/default.project.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: project middleware: project
schema: ~ schema: ~

View file

@ -9,8 +9,7 @@ metadata:
Path: /foo.project.json5 Path: /foo.project.json5
relevant_paths: relevant_paths:
- /foo.project.json5 - /foo.project.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~
@ -29,8 +28,7 @@ children:
$className: Model $className: Model
parent_class: Folder parent_class: Folder
relevant_paths: [] relevant_paths: []
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -10,8 +10,7 @@ metadata:
relevant_paths: relevant_paths:
- /foo/other.project.json5 - /foo/other.project.json5
- /foo/default.project.json5 - /foo/default.project.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: project middleware: project
schema: ~ schema: ~

View file

@ -10,8 +10,7 @@ metadata:
relevant_paths: relevant_paths:
- /foo/other.project.json5 - /foo/other.project.json5
- /foo/default.project.json5 - /foo/default.project.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: project middleware: project
schema: ~ schema: ~
@ -30,8 +29,7 @@ children:
$className: Model $className: Model
parent_class: Folder parent_class: Folder
relevant_paths: [] relevant_paths: []
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -11,8 +11,7 @@ metadata:
- /foo/other.txt - /foo/other.txt
- /foo/other.meta.json5 - /foo/other.meta.json5
- /foo/default.project.json5 - /foo/default.project.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: text middleware: text
schema: ~ schema: ~

View file

@ -9,8 +9,7 @@ metadata:
Path: /foo.project.json5 Path: /foo.project.json5
relevant_paths: relevant_paths:
- /foo.project.json5 - /foo.project.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -9,8 +9,7 @@ metadata:
Path: /foo.project.json5 Path: /foo.project.json5
relevant_paths: relevant_paths:
- /foo.project.json5 - /foo.project.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -10,8 +10,7 @@ metadata:
relevant_paths: relevant_paths:
- /foo.toml - /foo.toml
- /foo.meta.json5 - /foo.meta.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -10,8 +10,7 @@ metadata:
relevant_paths: relevant_paths:
- foo.toml - foo.toml
- foo.meta.json5 - foo.meta.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: manually specified specified_id: manually specified
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -10,8 +10,7 @@ metadata:
relevant_paths: relevant_paths:
- /foo.txt - /foo.txt
- /foo.meta.json5 - /foo.meta.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -10,8 +10,7 @@ metadata:
relevant_paths: relevant_paths:
- /foo.txt - /foo.txt
- /foo.meta.json5 - /foo.meta.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: manually specified specified_id: manually specified
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -10,8 +10,7 @@ metadata:
relevant_paths: relevant_paths:
- /foo.yaml - /foo.yaml
- /foo.meta.json5 - /foo.meta.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: ~ specified_id: ~
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -10,8 +10,7 @@ metadata:
relevant_paths: relevant_paths:
- foo.yaml - foo.yaml
- foo.meta.json5 - foo.meta.json5
context: context: {}
decode_windows_invalid_chars: true
specified_id: manually specified specified_id: manually specified
middleware: ~ middleware: ~
schema: ~ schema: ~

View file

@ -6,9 +6,8 @@ use rbx_dom_weak::types::Variant;
use rbx_dom_weak::ustr; use rbx_dom_weak::ustr;
use crate::{ use crate::{
path_encoding::encode_path_name,
snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot}, snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot},
syncback::{FsSnapshot, SyncbackReturn, SyncbackSnapshot}, syncback::{name_needs_slugify, slugify_name, FsSnapshot, SyncbackReturn, SyncbackSnapshot},
}; };
use super::{meta_file::AdjacentMetadata, PathExt as _}; use super::{meta_file::AdjacentMetadata, PathExt as _};
@ -59,10 +58,11 @@ pub fn syncback_txt<'sync>(
if !meta.is_empty() { if !meta.is_empty() {
let parent = snapshot.path.parent_err()?; let parent = snapshot.path.parent_err()?;
let meta_name = if snapshot.encode_windows_invalid_chars() { let instance_name = &new_inst.name;
encode_path_name(&new_inst.name) let meta_name = if name_needs_slugify(instance_name) {
slugify_name(instance_name)
} else { } else {
new_inst.name.clone() instance_name.clone()
}; };
fs_snapshot.add_file( fs_snapshot.add_file(
parent.join(format!("{}.meta.json5", meta_name)), parent.join(format!("{}.meta.json5", meta_name)),

File diff suppressed because it is too large Load diff

View file

@ -29,7 +29,10 @@ use crate::{
Project, Project,
}; };
pub use file_names::{extension_for_middleware, name_for_inst, validate_file_name}; pub use file_names::{
deduplicate_name, extension_for_middleware, name_for_inst, name_needs_slugify, slugify_name,
validate_file_name,
};
pub use fs_snapshot::FsSnapshot; pub use fs_snapshot::FsSnapshot;
pub use hash::*; pub use hash::*;
pub use property_filter::{ pub use property_filter::{
@ -996,12 +999,6 @@ pub struct SyncbackRules {
/// generally a better UX. /// generally a better UX.
#[serde(skip_serializing_if = "Option::is_none")] #[serde(skip_serializing_if = "Option::is_none")]
create_ignore_dir_paths: Option<bool>, create_ignore_dir_paths: Option<bool>,
/// Whether to encode Windows-invalid characters in file names during
/// syncback. Characters like `<`, `>`, `:`, `"`, `/`, `\`, `|`, `?`, `*`
/// will be encoded as `%LT%`, `%GT%`, `%COLON%`, etc.
/// Defaults to `true`.
#[serde(skip_serializing_if = "Option::is_none")]
encode_windows_invalid_chars: Option<bool>,
/// When enabled, only "visible" services will be synced back. This includes /// When enabled, only "visible" services will be synced back. This includes
/// commonly used services like Workspace, ReplicatedStorage, ServerScriptService, /// commonly used services like Workspace, ReplicatedStorage, ServerScriptService,
/// etc., while ignoring internal/hidden services like Chat, HttpService, etc. /// etc., while ignoring internal/hidden services like Chat, HttpService, etc.
@ -1054,13 +1051,6 @@ impl SyncbackRules {
Ok(globs) Ok(globs)
} }
/// Returns whether special characters (including periods) should be encoded
/// in file names during syncback. Defaults to `true`.
#[inline]
pub fn encode_windows_invalid_chars(&self) -> bool {
self.encode_windows_invalid_chars.unwrap_or(true)
}
/// Returns whether hidden/internal services should be ignored during /// Returns whether hidden/internal services should be ignored during
/// syncback. When `true`, only visible services like Workspace, /// syncback. When `true`, only visible services like Workspace,
/// ReplicatedStorage, etc. will be synced. Defaults to `true`. /// ReplicatedStorage, etc. will be synced. Defaults to `true`.

View file

@ -1,5 +1,6 @@
use indexmap::IndexMap; use indexmap::IndexMap;
use memofs::Vfs; use memofs::Vfs;
use std::collections::HashSet;
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
use crate::{ use crate::{
@ -56,7 +57,12 @@ impl<'sync> SyncbackSnapshot<'sync> {
/// In clean mode (incremental=false), old_ref is ignored and set to None, /// In clean mode (incremental=false), old_ref is ignored and set to None,
/// ensuring fresh filenames and middleware choices for all children. /// ensuring fresh filenames and middleware choices for all children.
#[inline] #[inline]
pub fn with_joined_path(&self, new_ref: Ref, old_ref: Option<Ref>) -> anyhow::Result<Self> { pub fn with_joined_path(
&self,
new_ref: Ref,
old_ref: Option<Ref>,
taken_names: &HashSet<String>,
) -> anyhow::Result<(Self, bool)> {
// In clean mode, ignore old_ref to ensure fresh structure // In clean mode, ignore old_ref to ensure fresh structure
let effective_old_ref = if self.data.incremental { old_ref } else { None }; let effective_old_ref = if self.data.incremental { old_ref } else { None };
let mut snapshot = Self { let mut snapshot = Self {
@ -67,16 +73,15 @@ impl<'sync> SyncbackSnapshot<'sync> {
middleware: None, middleware: None,
}; };
let middleware = get_best_middleware(&snapshot); let middleware = get_best_middleware(&snapshot);
let encode_invalid_chars = self.encode_windows_invalid_chars(); let (name, needs_meta_name) = name_for_inst(
let name = name_for_inst(
middleware, middleware,
snapshot.new_inst(), snapshot.new_inst(),
snapshot.old_inst(), snapshot.old_inst(),
encode_invalid_chars, taken_names,
)?; )?;
snapshot.path = self.path.join(name.as_ref()); snapshot.path = self.path.join(&*name);
Ok(snapshot) Ok((snapshot, needs_meta_name))
} }
/// Constructs a SyncbackSnapshot from the provided refs and a base path, /// Constructs a SyncbackSnapshot from the provided refs and a base path,
@ -93,7 +98,8 @@ impl<'sync> SyncbackSnapshot<'sync> {
base_path: &Path, base_path: &Path,
new_ref: Ref, new_ref: Ref,
old_ref: Option<Ref>, old_ref: Option<Ref>,
) -> anyhow::Result<Self> { taken_names: &HashSet<String>,
) -> anyhow::Result<(Self, bool)> {
// In clean mode, ignore old_ref to ensure fresh structure // In clean mode, ignore old_ref to ensure fresh structure
let effective_old_ref = if self.data.incremental { old_ref } else { None }; let effective_old_ref = if self.data.incremental { old_ref } else { None };
let mut snapshot = Self { let mut snapshot = Self {
@ -104,16 +110,15 @@ impl<'sync> SyncbackSnapshot<'sync> {
middleware: None, middleware: None,
}; };
let middleware = get_best_middleware(&snapshot); let middleware = get_best_middleware(&snapshot);
let encode_invalid_chars = self.encode_windows_invalid_chars(); let (name, needs_meta_name) = name_for_inst(
let name = name_for_inst(
middleware, middleware,
snapshot.new_inst(), snapshot.new_inst(),
snapshot.old_inst(), snapshot.old_inst(),
encode_invalid_chars, taken_names,
)?; )?;
snapshot.path = base_path.join(name.as_ref()); snapshot.path = base_path.join(&*name);
Ok(snapshot) Ok((snapshot, needs_meta_name))
} }
/// Constructs a SyncbackSnapshot with the provided path and refs while /// Constructs a SyncbackSnapshot with the provided path and refs while
@ -319,18 +324,6 @@ impl<'sync> SyncbackSnapshot<'sync> {
.unwrap_or_default() .unwrap_or_default()
} }
/// Returns whether Windows-invalid characters should be encoded in file
/// names during syncback. Defaults to `true`.
#[inline]
pub fn encode_windows_invalid_chars(&self) -> bool {
self.data
.project
.syncback_rules
.as_ref()
.map(|rules| rules.encode_windows_invalid_chars())
.unwrap_or(true)
}
/// Returns whether to emit warnings when duplicate child names are /// Returns whether to emit warnings when duplicate child names are
/// encountered during syncback. Defaults to `false` (suppressed). /// encountered during syncback. Defaults to `false` (suppressed).
#[inline] #[inline]

File diff suppressed because it is too large Load diff

View file

@ -1232,28 +1232,31 @@ fn get_encoded_names_instance(
} }
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
// Tests: Encoded name directory conversion (path encoding fix) // Tests: Slugified name directory conversion (path encoding fix)
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
/// Test 22: Adding a child to a standalone ModuleScript with Rojo-encoded /// Test 22: Adding a child to a standalone ModuleScript with slugified
/// characters (e.g., `What%QUESTION%Module.luau`) creates the directory using /// characters (e.g., `What_Module.luau`) creates the directory using
/// the filesystem-encoded name (`What%QUESTION%Module/`), NOT the decoded /// the slugified filesystem name (`What_Module/`), NOT the decoded
/// instance name (`What?Module/`). /// instance name (`What?Module/`).
/// ///
/// This is the core regression test for the path encoding fix in /// This is the core regression test for the slugified name fix in
/// `convert_standalone_script_to_directory`. /// `convert_standalone_script_to_directory`.
#[test] #[test]
fn add_child_to_encoded_module_creates_encoded_directory() { fn add_child_to_encoded_module_creates_encoded_directory() {
run_serve_test("syncback_encoded_names", |session, _redactions| { run_serve_test("syncback_encoded_names", |session, _redactions| {
let (session_id, encoded_module_id) = get_encoded_names_instance(&session, "What?Module"); let (session_id, encoded_module_id) = get_encoded_names_instance(&session, "What?Module");
let standalone_file = session.path().join("src").join("What%QUESTION%Module.luau"); let standalone_file = session.path().join("src").join("What_Module.luau");
// The CORRECT directory name uses the encoded filesystem name // The CORRECT directory name uses the slugified filesystem name
let encoded_dir = session.path().join("src").join("What%QUESTION%Module"); let encoded_dir = session.path().join("src").join("What_Module");
// The WRONG directory name would use the decoded instance name // The WRONG directory name would use the decoded instance name
let decoded_dir = session.path().join("src").join("What?Module"); let decoded_dir = session.path().join("src").join("What?Module");
assert_file_exists(&standalone_file, "Encoded standalone file before child add"); assert_file_exists(
&standalone_file,
"Slugified standalone file before child add",
);
// Add a child to trigger standalone → directory conversion // Add a child to trigger standalone → directory conversion
let mut properties = HashMap::new(); let mut properties = HashMap::new();
@ -1285,10 +1288,10 @@ fn add_child_to_encoded_module_creates_encoded_directory() {
"Standalone file after directory conversion", "Standalone file after directory conversion",
); );
// Directory should use ENCODED name (the fix) // Directory should use SLUGIFIED name (the fix)
assert!( assert!(
encoded_dir.is_dir(), encoded_dir.is_dir(),
"Directory should use encoded name (What%QUESTION%Module/): {}", "Directory should use slugified name (What_Module/): {}",
encoded_dir.display() encoded_dir.display()
); );
@ -1299,38 +1302,35 @@ fn add_child_to_encoded_module_creates_encoded_directory() {
decoded_dir.display() decoded_dir.display()
); );
// init.luau should exist inside the encoded directory // init.luau should exist inside the slugified directory
let init_file = encoded_dir.join("init.luau"); let init_file = encoded_dir.join("init.luau");
assert_file_exists(&init_file, "init.luau inside encoded directory"); assert_file_exists(&init_file, "init.luau inside slugified directory");
// Child should be created inside the encoded directory // Child should be created inside the slugified directory
let child_file = encoded_dir.join("EncodedChild.luau"); let child_file = encoded_dir.join("EncodedChild.luau");
assert_file_exists(&child_file, "Child inside encoded directory"); assert_file_exists(&child_file, "Child inside slugified directory");
}); });
} }
/// Test 23: Adding a child to a standalone Script with Rojo-encoded characters /// Test 23: Adding a child to a standalone Script with slugified characters
/// AND a script suffix (e.g., `Key%COLON%Script.server.luau`) strips the suffix /// AND a script suffix (e.g., `Key_Script.server.luau`) strips the suffix
/// correctly when creating the directory name. /// correctly when creating the directory name.
/// ///
/// The directory should be `Key%COLON%Script/`, not `Key%COLON%Script.server/` /// The directory should be `Key_Script/`, not `Key_Script.server/`
/// or `Key:Script/`. /// or `Key:Script/`.
#[test] #[test]
fn add_child_to_encoded_script_strips_suffix_correctly() { fn add_child_to_encoded_script_strips_suffix_correctly() {
run_serve_test("syncback_encoded_names", |session, _redactions| { run_serve_test("syncback_encoded_names", |session, _redactions| {
let (session_id, encoded_script_id) = get_encoded_names_instance(&session, "Key:Script"); let (session_id, encoded_script_id) = get_encoded_names_instance(&session, "Key:Script");
let standalone_file = session let standalone_file = session.path().join("src").join("Key_Script.server.luau");
.path() let encoded_dir = session.path().join("src").join("Key_Script");
.join("src")
.join("Key%COLON%Script.server.luau");
let encoded_dir = session.path().join("src").join("Key%COLON%Script");
// Wrong: decoded name // Wrong: decoded name
let decoded_dir = session.path().join("src").join("Key:Script"); let decoded_dir = session.path().join("src").join("Key:Script");
// Wrong: suffix not stripped // Wrong: suffix not stripped
let unsuffixed_dir = session.path().join("src").join("Key%COLON%Script.server"); let unsuffixed_dir = session.path().join("src").join("Key_Script.server");
assert_file_exists(&standalone_file, "Encoded script before child add"); assert_file_exists(&standalone_file, "Slugified script before child add");
let mut properties = HashMap::new(); let mut properties = HashMap::new();
properties.insert( properties.insert(
@ -1359,7 +1359,7 @@ fn add_child_to_encoded_script_strips_suffix_correctly() {
assert!( assert!(
encoded_dir.is_dir(), encoded_dir.is_dir(),
"Directory should be Key%COLON%Script/ (suffix stripped, encoded): {}", "Directory should be Key_Script/ (suffix stripped, slugified): {}",
encoded_dir.display() encoded_dir.display()
); );
assert!( assert!(
@ -1375,16 +1375,16 @@ fn add_child_to_encoded_script_strips_suffix_correctly() {
// Script → init.server.luau (preserves class) // Script → init.server.luau (preserves class)
let init_file = encoded_dir.join("init.server.luau"); let init_file = encoded_dir.join("init.server.luau");
assert_file_exists(&init_file, "init.server.luau inside encoded directory"); assert_file_exists(&init_file, "init.server.luau inside slugified directory");
let child_file = encoded_dir.join("ScriptChild.luau"); let child_file = encoded_dir.join("ScriptChild.luau");
assert_file_exists(&child_file, "Child inside encoded script directory"); assert_file_exists(&child_file, "Child inside slugified script directory");
}); });
} }
/// Test 24: Adding a child to a non-script standalone instance with Rojo-encoded /// Test 24: Adding a child to a non-script standalone instance with slugified
/// characters (e.g., `What%QUESTION%Model.model.json5`) creates the directory /// characters (e.g., `What_Model.model.json5`) creates the directory
/// using the encoded name with the compound extension stripped. /// using the slugified name with the compound extension stripped.
/// ///
/// This exercises `convert_standalone_instance_to_directory`. /// This exercises `convert_standalone_instance_to_directory`.
#[test] #[test]
@ -1392,14 +1392,11 @@ fn add_child_to_encoded_model_creates_encoded_directory() {
run_serve_test("syncback_encoded_names", |session, _redactions| { run_serve_test("syncback_encoded_names", |session, _redactions| {
let (session_id, encoded_model_id) = get_encoded_names_instance(&session, "What?Model"); let (session_id, encoded_model_id) = get_encoded_names_instance(&session, "What?Model");
let standalone_file = session let standalone_file = session.path().join("src").join("What_Model.model.json5");
.path() let encoded_dir = session.path().join("src").join("What_Model");
.join("src")
.join("What%QUESTION%Model.model.json5");
let encoded_dir = session.path().join("src").join("What%QUESTION%Model");
let decoded_dir = session.path().join("src").join("What?Model"); let decoded_dir = session.path().join("src").join("What?Model");
assert_file_exists(&standalone_file, "Encoded model file before child add"); assert_file_exists(&standalone_file, "Slugified model file before child add");
let mut properties = HashMap::new(); let mut properties = HashMap::new();
properties.insert( properties.insert(
@ -1428,7 +1425,7 @@ fn add_child_to_encoded_model_creates_encoded_directory() {
assert!( assert!(
encoded_dir.is_dir(), encoded_dir.is_dir(),
"Directory should use encoded name (What%QUESTION%Model/): {}", "Directory should use slugified name (What_Model/): {}",
encoded_dir.display() encoded_dir.display()
); );
assert!( assert!(
@ -1439,32 +1436,32 @@ fn add_child_to_encoded_model_creates_encoded_directory() {
// init.meta.json5 should exist inside the directory (converted from .model.json5) // init.meta.json5 should exist inside the directory (converted from .model.json5)
let init_meta = encoded_dir.join("init.meta.json5"); let init_meta = encoded_dir.join("init.meta.json5");
assert_file_exists(&init_meta, "init.meta.json5 inside encoded model directory"); assert_file_exists(
&init_meta,
"init.meta.json5 inside slugified model directory",
);
let child_file = encoded_dir.join("ModelChild.luau"); let child_file = encoded_dir.join("ModelChild.luau");
assert_file_exists(&child_file, "Child inside encoded model directory"); assert_file_exists(&child_file, "Child inside slugified model directory");
}); });
} }
/// Test 25: Non-Source property update on a standalone script with Rojo-encoded /// Test 25: Non-Source property update on a standalone script with slugified
/// name creates the adjacent meta file using the encoded filesystem name, /// name creates the adjacent meta file using the slugified filesystem name,
/// not the decoded instance name. /// not the decoded instance name.
/// ///
/// e.g., `What%QUESTION%Module.meta.json5` NOT `What?Module.meta.json5` /// e.g., `What_Module.meta.json5` NOT `What?Module.meta.json5`
#[test] #[test]
fn property_update_on_encoded_script_uses_encoded_meta_path() { fn property_update_on_encoded_script_uses_encoded_meta_path() {
run_serve_test("syncback_encoded_names", |session, _redactions| { run_serve_test("syncback_encoded_names", |session, _redactions| {
let (session_id, encoded_module_id) = get_encoded_names_instance(&session, "What?Module"); let (session_id, encoded_module_id) = get_encoded_names_instance(&session, "What?Module");
let script_file = session.path().join("src").join("What%QUESTION%Module.luau"); let script_file = session.path().join("src").join("What_Module.luau");
let encoded_meta = session let encoded_meta = session.path().join("src").join("What_Module.meta.json5");
.path()
.join("src")
.join("What%QUESTION%Module.meta.json5");
let decoded_meta = session.path().join("src").join("What?Module.meta.json5"); let decoded_meta = session.path().join("src").join("What?Module.meta.json5");
assert_file_exists(&script_file, "Encoded script file before property update"); assert_file_exists(&script_file, "Slugified script file before property update");
assert_not_exists(&encoded_meta, "Meta file before property update"); assert_file_exists(&encoded_meta, "Meta file exists from fixture (name field)");
let mut attrs = rbx_dom_weak::types::Attributes::new(); let mut attrs = rbx_dom_weak::types::Attributes::new();
attrs.insert("EncodedAttr".to_string(), Variant::Float64(99.0)); attrs.insert("EncodedAttr".to_string(), Variant::Float64(99.0));
@ -1483,10 +1480,10 @@ fn property_update_on_encoded_script_uses_encoded_meta_path() {
}, },
); );
// Meta file should use encoded name // Meta file should use slugified name
assert_file_exists( assert_file_exists(
&encoded_meta, &encoded_meta,
"Meta file should use encoded name (What%QUESTION%Module.meta.json5)", "Meta file should use slugified name (What_Module.meta.json5)",
); );
assert!( assert!(
!decoded_meta.exists(), !decoded_meta.exists(),
@ -2453,15 +2450,12 @@ fn encoded_name_rapid_rename_chain() {
wait_for_settle(); wait_for_settle();
// Final file should use encoded name: Done%QUESTION%Now.luau // Final file should use slugified name: Done_Now.luau
let final_file = src.join("Done%QUESTION%Now.luau"); let final_file = src.join("Done_Now.luau");
assert_file_exists(&final_file, "Final encoded file"); assert_file_exists(&final_file, "Final encoded file");
// Original should be gone // Original should be gone
assert_not_exists( assert_not_exists(&src.join("What_Module.luau"), "Original encoded file");
&src.join("What%QUESTION%Module.luau"),
"Original encoded file",
);
}); });
} }
@ -4151,8 +4145,8 @@ fn encoded_rename_and_source_rapid() {
} }
wait_for_settle(); wait_for_settle();
// Final: Final:Encoded -> Final%COLON%Encoded.luau // Final: Final:Encoded -> Final_Encoded.luau
let final_file = src.join("Final%COLON%Encoded.luau"); let final_file = src.join("Final_Encoded.luau");
assert_file_exists(&final_file, "Final encoded file"); assert_file_exists(&final_file, "Final encoded file");
let content = fs::read_to_string(&final_file).unwrap(); let content = fs::read_to_string(&final_file).unwrap();
assert!( assert!(
@ -4193,8 +4187,8 @@ fn encoded_rename_and_classchange_rapid() {
} }
wait_for_settle(); wait_for_settle();
// Final: Enc:Final as Script -> Enc%COLON%Final.server.luau // Final: Enc:Final as Script -> Enc_Final.server.luau
let final_file = src.join("Enc%COLON%Final.server.luau"); let final_file = src.join("Enc_Final.server.luau");
assert_file_exists(&final_file, "Final encoded+class file"); assert_file_exists(&final_file, "Final encoded+class file");
}); });
} }
@ -4229,8 +4223,8 @@ fn encoded_all_three_combined_rapid() {
} }
wait_for_settle(); wait_for_settle();
// Final: End?Now as LocalScript -> End%QUESTION%Now.local.luau // Final: End?Now as LocalScript -> End_Now.local.luau
let final_file = src.join("End%QUESTION%Now.local.luau"); let final_file = src.join("End_Now.local.luau");
assert_file_exists(&final_file, "Final all-three encoded file"); assert_file_exists(&final_file, "Final all-three encoded file");
let content = fs::read_to_string(&final_file).unwrap(); let content = fs::read_to_string(&final_file).unwrap();
assert!( assert!(

@ -1 +1 @@
Subproject commit 035c12d9ed499942a1e8b024d61b99675eab232d Subproject commit 2e37c0ee0f6801e4a159289a2e2a62292fe7e111