From f959a32f1fb09449d3c3af766923a1fcd35afa6e Mon Sep 17 00:00:00 2001 From: Adam Spiers Date: Thu, 15 Aug 2024 22:39:24 +0100 Subject: [PATCH 1/2] fix(rules): correct multi-line tests for body-max-line-length Previously the multi-line test fixtures in the test suite for the `body-max-line-length` rule were defined but never used; instead, the single line fixtures were being used in tests supposed to test multi-line commit messages. This was presumably an innocent copy-and-paste error, so fix the multi-line tests to use the multi-line fixtures. --- @commitlint/rules/src/body-max-line-length.test.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/@commitlint/rules/src/body-max-line-length.test.ts b/@commitlint/rules/src/body-max-line-length.test.ts index 26974ca6cc..b9bcf30443 100644 --- a/@commitlint/rules/src/body-max-line-length.test.ts +++ b/@commitlint/rules/src/body-max-line-length.test.ts @@ -19,6 +19,8 @@ const parsed = { empty: parse(messages.empty), short: parse(messages.short), long: parse(messages.long), + shortMultipleLines: parse(messages.shortMultipleLines), + longMultipleLines: parse(messages.longMultipleLines), }; test("with empty should succeed", async () => { @@ -40,13 +42,21 @@ test("with long should fail", async () => { }); test("with short with multiple lines should succeed", async () => { - const [actual] = bodyMaxLineLength(await parsed.short, undefined, value); + const [actual] = bodyMaxLineLength( + await parsed.shortMultipleLines, + undefined, + value + ); const expected = true; expect(actual).toEqual(expected); }); test("with long with multiple lines should fail", async () => { - const [actual] = bodyMaxLineLength(await parsed.long, undefined, value); + const [actual] = bodyMaxLineLength( + await parsed.longMultipleLines, + undefined, + value + ); const expected = false; expect(actual).toEqual(expected); }); From 13a7b75576fd4ac98fe8daeb9ec609197a8bf207 Mon Sep 17 00:00:00 2001 From: Adam Spiers Date: Thu, 15 Aug 2024 23:06:34 +0100 Subject: [PATCH 2/2] feat(rules): make body-max-line-length ignore lines with URLs Whilst the `body-max-line-length` is almost always desirable, there is one fairly frequent scenario in which it gets in the way: when lines contain URLs exceeding the maximum line length. In this case, the URL cannot be split across lines without breaking it, and minifying it results in obfuscation and a dependency on a third-party service, both of which are undesirable. So make an exception in this rule for any line which contains a URL. Allow other content on the same line, in order to support use of various rich-text formats such as Markdown, which are often used in order to render links in more elegant ways. --- @commitlint/ensure/src/max-line-length.ts | 10 +++- .../rules/src/body-max-line-length.test.ts | 54 +++++++++++++++---- docs/reference/rules.md | 4 +- 3 files changed, 56 insertions(+), 12 deletions(-) diff --git a/@commitlint/ensure/src/max-line-length.ts b/@commitlint/ensure/src/max-line-length.ts index 02f908c374..b25a2b6d1e 100644 --- a/@commitlint/ensure/src/max-line-length.ts +++ b/@commitlint/ensure/src/max-line-length.ts @@ -1,5 +1,13 @@ import ensure from "./max-length.js"; +// Allow an exception for long lines which contain URLs. +// +// This is overly lenient, in order to avoid costly regexps which +// have to worry about all the many edge cases of valid URLs. +const URL_REGEX = /\bhttps?:\/\/\S+/; + export default (value: string, max: number): boolean => typeof value === "string" && - value.split(/\r?\n/).every((line) => ensure(line, max)); + value + .split(/\r?\n/) + .every((line) => URL_REGEX.test(line) || ensure(line, max)); diff --git a/@commitlint/rules/src/body-max-line-length.test.ts b/@commitlint/rules/src/body-max-line-length.test.ts index b9bcf30443..7a599b30a3 100644 --- a/@commitlint/rules/src/body-max-line-length.test.ts +++ b/@commitlint/rules/src/body-max-line-length.test.ts @@ -1,9 +1,11 @@ import { test, expect } from "vitest"; import parse from "@commitlint/parse"; +import type { Commit } from "conventional-commits-parser"; import { bodyMaxLineLength } from "./body-max-line-length.js"; const short = "a"; const long = "ab"; +const url = "https://example.com/URL/with/a/very/long/path"; const value = short.length; @@ -13,16 +15,30 @@ const messages = { long: `test: subject\n${long}`, shortMultipleLines: `test:subject\n${short}\n${short}\n${short}`, longMultipleLines: `test:subject\n${short}\n${long}\n${short}`, -}; + urlStandalone: `test:subject\n${short}\n${url}\n${short}`, + urlMarkdownLinkInline: `test:subject + +This is a [link](${url}).`, + urlMarkdownLinkInList: `test:subject + +Link in a list: + +- ${url}`, + urlMarkdownLinkInFooter: `test:subject + +Finally, [link][] via footer. -const parsed = { - empty: parse(messages.empty), - short: parse(messages.short), - long: parse(messages.long), - shortMultipleLines: parse(messages.shortMultipleLines), - longMultipleLines: parse(messages.longMultipleLines), +[link]: ${url}`, }; +const parsed = Object.entries(messages).reduce( + (_parsed, [key, message]) => + Object.assign(_parsed, { + [key]: parse(message), + }), + {} as Record>, +); + test("with empty should succeed", async () => { const [actual] = bodyMaxLineLength(await parsed.empty, undefined, value); const expected = true; @@ -45,7 +61,7 @@ test("with short with multiple lines should succeed", async () => { const [actual] = bodyMaxLineLength( await parsed.shortMultipleLines, undefined, - value + value, ); const expected = true; expect(actual).toEqual(expected); @@ -55,8 +71,28 @@ test("with long with multiple lines should fail", async () => { const [actual] = bodyMaxLineLength( await parsed.longMultipleLines, undefined, - value + value, ); const expected = false; expect(actual).toEqual(expected); }); + +test("with multiple lines and standalone URL should succeed", async () => { + const [actual] = bodyMaxLineLength( + await parsed.urlStandalone, + undefined, + value, + ); + const expected = true; + expect(actual).toEqual(expected); +}); + +test("with multiple lines and URL in inline Markdown link should succeed", async () => { + const [actual] = bodyMaxLineLength( + await parsed.urlMarkdownLinkInline, + undefined, + 30, + ); + const expected = true; + expect(actual).toEqual(expected); +}); diff --git a/docs/reference/rules.md b/docs/reference/rules.md index 8180e753e0..5ab5f77a0f 100644 --- a/docs/reference/rules.md +++ b/docs/reference/rules.md @@ -32,7 +32,7 @@ ## body-max-line-length -- **condition**: `body` lines has `value` or less characters +- **condition**: `body` lines have `value` or less characters, or contain a URL - **rule**: `always` - **value** @@ -97,7 +97,7 @@ ## footer-max-line-length -- **condition**: `footer` lines has `value` or less characters +- **condition**: `footer` lines have `value` or less characters - **rule**: `always` - **value**