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) ` +