lines-changed-only: true -> always analyses +1 line

[mkmer]

What events trigger your workflow?

name: "Pre-commit checks"

on:
   pull_request:
      types: [opened, synchronize, reopened]

permissions:
  contents: write
  actions: write
  pull-requests: write

jobs:
   codespell:
    name: "Check for spelling errors"
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
        with:
          persist-credentials: false
      - uses: codespell-project/actions-codespell@v2
        with:
          check_filenames: true
          ignore_words_file: ./.dev/.codespellignore
          # When using this Action in other repos, the --skip option below can be removed
          # and the .codespellignore file can be used to ignore files or words.          
   lint:
    name: "C-lint tools"
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - uses: cpp-linter/cpp-linter-action@v2
        id: linter
        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
        with:
          version: '19'
          lines-changed-only: true
          format-review: true
          style: 'file:./.dev/.clang-format'  # Use .clang-format config file
          tidy-checks: '-*' # Use .clang-tidy config file
          # only 'update' a single comment in a pull request thread.
          thread-comments: ${{ github.event_name == 'pull_request' && 'update' }}
      - name: Fail fast?!
        if: steps.linter.outputs.checks-failed > 0
        run: exit 1

What OS does your workflow use?

Github action container.
Linux

If you need more info, see the action run:(https://github.com/AllStarLink/app_rpt/actions/runs/20413702845/job/58654206133)

How is cpp-linter-action configured?

lint:
    name: "C-lint tools"
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - uses: cpp-linter/cpp-linter-action@v2
        id: linter
        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
        with:
          version: '19'
          lines-changed-only: true
          format-review: true
          style: 'file:./.dev/.clang-format'  # Use .clang-format config file
          tidy-checks: '-*' # Use .clang-tidy config file
          # only 'update' a single comment in a pull request thread.
          thread-comments: ${{ github.event_name == 'pull_request' && 'update' }}
      - name: Fail fast?!
        if: steps.linter.outputs.checks-failed > 0
        run: exit 1

What was the unexpected behavior?

Github action container.

INFO:CPP Linter:Running "/usr/bin/clang-format-19 -style=file:./.dev/.clang-format --output-replacements-xml --lines=758:761 --lines=829:838 --lines=893:894 --lines=925:926 --lines=937:939 --lines=958:960 --lines=985:989 --lines=1022:1024 --lines=1032:1039 --lines=1087:1088 --lines=1089:1090 apps/app_rpt/app_rpt.h"

All of the above "lines=" are +1 line more than what is actually changed.
ex: --lines=758:761 should be --lines=758-760

Only look at the changed lines as the "docs" explain.

[coderabbitai]

📝 CodeRabbit Plan Mode

Generate an implementation plan and prompts that you can use with your favorite coding agent.

• Create Plan

Examples

• Example 1
• Example 2

---

🔗 Similar Issues

Possible Duplicates

• ISSUE-297

Related Issues

• clang-tidy cannot be disabled since v2.16.1 #323
• Lines-changed-only seems not to work for clang-tidy #297
• Clang-tidy detects issues in code, but WarningsAsErrors pervents cpp-linter from reporting them #347
• unexpected source files has been included #321
• an not dismiss a commented pull request review #290

🔗 Related PRs

cpp-linter/clang-tools-static-binaries#42 - fixup! feat: add pre-commit hooks and fix build issues [merged]
#293 - docs: add used by section [merged]
#324 - fix: pass inputs as uninterpretted strings [merged]
cpp-linter/.github#42 - feat: use full commit SHA hash for dependency [merged]
cpp-linter/.github#51 - add: statically analyze CI workflows [merged]

👤 Suggested Assignees

• shenxianpeng
• 2bndy5
• kleisauke
• regit
• FelipeGdM

---

🧪 Issue enrichment is currently in open beta.

You can configure auto-planning by selecting labels in the issue_enrichment configuration.

To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

issue_enrichment:
  auto_enrich:
    enabled: false

💬 Have feedback or questions? Drop into our discord or schedule a call!

[2bndy5]

The upper bounds are not inclusive for the range of line numbers. At least that's how the upper bounds behaved when I tested/developed the lines-changed-only feature. Maybe the upper bounds changed from exclusive to inclusive at some major version of clang. Please check the clang-format docs.

Are you trying to report a problem with what cpp-linter returns from clang-format output? Or are you just noticing the args passed to clang-format?

[mkmer]

Well... Our Github workflow does not match our local clang-format behavior. When run locally (same version 19.x) with the "same" options we don't have +1 line check. This unfortunately makes for a lot of pre-checks that pass but then fail when github runs the work flow.
The args passed to clang-format are what is in "error"

[2bndy5]

I guess the only way to definitively know if the upper bounds are exclusive is to intentionally create a format error on the last line of a diff chunk:

-    unsigned int deleted:1;
+  unsigned   int deleted  :    1;

And then run clang-format locally without a +1 to the upper bounds.

Warning

A single line in a range of --lines may not accurately yield expected results because the starting line number is inclusive.

Its better to experiment with --lines on a range of multiple lines.

For example, --lines=760:760 means only line 760, but --lines=758:760 would mean lines 758 and 759 (and 760 if your assumption is correct about the upper bounds being inclusive).

If it still passes, you know for sure that the upper bounds is exclusive.

FYI, this is what our unit tests do in our cpp-linter/cpp-linter (python) project.

[mkmer]

I know it is testing the "extra" line as we continually get format errors on the +1 (unchanged) line after a commit on the git action.

[2bndy5]

I'm not seeing that in your PR AllStarLink/app_rpt#881 (which instigated the run linked in the OP). In fact, all the lines that clang-format (and thus cpp-linter) flags are within the ranges of changed lines.

Please excuse my skepticism. I'm very hesitant to change something about our most popular feature. So far, you have not provided any proof that the upper bounds are inclusive instead of exclusive.

[mkmer]

Here is one of MANY: We change line 600, yet it suggested a change on 601.
AllStarLink/app_rpt#881 (comment)

lines_changed_only (if I understand correctly) should only look at 600 (right?)
I believe the root issue is that the command is called with --lines=600:601

[2bndy5]

Ah, thank you. I was having trouble navigating the PR review suggestions (specifically the ones from cpp-linter) and corresponding them with the CI workflow runs that generated the suggestions.

1. The lines used in PR review suggestions (format-review) are actually gotten from diff-ing the original file contents with changes from running clang-format (with the --lines args you have noted) on the content.
2. We have noticed in the past that clang-format does not strictly adhere to the --lines value(s). Every so often we get reported cases like this where the lines surrounding the changed lines are also formatted.
3. The file-annotations are filtered down to lines that are actually changed (due to the reports referenced in point 2). The PR review suggestions are not filtered the same way as file-annotations. Instead, we rely on setting the number of context lines to 0 in the diff of suggestions. Then we only make sure that the suggestions can fit within the diff of the PR's "Changed Files" tab. FWIW, diff algorithms are very complex, and we're not trying to reinvent the wheel here.

Lastly, I'm not trying to be infuriating. I'm only trying to be precise; hearsay is not evidence.
Now that I have a real-life undesirable scenario, I can conduct experiments to find a cause/solution. The thing I'm most worried about is regressions.

[mkmer]

The issue (as it stands) - we are formatting old code as we commit to the repo. When it reaches +1 (or more) lines past a commit, it becomes "endless" as the fix adds 1 more line. The next commit looks +1 more line and so on.
Your support and effort is VERY much appreciated.
FWIW, running this locally: git clang-format --style=file:./.dev/.clang-format --staged only looks at the changed lines (hence the conflict between local and remote checks).

[2bndy5]

git clang-format --style=file:./.dev/.clang-format --staged

This actually uses a python script maintained officially by LLVM. It may be worth examining how they handle --lines. However, there are a few more steps in cpp-linter to abstract PR review suggestions.

[2bndy5]

git clang-format --style=file:./.dev/.clang-format --staged

This actually uses a python script maintained officially by LLVM. It may be worth examining how they handle --lines.

Found it! That LLVM python script uses --lines like so:

"--lines=%s:%s" % (start_line, start_line + line_count - 1)

Where the start_line and line_count is from the diff hunk header (added lines, eg. @@ ... +750,3 @@), notice the end line number is subtracted by 1 (probably because line numbers are a 1-based count). This means that the end line number is treated as inclusive.