Skip to content

Conversation

@ScottNotFound
Copy link
Contributor

@ScottNotFound ScottNotFound commented Jan 22, 2026

Changes in this PR only affect behavior in a non CI or non GitHub environment.

Resolves #179

Previous behavior

If staged files exist, then use diff between HEAD and index (staging).
Otherwise, use the diff between HEAD~1 and HEAD.

New behavior

Previous behavior is unchanged. Options were added to allow:

  1. --diff-base BASE and empty index: diff between BASE and HEAD
  2. --diff-base BASE and populated index: diff between BASE and index
  3. --ignore-index: follows behavior as if the index is empty.

Thus, --diff-base BASE and --ignore-index --> always diff between BASE and HEAD

Summary by CodeRabbit

  • New Features

    • Added --diff-base / -b (accepts integer or string) and --ignore-index flags to the CLI.
  • Behavior

    • Diff resolution now respects an explicit base and whether staged/indexed changes are considered, changing which commits/trees are compared.
  • Tests

    • CLI tests extended for new flags; added tests covering SHA/base resolution.
  • Documentation

    • Release matrix updated to list support for diff_base and ignore_index.

@ScottNotFound ScottNotFound changed the title Allow command line users to specify the base commit to use and flag to ignore the index for producing diffs feat: Allow command line users to specify the base commit to use and flag to ignore the index for producing diffs Jan 22, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 22, 2026

Warning

Rate limit exceeded

@2bndy5 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 26 minutes and 14 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

Walkthrough

Adds optional diff selection and index-ignoring flags: diff_base and ignore_index are introduced to the CLI, git utilities, REST/Git clients, and are forwarded through main execution paths; tests and docs updated to exercise parsing and SHA/diff resolution behaviors.

Changes

Cohort / File(s) Summary
CLI Interface
cpp_linter/cli.py
Added Union import; new Args fields diff_base: Optional[Union[int,str]] and ignore_index: bool; added -b/--diff-base and --ignore-index parser flags.
Library entry / main
cpp_linter/__init__.py
main() now forwards diff_base and ignore_index into GithubApiClient.get_list_of_changed_files(...) in both execution branches.
Git utilities
cpp_linter/git/... cpp_linter/git/__init__.py
get_sha() accepts parent: Optional[Union[int,str]]; get_diff() signature changed to parents: Optional[Union[int,str]] = None, ignore_index: bool = False; diff-base resolution and staged-vs-index logic updated to respect these args and to name/cache diffs accordingly.
REST API client
cpp_linter/rest_api/__init__.py, cpp_linter/rest_api/github_api.py
Added Union import; extended RestApiClient.get_list_of_changed_files() and GithubApiClient.get_list_of_changed_files() signatures with diff_base and ignore_index; docs updated; non-CI path now calls get_diff(diff_base, ignore_index) and parses that diff.
Tests
tests/test_cli_args.py, tests/capture_tools_output/test_tools_output.py
CLI argument tests accept Optional values for diff-base (int/str) and ignore-index; added/updated test_get_sha to validate SHA resolution; typing imports adjusted (Optional, Tuple, Union).
Docs / Config
docs/conf.py
Added REQUIRED_VERSIONS["1.12.0"] = ["diff_base", "ignore_index"].

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • shenxianpeng
  • 2bndy5
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely describes the main change: allowing users to specify the base commit for local (non-CI) diffs, which is the core feature added in this PR.
Linked Issues check ✅ Passed The PR fully addresses issue #179 by implementing CLI options (--diff-base and --ignore-index) to allow users to explicitly choose the base commit reference for diffs in non-CI environments.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing the requested feature. The modifications span CLI argument parsing, git operations, REST API client methods, and corresponding tests—all necessary for the feature.
Docstring Coverage ✅ Passed Docstring coverage is 88.89% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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 and usage tips.

coderabbitai[bot]

This comment was marked as resolved.

@codecov
Copy link

codecov bot commented Jan 22, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.30%. Comparing base (4dc4d9d) to head (3eb723b).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #180      +/-   ##
==========================================
+ Coverage   98.27%   98.30%   +0.02%     
==========================================
  Files          24       24              
  Lines        1916     1947      +31     
==========================================
+ Hits         1883     1914      +31     
  Misses         33       33              

☔ View full report in Codecov by Sentry.
📢 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.

@2bndy5 2bndy5 changed the title feat: Allow command line users to specify the base commit to use and flag to ignore the index for producing diffs feat: Allow specifying the base commit for local (non-CI) diffs Jan 22, 2026
coderabbitai[bot]

This comment was marked as resolved.

@2bndy5

This comment was marked as resolved.

@ScottNotFound ScottNotFound force-pushed the allow-specify-parent-index branch from c4de8ea to f9b56dc Compare January 22, 2026 22:14
@ScottNotFound
Copy link
Contributor Author

I did not mean to close this.

@ScottNotFound

This comment was marked as outdated.

@ScottNotFound ScottNotFound reopened this Jan 22, 2026
coderabbitai[bot]

This comment was marked as resolved.

Co-authored-by: Brendan <2bndy5@gmail.com>
coderabbitai[bot]

This comment was marked as resolved.

Copy link
Contributor

@2bndy5 2bndy5 left a comment

Choose a reason for hiding this comment

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

This looks great, thanks!

I have some final requests about the docs and --ignore-index CLI parsing.

Also, could you apply the following patch to conf.py:

diff --git a/docs/conf.py b/docs/conf.py
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -238,6 +238,7 @@ REQUIRED_VERSIONS = {
     "1.8.1": ["jobs"],
     "1.9.0": ["ignore_tidy", "ignore_format"],
     "1.10.0": ["passive_reviews"],
+    "1.12.0": ["diff_base", "ignore_index"],
 }
 SUBCOMMAND_VERSIONS = {"version": "1.11.0"}

It will help when generating the CLI doc from the --help output. Currently, it is incorrect:
image

Yes, I intend to bump the minor version because this is a new feature.

@shenxianpeng shenxianpeng added the enhancement New feature or request label Jan 28, 2026
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@cpp_linter/rest_api/__init__.py`:
- Around line 171-180: The docstring for parameter diff_base in __init__.py has
the diff ranges reversed; update the CSV table so ranges are "older..newer"
(base → head/index): replace "HEAD..HEAD~1" with "HEAD~1..HEAD" and replace
"index..HEAD" with "HEAD..index" so the examples for diff_base (in the table
under the diff_base param) show the correct base→target ordering.

2bndy5
2bndy5 previously requested changes Feb 4, 2026
Copy link
Contributor

@2bndy5 2bndy5 left a comment

Choose a reason for hiding this comment

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

Small doc revision about CLI option vs switch.

2bndy5 and others added 2 commits February 4, 2026 13:29
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@2bndy5 2bndy5 dismissed their stale review February 4, 2026 21:30

Resolved

Copy link
Contributor

@2bndy5 2bndy5 left a comment

Choose a reason for hiding this comment

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

🎉

@2bndy5 2bndy5 merged commit b3c28b6 into cpp-linter:main Feb 4, 2026
41 checks passed
@2bndy5 2bndy5 added the minor A minor version bump label Feb 5, 2026
2bndy5 added a commit to cpp-linter/cpp-linter-rs that referenced this pull request Feb 5, 2026
2bndy5 added a commit to cpp-linter/cpp-linter-rs that referenced this pull request Feb 5, 2026
- ref cpp-linter/cpp-linter#180
- as discussed in cpp-linter/cpp-linter#179

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Added --diff-base and --ignore-index CLI options to control base diffs
and staged-file handling.

* **Documentation**
  * Documented new CLI options (minimum version 1.12.0).
* Improved CLI argument header formatting to handle missing short names.

* **Chores**
  * Default test filter updated to exclude a specific test.

* **Tests**
* Tests expanded to cover diff-base and ignore-index behaviors and
updated call sites.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
2bndy5 added a commit that referenced this pull request Feb 6, 2026
In porting #180 to rust (cpp-inter/cpp-linter-rs#260),
I found a few spots for improvement.
These changes should be more performant when
there are staged changes.

I also made some typing-related changes to satisfy mypy.
2bndy5 added a commit that referenced this pull request Feb 6, 2026
In porting #180 to rust (cpp-linter/cpp-linter-rs#260),
I found a few spots for improvement.
These changes should be more performant when
there are staged changes.
2bndy5 added a commit that referenced this pull request Feb 6, 2026
In porting #180 to rust (cpp-linter/cpp-linter-rs#260),
I found a few spots for improvement.
These changes should be more performant when
there are staged changes.
2bndy5 added a commit that referenced this pull request Feb 6, 2026
In porting #180 to rust (cpp-linter/cpp-linter-rs#260),
I found a few spots for improvement.
These changes should be more performant when
there are staged changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request minor A minor version bump

Projects

None yet

Development

Successfully merging this pull request may close these issues.

support explicitly choosing the ref commits to use for diffs

3 participants