Skip to content

fix(rules): ignore footer lines in body max length#4813

Open
puneetdixit200 wants to merge 1 commit into
conventional-changelog:masterfrom
puneetdixit200:fix-body-line-length-footer
Open

fix(rules): ignore footer lines in body max length#4813
puneetdixit200 wants to merge 1 commit into
conventional-changelog:masterfrom
puneetdixit200:fix-body-line-length-footer

Conversation

@puneetdixit200

Copy link
Copy Markdown

Summary

  • ignore trailing footer/trailer paragraphs when checking body line lengths
  • add a regression test for long Signed-off-by footer lines with short body lines

Fixes #4088

Tests

  • pnpm exec vitest run @commitlint/rules/src/body-max-line-length.test.ts @commitlint/rules/src/footer-max-line-length.test.ts
  • node scripts/check-no-focused-tests.js @commitlint/rules/src/body-max-line-length.test.ts
  • pnpm exec oxfmt --check --ignore-path .oxfmtignore @commitlint/rules/src/body-max-line-length.ts @commitlint/rules/src/body-max-line-length.test.ts
  • pnpm exec oxlint @commitlint/rules/src/body-max-line-length.ts @commitlint/rules/src/body-max-line-length.test.ts
  • git diff --check
  • pnpm build

@qodo-code-review

Copy link
Copy Markdown

Review Summary by Qodo

Ignore footer lines in body max length validation

🐞 Bug fix 🧪 Tests

Grey Divider

Walkthroughs

Description
• Ignore footer/trailer lines when checking body max line length
• Add regression test for long footer lines with short body
• Implement trailer token detection to identify footer paragraphs
• Exclude trailing footers from body line length validation
Diagram
flowchart LR
  A["body-max-line-length rule"] -->|parse body and footer| B["withoutTrailingFooter function"]
  B -->|detect trailer tokens| C["TRAILER_TOKEN regex"]
  C -->|identify footer lines| D["remove footer from validation"]
  D -->|validate remaining body| E["maxLineLength check"]
  E -->|return result| F["pass/fail decision"]

Loading

Grey Divider

File Changes

1. @commitlint/rules/src/body-max-line-length.ts 🐞 Bug fix +39/-2

Implement footer detection and exclusion logic

• Add withoutTrailingFooter() function to strip trailing footer paragraphs
• Implement TRAILER_TOKEN regex pattern to identify trailer lines
• Import toLines utility for line parsing
• Modify rule to exclude footers from body line length validation

@commitlint/rules/src/body-max-line-length.ts


2. @commitlint/rules/src/body-max-line-length.test.ts 🧪 Tests +8/-0

Add regression test for long footer lines

• Add longFooter test constant with long Signed-off-by line
• Add longFooter message test case combining short body with long footer
• Add regression test "with long footer should succeed"

@commitlint/rules/src/body-max-line-length.test.ts


Grey Divider

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented Jun 4, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (1)

Context used

Grey Divider


Action required

1. TRAILER_TOKEN excludes valid footers 📎 Requirement gap ≡ Correctness
Description
withoutTrailingFooter() only treats the last paragraph as a footer if every line matches
TRAILER_TOKEN, but the regex requires a hyphenated token and will miss valid trailers like
Fixes:/Refs:/BREAKING CHANGE:. As a result, some footer lines can still be checked by
body-max-line-length, violating the requirement that footer line-length is not enforced by the
body rule.
Code

@commitlint/rules/src/body-max-line-length.ts[R5-33]

+const TRAILER_TOKEN = /^[A-Za-z0-9]+(?:-[A-Za-z0-9]+)+:\s\S/;
+
+function withoutTrailingFooter(body: string, footer?: string | null): string {
+	const footerStart = footer ? body.length - footer.length : -1;
+	const input =
+		footer &&
+		footerStart >= 0 &&
+		body.endsWith(footer) &&
+		(footerStart === 0 || body[footerStart - 1] === "\n")
+			? body.slice(0, footerStart).replace(/(\r?\n)+$/, "")
+			: body;
+
+	const lines = toLines(input);
+	let last = lines.length - 1;
+
+	while (last >= 0 && lines[last] === "") {
+		last--;
+	}
+
+	let first = last;
+
+	while (first >= 0 && lines[first] !== "") {
+		first--;
+	}
+
+	const footerLines = lines.slice(first + 1, last + 1);
+
+	if (footerLines.length === 0 || !footerLines.every((line) => TRAILER_TOKEN.test(line))) {
+		return input;
Evidence
The compliance rule requires that footer lines are not validated by body-max-line-length. The
added TRAILER_TOKEN regex only matches hyphenated trailer keys ((?:-[A-Za-z0-9]+)+), and
withoutTrailingFooter() returns the unmodified input when trailer lines don’t match it, leaving
such footers to be checked by maxLineLength() in the body rule.

Respect footer-max-line-length for footer lines (do not fail body-max-line-length)
@commitlint/rules/src/body-max-line-length.ts[5-36]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`body-max-line-length` is intended to ignore footer/trailer lines so they are not validated against the body max length. The current trailer detection regex (`TRAILER_TOKEN`) only matches hyphenated tokens (e.g., `Signed-off-by:`), which means other valid trailer/footer tokens (e.g., `Fixes:`, `Refs:`, `BREAKING CHANGE:`) may remain in the body input and still trigger `body-max-line-length` failures.

## Issue Context
Compliance requires that footer lines are not validated by `body-max-line-length` (they should be governed separately, e.g. by `footer-max-line-length`). The current implementation gates footer stripping on `TRAILER_TOKEN`, so missed trailer formats can violate that requirement.

## Fix Focus Areas
- @commitlint/rules/src/body-max-line-length.ts[5-37]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Multi-trailer body not removed 🐞 Bug ≡ Correctness
Description
withoutTrailingFooter() removes a detected trailer paragraph via lines.slice(0, first), but when
the body contains only multiple trailer lines (no blank separator), first becomes -1 and the
function returns all but the last trailer line. This can still trigger bodyMaxLineLength failures
for long trailer lines in commits that contain only trailers (e.g., Signed-off-by +
Co-authored-by).
Code

@commitlint/rules/src/body-max-line-length.ts[R24-37]

+	let first = last;
+
+	while (first >= 0 && lines[first] !== "") {
+		first--;
+	}
+
+	const footerLines = lines.slice(first + 1, last + 1);
+
+	if (footerLines.length === 0 || !footerLines.every((line) => TRAILER_TOKEN.test(line))) {
+		return input;
+	}
+
+	return lines.slice(0, first).join("\n");
+}
Evidence
The code can set first to -1 when there is no blank line in input; in that case
lines.slice(0, first) becomes slice(0, -1) which keeps all elements except the last, leaving
trailer lines behind. toLines() splits the body into multiple lines, so a trailer-only body with
multiple trailer lines triggers this path.

@commitlint/rules/src/body-max-line-length.ts[17-37]
@commitlint/to-lines/src/index.ts[1-7]
@commitlint/rules/src/trailer-exists.ts[6-12]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`withoutTrailingFooter()` can detect that the entire body is made of trailer lines, but when there is no blank line separator, `first` ends up as `-1` and `lines.slice(0, first)` drops only the last trailer line (because `slice(0, -1)` keeps all-but-last). This leaves remaining trailer lines in the body, so `body-max-line-length` can still fail on long trailer lines.

## Issue Context
This happens when the commit has no body and contains multiple trailers (e.g., `Signed-off-by: ...` followed by `Co-authored-by: ...`) that are present in `parsed.body` (trailers are commonly handled via raw parsing / `git interpret-trailers`).

## Fix Focus Areas
- @commitlint/rules/src/body-max-line-length.ts[24-37]

### Suggested change
Special-case `first < 0` to remove the entire input (return empty string) when the last paragraph is detected as trailers.

### Add regression test
Add a test where the commit message has *no body* and *multiple* trailer lines, and ensure `bodyMaxLineLength` returns `true` when the trailers exceed the body limit (since they should be ignored).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment on lines +5 to +33
const TRAILER_TOKEN = /^[A-Za-z0-9]+(?:-[A-Za-z0-9]+)+:\s\S/;

function withoutTrailingFooter(body: string, footer?: string | null): string {
const footerStart = footer ? body.length - footer.length : -1;
const input =
footer &&
footerStart >= 0 &&
body.endsWith(footer) &&
(footerStart === 0 || body[footerStart - 1] === "\n")
? body.slice(0, footerStart).replace(/(\r?\n)+$/, "")
: body;

const lines = toLines(input);
let last = lines.length - 1;

while (last >= 0 && lines[last] === "") {
last--;
}

let first = last;

while (first >= 0 && lines[first] !== "") {
first--;
}

const footerLines = lines.slice(first + 1, last + 1);

if (footerLines.length === 0 || !footerLines.every((line) => TRAILER_TOKEN.test(line))) {
return input;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. trailer_token excludes valid footers 📎 Requirement gap ≡ Correctness

withoutTrailingFooter() only treats the last paragraph as a footer if every line matches
TRAILER_TOKEN, but the regex requires a hyphenated token and will miss valid trailers like
Fixes:/Refs:/BREAKING CHANGE:. As a result, some footer lines can still be checked by
body-max-line-length, violating the requirement that footer line-length is not enforced by the
body rule.
Agent Prompt
## Issue description
`body-max-line-length` is intended to ignore footer/trailer lines so they are not validated against the body max length. The current trailer detection regex (`TRAILER_TOKEN`) only matches hyphenated tokens (e.g., `Signed-off-by:`), which means other valid trailer/footer tokens (e.g., `Fixes:`, `Refs:`, `BREAKING CHANGE:`) may remain in the body input and still trigger `body-max-line-length` failures.

## Issue Context
Compliance requires that footer lines are not validated by `body-max-line-length` (they should be governed separately, e.g. by `footer-max-line-length`). The current implementation gates footer stripping on `TRAILER_TOKEN`, so missed trailer formats can violate that requirement.

## Fix Focus Areas
- @commitlint/rules/src/body-max-line-length.ts[5-37]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +24 to +37
let first = last;

while (first >= 0 && lines[first] !== "") {
first--;
}

const footerLines = lines.slice(first + 1, last + 1);

if (footerLines.length === 0 || !footerLines.every((line) => TRAILER_TOKEN.test(line))) {
return input;
}

return lines.slice(0, first).join("\n");
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

2. Multi-trailer body not removed 🐞 Bug ≡ Correctness

withoutTrailingFooter() removes a detected trailer paragraph via lines.slice(0, first), but when
the body contains only multiple trailer lines (no blank separator), first becomes -1 and the
function returns all but the last trailer line. This can still trigger bodyMaxLineLength failures
for long trailer lines in commits that contain only trailers (e.g., Signed-off-by +
Co-authored-by).
Agent Prompt
## Issue description
`withoutTrailingFooter()` can detect that the entire body is made of trailer lines, but when there is no blank line separator, `first` ends up as `-1` and `lines.slice(0, first)` drops only the last trailer line (because `slice(0, -1)` keeps all-but-last). This leaves remaining trailer lines in the body, so `body-max-line-length` can still fail on long trailer lines.

## Issue Context
This happens when the commit has no body and contains multiple trailers (e.g., `Signed-off-by: ...` followed by `Co-authored-by: ...`) that are present in `parsed.body` (trailers are commonly handled via raw parsing / `git interpret-trailers`).

## Fix Focus Areas
- @commitlint/rules/src/body-max-line-length.ts[24-37]

### Suggested change
Special-case `first < 0` to remove the entire input (return empty string) when the last paragraph is detected as trailers.

### Add regression test
Add a test where the commit message has *no body* and *multiple* trailer lines, and ensure `bodyMaxLineLength` returns `true` when the trailers exceed the body limit (since they should be ignored).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@escapedcat

Copy link
Copy Markdown
Member

@puneetdixit200 thanks for the PR! Could you please:

  1. Split up the commit into two commits
    • one adding the test only, push it, CI will fail
    • a second one that pushes the fix, CI will be green
      This will be a more transparent TDD approach
  2. Have a look at the feedback from qodo and copilot, either tackle it or comment why it's not valid

Thanks!

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

Pull request overview

This PR updates the body-max-line-length rule to ignore trailing footer/trailer paragraphs when evaluating body line lengths, fixing cases where long trailer lines (e.g. Signed-off-by) incorrectly fail the body rule instead of being handled by footer/trailer-related rules.

Changes:

  • Add logic to strip a trailing “trailer paragraph” (and optionally an already-parsed footer) from parsed.body before applying maxLineLength.
  • Add a regression test ensuring a long Signed-off-by: trailer line doesn’t cause body-max-line-length to fail.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
@commitlint/rules/src/body-max-line-length.ts Introduces footer/trailer-stripping logic prior to applying the body line-length check.
@commitlint/rules/src/body-max-line-length.test.ts Adds a regression test for long trailer lines to ensure body line-length validation ignores them.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

return input;
}

return lines.slice(0, first).join("\n");
const expected = true;
expect(actual).toEqual(expected);
});

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

fix: body-max-line-length is not respecting footer-max-line-length and failing for long footers

5 participants