From 15939b1647e12f317dd3e11bba716d17b4544511 Mon Sep 17 00:00:00 2001 From: EgoMoose Date: Tue, 2 Jun 2026 23:40:12 -0400 Subject: [PATCH] Add ignorable glob support (#1256) --- CHANGELOG.md | 2 + src/glob.rs | 58 +++++++++++++++++++++++ src/project.rs | 56 +++++++++++++++++++++- src/snapshot/metadata.rs | 29 ++++++++++-- src/snapshot_middleware/dir.rs | 12 ++--- src/syncback/mod.rs | 85 +++++++++++++++++++++++++++++++--- 6 files changed, 222 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b126607..e9e92892 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,7 @@ Making a new release? Simply add the new header with the version and date undern * Fixed a bug where the notification timeout thread would fail to cancel on unmount ([#1211]) * Added a "Forget" option to the sync reminder notification to avoid being reminded for that place in the future ([#1215]) * Improves relative path calculation for sourcemap generation to avoid issues with Windows UNC paths. ([#1217]) +* Add support for gitignore-style negation in `globIgnorePaths` and syncback's `ignorePaths` ([#1256]) * Fixed the sync fallback scrambling sibling order; replacements are now re-parented ancestors-first and in their original child order. ([#1265]) * Instances that share a name and class are now robustly matched on resync by comparing their properties, instead of relying on child order alone. ([#1266]) * Rojo now reports a clear error instead of panicking in several cases, including when the `serve` port is already in use, when a synced file is read-only or locked, when the filesystem watcher can't be created, and when the working directory is inaccessible. ([#1267]) @@ -51,6 +52,7 @@ Making a new release? Simply add the new header with the version and date undern [#1211]: https://github.com/rojo-rbx/rojo/pull/1211 [#1215]: https://github.com/rojo-rbx/rojo/pull/1215 [#1217]: https://github.com/rojo-rbx/rojo/pull/1217 +[#1256]: https://github.com/rojo-rbx/rojo/pull/1256 [#1265]: https://github.com/rojo-rbx/rojo/pull/1265 [#1266]: https://github.com/rojo-rbx/rojo/pull/1266 [#1267]: https://github.com/rojo-rbx/rojo/pull/1267 diff --git a/src/glob.rs b/src/glob.rs index 96437c38..c0146863 100644 --- a/src/glob.rs +++ b/src/glob.rs @@ -48,3 +48,61 @@ impl<'de> Deserialize<'de> for Glob { Glob::new(&glob).map_err(D::Error::custom) } } + +/// A glob with optional gitignore-style negation. A leading `!` marks the +/// pattern as a negation (re-includes paths that an earlier rule excluded). +/// To match a literal `!` at the start of a pattern, escape it with `\!`. +#[derive(Debug, Clone)] +pub struct IgnorableGlob { + glob: Glob, + negated: bool, + raw: String, +} + +impl IgnorableGlob { + pub fn new(pattern: &str) -> Result { + let (negated, body) = if let Some(rest) = pattern.strip_prefix('!') { + (true, rest) + } else if pattern.starts_with(r"\!") { + (false, &pattern[1..]) + } else { + (false, pattern) + }; + + Ok(IgnorableGlob { + glob: Glob::new(body)?, + negated, + raw: pattern.to_owned(), + }) + } + + pub fn is_match>(&self, path: P) -> bool { + self.glob.is_match(path) + } + + pub fn is_negation(&self) -> bool { + self.negated + } +} + +impl PartialEq for IgnorableGlob { + fn eq(&self, other: &Self) -> bool { + self.negated == other.negated && self.glob == other.glob + } +} + +impl Eq for IgnorableGlob {} + +impl Serialize for IgnorableGlob { + fn serialize(&self, serializer: S) -> Result { + serializer.serialize_str(&self.raw) + } +} + +impl<'de> Deserialize<'de> for IgnorableGlob { + fn deserialize>(deserializer: D) -> Result { + let pattern = String::deserialize(deserializer)?; + + IgnorableGlob::new(&pattern).map_err(D::Error::custom) + } +} diff --git a/src/project.rs b/src/project.rs index 5cc8278e..bfcc7f0e 100644 --- a/src/project.rs +++ b/src/project.rs @@ -12,7 +12,8 @@ use serde::{Deserialize, Serialize}; use thiserror::Error; use crate::{ - glob::Glob, json, resolution::UnresolvedValue, snapshot::SyncRule, syncback::SyncbackRules, + glob::IgnorableGlob, json, resolution::UnresolvedValue, snapshot::SyncRule, + syncback::SyncbackRules, }; /// Represents 'default' project names that act as `init` files @@ -114,7 +115,7 @@ pub struct Project { /// A list of globs, relative to the folder the project file is in, that /// match files that should be excluded if Rojo encounters them. #[serde(default, skip_serializing_if = "Vec::is_empty")] - pub glob_ignore_paths: Vec, + pub glob_ignore_paths: Vec, /// A list of rules for syncback with this project file. #[serde(skip_serializing_if = "Option::is_none")] @@ -593,4 +594,55 @@ mod test { assert!(project.sync_rules[0].include.is_match("data.data.json")); assert!(project.sync_rules[1].include.is_match("init.module.lua")); } + + #[test] + fn glob_ignore_paths_negation() { + let project_json = r#"{ + "name": "TestProject", + "tree": { "$path": "src" }, + "globIgnorePaths": [ + "**/*.spec.lua", + "!keep.spec.lua", + "\\!literal.lua" + ] + }"#; + + let project = Project::load_from_slice( + project_json.as_bytes(), + PathBuf::from("/test/default.project.json"), + None, + ) + .expect("project should parse"); + + let paths = &project.glob_ignore_paths; + assert_eq!(paths.len(), 3); + + assert!(!paths[0].is_negation()); + assert!(paths[0].is_match("foo.spec.lua")); + + assert!(paths[1].is_negation()); + assert!(paths[1].is_match("keep.spec.lua")); + + // `\!literal.lua` should match a file literally named `!literal.lua`, + // not be parsed as a negation. + assert!(!paths[2].is_negation()); + assert!(paths[2].is_match("!literal.lua")); + + let rules: Vec<_> = paths + .iter() + .map(|g| crate::snapshot::PathIgnoreRule { + base_path: PathBuf::from("/test"), + glob: g.clone(), + }) + .collect(); + assert!(crate::snapshot::is_path_ignored( + &rules, + "/test/foo.spec.lua" + )); + assert!(!crate::snapshot::is_path_ignored( + &rules, + "/test/keep.spec.lua" + )); + assert!(!crate::snapshot::is_path_ignored(&rules, "/test/plain.lua")); + } } diff --git a/src/snapshot/metadata.rs b/src/snapshot/metadata.rs index 79f7a2d0..a1bf0afe 100644 --- a/src/snapshot/metadata.rs +++ b/src/snapshot/metadata.rs @@ -8,7 +8,7 @@ use anyhow::Context; use serde::{Deserialize, Serialize}; use crate::{ - glob::Glob, + glob::{Glob, IgnorableGlob}, path_serializer, project::ProjectNode, snapshot_middleware::{emit_legacy_scripts_default, Middleware}, @@ -222,18 +222,37 @@ pub struct PathIgnoreRule { pub base_path: PathBuf, /// The actual glob that can be matched against the input path. - pub glob: Glob, + pub glob: IgnorableGlob, } impl PathIgnoreRule { - pub fn passes>(&self, path: P) -> bool { + pub fn matches>(&self, path: P) -> bool { let path = path.as_ref(); match path.strip_prefix(&self.base_path) { - Ok(suffix) => !self.glob.is_match(suffix), - Err(_) => true, + Ok(suffix) => self.glob.is_match(suffix), + Err(_) => false, } } + + pub fn is_negation(&self) -> bool { + self.glob.is_negation() + } +} + +/// Evaluates an ordered list of [`PathIgnoreRule`]s against a path using +/// gitignore-style "last match wins" semantics: a path is ignored if the last +/// rule whose pattern matches it is non-negated. Paths matched by no rule are +/// not ignored. +pub fn is_path_ignored>(rules: &[PathIgnoreRule], path: P) -> bool { + let path = path.as_ref(); + let mut ignored = false; + for rule in rules { + if rule.matches(path) { + ignored = !rule.is_negation(); + } + } + ignored } /// Represents where a particular Instance or InstanceSnapshot came from. diff --git a/src/snapshot_middleware/dir.rs b/src/snapshot_middleware/dir.rs index fb444c37..fac1d0ff 100644 --- a/src/snapshot_middleware/dir.rs +++ b/src/snapshot_middleware/dir.rs @@ -7,7 +7,9 @@ use anyhow::Context; use memofs::{DirEntry, Vfs}; use crate::{ - snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot, InstigatingSource}, + snapshot::{ + is_path_ignored, InstanceContext, InstanceMetadata, InstanceSnapshot, InstigatingSource, + }, syncback::{hash_instance, FsSnapshot, SyncbackReturn, SyncbackSnapshot}, }; @@ -41,12 +43,8 @@ pub fn snapshot_dir_no_meta( path: &Path, name: &str, ) -> anyhow::Result> { - let passes_filter_rules = |child: &DirEntry| { - context - .path_ignore_rules - .iter() - .all(|rule| rule.passes(child.path())) - }; + let passes_filter_rules = + |child: &DirEntry| !is_path_ignored(&context.path_ignore_rules, child.path()); let mut snapshot_children = Vec::new(); diff --git a/src/syncback/mod.rs b/src/syncback/mod.rs index 5dc3028f..e1666abc 100644 --- a/src/syncback/mod.rs +++ b/src/syncback/mod.rs @@ -21,7 +21,7 @@ use std::{ }; use crate::{ - glob::Glob, + glob::{Glob, IgnorableGlob}, snapshot::{InstanceWithMeta, RojoTree}, snapshot_middleware::Middleware, syncback::ref_properties::{collect_referents, link_referents}, @@ -414,18 +414,18 @@ pub struct SyncbackRules { } impl SyncbackRules { - pub fn compile_globs(&self) -> anyhow::Result> { + pub fn compile_globs(&self) -> anyhow::Result> { let mut globs = Vec::with_capacity(self.ignore_paths.len()); let dir_ignore_paths = self.create_ignore_dir_paths.unwrap_or(true); for pattern in &self.ignore_paths { - let glob = Glob::new(pattern) + let glob = IgnorableGlob::new(pattern) .with_context(|| format!("the pattern '{pattern}' is not a valid glob"))?; globs.push(glob); if dir_ignore_paths { if let Some(dir_pattern) = pattern.strip_suffix("/**") { - if let Ok(glob) = Glob::new(dir_pattern) { + if let Ok(glob) = IgnorableGlob::new(dir_pattern) { globs.push(glob) } } @@ -436,7 +436,7 @@ impl SyncbackRules { } } -fn is_valid_path(globs: &Option>, base_path: &Path, path: &Path) -> bool { +fn is_valid_path(globs: &Option>, base_path: &Path, path: &Path) -> bool { let git_glob = GIT_IGNORE_GLOB.get_or_init(|| Glob::new(".git/**").unwrap()); let test_path = match path.strip_prefix(base_path) { Ok(suffix) => suffix, @@ -446,11 +446,16 @@ fn is_valid_path(globs: &Option>, base_path: &Path, path: &Path) -> bo return false; } if let Some(ref ignore_paths) = globs { + // Gitignore-style "last match wins" + let mut ignored = false; for glob in ignore_paths { if glob.is_match(test_path) { - return false; + ignored = !glob.is_negation(); } } + if ignored { + return false; + } } true } @@ -538,3 +543,71 @@ fn strip_unknown_root_children(new: &mut WeakDom, old: &RojoTree) { new.destroy(child_ref); } } + +#[cfg(test)] +mod test { + use super::*; + + fn rules(ignore_paths: &[&str], create_ignore_dir_paths: Option) -> SyncbackRules { + SyncbackRules { + ignore_trees: Vec::new(), + ignore_paths: ignore_paths.iter().map(|s| s.to_string()).collect(), + ignore_properties: IndexMap::new(), + sync_current_camera: None, + sync_unscriptable: None, + ignore_referents: None, + create_ignore_dir_paths, + } + } + + #[test] + fn ignore_paths_negation() { + let globs = Some( + rules(&["**/*.lua", "!keep.lua"], Some(false)) + .compile_globs() + .unwrap(), + ); + let base = Path::new("/test"); + + // A later negation re-includes a path matched by an earlier pattern. + assert!(!is_valid_path(&globs, base, Path::new("/test/foo.lua"))); + assert!(is_valid_path(&globs, base, Path::new("/test/keep.lua"))); + // Paths matched by no rule are valid. + assert!(is_valid_path(&globs, base, Path::new("/test/plain.txt"))); + } + + #[test] + fn ignore_paths_negation_with_dir_expansion() { + // With `create_ignore_dir_paths`, a negated `foo/**` pattern should also + // re-include the `foo` directory itself, mirroring the file rule. + let globs = Some( + rules(&["**/*", "!keep/**"], Some(true)) + .compile_globs() + .unwrap(), + ); + let base = Path::new("/test"); + + assert!(!is_valid_path(&globs, base, Path::new("/test/drop/a.lua"))); + assert!(is_valid_path(&globs, base, Path::new("/test/keep"))); + assert!(is_valid_path(&globs, base, Path::new("/test/keep/a.lua"))); + } + + #[test] + fn ignore_paths_escaped_bang_is_literal() { + // `\!literal.lua` should ignore a file literally named `!literal.lua` + // rather than being parsed as a negation. + let globs = Some( + rules(&[r"\!literal.lua"], Some(false)) + .compile_globs() + .unwrap(), + ); + let base = Path::new("/test"); + + assert!(!globs.as_ref().unwrap()[0].is_negation()); + assert!(!is_valid_path( + &globs, + base, + Path::new("/test/!literal.lua") + )); + } +}