Instruct how to remedy the issues (#68)

In many teams, the wider team is not aware how the GitHub action has been set up and merely sees the messages in their CI reports. These users are ultimately lost and need further guidance on how to fix the errors in their commit messages. In particular, sparse error messages with no hints how to remedy the commit messages are almost useless leaving the users utterly frustrated. This change introduces very verbose error messages which include the hints what can be done about the message. While the messages are now quite long and seem cluttered, they are hopefully much more useful to the inexperienced users.
mristin · Sep 9, 2020 · a884d91 · a884d91
1 parent eadfa06
commit a884d91
Show file tree

Hide file tree

Showing 12 changed files with 909 additions and 335 deletions.
diff --git a/.eslintrc.json b/.eslintrc.json
@@ -12,7 +12,7 @@
     "import/no-namespace": "off",
     "no-unused-vars": "off",
     "@typescript-eslint/no-unused-vars": "error",
-    "@typescript-eslint/explicit-member-accessibility": ["error", {"accessibility": "no-public"}],
+    "@typescript-eslint/explicit-member-accessibility": "off",
     "@typescript-eslint/no-require-imports": "error",
     "@typescript-eslint/array-type": "error",
     "@typescript-eslint/await-thenable": "error",

diff --git a/dist/index.js b/dist/index.js
diff --git a/local/powershell/OpinionatedCommitMessage.Tests.ps1 b/local/powershell/OpinionatedCommitMessage.Tests.ps1
@@ -30,13 +30,62 @@ function TestFailTooFewLines
     Write-Host "TestFailTooFewLines: OK"
 }
 
+function TestFailSubjectWithNonWord
+{
+    $message = "ABC15 does something`n`nThis patch does something."
+    $got = (powershell -File OpinionatedCommitMessage.ps1 -message $message -dontThrow)|Out-String
+
+    $nl = [Environment]::NewLine
+    $expected = (
+        "* Expected the subject to start with a verb in imperative mood consisting " +
+        "of letters and possibly dashes in-between, but the subject was: `"ABC15 does something`". " +
+        "Please re-write the subject so that it starts with a verb in imperative mood.${nl}"
+    )
+
+    if ($got -ne $expected)
+    {
+        Write-Host "TestFailSubjectWithNonWord: FAILED"
+        WriteExpectedGot -expected $expected -got $got
+        return $false
+    }
+
+    Write-Host "TestFailSubjectWithNonWord: OK"
+    return $true
+}
+
+function TestFailSubjectWithNonCapitalized
+{
+    $message = "do something`n`nThis patch does something."
+    $got = (powershell -File OpinionatedCommitMessage.ps1 -message $message -dontThrow)|Out-String
+
+    $nl = [Environment]::NewLine
+    $expected = (
+        "* The subject must start with a capitalized word, but the current first word is: `"do`". " +
+        "Please capitalize to: `"Do`".${nl}"
+    )
+
+    if ($got -ne $expected)
+    {
+        Write-Host "TestFailSubjectWithNonCapitalized: FAILED"
+        WriteExpectedGot -expected $expected -got $got
+        return $false
+    }
+
+    Write-Host "TestFailSubjectWithNonCapitalized: OK"
+    return $true
+}
+
 function TestFailSubjectWithNonVerb
 {
-    $message = "ABC does something`n`nThis patch does something."
+    $message = "Abc does something`n`nThis patch does something."
     $got = (powershell -File OpinionatedCommitMessage.ps1 -message $message -dontThrow)|Out-String
 
     $nl = [Environment]::NewLine
-    $expected = "* The subject must start with a capitalized verb (e.g., `"Change`").${nl}"
+    $expected = (
+        "* The subject must start with a verb in imperative mood (according to a whitelist), " +
+        "but got: `"Abc`"; if this is a false positive, consider adding the verb to -additionalVerbs or " +
+        "to the file referenced by -pathToAdditionalVerbs.${nl}"
+    )
 
     if ($got -ne $expected)
     {
@@ -55,7 +104,7 @@ function TestFailSubjectWithTrailingDot
     $got = (powershell -File OpinionatedCommitMessage.ps1 -message $message -dontThrow)|Out-String
 
     $nl = [Environment]::NewLine
-    $expected = "* The subject must not end with a dot ('.').${nl}"
+    $expected = "* The subject must not end with a dot ('.'). Please remove the trailing dot(s).${nl}"
 
     if ($got -ne $expected)
     {
@@ -74,9 +123,17 @@ function TestFailSubjectTooLong
     $got = (powershell -File OpinionatedCommitMessage.ps1 -message $message -dontThrow)|Out-String
 
     $nl = [Environment]::NewLine
-    $expected = "* The subject exceeds the limit of 50 characters " + `
-        "(got: 51): `"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA`"${nl}" + `
-        "* The subject must start with a capitalized verb (e.g., `"Change`").${nl}"
+    $expected = (
+        "* The subject exceeds the limit of 50 characters " +
+        "(got: 51): `"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA`". " +
+        "Please shorten the subject to make it more succinct.${nl}" +
+        "* The subject must start with a capitalized word, but " +
+        "the current first word is: `"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA`". " +
+        "Please capitalize to: `"Aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa`".${nl}" +
+        "* The subject must start with a verb in imperative mood (according to a whitelist), " +
+        "but got: `"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA`"; if this is a false positive, " +
+        "consider adding the verb to -additionalVerbs or to the file referenced by -pathToAdditionalVerbs.${nl}"
+    )
 
     if ($got -ne $expected)
     {
@@ -96,8 +153,11 @@ function TestFailBodyTooLong
     $got = (powershell -File OpinionatedCommitMessage.ps1 -message $message -dontThrow)|Out-String
 
     $nl = [Environment]::NewLine
-    $expected = "* The line 3 of the message (line 1 of the body) exceeds the limit of 72 characters. " + `
-        "The line contains 73 characters: `"${bodyLine}`".${nl}"
+    $expected = (
+        "* The line 3 of the message (line 1 of the body) exceeds the limit of 72 characters. " +
+        "The line contains 73 characters: `"${bodyLine}`". " +
+        "Please reformat the body so that all the lines fit 72 characters.${nl}"
+    )
 
     if ($got -ne $expected)
     {
@@ -116,7 +176,12 @@ function TestFailIdenticalFirstWordInBodyAndSubject
     $got = (powershell -File OpinionatedCommitMessage.ps1 -message $message -dontThrow)|Out-String
 
     $nl = [Environment]::NewLine
-    $expected = "* The first word of the subject (`"Do`") must not match the first word of the body.${nl}"
+    $expected = (
+        "* The first word of the subject (`"Do`") must not match the first word of the body. " +
+        "Please make the body more informative by adding more information instead of repeating " +
+        "the subject. For example, start by explaining the problem that this change is " +
+        'intendended to solve or what was previously missing (e.g., "Previously, ....").' + $nl
+    )
 
     if ($got -ne $expected)
     {
@@ -280,7 +345,7 @@ function TestFailWithAllowOneLiners
     )|Out-String
 
     $nl = [Environment]::NewLine
-    $expected = "* The subject must not end with a dot ('.').${nl}"
+    $expected = "* The subject must not end with a dot ('.'). Please remove the trailing dot(s).${nl}"
 
     if ($got -ne $expected)
     {
@@ -345,7 +410,10 @@ function Main
 
     try
     {
-        $success = TestFailTooFewLines
+        $success = $true
+        $success = TestFailTooFewLines -and $success
+        $success = TestFailSubjectWithNonWord -and $success
+        $success = TestFailSubjectWithNonCapitalized -and $success
         $success = TestFailSubjectWithNonVerb -and $success
         $success = TestFailSubjectWithTrailingDot -and $success
         $success = TestFailSubjectTooLong -and $success

diff --git a/local/powershell/OpinionatedCommitMessage.ps1 b/local/powershell/OpinionatedCommitMessage.ps1
@@ -410,6 +410,7 @@ $frequentVerbs = @(
 'inline',
 'insist',
 'install',
+'instruct',
 'integrate',
 'intend',
 'intercept',
@@ -817,9 +818,47 @@ function ParseAdditionalVerbs($text)
     return $verbs
 }
 
-$capitalizedWordRe = [Regex]::new('^([A-Z][a-z]*)[^a-zA-Z]')
+$allLettersRe = [Regex]::new('^[a-zA-Z][a-zA-Z-]+$')
+$firstWordBeforeSpaceRe = [Regex]::new('^([a-zA-Z][a-zA-Z-]+)\s');
 $suffixHashCodeRe = [Regex]::new('\s?\(\s*#[a-zA-Z_0-9]+\s*\)$')
 
+function ExtractFirstWord([string]$Text)
+{
+    if ($Text.Length -eq 0)
+    {
+        return $null
+    }
+
+    if ($allLettersRe.IsMatch($Text))
+    {
+        return $Text
+    }
+
+    $matches = $firstWordBeforeSpaceRe.Match($Text)
+    if (!$matches.Success)
+    {
+        return $null
+    }
+
+    $firstWord = $matches[0].Groups[1].Value
+    return $firstWord
+}
+
+function Capitalize([string]$word)
+{
+    if ($word.Length -eq 0)
+    {
+        return ""
+    }
+
+    if ($word.Length -eq 1)
+    {
+        return $word.ToUpperInvariant()
+    }
+
+    return $word.Substring(0, 1).ToUpperInvariant() + $word.Substring(1).ToLowerInvariant()
+}
+
 function CheckSubject([string]$subject, [hashtable]$verbs)
 {
     # Precondition
@@ -842,31 +881,47 @@ function CheckSubject([string]$subject, [hashtable]$verbs)
     if ($subjectWoCode.Length -gt 50)
     {
         $errors += "The subject exceeds the limit of 50 characters " +    `
-              "(got: $( $subjectWoCode.Length )): $( $subjectWoCode|ConvertTo-Json )"
+              "(got: $( $subjectWoCode.Length )): $( $subjectWoCode|ConvertTo-Json ). " +
+              "Please shorten the subject to make it more succinct."  
     }
 
-    $matches = $capitalizedWordRe.Matches($subjectWoCode)
-    if ($matches.Count -ne 1)
+    $firstWord = ExtractFirstWord -Text $subjectWoCode
+    if ($null -eq $firstWord)
     {
-        $errors += 'The subject must start with a capitalized verb (e.g., "Change").'
-    }
-    else
-    {
-        $match = $matches[0]
-        $word = $match.Groups[1].Value
-        $wordLower = $word.ToLower()
+        $errors += (
+            "Expected the subject to start with a verb in imperative mood " +
+            "consisting of letters and possibly dashes in-between, " +
+            "but the subject was: $($subjectWoCode|ConvertTo-Json). " +
+            "Please re-write the subject so that it starts with " +
+            "a verb in imperative mood."
+        )
+    } else {
+        $capitalized = Capitalize -Word $firstWord
+        if ($capitalized -cne $firstWord)
+        {
+            $errors += (
+                "The subject must start with a capitalized word, " +
+                "but the current first word is: $( $firstWord|ConvertTo-Json ). " +
+                "Please capitalize to: $( $capitalized|ConvertTo-Json )."
+            )
+        }
+
+        $firstWordLower = $firstWord.ToLower()
 
-        if (!$verbs.Contains($wordLower) -or ($false -eq $verbs[$wordLower]))
+        if (!$verbs.Contains($firstWordLower) -or ($false -eq $verbs[$firstWordLower]))
         {
             $errors += "The subject must start with a verb in imperative mood (according to a whitelist), " +   `
-                  "but got: $($word|ConvertTo-Json); if this is a false positive, consider adding the verb " + `
+                  "but got: $($firstWord|ConvertTo-Json); if this is a false positive, consider adding the verb " + `
                   "to -additionalVerbs or to the file referenced by -pathToAdditionalVerbs."
         }
     }
 
     if ( $subjectWoCode.EndsWith("."))
     {
-        $errors += "The subject must not end with a dot ('.')."
+        $errors += (
+            "The subject must not end with a dot ('.'). " +
+            "Please remove the trailing dot(s)."
+        )
     }
 
     return $errors
@@ -902,26 +957,30 @@ function CheckBody([string]$subject, [string[]] $bodyLines)
 
         if($line.Length -gt 72)
         {
-            $errors += "The line $($i + 3) of the message (line $($i + 1) of the body) " + `
-                "exceeds the limit of 72 characters. The line contains $($line.Length) characters: " + `
-                "$($line|ConvertTo-Json)."
+            $errors += (
+                "The line $($i + 3) of the message (line $($i + 1) of the body) " +
+                "exceeds the limit of 72 characters. The line contains $($line.Length) characters: " +
+                "$($line|ConvertTo-Json). " +
+                "Please reformat the body so that all the lines fit 72 characters."
+            )
         }
     }
 
-    $bodyFirstWordMatches = $capitalizedWordRe.Matches($bodyLines[0])
-    if($bodyFirstWordMatches.Count -eq 1)
+    $bodyFirstWord = ExtractFirstWord -Text $bodyLines[0]
+    if($null -ne $bodyFirstWord)
     {
-        $bodyFirstWord = $bodyFirstWordMatches[0].Groups[1].Value
-
-        $subjectFirstWordMatches = $capitalizedWordRe.Matches($subject)
-        if($subjectFirstWordMatches.Count -eq 1)
+        $subjectFirstWord = ExtractFirstWord -Text $subject
+        if($null -ne $subjectFirstWord)
         {
-            $subjectFirstWord = $subjectFirstWordMatches[0].Groups[1].Value
-
             if($subjectFirstWord.ToLower() -eq $bodyFirstWord.ToLower())
             {
-                $errors += "The first word of the subject ($($subjectFirstWord|ConvertTo-Json)) must not match " + `
-                    "the first word of the body."
+                $errors += (
+                    "The first word of the subject ($($subjectFirstWord|ConvertTo-Json)) must not match " +
+                    "the first word of the body. " +
+                    "Please make the body more informative by adding more information instead of repeating " +
+                    "the subject. For example, start by explaining the problem that this change is " +
+                    'intendended to solve or what was previously missing (e.g., "Previously, ....").'
+                )
             }
         }
     }