Skip to content

Fix exclude filters skipping adjacent files and directories - #176

Merged
ryanmrichard merged 1 commit into
CMakePP:masterfrom
rkent:fix-exclude-skips-adjacent
Oct 22, 2026
Merged

ryanmrichard merged 1 commit into
CMakePP:masterfrom
rkent:fix-exclude-skips-adjacent

Conversation

@rkent

@rkent rkent commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

While implementing CMinx to document ament_cmake, Claude Opus 5.5 reported as an aside that there was a bug in CMinx. Then when running CMinx with a number of files excluded, some files showed that were not supposed to because of the bug.

Claude seemed to know what to do, so I let it do it. The result is below. It looks useful to me, but I don't really know CMinx other than as a user.


Claude says:

When document() applied exclude filters, it removed matching entries from the subdirs and filenames lists while iterating over those same lists. Each removal shifted the list, so the entry following an excluded one was never checked, and an excluded file or directory next to another excluded one was still documented.

Iterate over a copy of each list, as is already done for the auto_exclude_directories_without_cmake check.

Is this pull request associated with an issue(s)?
No

Description
This will prevent excludes from inadvertently including adjacent directories.

When document() applied exclude filters, it removed matching entries
from the subdirs and filenames lists while iterating over those same
lists. Each removal shifted the list, so the entry following an
excluded one was never checked, and an excluded file or directory
next to another excluded one was still documented.

Iterate over a copy of each list, as is already done for the
auto_exclude_directories_without_cmake check.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: R Kent James <kent@caspia.com>
@CLAassistant

CLAassistant commented Oct 21, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@ryanmrichard
ryanmrichard requested a balanced review from Copilot October 22, 2026 16:20
@codecov

codecov Bot commented Oct 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.16%. Comparing base (65f17f0) to head (e128d0f).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #176   +/-   ##
=======================================
  Coverage   97.16%   97.16%           
=======================================
  Files           8        8           
  Lines         986      986           
=======================================
  Hits          958      958           
  Misses         28       28           
Flag Coverage Δ
unittests 97.16% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI 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.

🟢 Approval recommended

The focused changes fix both affected loops, preserve directory pruning, and include regression coverage with no unresolved findings.

No open issues 🎉

What changed in this PR

This PR fixes CMinx exclusion filtering so adjacent excluded files and directories are not accidentally documented.

Changes:

  • Iterate over copies of directory and filename lists while removing excluded entries from the originals.
  • Add a regression test that verifies adjacent exclusions and confirms retained entries are documented.
File Description
tests/​unit_tests/​test_config.py Adds regression coverage for adjacent file and directory exclusions.
src/​cminx/​__init__.py Prevents skipped entries while preserving in-place directory pruning.

Review effort: Balanced

@ryanmrichard

Copy link
Copy Markdown
Collaborator

Thanks for the contribution! The changes look great!

@ryanmrichard
ryanmrichard merged commit af446f6 into CMakePP:master Oct 22, 2026
15 checks passed
@github-actions

Copy link
Copy Markdown

🚀 [bumpr] Bumped!
New version:v1.1.14
Changes:v1.1.13...v1.1.14

@rkent
rkent deleted the fix-exclude-skips-adjacent branch October 28, 2026 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants