refactor(pi-natives-cache/search): restructured node-modules scan cache
- Added skip_node_modules to cache keys and scan calls, preventing stale cache reuse across policies. - Updated build_walker to filter .git and optionally prune node_modules during traversal. - Threaded skip-node-modules decisions through ast, glob, grep, and fff scan paths for consistency. - Adjusted get_or_scan and force_rescan to honor the same skip-node_modules policy. - Added tests verifying node_modules and .git behavior in walker, collect_entries, and force_rescan.
This commit is contained in:
@@ -369,7 +369,8 @@ fn collect_candidates(
|
||||
|
||||
let glob_set = glob_util::try_compile_glob(glob, false)?;
|
||||
let mentions_node_modules = glob.is_some_and(|value| value.contains("node_modules"));
|
||||
let scan = fs_cache::get_or_scan(&search_path, true, true, ct)?;
|
||||
let skip_node_modules = !mentions_node_modules;
|
||||
let scan = fs_cache::get_or_scan(&search_path, true, true, skip_node_modules, ct)?;
|
||||
let mut files = collect_from_entries(
|
||||
&search_path,
|
||||
&scan.entries,
|
||||
@@ -379,7 +380,7 @@ fn collect_candidates(
|
||||
)?;
|
||||
|
||||
if files.is_empty() && scan.cache_age_ms >= fs_cache::empty_recheck_ms() {
|
||||
let fresh = fs_cache::force_rescan(&search_path, true, true, true, ct)?;
|
||||
let fresh = fs_cache::force_rescan(&search_path, true, true, skip_node_modules, true, ct)?;
|
||||
files =
|
||||
collect_from_entries(&search_path, &fresh, glob_set.as_ref(), mentions_node_modules, ct)?;
|
||||
}
|
||||
|
||||
@@ -280,7 +280,7 @@ fn fuzzy_find_sync(
|
||||
let query = config.query.trim().to_string();
|
||||
let use_cache = config.cache.unwrap_or(false);
|
||||
if use_cache {
|
||||
let scan = fs_cache::get_or_scan(&root, include_hidden, respect_gitignore, &ct)?;
|
||||
let scan = fs_cache::get_or_scan(&root, include_hidden, respect_gitignore, true, &ct)?;
|
||||
let mut results = run_fff_search(
|
||||
&root,
|
||||
&scan.entries,
|
||||
@@ -295,7 +295,7 @@ fn fuzzy_find_sync(
|
||||
&& !query.is_empty()
|
||||
&& scan.cache_age_ms >= fs_cache::empty_recheck_ms()
|
||||
{
|
||||
let fresh = fs_cache::force_rescan(&root, include_hidden, respect_gitignore, true, &ct)?;
|
||||
let fresh = fs_cache::force_rescan(&root, include_hidden, respect_gitignore, true, true, &ct)?;
|
||||
results = run_fff_search(
|
||||
&root,
|
||||
&fresh,
|
||||
@@ -310,7 +310,7 @@ fn fuzzy_find_sync(
|
||||
return Ok(results);
|
||||
}
|
||||
|
||||
let fresh = fs_cache::force_rescan(&root, include_hidden, respect_gitignore, false, &ct)?;
|
||||
let fresh = fs_cache::force_rescan(&root, include_hidden, respect_gitignore, true, false, &ct)?;
|
||||
run_fff_search(&root, &fresh, &query, max_results, include_hidden, respect_gitignore, db, &ct)
|
||||
}
|
||||
|
||||
|
||||
@@ -84,9 +84,10 @@ pub fn max_cache_entries() -> usize {
|
||||
|
||||
#[derive(Clone, Debug, Eq, Hash, PartialEq)]
|
||||
struct CacheKey {
|
||||
root: PathBuf,
|
||||
include_hidden: bool,
|
||||
use_gitignore: bool,
|
||||
root: PathBuf,
|
||||
include_hidden: bool,
|
||||
use_gitignore: bool,
|
||||
skip_node_modules: bool,
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
@@ -199,12 +200,34 @@ pub fn classify_file_type(path: &Path) -> Option<(FileType, Option<f64>)> {
|
||||
|
||||
/// Builds a deterministic filesystem walker configured for visibility and
|
||||
/// ignore rules.
|
||||
pub fn build_walker(root: &Path, include_hidden: bool, use_gitignore: bool) -> WalkBuilder {
|
||||
///
|
||||
/// When `skip_node_modules` is true, `node_modules` directories are pruned at
|
||||
/// traversal time (not just filtered post-scan). `.git` is always skipped.
|
||||
pub fn build_walker(
|
||||
root: &Path,
|
||||
include_hidden: bool,
|
||||
use_gitignore: bool,
|
||||
skip_node_modules: bool,
|
||||
) -> WalkBuilder {
|
||||
let mut builder = WalkBuilder::new(root);
|
||||
builder
|
||||
.hidden(!include_hidden)
|
||||
.follow_links(false)
|
||||
.sort_by_file_path(|a, b| a.cmp(b));
|
||||
.sort_by_file_path(|a, b| a.cmp(b))
|
||||
// filter_entry controls whether to yield an entry AND whether to descend
|
||||
// into a directory. Returning false for a directory skips the entire subtree.
|
||||
.filter_entry(move |entry| {
|
||||
let name = entry.file_name().to_str().unwrap_or_default();
|
||||
// Always skip .git
|
||||
if name == ".git" {
|
||||
return false;
|
||||
}
|
||||
// Skip node_modules when skip_node_modules is true
|
||||
if skip_node_modules && name == "node_modules" {
|
||||
return false;
|
||||
}
|
||||
true
|
||||
});
|
||||
|
||||
if use_gitignore {
|
||||
// Honor repository and global ignore files for repo-like behavior.
|
||||
@@ -229,16 +252,14 @@ pub fn build_walker(root: &Path, include_hidden: bool, use_gitignore: bool) -> W
|
||||
|
||||
/// Scans filesystem entries and records normalized relative paths with file
|
||||
/// metadata.
|
||||
///
|
||||
/// Always stores `node_modules` entries; caller-side filtering handles
|
||||
/// exclusion.
|
||||
fn collect_entries(
|
||||
root: &Path,
|
||||
include_hidden: bool,
|
||||
use_gitignore: bool,
|
||||
skip_node_modules: bool,
|
||||
ct: &task::CancelToken,
|
||||
) -> Result<Vec<GlobMatch>> {
|
||||
let builder = build_walker(root, include_hidden, use_gitignore);
|
||||
let builder = build_walker(root, include_hidden, use_gitignore, skip_node_modules);
|
||||
let mut entries = Vec::new();
|
||||
|
||||
for entry in builder.build() {
|
||||
@@ -246,10 +267,6 @@ fn collect_entries(
|
||||
|
||||
let Ok(entry) = entry else { continue };
|
||||
let path = entry.path();
|
||||
if should_skip_path(path, true) {
|
||||
// The cache always stores node_modules; caller-side filtering is applied later.
|
||||
continue;
|
||||
}
|
||||
|
||||
let relative = normalize_relative_path(root, path);
|
||||
if relative.is_empty() {
|
||||
@@ -281,16 +298,17 @@ pub fn get_or_scan(
|
||||
root: &Path,
|
||||
include_hidden: bool,
|
||||
use_gitignore: bool,
|
||||
skip_node_modules: bool,
|
||||
ct: &task::CancelToken,
|
||||
) -> Result<ScanResult> {
|
||||
let ttl = *CACHE_TTL_MS;
|
||||
if ttl == 0 {
|
||||
// Caching disabled – always scan fresh.
|
||||
let entries = collect_entries(root, include_hidden, use_gitignore, ct)?;
|
||||
let entries = collect_entries(root, include_hidden, use_gitignore, skip_node_modules, ct)?;
|
||||
return Ok(ScanResult { entries, cache_age_ms: 0 });
|
||||
}
|
||||
|
||||
let key = CacheKey { root: root.to_path_buf(), include_hidden, use_gitignore };
|
||||
let key = CacheKey { root: root.to_path_buf(), include_hidden, use_gitignore, skip_node_modules };
|
||||
|
||||
let now = Instant::now();
|
||||
if let Some(entry) = FS_CACHE.get(&key) {
|
||||
@@ -305,7 +323,7 @@ pub fn get_or_scan(
|
||||
FS_CACHE.remove(&key);
|
||||
}
|
||||
|
||||
let entries = collect_entries(root, include_hidden, use_gitignore, ct)?;
|
||||
let entries = collect_entries(root, include_hidden, use_gitignore, skip_node_modules, ct)?;
|
||||
FS_CACHE.insert(key, CacheEntry { created_at: now, entries: entries.clone() });
|
||||
evict_oldest();
|
||||
Ok(ScanResult { entries, cache_age_ms: 0 })
|
||||
@@ -320,13 +338,14 @@ pub fn force_rescan(
|
||||
root: &Path,
|
||||
include_hidden: bool,
|
||||
use_gitignore: bool,
|
||||
skip_node_modules: bool,
|
||||
store: bool,
|
||||
ct: &task::CancelToken,
|
||||
) -> Result<Vec<GlobMatch>> {
|
||||
let key = CacheKey { root: root.to_path_buf(), include_hidden, use_gitignore };
|
||||
let key = CacheKey { root: root.to_path_buf(), include_hidden, use_gitignore, skip_node_modules };
|
||||
FS_CACHE.remove(&key);
|
||||
|
||||
let entries = collect_entries(root, include_hidden, use_gitignore, ct)?;
|
||||
let entries = collect_entries(root, include_hidden, use_gitignore, skip_node_modules, ct)?;
|
||||
if store {
|
||||
let now = Instant::now();
|
||||
FS_CACHE.insert(key, CacheEntry { created_at: now, entries: entries.clone() });
|
||||
@@ -449,4 +468,85 @@ mod tests {
|
||||
|
||||
assert_eq!(classify_file_type(&fifo), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn build_walker_skips_git_and_node_modules() {
|
||||
let root = TempDirGuard::new();
|
||||
fs::create_dir_all(root.path().join(".git/objects")).unwrap();
|
||||
fs::write(root.path().join(".git/objects/a.txt"), "git obj").unwrap();
|
||||
fs::create_dir_all(root.path().join("node_modules/pkg")).unwrap();
|
||||
fs::write(root.path().join("node_modules/pkg/index.js"), "nm").unwrap();
|
||||
fs::write(root.path().join("real.txt"), "ok").unwrap();
|
||||
|
||||
// skip_node_modules: true -> should only see real.txt
|
||||
let walker = super::build_walker(root.path(), true, false, true);
|
||||
let paths: Vec<String> = walker
|
||||
.build()
|
||||
.filter_map(|e| e.ok())
|
||||
.filter(|e| e.path() != root.path())
|
||||
.map(|e| e.path().strip_prefix(root.path()).unwrap().to_string_lossy().into_owned())
|
||||
.collect();
|
||||
assert!(
|
||||
!paths.iter().any(|p| p.contains("node_modules") || p.contains(".git")),
|
||||
"expected no .git or node_modules entries, got: {paths:?}"
|
||||
);
|
||||
assert!(paths.iter().any(|p| p == "real.txt"), "expected real.txt, got: {paths:?}");
|
||||
|
||||
// skip_node_modules: false -> should see node_modules but not .git
|
||||
let walker = super::build_walker(root.path(), true, false, false);
|
||||
let paths: Vec<String> = walker
|
||||
.build()
|
||||
.filter_map(|e| e.ok())
|
||||
.filter(|e| e.path() != root.path())
|
||||
.map(|e| e.path().strip_prefix(root.path()).unwrap().to_string_lossy().into_owned())
|
||||
.collect();
|
||||
assert!(
|
||||
!paths.iter().any(|p| p.contains(".git")),
|
||||
"expected no .git entries, got: {paths:?}"
|
||||
);
|
||||
assert!(
|
||||
paths.iter().any(|p| p.contains("node_modules")),
|
||||
"expected node_modules entries, got: {paths:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn collect_entries_skips_node_modules() {
|
||||
let root = TempDirGuard::new();
|
||||
fs::create_dir_all(root.path().join("node_modules/pkg")).unwrap();
|
||||
fs::write(root.path().join("node_modules/pkg/index.js"), "nm").unwrap();
|
||||
fs::write(root.path().join("real.txt"), "ok").unwrap();
|
||||
|
||||
let ct = crate::task::CancelToken::default();
|
||||
let entries = super::collect_entries(root.path(), true, false, true, &ct).unwrap();
|
||||
let paths: Vec<&str> = entries.iter().map(|e| e.path.as_str()).collect();
|
||||
assert!(
|
||||
!paths.iter().any(|p| p.contains("node_modules")),
|
||||
"expected no node_modules entries, got: {paths:?}"
|
||||
);
|
||||
assert!(paths.iter().any(|p| p == &"real.txt"), "expected real.txt, got: {paths:?}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn force_rescan_respects_skip_node_modules() {
|
||||
let root = TempDirGuard::new();
|
||||
// Create a nested node_modules with many files
|
||||
for i in 0..100 {
|
||||
let pkg_dir = root.path().join(format!("node_modules/pkg-{i}"));
|
||||
fs::create_dir_all(&pkg_dir).unwrap();
|
||||
fs::write(pkg_dir.join("index.js"), "x").unwrap();
|
||||
}
|
||||
fs::write(root.path().join("app.js"), "ok").unwrap();
|
||||
|
||||
let ct = crate::task::CancelToken::default();
|
||||
|
||||
// With skip: should only get app.js
|
||||
let entries = super::force_rescan(root.path(), true, false, true, false, &ct).unwrap();
|
||||
assert_eq!(entries.len(), 1, "skip=true got: {}", entries.len());
|
||||
assert_eq!(entries[0].path, "app.js");
|
||||
|
||||
// Without skip: should get app.js + 100 node_modules files + directories
|
||||
let entries = super::force_rescan(root.path(), true, false, false, false, &ct).unwrap();
|
||||
assert!(entries.len() > 100, "skip=false got: {}", entries.len());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -239,8 +239,9 @@ fn run_glob(
|
||||
{
|
||||
collect_files_from_picker(&config.root, &glob_set, &config, db, on_match, &ct)?
|
||||
} else if config.use_cache {
|
||||
let skip_node_modules = !config.mentions_node_modules;
|
||||
let scan =
|
||||
fs_cache::get_or_scan(&config.root, config.include_hidden, config.use_gitignore, &ct)?;
|
||||
fs_cache::get_or_scan(&config.root, config.include_hidden, config.use_gitignore, skip_node_modules, &ct)?;
|
||||
let mut matches = filter_entries(&scan.entries, &glob_set, &config, on_match, &ct)?;
|
||||
// Empty-result recheck: if we got zero matches from a cached scan that's old
|
||||
// enough, force a rescan and try once more before returning empty.
|
||||
@@ -249,6 +250,7 @@ fn run_glob(
|
||||
&config.root,
|
||||
config.include_hidden,
|
||||
config.use_gitignore,
|
||||
skip_node_modules,
|
||||
true,
|
||||
&ct,
|
||||
)?;
|
||||
@@ -256,10 +258,12 @@ fn run_glob(
|
||||
}
|
||||
matches
|
||||
} else {
|
||||
let skip_node_modules = !config.mentions_node_modules;
|
||||
let fresh = fs_cache::force_rescan(
|
||||
&config.root,
|
||||
config.include_hidden,
|
||||
config.use_gitignore,
|
||||
skip_node_modules,
|
||||
false,
|
||||
&ct,
|
||||
)?;
|
||||
|
||||
@@ -1509,17 +1509,17 @@ fn grep_sync(
|
||||
&ct,
|
||||
)?
|
||||
} else if use_cache {
|
||||
let scan = fs_cache::get_or_scan(&search_path, include_hidden, use_gitignore, &ct)?;
|
||||
let scan = fs_cache::get_or_scan(&search_path, include_hidden, use_gitignore, true, &ct)?;
|
||||
let mut entries =
|
||||
collect_files(&search_path, &scan.entries, glob_set.as_ref(), type_filter.as_ref());
|
||||
if entries.is_empty() && scan.cache_age_ms >= fs_cache::empty_recheck_ms() {
|
||||
let fresh =
|
||||
fs_cache::force_rescan(&search_path, include_hidden, use_gitignore, true, &ct)?;
|
||||
fs_cache::force_rescan(&search_path, include_hidden, use_gitignore, true, true, &ct)?;
|
||||
entries = collect_files(&search_path, &fresh, glob_set.as_ref(), type_filter.as_ref());
|
||||
}
|
||||
entries
|
||||
} else {
|
||||
let fresh = fs_cache::force_rescan(&search_path, include_hidden, use_gitignore, false, &ct)?;
|
||||
let fresh = fs_cache::force_rescan(&search_path, include_hidden, use_gitignore, true, false, &ct)?;
|
||||
collect_files(&search_path, &fresh, glob_set.as_ref(), type_filter.as_ref())
|
||||
};
|
||||
// Check cancellation before heavy work
|
||||
|
||||
Reference in New Issue
Block a user