Skip to content

fix: match exclude_files against relative paths - #463

Open
yangfan-yf-yf wants to merge 2 commits into
cpplint:developfrom
yangfan-yf-yf:fix/exclude-files-relative-path
Open

fix: match exclude_files against relative paths#463
yangfan-yf-yf wants to merge 2 commits into
cpplint:developfrom
yangfan-yf-yf:fix/exclude-files-relative-path

Conversation

@yangfan-yf-yf

@yangfan-yf-yf yangfan-yf-yf commented Aug 9, 2026 •
edited
Loading

Copy link
Copy Markdown

Fixes #254.

Summary

  • match each exclude_files expression against the target path relative to the CPPLINT.cfg that defines it
  • normalize path components to / on every platform
  • preserve same-directory matching, parent configuration traversal, and set noparent
  • document the relative-path behavior and its compatibility boundary

An anchored directory-only pattern such as ^bar$ should be written as ^bar/ when the intent is to exclude that subtree.

Validation

  • focused configuration tests: 7 passed
  • full test suite: 238 passed
  • Ruff 0.15.20 check and format check passed
  • Mypy, Pylint, Codespell, and git diff --check passed

@coderabbitai

coderabbitai Bot commented Aug 9, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

exclude_files patterns now match source paths relative to their CPPLINT.cfg, using forward slashes. Tests cover nested paths, inherited exclusions, and set noparent.

Changes

Relative exclude_files matching

Layer / File(s) Summary
Compute and apply relative exclusion paths
cpplint.py, CHANGELOG.rst		 cpplint.py builds configuration-relative paths, matches exclude_files against those paths, and reports the matching path and pattern. The changelog documents the behavior.
Validate configuration inheritance and matching
cpplint_unittest.py		 Tests cover exact, nested, and nonmatching patterns, parent exclusion inheritance, and set noparent.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes implement relative-path exclude_files matching for subdirectory files and preserve nested configuration and set noparent behavior required by issue #254.
Out of Scope Changes check ✅ Passed The code, tests, and changelog changes directly support the relative-path exclude_files objective and contain no unrelated scope.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change to match exclude_files patterns against relative paths.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@yangfan-yf-yf
yangfan-yf-yf marked this pull request as ready for review August 9, 2026 17:03
coderabbitai[bot]
coderabbitai Bot reviewed Aug 9, 2026
View reviewed changes

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cpplint.py`:
- Around line 270-273: Document in the exclude_files path-matching guidance in
cpplint.py (lines 270-273) that anchored directory patterns must be changed from
^bar$ to ^bar/. Add ("bar/baz.cc", "^bar$", True) to the relevant compatibility
test cases in cpplint_unittest.py (lines 7295-7302) to lock the behavior that
^bar$ does not exclude the bar subtree.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 68156488-6931-42bb-9d53-40dde9ee4fd0

📥 Commits

Reviewing files that changed from the base of the PR and between 96db7ae and 5f7c736.

📒 Files selected for processing (3)
  • CHANGELOG.rst
  • cpplint.py
  • cpplint_unittest.py

Comment thread cpplint.py Outdated
PNHD
PNHD approved these changes Aug 14, 2026
View reviewed changes

@PNHD PNHD left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed current head db3dfc9. The implementation consistently matches exclude_files against the path relative to the defining CPPLINT.cfg, normalizes separators, preserves parent traversal and set noparent, and documents/tests the anchored-directory compatibility boundary (^bar$ vs ^bar/). The prior review finding about that boundary is addressed on this head. The PR reports 7 focused configuration tests and 238 full-suite tests passing, together with Ruff, Mypy, Pylint, Codespell, and git diff --check. LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments
+1 more reviewer
@PNHD PNHD PNHD approved these changes
Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Allow exclude_files option in the config file to exclude a specific file in a subdirectory

2 participants

@yangfan-yf-yf @PNHD