From ea97f7e115e19e755722e5a7a2c63f1f2a4d4e82 Mon Sep 17 00:00:00 2001 From: Marko Ristin Date: Fri, 14 Aug 2020 15:57:08 +0200 Subject: [PATCH] Exempt URLs on separate lines from the checks This patch allows long URLs on separate lines or as link definitions in the body. This is necessary so that the link functionality does not break in the readers such as terminals and Github. Fixes #59. --- README.md | 26 +++++++ .../OpinionatedCommitMessage.Tests.ps1 | 49 ++++++++++++- local/powershell/OpinionatedCommitMessage.ps1 | 9 +++ .../OpinionatedCommitMessage.ps1.template | 9 +++ src/__tests__/inspection.test.ts | 73 +++++++++++++++++++ src/inspection.ts | 7 ++ 6 files changed, 171 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 4b45dfa..a00d095 100644 --- a/README.md +++ b/README.md @@ -116,6 +116,32 @@ as input: path-to-additional-verbs: 'src/additional-verbs.txt' ``` +## Long URLs + +Splitting URLs on separate lines to satisfy the maximum character lenght +breaks the link functionality on most readers +(*e.g.*, in a terminal or on Github). Therefore we need to tolerate long URLs +in the message body. + +Nevertheless, in order to make the text readable the URL should be put either on +a separate line or defined as a link in markdown. + +For example, this is how you can write a message with an URL on a separate line: + +``` +Please see this page for more details: +http://some-domain.com/very/long/long/long/long/long/long/long/long/long/path.html +or read the manual. +``` + +Here is the same message with a link definition (arguably a bit more readable): + +``` +Please see [this page for more details][1] or read the manual. + +[1]: http://some-domain.com/very/long/long/long/long/long/long/long/long/long/path.html +``` + ## One-liners Usually, you need to write elaborate commit messages with a shorter header diff --git a/local/powershell/OpinionatedCommitMessage.Tests.ps1 b/local/powershell/OpinionatedCommitMessage.Tests.ps1 index be84022..1dd253b 100755 --- a/local/powershell/OpinionatedCommitMessage.Tests.ps1 +++ b/local/powershell/OpinionatedCommitMessage.Tests.ps1 @@ -293,6 +293,51 @@ function TestFailWithAllowOneLiners return $true } +function TestOKWithURLOnSeparateLine +{ + $url = ("http://mristin@some-domain.com/some/very/very/very/very/" + + "very/very/very/long/path/index.html") + + $message = "Do something`n`nThis does something with URL:`n$url" + $got = (powershell -File OpinionatedCommitMessage.ps1 -message $message -dontThrow)|Out-String + + $nl = [Environment]::NewLine + $expected = "The message is OK.${nl}" + + if ($got -ne $expected) + { + Write-Host "TestOKWithURLOnSeparateLine: FAILED" + WriteExpectedGot -expected $expected -got $got + return $false + } + + Write-Host "TestOKWithURLOnSeparateLine: OK" + return $true +} + +function TestOKWithLinkDefinition +{ + $url = ("http://mristin@some-domain.com/some/very/very/very/very/" + + "very/very/very/long/path/index.html") + + $message = "Do something`n`nThis does something with URL: [1]`n`n[1]: $url" + $got = (powershell -File OpinionatedCommitMessage.ps1 -message $message -dontThrow)|Out-String + + $nl = [Environment]::NewLine + $expected = "The message is OK.${nl}" + + if ($got -ne $expected) + { + Write-Host "TestOKWithLinkDefinition: FAILED" + WriteExpectedGot -expected $expected -got $got + return $false + } + + Write-Host "TestOKWithLinkDefinition: OK" + return $true +} + + function Main { Push-Location @@ -313,8 +358,8 @@ function Main $success = TestOKPathToAdditionalVerbs -and $success $success = TestOKWithAllowOneLiners -and $success $success = TestFailWithAllowOneLiners -and $success - - # TODO: TestFailAllowOneLiners + $success = TestOKWithURLOnSeparateLine -and $success + $success = TestOKWithLinkDefinition -and $success if(!$success) { diff --git a/local/powershell/OpinionatedCommitMessage.ps1 b/local/powershell/OpinionatedCommitMessage.ps1 index 5dc6beb..920dba1 100644 --- a/local/powershell/OpinionatedCommitMessage.ps1 +++ b/local/powershell/OpinionatedCommitMessage.ps1 @@ -870,6 +870,9 @@ function CheckSubject([string]$subject, [hashtable]$verbs) return $errors } +$urlLineRe = [Regex]::new('^[^ ]+://[^ ]+$') +$linkDefinitionRe = [Regex]::new('^\[[^\]]+]\s*:\s*[^ ]+://[^ ]+$') + function CheckBody([string]$subject, [string[]] $bodyLines) { $errors = @() @@ -889,6 +892,12 @@ function CheckBody([string]$subject, [string[]] $bodyLines) for($i = 0; $i -lt $bodyLines.Count; $i++) { $line = $bodyLines[$i] + + if ($urlLineRe.IsMatch($line) -or $linkDefinitionRe.IsMatch($line)) + { + continue; + } + if($line.Length -gt 72) { $errors += "The line $($i + 3) of the message (line $($i + 1) of the body) " + ` diff --git a/local/powershell/OpinionatedCommitMessage.ps1.template b/local/powershell/OpinionatedCommitMessage.ps1.template index 7a514e3..3151bdc 100755 --- a/local/powershell/OpinionatedCommitMessage.ps1.template +++ b/local/powershell/OpinionatedCommitMessage.ps1.template @@ -132,6 +132,9 @@ function CheckSubject([string]$subject, [hashtable]$verbs) return $errors } +$urlLineRe = [Regex]::new('^[^ ]+://[^ ]+$') +$linkDefinitionRe = [Regex]::new('^\[[^\]]+]\s*:\s*[^ ]+://[^ ]+$') + function CheckBody([string]$subject, [string[]] $bodyLines) { $errors = @() @@ -151,6 +154,12 @@ function CheckBody([string]$subject, [string[]] $bodyLines) for($i = 0; $i -lt $bodyLines.Count; $i++) { $line = $bodyLines[$i] + + if ($urlLineRe.IsMatch($line) -or $linkDefinitionRe.IsMatch($line)) + { + continue; + } + if($line.Length -gt 72) { $errors += "The line $($i + 3) of the message (line $($i + 1) of the body) " + ` diff --git a/src/__tests__/inspection.test.ts b/src/__tests__/inspection.test.ts index c6289c9..53b45f2 100644 --- a/src/__tests__/inspection.test.ts +++ b/src/__tests__/inspection.test.ts @@ -234,3 +234,76 @@ it('ignores merge messages.', () => { const errors = inspection.check(message, new Set(), false); expect(errors).toEqual([]); }); + +it('ignores URL on a separate line.', () => { + const url = + 'http://mristin@some-domain.com/some/very/very/very/very/' + + 'very/very/very/long/path/index.html'; + + const message = `Do something + +This patch does something with the URL: +${url} +The next line conforms to the line length.`; + + const errors = inspection.check(message, new Set(), false); + expect(errors).toEqual([]); +}); + +it('ignores URL on a separate line, but reports non-conform lines.', () => { + const long = 'long, long, long, long, long, long, long, long, long'; + const url = + 'http://mristin@some-domain.com/some/very/very/very/very/' + + 'very/very/very/long/path/index.html'; + + const message = `Do something + +This ${long} patch does something with the URL. +${url}`; + + const errors = inspection.check(message, new Set(), false); + expect(errors).toEqual([ + 'The line 3 of the message (line 1 of the body) exceeds ' + + 'the limit of 72 characters. The line contains 92 characters: ' + + `"This ${long} patch does something with the URL.".` + ]); +}); + +it('ignores link definitions.', () => { + const url = + 'http://mristin@some-domain.com/some/very/very/very/very/' + + 'very/very/very/long/path/index.html'; + + const message = `Do something + +This patch does something with the URL: [1] + +[1]: ${url} + +The next line conforms to the line length.`; + + const errors = inspection.check(message, new Set(), false); + expect(errors).toEqual([]); +}); + +it('ignores link definitions, but reports non-conform lines.', () => { + const url = + 'http://mristin@some-domain.com/some/very/very/very/very/' + + 'very/very/very/long/path/index.html'; + const long = 'long, long, long, long, long, long, long, long, long'; + + const message = `Do something + +This patch does something with the URL: [1] + +[1]: ${url} + +The ${long} line is too long.`; + + const errors = inspection.check(message, new Set(), false); + expect(errors).toEqual([ + 'The line 7 of the message (line 5 of the body) exceeds ' + + 'the limit of 72 characters. The line contains 74 characters: ' + + `"The ${long} line is too long.".` + ]); +}); diff --git a/src/inspection.ts b/src/inspection.ts index e505d9d..eaa0f28 100644 --- a/src/inspection.ts +++ b/src/inspection.ts @@ -129,6 +129,9 @@ function checkSubject(subject: string, additionalVerbs: Set): string[] { return errors; } +const urlLineRe = new RegExp('^[^ ]+://[^ ]+$'); +const linkDefinitionRe = new RegExp('^\\[[^\\]]+]\\s*:\\s*[^ ]+://[^ ]+$'); + function checkBody(subject: string, bodyLines: string[]): string[] { const errors: string[] = []; @@ -145,6 +148,10 @@ function checkBody(subject: string, bodyLines: string[]): string[] { } for (const [i, line] of bodyLines.entries()) { + if (urlLineRe.test(line) || linkDefinitionRe.test(line)) { + continue; + } + if (line.length > 72) { errors.push( `The line ${i + 3} of the message (line ${i + 1} of the body) ` +