mirror of
https://github.com/rojo-rbx/rojo.git
synced 2026-10-25 13:31:37 +00:00
Refactor file path handling in JobThreadContext to support combined rename and ClassName changes
- Updated the logic to use the effective file path when handling script-to-script transitions, ensuring that the correct path is used even after a rename operation. - Enhanced the test suite with a new test case for combined rename and ClassName changes, verifying that the extension change is applied correctly after a file rename. - Improved comments for clarity on the renaming process and its implications for file handling during updates.
This commit is contained in:
parent
9c92204956
commit
5b2ea2a23f
2 changed files with 73 additions and 7 deletions
|
|
@ -654,21 +654,29 @@ impl JobThreadContext {
|
||||||
"ModuleScript" | "Script" | "LocalScript"
|
"ModuleScript" | "Script" | "LocalScript"
|
||||||
);
|
);
|
||||||
|
|
||||||
if old_is_script && new_is_script && path.exists() {
|
// If a rename handler already moved the file,
|
||||||
|
// use the new path instead of the stale
|
||||||
|
// instigating_source path.
|
||||||
|
let effective_path = overridden_source_path
|
||||||
|
.as_deref()
|
||||||
|
.unwrap_or(path.as_path());
|
||||||
|
|
||||||
|
if old_is_script && new_is_script && effective_path.exists() {
|
||||||
// Script-to-script transition: rename the file extension.
|
// Script-to-script transition: rename the file extension.
|
||||||
// For directory-format scripts, the path is the directory
|
// For directory-format scripts, the path is the directory
|
||||||
// (e.g., src/MyModule/), not the init file inside. We must
|
// (e.g., src/MyModule/), not the init file inside. We must
|
||||||
// find and rename the init file, not the directory.
|
// find and rename the init file, not the directory.
|
||||||
// Resolve the actual file to rename. For directories,
|
// Resolve the actual file to rename. For directories,
|
||||||
// find the init file inside; for files, use directly.
|
// find the init file inside; for files, use directly.
|
||||||
let init_result = if path.is_dir() {
|
let init_result = if effective_path.is_dir() {
|
||||||
Self::find_init_file(path)
|
Self::find_init_file(effective_path)
|
||||||
.map(|f| (f.clone(), path.to_path_buf()))
|
.map(|f| (f.clone(), effective_path.to_path_buf()))
|
||||||
} else {
|
} else {
|
||||||
Some((
|
Some((
|
||||||
path.clone(),
|
effective_path.to_path_buf(),
|
||||||
path.parent()
|
effective_path
|
||||||
.unwrap_or(path.as_path())
|
.parent()
|
||||||
|
.unwrap_or(effective_path)
|
||||||
.to_path_buf(),
|
.to_path_buf(),
|
||||||
))
|
))
|
||||||
};
|
};
|
||||||
|
|
|
||||||
|
|
@ -932,6 +932,64 @@ fn combined_classname_and_source_update() {
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Test 19b: Combined rename + ClassName change in a single update.
|
||||||
|
/// The rename handler moves the file first (existing.luau → Renamed.luau),
|
||||||
|
/// then the ClassName handler must use the renamed path to apply the extension
|
||||||
|
/// change (Renamed.luau → Renamed.server.luau). Without the fix, the ClassName
|
||||||
|
/// handler checks the OLD path which no longer exists, silently skipping the
|
||||||
|
/// extension rename.
|
||||||
|
#[test]
|
||||||
|
fn combined_rename_and_classname_change() {
|
||||||
|
run_serve_test("syncback_write", |session, _redactions| {
|
||||||
|
let (session_id, _rs_id, existing_id) = get_rs_and_existing(&session);
|
||||||
|
|
||||||
|
let old_path = session.path().join("src").join("existing.luau");
|
||||||
|
// After rename + ClassName change: should end up as Renamed.server.luau
|
||||||
|
let final_path = session.path().join("src").join("Renamed.server.luau");
|
||||||
|
// Should NOT exist: renamed but without extension change
|
||||||
|
let renamed_only = session.path().join("src").join("Renamed.luau");
|
||||||
|
|
||||||
|
assert_file_exists(&old_path, "existing.luau before combined update");
|
||||||
|
|
||||||
|
let mut props = UstrMap::default();
|
||||||
|
props.insert(
|
||||||
|
ustr("Source"),
|
||||||
|
Some(Variant::String(
|
||||||
|
"-- After rename + class change".to_string(),
|
||||||
|
)),
|
||||||
|
);
|
||||||
|
send_update(
|
||||||
|
&session,
|
||||||
|
&session_id,
|
||||||
|
InstanceUpdate {
|
||||||
|
id: existing_id,
|
||||||
|
changed_name: Some("Renamed".to_string()),
|
||||||
|
changed_class_name: Some(ustr("Script")),
|
||||||
|
changed_properties: props,
|
||||||
|
changed_metadata: None,
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
|
assert_not_exists(&old_path, "Original file after combined rename+classname");
|
||||||
|
assert_not_exists(
|
||||||
|
&renamed_only,
|
||||||
|
"Renamed.luau should not exist — ClassName handler should have \
|
||||||
|
applied the .server extension",
|
||||||
|
);
|
||||||
|
assert_file_exists(
|
||||||
|
&final_path,
|
||||||
|
"Renamed.server.luau should exist after combined rename+classname",
|
||||||
|
);
|
||||||
|
|
||||||
|
let content = fs::read_to_string(&final_path).unwrap();
|
||||||
|
assert!(
|
||||||
|
content.contains("After rename + class change"),
|
||||||
|
"Source should be in the final file, got: {}",
|
||||||
|
content
|
||||||
|
);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
// Tests: Standalone → directory conversion & suffix-aware removal
|
// Tests: Standalone → directory conversion & suffix-aware removal
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue