diff --git a/README.md b/README.md index 6774102..106e506 100644 --- a/README.md +++ b/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. diff --git a/action.yml b/action.yml index 9b85bb2..7d1340b 100644 --- a/action.yml +++ b/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 diff --git a/dist/index.js b/dist/index.js index 0e8c211..ee7307a 100644 --- a/dist/index.js +++ b/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,7 +1750,7 @@ 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 @@ -1742,9 +1758,11 @@ function runWithExceptions() { 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); diff --git a/src/__tests__/input.test.ts b/src/__tests__/input.test.ts index 1dc40e8..561d72e 100644 --- a/src/__tests__/input.test.ts +++ b/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(['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(); }); diff --git a/src/__tests__/inspection.test.ts b/src/__tests__/inspection.test.ts index 67ba070..5ceb64a 100644 --- a/src/__tests__/inspection.test.ts +++ b/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('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 }); it('rejects invalid sign-offs.', () => { - const inputs = input.parseInputs('', '', '', 'true', '').mustInputs(); + const inputs = input.parseInputs('', '', '', '', '', 'true', '').mustInputs(); const message = `Do something diff --git a/src/input.ts b/src/input.ts index e398f26..978bc77 100644 --- a/src/input.ts +++ b/src/input.ts @@ -4,6 +4,8 @@ export class Inputs { public hasAdditionalVerbsInput: boolean; public pathToAdditionalVerbs: string; public allowOneLiners: boolean; + public maxSubjectLength: number; + public maxBodyLineLength: number; public skipBodyCheck: boolean; // This is a complete appendix to the whiltelist parsed both from @@ -18,6 +20,8 @@ export class Inputs { pathToAdditionalVerbs: string, allowOneLiners: boolean, additionalVerbs: Set, + maxSubjectLength: number, + maxBodyLineLength: number, enforceSignOff: boolean, skipBodyCheck: boolean ) { @@ -25,6 +29,8 @@ export class Inputs { this.pathToAdditionalVerbs = pathToAdditionalVerbs; this.allowOneLiners = allowOneLiners; this.additionalVerbs = additionalVerbs; + this.maxSubjectLength = maxSubjectLength; + this.maxBodyLineLength = maxBodyLineLength; this.enforceSignOff = enforceSignOff; this.skipBodyCheck = skipBodyCheck; } @@ -64,6 +70,8 @@ export function parseInputs( additionalVerbsInput: string, pathToAdditionalVerbsInput: string, allowOneLinersInput: string, + maxSubjectLengthInput: string, + maxBodyLineLengthInput: string, enforceSignOffInput: string, skipBodyCheckInput: string ): MaybeInputs { @@ -105,6 +113,30 @@ export function parseInputs( ); } + const maxSubjectLength: number = !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: number = !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: boolean | null = !enforceSignOffInput ? false : parseBooleanFromString(enforceSignOffInput); @@ -135,6 +167,8 @@ export function parseInputs( pathToAdditionalVerbsInput, allowOneLiners, additionalVerbs, + maxSubjectLength, + maxBodyLineLength, enforceSignOff, skipBodyCheck ), diff --git a/src/inspection.ts b/src/inspection.ts index e648989..72a245d 100644 --- a/src/inspection.ts +++ b/src/inspection.ts @@ -151,9 +151,9 @@ function checkSubject(subject: string, inputs: input.Inputs): string[] { // similar services. const subjectWoCode = subject.replace(suffixHashCodeRe, ''); - if (subjectWoCode.length > 50) { + if (subjectWoCode.length > inputs.maxSubjectLength) { errors.push( - `The subject exceeds the limit of 50 characters ` + + `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.' ); @@ -209,7 +209,11 @@ function checkSubject(subject: string, inputs: input.Inputs): string[] { const urlLineRe = new RegExp('^[^ ]+://[^ ]+$'); const linkDefinitionRe = new RegExp('^\\[[^\\]]+]\\s*:\\s*[^ ]+://[^ ]+$'); -function checkBody(subject: string, bodyLines: string[]): string[] { +function checkBody( + subject: string, + bodyLines: string[], + inputs: input.Inputs +): string[] { const errors: string[] = []; if (bodyLines.length === 0) { @@ -229,14 +233,14 @@ function checkBody(subject: string, bodyLines: string[]): string[] { 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.` ); } } @@ -321,7 +325,9 @@ export function check(message: string, inputs: input.Inputs): string[] { 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) { diff --git a/src/mainImpl.ts b/src/mainImpl.ts index 24c0ebd..1f47495 100644 --- a/src/mainImpl.ts +++ b/src/mainImpl.ts @@ -21,6 +21,12 @@ function runWithExceptions(): void { const allowOneLinersInput = core.getInput('allow-one-liners', {required: false}) ?? ''; + const maxSubjectLengthInput = + core.getInput('max-subject-line-length', {required: false}) ?? ''; + + const maxBodyLineLengthInput = + core.getInput('max-body-line-length', {required: false}) ?? ''; + const enforceSignOffInput = core.getInput('enforce-sign-off', {required: false}) ?? ''; @@ -31,6 +37,8 @@ function runWithExceptions(): void { additionalVerbsInput, pathToAdditionalVerbsInput, allowOneLinersInput, + maxSubjectLengthInput, + maxBodyLineLengthInput, enforceSignOffInput, skipBodyCheckInput );