Add p4ignore.ini support - #92
Merged
Merged
Conversation
Recognize root-level p4ignore.ini rules across indexing, metadata scans, file counting, and watcher filtering. Normalize Windows separators and preserve negated re-inclusions while pruning ignored subtrees. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 379a17fb-47de-4e29-9192-322b10af777b
Contributor
There was a problem hiding this comment.
Pull request overview
Adds Perforce-style ignore support by loading root-level p4ignore.ini rules alongside existing Git ignore behavior, and applies those rules consistently during directory walks, metadata scans, and watcher filtering.
Changes:
- Introduce
p4ignore.iniparsing/matching (including Windows separator normalization and!re-includes). - Prune p4ignored subtrees during indexing walks, file metadata scans, and
.gitignorediscovery for watcher matchers. - Update CLI/README docs to mention
p4ignore.iniunder--no-ignore.
Show a summary per file
| File | Description |
|---|---|
| tgrep-core/src/walker.rs | Adds p4ignore-based pruning to repo walks and metadata scans; includes tests. |
| tgrep-core/src/gitignore.rs | Implements p4ignore parsing/matcher and integrates rules into point-query ignore matcher. |
| tgrep-cli/src/main.rs | Updates --no-ignore help text to mention p4ignore.ini. |
| README.md | Documents p4ignore support and updates --no-ignore description. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 4/4 changed files
- Comments generated: 4
- Review effort level: Lite
Propagate --no-ignore through index and serve discovery, include global Git excludes in watcher matching, and reconcile live indexes when ignore files change. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 379a17fb-47de-4e29-9192-322b10af777b
Signed-off-by: Shengyu Fu <shengyfu@microsoft.com>
Keep .gitignore files discoverable when p4ignore.ini excludes them, and consolidate serve options after the latest main merge. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 379a17fb-47de-4e29-9192-322b10af777b
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 379a17fb-47de-4e29-9192-322b10af777b
Contributor
There was a problem hiding this comment.
Review details
Suppressed comments (4)
Previously missed (4) — in code that hasn't changed since the last review.
tgrep-core/src/walker.rs:206
p4ignore.inirules are added to the point-query builder before.gitignorerules. Since gitignore pattern order is “last match wins”, a later.gitignorewhitelist can override an earlier p4ignore ignore, but the actual directory walk applies p4ignore as a hardfilter_entry(so.gitignorecannot override it). This mismatch can let the watcher upsert files the initial walk would never index, causing divergence. To mirror the walker's behavior, add p4ignore rules after.gitignorerules so p4ignore has higher precedence (or otherwise make both paths use the same precedence model).
let info_exclude = root.join(".git").join("info").join("exclude");
if info_exclude.is_file() {
let _ = builder.add(&info_exclude);
}
crate::gitignore::add_p4ignore_rules(&mut builder, root);
tgrep-core/src/gitignore.rs:198
build_matcheraddsp4ignore.inirules before discovering/adding.gitignorerules. Because gitignore precedence is order-dependent, this allows a later.gitignorewhitelist to override an earlier p4ignore ignore, but the actual filesystem walk applies p4ignore via a separatefilter_entry(effectively higher precedence). That inconsistency can make the watcher treat paths as indexable that the walker would have pruned. A minimal way to align semantics is to (re-)add p4ignore rules after the.gitignorediscovery loop so p4ignore patterns come last.
for entry in walker.flatten() {
if entry.file_name() == ".gitignore" && entry.path().is_file() {
let _ = builder.add(entry.path());
}
}
tgrep-cli/src/serve.rs:1316
- When
indexed_pathsisSome(ignore-rule reconciliation) butold_stampsis empty (e.g., filestamps missing/corrupt), the current logic classifies every already-indexed file asaddedbecause the stamp lookup always returnsNone. That can trigger a full reindex on any ignore-rule change even though the reconciliation mode already hasindexed_pathsand could limit work to membership changes (newly-unignored / newly-ignored). Consider short-circuiting the stamp comparison whenindexed_paths.is_some()andold_stamps.is_empty()to avoid reindexing everything unnecessarily.
match old_stamps.get(&fm.relative_path) {
Some(old) if *old == stamp => {}
Some(_) => changed.push(fm.relative_path.clone()),
None => added.push(fm.relative_path.clone()),
}
Cargo.toml:6
- This PR is scoped to adding
p4ignore.inisupport, but it also bumps the workspace version from0.1.21to1.0.0. If this is intentional (a 1.0 release), it should be called out explicitly in the PR description / release notes; otherwise, consider splitting it into a separate release PR to keep changes focused.
[workspace.package]
version = "1.0.0"
- Files reviewed: 10/11 changed files
- Comments generated: 0 new
- Review effort level: Lite
Limit Perforce ignore refreshes to the supported root file and ensure watcher event bursts share a single refresh worker. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 379a17fb-47de-4e29-9192-322b10af777b
Contributor
There was a problem hiding this comment.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
Cargo.toml:6
- Workspace version is bumped to 1.0.0 as part of this PR, but the PR description is focused on ignore-rule behavior and doesn’t mention a release/versioning change. Since this affects downstream consumers and release notes, either document the version bump in the PR description (or changelog), or split the version bump into a dedicated release PR if that was not intended.
version = "1.0.0"
- Files reviewed: 10/11 changed files
- Comments generated: 1
- Review effort level: Lite
baopingz
approved these changes
Aug 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
p4ignore.inirules alongside Git ignore rules--no-ignoreconsistently through search, counting, indexing, serving, reloads, and stale scans.gitignoreorp4ignore.inichanges while serving--no-ignoresemanticsValidation
cargo test --workspace --quietcargo fmt --all -- --checkcargo clippy --workspace --all-targets --quiet -- -D warnings