Make max length of lines configurable (#96)

The line limit is great for strict terminal use and some
GUIs, but for teams that use only UIs such as GitHub Web it is an
unnecessary restriction.
This commit adds options to make this
configurable.

Signed-off-by: Luiz Ferraz <[email protected]>
Co-authored-by: Marko Ristin <[email protected]>

README.md

@@ -158,6 +158,38 @@ You can allow one-liner commit messages by setting the flag `allow-one-liners`:
           allow-one-liners: 'true'
 ```
 
+## Custom subject length
+
+For use in terminals and monospaced GUIs it is a good practice to limit length of the subject to 50 characters.
+For some projects, though, this limit is too restrictive.
+For example, if you include tags in the subject (such as `[FIX]`) there is not much space left for the actual subject.
+
+You can change the imposed maximum subject length by setting the flag `max-subject-line-length`:
+
+```yaml
+    steps:
+      - name: Check
+        uses: mristin/opinionated-commit-message@v2
+        with:
+          max-subject-line-length: '100'
+```
+
+## Custom line length on the body
+
+Similar to the subject line, for terminals and monospaced GUIs it is a good practice to limit the line length of the body to 72 characters.
+However, the restriction is unnecessarily harsh for teams that use modern GUIs such as GitHub Web.
+This is especially so when using a description of the pull request as the body, since there is no such limitation in the GitHub UI itself.
+
+You can change the imposed maximum line length by setting the flag `max-body-line-length`:
+
+```yaml
+    steps:
+      - name: Check
+        uses: mristin/opinionated-commit-message@v2
+        with:
+          max-body-line-length: '100'
+```
+
 ## Skip Body Check
 
 For some repositories only the subject matters while the body is allowed to be free-form.

action.yml

@@ -20,6 +20,14 @@ inputs:
     description: 'If set to "true", allows one-liner commit messages without body'
     required: false
     default: ''
+  max-subject-line-length:
+    description: 'Maximum length of the commit subject line'
+    required: false
+    default: ''
+  max-body-line-length:
+    description: 'Maximum length of a line in the body of the commit message'
+    required: false
+    default: ''
   enforce-sign-off:
     description: 'If set to "true", signed-off-by is required in the commit message body'
     required: false

dist/index.js

@@ -412,8 +412,8 @@ function checkSubject(subject, inputs) {
     // These hash codes are usually added automatically by GitHub and
     // similar services.
     const subjectWoCode = subject.replace(suffixHashCodeRe, '');
-    if (subjectWoCode.length > 50) {
-        errors.push(`The subject exceeds the limit of 50 characters ` +
+    if (subjectWoCode.length > inputs.maxSubjectLength) {
+        errors.push(`The subject exceeds the limit of ${inputs.maxSubjectLength} characters ` +
             `(got: ${subject.length}, JSON: ${JSON.stringify(subjectWoCode)}).` +
             'Please shorten the subject to make it more succinct.');
     }
@@ -454,7 +454,7 @@ function checkSubject(subject, inputs) {
 }
 const urlLineRe = new RegExp('^[^ ]+://[^ ]+$');
 const linkDefinitionRe = new RegExp('^\\[[^\\]]+]\\s*:\\s*[^ ]+://[^ ]+$');
-function checkBody(subject, bodyLines) {
+function checkBody(subject, bodyLines, inputs) {
     const errors = [];
     if (bodyLines.length === 0) {
         errors.push('At least one line is expected in the body, but got empty body.');
@@ -468,13 +468,13 @@ function checkBody(subject, bodyLines) {
         if (urlLineRe.test(line) || linkDefinitionRe.test(line)) {
             continue;
         }
-        if (line.length > 72) {
+        if (line.length > inputs.maxBodyLineLength) {
             errors.push(`The line ${i + 3} of the message (line ${i + 1} of the body) ` +
-                'exceeds the limit of 72 characters. ' +
+                `exceeds the limit of ${inputs.maxBodyLineLength} characters. ` +
                 `The line contains ${line.length} characters: ` +
                 `${JSON.stringify(line)}. ` +
                 'Please reformat the body so that all the lines fit ' +
-                '72 characters.');
+                `${inputs.maxBodyLineLength} characters.`);
         }
     }
     const bodyFirstWord = extractFirstWord(bodyLines[0]);
@@ -537,7 +537,7 @@ function check(message, inputs) {
             const subjectBody = maybeSubjectBody.subjectBody;
             errors.push(...checkSubject(subjectBody.subject, inputs));
             if (!inputs.skipBodyCheck) {
-                errors.push(...checkBody(subjectBody.subject, subjectBody.bodyLines));
+                errors.push(...checkBody(subjectBody.subject, subjectBody.bodyLines, inputs));
             }
             if (inputs.enforceSignOff) {
                 errors.push(...checkSignedOff(subjectBody.bodyLines));
@@ -1267,11 +1267,13 @@ Object.defineProperty(exports, "__esModule", { value: true });
 exports.parseVerbs = exports.parseInputs = exports.MaybeInputs = exports.Inputs = void 0;
 const fs_1 = __importDefault(__webpack_require__(747));
 class Inputs {
-    constructor(hasAdditionalVerbsInput, pathToAdditionalVerbs, allowOneLiners, additionalVerbs, enforceSignOff, skipBodyCheck) {
+    constructor(hasAdditionalVerbsInput, pathToAdditionalVerbs, allowOneLiners, additionalVerbs, maxSubjectLength, maxBodyLineLength, enforceSignOff, skipBodyCheck) {
         this.hasAdditionalVerbsInput = hasAdditionalVerbsInput;
         this.pathToAdditionalVerbs = pathToAdditionalVerbs;
         this.allowOneLiners = allowOneLiners;
         this.additionalVerbs = additionalVerbs;
+        this.maxSubjectLength = maxSubjectLength;
+        this.maxBodyLineLength = maxBodyLineLength;
         this.enforceSignOff = enforceSignOff;
         this.skipBodyCheck = skipBodyCheck;
     }
@@ -1297,7 +1299,7 @@ class MaybeInputs {
     }
 }
 exports.MaybeInputs = MaybeInputs;
-function parseInputs(additionalVerbsInput, pathToAdditionalVerbsInput, allowOneLinersInput, enforceSignOffInput, skipBodyCheckInput) {
+function parseInputs(additionalVerbsInput, pathToAdditionalVerbsInput, allowOneLinersInput, maxSubjectLengthInput, maxBodyLineLengthInput, enforceSignOffInput, skipBodyCheckInput) {
     const additionalVerbs = new Set();
     const hasAdditionalVerbsInput = additionalVerbsInput.length > 0;
     if (additionalVerbsInput) {
@@ -1322,6 +1324,20 @@ function parseInputs(additionalVerbsInput, pathToAdditionalVerbsInput, allowOneL
         return new MaybeInputs(null, 'Unexpected value for allow-one-liners. ' +
             `Expected either 'true' or 'false', got: ${allowOneLinersInput}`);
     }
+    const maxSubjectLength = !maxSubjectLengthInput
+        ? 50
+        : parseInt(maxSubjectLengthInput, 10);
+    if (Number.isNaN(maxSubjectLength)) {
+        return new MaybeInputs(null, 'Unexpected value for max-subject-line-length. ' +
+            `Expected a number or nothing, got ${maxSubjectLengthInput}`);
+    }
+    const maxBodyLineLength = !maxBodyLineLengthInput
+        ? 72
+        : parseInt(maxBodyLineLengthInput, 10);
+    if (Number.isNaN(maxBodyLineLength)) {
+        return new MaybeInputs(null, 'Unexpected value for max-body-line-length. ' +
+            `Expected a number or nothing, got ${maxBodyLineLengthInput}`);
+    }
     const enforceSignOff = !enforceSignOffInput
         ? false
         : parseBooleanFromString(enforceSignOffInput);
@@ -1336,7 +1352,7 @@ function parseInputs(additionalVerbsInput, pathToAdditionalVerbsInput, allowOneL
         return new MaybeInputs(null, 'Unexpected value for skip-body-check. ' +
             `Expected either 'true' or 'false', got: ${skipBodyCheckInput}`);
     }
-    return new MaybeInputs(new Inputs(hasAdditionalVerbsInput, pathToAdditionalVerbsInput, allowOneLiners, additionalVerbs, enforceSignOff, skipBodyCheck), null);
+    return new MaybeInputs(new Inputs(hasAdditionalVerbsInput, pathToAdditionalVerbsInput, allowOneLiners, additionalVerbs, maxSubjectLength, maxBodyLineLength, enforceSignOff, skipBodyCheck), null);
 }
 exports.parseInputs = parseInputs;
 function parseVerbs(text) {
@@ -1734,17 +1750,19 @@ const inspection = __importStar(__webpack_require__(117));
 const represent = __importStar(__webpack_require__(110));
 const input = __importStar(__webpack_require__(265));
 function runWithExceptions() {
-    var _a, _b, _c, _d, _e;
+    var _a, _b, _c, _d, _e, _f, _g;
     const messages = commitMessages.retrieve();
     ////
     // Parse inputs
     ////
     const additionalVerbsInput = (_a = core.getInput('additional-verbs', { required: false })) !== null && _a !== void 0 ? _a : '';
     const pathToAdditionalVerbsInput = (_b = core.getInput('path-to-additional-verbs', { required: false })) !== null && _b !== void 0 ? _b : '';
     const allowOneLinersInput = (_c = core.getInput('allow-one-liners', { required: false })) !== null && _c !== void 0 ? _c : '';
-    const enforceSignOffInput = (_d = core.getInput('enforce-sign-off', { required: false })) !== null && _d !== void 0 ? _d : '';
-    const skipBodyCheckInput = (_e = core.getInput('skip-body-check', { required: false })) !== null && _e !== void 0 ? _e : '';
-    const maybeInputs = input.parseInputs(additionalVerbsInput, pathToAdditionalVerbsInput, allowOneLinersInput, enforceSignOffInput, skipBodyCheckInput);
+    const maxSubjectLengthInput = (_d = core.getInput('max-subject-line-length', { required: false })) !== null && _d !== void 0 ? _d : '';
+    const maxBodyLineLengthInput = (_e = core.getInput('max-body-line-length', { required: false })) !== null && _e !== void 0 ? _e : '';
+    const enforceSignOffInput = (_f = core.getInput('enforce-sign-off', { required: false })) !== null && _f !== void 0 ? _f : '';
+    const skipBodyCheckInput = (_g = core.getInput('skip-body-check', { required: false })) !== null && _g !== void 0 ? _g : '';
+    const maybeInputs = input.parseInputs(additionalVerbsInput, pathToAdditionalVerbsInput, allowOneLinersInput, maxSubjectLengthInput, maxBodyLineLengthInput, enforceSignOffInput, skipBodyCheckInput);
     if (maybeInputs.error !== null) {
         core.error(maybeInputs.error);
         core.setFailed(maybeInputs.error);

src/__tests__/input.test.ts

@@ -34,6 +34,8 @@ it('parses the inputs.', () => {
     'integrate\nanalyze',
     pathToVerbs,
     'true',
+    '90',
+    '100',
     'true',
     'true'
   );
@@ -47,6 +49,8 @@ it('parses the inputs.', () => {
     new Set<string>(['rewrap', 'table', 'integrate', 'analyze'])
   );
   expect(inputs.allowOneLiners).toBeTruthy();
+  expect(inputs.maxSubjectLength).toEqual(90);
+  expect(inputs.maxBodyLineLength).toEqual(100);
   expect(inputs.enforceSignOff).toBeTruthy();
   expect(inputs.skipBodyCheck).toBeTruthy();
 });

src/__tests__/inspection.test.ts

@@ -2,7 +2,7 @@ import * as input from '../input';
 import * as inspection from '../inspection';
 
 const defaultInputs: input.Inputs = input
-  .parseInputs('', '', '', '', '')
+  .parseInputs('', '', '', '', '', '', '')
   .mustInputs();
 
 it('reports no errors on correct multi-line message.', () => {
@@ -28,7 +28,7 @@ it('reports no errors on OK multi-line message with allowed one-liners.', () =>
 });
 
 it('reports no errors on OK single-line message with allowed one-liners.', () => {
-  const inputs = input.parseInputs('', '', 'true', '', '').mustInputs();
+  const inputs = input.parseInputs('', '', 'true', '', '', '', '').mustInputs();
 
   const message = 'Change SomeClass to OtherClass';
 
@@ -63,12 +63,14 @@ it('reports no errors on any message when body check is disabled.', () => {
     'since Some class was deprecated. This is long message should not ' +
     'be checked.';
 
-  const inputCheckingBody = input.parseInputs('', '', '', '', '').mustInputs();
+  const inputCheckingBody = input
+    .parseInputs('', '', '', '', '', '', '')
+    .mustInputs();
 
   expect(inspection.check(message, inputCheckingBody)).not.toEqual([]);
 
   const inputNotCheckingBody = input
-    .parseInputs('', '', '', '', 'true')
+    .parseInputs('', '', '', '', '', '', 'true')
     .mustInputs();
 
   expect(inspection.check(message, inputNotCheckingBody)).toEqual([]);
@@ -81,7 +83,7 @@ it('reports missing body with disallowed one-liners.', () => {
 });
 
 it('reports missing body with allowed one-liners.', () => {
-  const inputs = input.parseInputs('', '', 'true', '', '').mustInputs();
+  const inputs = input.parseInputs('', '', 'true', '', '', '', '').mustInputs();
 
   const message = 'Change SomeClass to OtherClass\n';
   const errors = inspection.check(message, inputs);
@@ -167,7 +169,9 @@ it(
   'reports the subject starting with a non-verb ' +
     'with additional verbs given as direct input.',
   () => {
-    const inputs = input.parseInputs('table', '', 'false', '', '').mustInputs();
+    const inputs = input
+      .parseInputs('table', '', 'false', '', '', '', '')
+      .mustInputs();
 
     const message =
       'Replaced SomeClass to OtherClass\n' +
@@ -205,6 +209,8 @@ it(
       '/some/path',
       false,
       new Set<string>('table'),
+      50,
+      72,
       false,
       false
     );
@@ -237,7 +243,9 @@ it(
 );
 
 it('accepts the subject starting with an additional verb.', () => {
-  const inputs = input.parseInputs('table', '', 'false', '', '').mustInputs();
+  const inputs = input
+    .parseInputs('table', '', 'false', '', '', '', '')
+    .mustInputs();
 
   const message = 'Table that for me\n\nThis is a dummy commit.';
   const errors = inspection.check(message, inputs);
@@ -259,7 +267,7 @@ it('reports the subject ending in a dot.', () => {
 });
 
 it('reports an incorrect one-line message with allowed one-liners.', () => {
-  const inputs = input.parseInputs('', '', 'true', '', '').mustInputs();
+  const inputs = input.parseInputs('', '', 'true', '', '', '', '').mustInputs();
 
   const message = 'Change SomeClass to OtherClass.';
 
@@ -270,6 +278,38 @@ it('reports an incorrect one-line message with allowed one-liners.', () => {
   ]);
 });
 
+it('reports too long a subject line.', () => {
+  const message =
+    'Change SomeClass to OtherClass in order to handle upstream deprecation\n' +
+    '\n' +
+    'This replaces the SomeClass with OtherClass in all of the module\n' +
+    'since Some class was deprecated.';
+
+  const errors = inspection.check(message, defaultInputs);
+  expect(errors).toEqual([
+    `The subject exceeds the limit of 50 characters ` +
+      `(got: 70, JSON: "Change SomeClass to OtherClass in order to handle upstream deprecation").` +
+      'Please shorten the subject to make it more succinct.'
+  ]);
+});
+
+it('reports too long a subject line with custom max length.', () => {
+  const message =
+    'Change SomeClass to OtherClass in order to handle upstream deprecation\n' +
+    '\n' +
+    'This replaces the SomeClass with OtherClass in all of the module\n' +
+    'since Some class was deprecated.';
+
+  const inputs = input.parseInputs('', '', '', '60', '', '', '').mustInputs();
+
+  const errors = inspection.check(message, inputs);
+  expect(errors).toEqual([
+    `The subject exceeds the limit of 60 characters ` +
+      `(got: 70, JSON: "Change SomeClass to OtherClass in order to handle upstream deprecation").` +
+      'Please shorten the subject to make it more succinct.'
+  ]);
+});
+
 it('reports too long a body line.', () => {
   const message =
     'Change SomeClass to OtherClass\n' +
@@ -287,6 +327,25 @@ it('reports too long a body line.', () => {
   ]);
 });
 
+it('reports too long a body line with custom max length.', () => {
+  const message =
+    'Change SomeClass to OtherClass\n' +
+    '\n' +
+    'This replaces the SomeClass with OtherClass in all of the module ' +
+    'since Some class was deprecated.';
+
+  const inputs = input.parseInputs('', '', '', '', '90', '', '').mustInputs();
+
+  const errors = inspection.check(message, inputs);
+  expect(errors).toEqual([
+    'The line 3 of the message (line 1 of the body) exceeds the limit of ' +
+      '90 characters. The line contains 97 characters: ' +
+      '"This replaces the SomeClass with OtherClass in all of the module since ' +
+      'Some class was deprecated.". ' +
+      'Please reformat the body so that all the lines fit 90 characters.'
+  ]);
+});
+
 it('accepts a body line of exactly 72 characters.', () => {
   const message =
     'Do something\n' +
@@ -434,7 +493,7 @@ The ${long} line is too long.`;
 });
 
 it('accepts the valid body when enforcing the sign-off.', () => {
-  const inputs = input.parseInputs('', '', '', 'true', '').mustInputs();
+  const inputs = input.parseInputs('', '', '', '', '', 'true', '').mustInputs();
 
   const message = `Do something
 
@@ -453,7 +512,7 @@ Signed-off-by: Somebody Else <[email protected]>
 });
 
 it('rejects invalid sign-offs.', () => {
-  const inputs = input.parseInputs('', '', '', 'true', '').mustInputs();
+  const inputs = input.parseInputs('', '', '', '', '', 'true', '').mustInputs();
 
   const message = `Do something