Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make max length of body lines configurable #96

Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,36 @@ 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.
Fryuni marked this conversation as resolved.
Show resolved Hide resolved

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.
Expand Down
8 changes: 8 additions & 0 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
46 changes: 32 additions & 14 deletions dist/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.');
}
Expand Down Expand Up @@ -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.');
Expand All @@ -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]);
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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;
}
Expand All @@ -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) {
Expand All @@ -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);
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 4 additions & 0 deletions src/__tests__/input.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ it('parses the inputs.', () => {
'integrate\nanalyze',
pathToVerbs,
'true',
'90',
'100',
'true',
'true'
);
Expand All @@ -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();
});
79 changes: 69 additions & 10 deletions src/__tests__/inspection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.', () => {
Expand All @@ -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';

Expand Down Expand Up @@ -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([]);
Expand All @@ -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);
Expand Down Expand Up @@ -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' +
Expand Down Expand Up @@ -205,6 +209,8 @@ it(
'/some/path',
false,
new Set<string>('table'),
50,
72,
false,
false
);
Expand Down Expand Up @@ -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);
Expand All @@ -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.';

Expand All @@ -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' +
Expand All @@ -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' +
Expand Down Expand Up @@ -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

Expand All @@ -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

Expand Down
Loading