mirror of
https://github.com/rojo-rbx/rojo.git
synced 2026-10-22 21:41:37 +00:00
Enhance git metadata handling and improve change detection logic
- Introduced a new function `getGitMetadata` in `ApiContext` to fetch current git metadata, including new file IDs. - Updated the `PatchTree.build` function to handle new file IDs, allowing for better differentiation between tracked and untracked files during patch processing. - Modified the `git_changed_files` function to return both tracked and untracked files, improving accuracy in change detection. - Enhanced the `App` component to debounce git metadata refreshes, ensuring the UI reflects the latest changes without unnecessary calls. These changes aim to improve the integration with git and enhance the accuracy of change detection in the application.
This commit is contained in:
parent
b8bd92d665
commit
640b2d3d97
7 changed files with 128 additions and 30 deletions
4
TODO.txt
4
TODO.txt
|
|
@ -338,7 +338,3 @@ make sure this is applied to plugin-based syncback, too.
|
|||
|
||||
[git]
|
||||
I notice we actually use the git cli in a lot of places. is there a git library we can use instead?
|
||||
|
||||
[auto select studio/atlas]
|
||||
when things get added AFTER the fact because we send the git history hashes on connect rather than continiously, it auto-selects Studio for things that are being edited in real-time in the sync dialog
|
||||
how should we handle new files? because it could have been that someone just deleted it in studio. we can actually check if its a truly new file in Studio, like A but not staged!
|
||||
|
|
|
|||
|
|
@ -271,6 +271,12 @@ function ApiContext:write(patch, stageIds)
|
|||
end)
|
||||
end
|
||||
|
||||
function ApiContext:getGitMetadata()
|
||||
local url = ("%s/api/git-metadata"):format(self.__baseUrl)
|
||||
|
||||
return Http.get(url):andThen(rejectFailedRequests):andThen(Http.Response.msgpack)
|
||||
end
|
||||
|
||||
function ApiContext:connectWebSocket(packetHandlers)
|
||||
local url = ("%s/api/socket/%s"):format(self.__baseUrl, self.__messageCursor)
|
||||
-- Convert HTTP/HTTPS URL to WS/WSS
|
||||
|
|
|
|||
|
|
@ -1000,6 +1000,9 @@ function App:startSession()
|
|||
return result
|
||||
end)
|
||||
|
||||
local pendingGitRefreshThread = nil
|
||||
local GIT_METADATA_DEBOUNCE = 0.5
|
||||
|
||||
serveSession:setPatchUpdateCallback(function(instanceMap, patch, changedIds)
|
||||
-- If all changes have been reverted, auto-accept the empty patch
|
||||
if PatchSet.isEmpty(patch) then
|
||||
|
|
@ -1009,14 +1012,40 @@ function App:startSession()
|
|||
return
|
||||
end
|
||||
|
||||
-- Update the patchTree when new changes arrive during confirmation
|
||||
-- The changedIds parameter contains IDs of items that were modified
|
||||
-- so their selections can be reset (user must re-review)
|
||||
-- Immediately rebuild with stale metadata so the UI reflects new items
|
||||
local gitMetadata = cachedServerInfo and cachedServerInfo.gitMetadata
|
||||
self:setState({
|
||||
patchTree = PatchTree.build(patch, instanceMap, { "Property", "Current", "Incoming" }, gitMetadata),
|
||||
changedIds = changedIds, -- Pass to UI so it can unselect changed items
|
||||
changedIds = changedIds,
|
||||
})
|
||||
|
||||
-- Debounce: schedule a fresh git metadata fetch after rapid patches settle
|
||||
if pendingGitRefreshThread then
|
||||
task.cancel(pendingGitRefreshThread)
|
||||
end
|
||||
pendingGitRefreshThread = task.delay(GIT_METADATA_DEBOUNCE, function()
|
||||
pendingGitRefreshThread = nil
|
||||
apiContext
|
||||
:getGitMetadata()
|
||||
:andThen(function(freshMetadata)
|
||||
if cachedServerInfo then
|
||||
cachedServerInfo.gitMetadata = freshMetadata
|
||||
end
|
||||
if self.state.appStatus == AppStatus.Confirming then
|
||||
self:setState({
|
||||
patchTree = PatchTree.build(
|
||||
patch,
|
||||
instanceMap,
|
||||
{ "Property", "Current", "Incoming" },
|
||||
freshMetadata
|
||||
),
|
||||
})
|
||||
end
|
||||
end)
|
||||
:catch(function(err)
|
||||
Log.warn("Failed to refresh git metadata: {}", tostring(err))
|
||||
end)
|
||||
end)
|
||||
end)
|
||||
|
||||
-- One-shot sync: disconnect after initial sync is fully complete (including any writes)
|
||||
|
|
|
|||
|
|
@ -250,7 +250,7 @@ local PatchTree = {}
|
|||
|
||||
-- Builds a new tree from a patch and instanceMap
|
||||
-- uses changeListHeaders in node.changeList
|
||||
-- gitMetadata is optional: { changedIds: {string}, scriptCommittedHashes: {[string]: {string}} }
|
||||
-- gitMetadata is optional: { changedIds: {string}, scriptCommittedHashes: {[string]: {string}}, newFileIds: {string}? }
|
||||
function PatchTree.build(patch, instanceMap, changeListHeaders, gitMetadata)
|
||||
Timer.start("PatchTree.build")
|
||||
local clock = os.clock()
|
||||
|
|
@ -267,6 +267,14 @@ function PatchTree.build(patch, instanceMap, changeListHeaders, gitMetadata)
|
|||
end
|
||||
end
|
||||
|
||||
local newFileIdSet = nil
|
||||
if gitMetadata and gitMetadata.newFileIds then
|
||||
newFileIdSet = {}
|
||||
for _, id in gitMetadata.newFileIds do
|
||||
newFileIdSet[id] = true
|
||||
end
|
||||
end
|
||||
|
||||
Timer.start("patch.updated")
|
||||
for _, change in patch.updated do
|
||||
clock = yieldIfNeeded(clock)
|
||||
|
|
@ -518,8 +526,14 @@ function PatchTree.build(patch, instanceMap, changeListHeaders, gitMetadata)
|
|||
table.insert(changeList, 1, changeListHeaders)
|
||||
end
|
||||
|
||||
-- Add this node to tree
|
||||
-- Default to nil (unselected) - user must explicitly choose Push/Pull/Skip
|
||||
-- Truly new files (untracked in git) default to "push" (Atlas).
|
||||
-- Tracked files appearing as additions are ambiguous (Studio may have
|
||||
-- deleted them), so they remain nil for user review.
|
||||
local defaultSelection = nil
|
||||
if newFileIdSet and newFileIdSet[id] then
|
||||
defaultSelection = "push"
|
||||
end
|
||||
|
||||
tree:addNode(change.Parent, {
|
||||
id = change.Id,
|
||||
patchType = "Add",
|
||||
|
|
@ -528,7 +542,7 @@ function PatchTree.build(patch, instanceMap, changeListHeaders, gitMetadata)
|
|||
changeInfo = changeInfo,
|
||||
changeList = changeList,
|
||||
instance = instanceMap.fromIds[id],
|
||||
defaultSelection = nil,
|
||||
defaultSelection = defaultSelection,
|
||||
})
|
||||
end
|
||||
Timer.stop()
|
||||
|
|
|
|||
71
src/git.rs
71
src/git.rs
|
|
@ -59,8 +59,23 @@ pub fn git_head_commit(repo_root: &Path) -> Option<String> {
|
|||
}
|
||||
}
|
||||
|
||||
fn git_changed_files(repo_root: &Path, initial_head: Option<&str>) -> Option<HashSet<PathBuf>> {
|
||||
let mut changed = HashSet::new();
|
||||
struct ChangedFiles {
|
||||
tracked: HashSet<PathBuf>,
|
||||
untracked: HashSet<PathBuf>,
|
||||
}
|
||||
|
||||
impl ChangedFiles {
|
||||
fn all(&self) -> HashSet<PathBuf> {
|
||||
self.tracked.union(&self.untracked).cloned().collect()
|
||||
}
|
||||
|
||||
fn is_empty(&self) -> bool {
|
||||
self.tracked.is_empty() && self.untracked.is_empty()
|
||||
}
|
||||
}
|
||||
|
||||
fn git_changed_files(repo_root: &Path, initial_head: Option<&str>) -> Option<ChangedFiles> {
|
||||
let mut tracked = HashSet::new();
|
||||
|
||||
let diff_base = initial_head.unwrap_or("HEAD");
|
||||
let diff_output = Command::new("git")
|
||||
|
|
@ -78,7 +93,7 @@ fn git_changed_files(repo_root: &Path, initial_head: Option<&str>) -> Option<Has
|
|||
for line in String::from_utf8_lossy(&diff_output.stdout).lines() {
|
||||
let trimmed = line.trim();
|
||||
if !trimmed.is_empty() {
|
||||
changed.insert(PathBuf::from(trimmed));
|
||||
tracked.insert(PathBuf::from(trimmed));
|
||||
}
|
||||
}
|
||||
} else {
|
||||
|
|
@ -88,6 +103,7 @@ fn git_changed_files(repo_root: &Path, initial_head: Option<&str>) -> Option<Has
|
|||
);
|
||||
}
|
||||
|
||||
let mut untracked = HashSet::new();
|
||||
let untracked_output = Command::new("git")
|
||||
.args([
|
||||
"-C",
|
||||
|
|
@ -103,16 +119,16 @@ fn git_changed_files(repo_root: &Path, initial_head: Option<&str>) -> Option<Has
|
|||
for line in String::from_utf8_lossy(&untracked_output.stdout).lines() {
|
||||
let trimmed = line.trim();
|
||||
if !trimmed.is_empty() {
|
||||
changed.insert(PathBuf::from(trimmed));
|
||||
untracked.insert(PathBuf::from(trimmed));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if changed.is_empty() && !diff_output.status.success() {
|
||||
if tracked.is_empty() && untracked.is_empty() && !diff_output.status.success() {
|
||||
return None;
|
||||
}
|
||||
|
||||
Some(changed)
|
||||
Some(ChangedFiles { tracked, untracked })
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
|
@ -278,6 +294,7 @@ pub fn compute_git_metadata(
|
|||
return GitMetadata {
|
||||
changed_ids: Vec::new(),
|
||||
script_committed_hashes: HashMap::new(),
|
||||
new_file_ids: Vec::new(),
|
||||
};
|
||||
}
|
||||
};
|
||||
|
|
@ -286,15 +303,19 @@ pub fn compute_git_metadata(
|
|||
return GitMetadata {
|
||||
changed_ids: Vec::new(),
|
||||
script_committed_hashes: HashMap::new(),
|
||||
new_file_ids: Vec::new(),
|
||||
};
|
||||
}
|
||||
|
||||
let untracked_set = &changed_files.untracked;
|
||||
let all_changed = changed_files.all();
|
||||
|
||||
// Brief lock: resolve changed file paths to instance Refs and class names
|
||||
let resolved: Vec<ResolvedInstance> = {
|
||||
let tree = tree_handle.lock().unwrap();
|
||||
let mut result = Vec::new();
|
||||
|
||||
for rel_path in &changed_files {
|
||||
for rel_path in &all_changed {
|
||||
let abs_path = repo_root.join(rel_path);
|
||||
let canonical_path = std::fs::canonicalize(&abs_path).ok();
|
||||
|
||||
|
|
@ -322,6 +343,11 @@ pub fn compute_git_metadata(
|
|||
}; // Lock released
|
||||
|
||||
let changed_ids: Vec<Ref> = resolved.iter().map(|ri| ri.id).collect();
|
||||
let new_file_ids: Vec<Ref> = resolved
|
||||
.iter()
|
||||
.filter(|ri| untracked_set.contains(&ri.rel_path))
|
||||
.map(|ri| ri.id)
|
||||
.collect();
|
||||
let mut script_committed_hashes: HashMap<Ref, Vec<String>> = HashMap::new();
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
|
|
@ -393,6 +419,7 @@ pub fn compute_git_metadata(
|
|||
GitMetadata {
|
||||
changed_ids,
|
||||
script_committed_hashes,
|
||||
new_file_ids,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -605,7 +632,8 @@ mod tests {
|
|||
fs::write(dir.path().join("script.luau"), "modified").unwrap();
|
||||
|
||||
let changed = git_changed_files(dir.path(), None).unwrap();
|
||||
assert!(changed.contains(&PathBuf::from("script.luau")));
|
||||
assert!(changed.tracked.contains(&PathBuf::from("script.luau")));
|
||||
assert!(changed.untracked.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
@ -618,7 +646,7 @@ mod tests {
|
|||
git_stage(dir.path(), "script.luau");
|
||||
|
||||
let changed = git_changed_files(dir.path(), None).unwrap();
|
||||
assert!(changed.contains(&PathBuf::from("script.luau")));
|
||||
assert!(changed.tracked.contains(&PathBuf::from("script.luau")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
@ -630,7 +658,8 @@ mod tests {
|
|||
fs::write(dir.path().join("new_file.luau"), "new").unwrap();
|
||||
|
||||
let changed = git_changed_files(dir.path(), None).unwrap();
|
||||
assert!(changed.contains(&PathBuf::from("new_file.luau")));
|
||||
assert!(changed.untracked.contains(&PathBuf::from("new_file.luau")));
|
||||
assert!(!changed.tracked.contains(&PathBuf::from("new_file.luau")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
@ -642,7 +671,7 @@ mod tests {
|
|||
fs::remove_file(dir.path().join("to_delete.luau")).unwrap();
|
||||
|
||||
let changed = git_changed_files(dir.path(), None).unwrap();
|
||||
assert!(changed.contains(&PathBuf::from("to_delete.luau")));
|
||||
assert!(changed.tracked.contains(&PathBuf::from("to_delete.luau")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
@ -657,9 +686,12 @@ mod tests {
|
|||
fs::write(dir.path().join("untracked.luau"), "brand new").unwrap();
|
||||
|
||||
let changed = git_changed_files(dir.path(), None).unwrap();
|
||||
assert!(changed.contains(&PathBuf::from("will_modify.luau")));
|
||||
assert!(changed.contains(&PathBuf::from("untracked.luau")));
|
||||
assert!(!changed.contains(&PathBuf::from("committed.luau")));
|
||||
let all = changed.all();
|
||||
assert!(all.contains(&PathBuf::from("will_modify.luau")));
|
||||
assert!(all.contains(&PathBuf::from("untracked.luau")));
|
||||
assert!(!all.contains(&PathBuf::from("committed.luau")));
|
||||
assert!(changed.tracked.contains(&PathBuf::from("will_modify.luau")));
|
||||
assert!(changed.untracked.contains(&PathBuf::from("untracked.luau")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
@ -670,7 +702,10 @@ mod tests {
|
|||
|
||||
let changed = git_changed_files(dir.path(), None);
|
||||
assert!(changed.is_some());
|
||||
assert!(changed.unwrap().contains(&PathBuf::from("new.luau")));
|
||||
assert!(changed
|
||||
.unwrap()
|
||||
.untracked
|
||||
.contains(&PathBuf::from("new.luau")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
@ -683,7 +718,7 @@ mod tests {
|
|||
fs::write(dir.path().join("src/module.luau"), "modified").unwrap();
|
||||
|
||||
let changed = git_changed_files(dir.path(), None).unwrap();
|
||||
assert!(changed.contains(&PathBuf::from("src/module.luau")));
|
||||
assert!(changed.tracked.contains(&PathBuf::from("src/module.luau")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
@ -700,13 +735,13 @@ mod tests {
|
|||
|
||||
let without = git_changed_files(dir.path(), None).unwrap();
|
||||
assert!(
|
||||
!without.contains(&PathBuf::from("script.luau")),
|
||||
!without.all().contains(&PathBuf::from("script.luau")),
|
||||
"without initial_head, committed file should NOT appear"
|
||||
);
|
||||
|
||||
let with = git_changed_files(dir.path(), Some(&initial_head)).unwrap();
|
||||
assert!(
|
||||
with.contains(&PathBuf::from("script.luau")),
|
||||
with.tracked.contains(&PathBuf::from("script.luau")),
|
||||
"with initial_head, committed file should appear"
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -160,6 +160,7 @@ pub async fn call(
|
|||
handle_api_syncback(request, &service, syncback_signal).await
|
||||
}
|
||||
(&Method::GET, "/api/validate-tree") => service.handle_api_validate_tree().await,
|
||||
(&Method::GET, "/api/git-metadata") => service.handle_api_git_metadata().await,
|
||||
|
||||
(_method, path) => msgpack(
|
||||
ErrorResponse::not_found(format!("Route not found: {}", path)),
|
||||
|
|
@ -405,6 +406,18 @@ impl ApiService {
|
|||
})
|
||||
}
|
||||
|
||||
async fn handle_api_git_metadata(&self) -> Response<Full<Bytes>> {
|
||||
let git_metadata = self.serve_session.git_repo_root().map(|repo_root| {
|
||||
crate::git::compute_git_metadata(
|
||||
&self.serve_session.tree_handle(),
|
||||
repo_root,
|
||||
self.serve_session.initial_head_commit(),
|
||||
)
|
||||
});
|
||||
|
||||
msgpack_ok(&git_metadata)
|
||||
}
|
||||
|
||||
/// Read-only tree freshness check for test infrastructure.
|
||||
/// Re-snapshots from disk and returns drift counts without applying corrections.
|
||||
async fn handle_api_validate_tree(&self) -> Response<Full<Bytes>> {
|
||||
|
|
|
|||
|
|
@ -214,6 +214,11 @@ pub struct GitMetadata {
|
|||
/// SHA1 hashes of prior versions (git blob format) for changed script instances.
|
||||
/// Contains 1-2 hashes per script: HEAD version and staged version (if different).
|
||||
pub script_committed_hashes: HashMap<Ref, Vec<String>>,
|
||||
/// Instance IDs whose backing files are untracked in git (truly new files,
|
||||
/// never committed). Used by the plugin to default new-file additions to
|
||||
/// "push" (Atlas) since they can't be "something Studio deleted."
|
||||
#[serde(default)]
|
||||
pub new_file_ids: Vec<Ref>,
|
||||
}
|
||||
|
||||
/// Response body from /api/rojo
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue