Skip to content

Commit

Permalink
Allow URLs as separate lines and link definitions
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mristin committed Aug 14, 2020
1 parent dd3aa72 commit f8ce336
Show file tree
Hide file tree
Showing 6 changed files with 171 additions and 2 deletions.
26 changes: 26 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
49 changes: 47 additions & 2 deletions local/powershell/OpinionatedCommitMessage.Tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
{
Expand Down
9 changes: 9 additions & 0 deletions local/powershell/OpinionatedCommitMessage.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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 = @()
Expand All @@ -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) " + `
Expand Down
9 changes: 9 additions & 0 deletions local/powershell/OpinionatedCommitMessage.ps1.template
Original file line number Diff line number Diff line change
Expand Up @@ -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 = @()
Expand All @@ -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) " + `
Expand Down
73 changes: 73 additions & 0 deletions src/__tests__/inspection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -234,3 +234,76 @@ it('ignores merge messages.', () => {
const errors = inspection.check(message, new Set<string>(), 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<string>(), 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<string>(), 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<string>(), 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<string>(), 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.".`
]);
});
7 changes: 7 additions & 0 deletions src/inspection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,9 @@ function checkSubject(subject: string, additionalVerbs: Set<string>): string[] {
return errors;
}

const urlLineRe = new RegExp('^[^ ]+://[^ ]+$');
const linkDefinitionRe = new RegExp('^\\[[^\\]]+]\\s*:\\s*[^ ]+://[^ ]+$');

function checkBody(subject: string, bodyLines: string[]): string[] {
const errors: string[] = [];

Expand All @@ -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) ` +
Expand Down

0 comments on commit f8ce336

Please sign in to comment.