Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Commit

Permalink
Merge pull request #1500 from atom/aw/commit-message-wrapping
Browse files Browse the repository at this point in the history 
Revisit commit message processing
  • Loading branch information
@smashwilson
smashwilson committed Jun 1, 2018
1 parent 710c020 commit 9b94879
Show file tree
Hide file tree
Showing 7 changed files with 183 additions and 61 deletions.
8 changes: 5 additions & 3 deletions lib/controllers/commit-controller.js
View file
Expand Up @@ -122,16 +122,18 @@ export default class CommitController extends React.Component {
this.subscriptions.dispose();
}

commit(message, coAuthors = [], amend) {
let msg;
commit(message, coAuthors = [], amend = false) {
let msg, verbatim;
if (this.isCommitMessageEditorExpanded()) {
msg = this.getCommitMessageEditors()[0].getText();
verbatim = false;
} else {
const wrapMessage = this.props.config.get('github.automaticCommitMessageWrapping');
msg = wrapMessage ? wrapCommitMessage(message) : message;
verbatim = true;
}

return this.props.commit(msg.trim(), {amend, coAuthors});
return this.props.commit(msg.trim(), {amend, coAuthors, verbatim});
}

setCommitMessage(message) {
Expand Down
24 changes: 19 additions & 5 deletions lib/git-shell-out-strategy.js
View file
Expand Up @@ -437,19 +437,33 @@ export default class GitShellOutStrategy {
return this.exec(args, {stdin: patch, writeOperation: true});
}

async commit(rawMessage, {allowEmpty, amend, coAuthors} = {}) {
const args = ['commit', '--cleanup=strip'];

async commit(rawMessage, {allowEmpty, amend, coAuthors, verbatim} = {}) {
const args = ['commit'];
let msg;

// if amending and no new message is passed, use last commit's message
// if amending and no new message is passed, use last commit's message. Ensure that we don't
// mangle it in the process.
if (amend && rawMessage.length === 0) {
const {unbornRef, messageBody, messageSubject} = await this.getHeadCommit();
msg = unbornRef ? rawMessage : `${messageSubject}\n\n${messageBody}`.trim();
if (unbornRef) {
msg = rawMessage;
} else {
msg = `${messageSubject}\n\n${messageBody}`.trim();
verbatim = true;
}
} else {
msg = rawMessage;
}

// Determine the cleanup mode.
if (verbatim) {
args.push('--cleanup=verbatim');
} else {
const configured = await this.getConfig('commit.cleanup');
const mode = (configured && configured !== 'default') ? configured : 'strip';
args.push(`--cleanup=${mode}`);
}

// add co-author commit trailers if necessary
if (coAuthors && coAuthors.length > 0) {
msg = await this.addCoAuthorsToMessage(msg, coAuthors);
Expand Down
16 changes: 9 additions & 7 deletions lib/helpers.js
View file
Expand Up @@ -332,19 +332,21 @@ export function destroyEmptyFilePatchPaneItems(workspace) {
}

export function extractCoAuthorsAndRawCommitMessage(commitMessage) {
let rawMessage = '';
const coAuthors = commitMessage.split(LINE_ENDING_REGEX).reduce((authors, line) => {
const messageLines = [];
const coAuthors = [];

for (const line of commitMessage.split(LINE_ENDING_REGEX)) {
const match = line.match(CO_AUTHOR_REGEX);
if (match) {
// eslint-disable-next-line no-unused-vars
const [_, name, email] = match;
authors.push({name, email});
coAuthors.push({name, email});
} else {
rawMessage += line;
messageLines.push(line);
}
return authors;
}, []);
return {message: rawMessage, coAuthors};
}

return {message: messageLines.join('\n'), coAuthors};
}

export function createItem(node, componentHolder = null, uri = null, extra = {}) {
Expand Down
2 changes: 1 addition & 1 deletion lib/models/repository.js
View file
Expand Up @@ -131,7 +131,7 @@ export default class Repository {
async getMergeMessage() {
try {
const contents = await fs.readFile(path.join(this.getGitDirectoryPath(), 'MERGE_MSG'), {encoding: 'utf8'});
return contents;
return contents.split(/\n/).filter(line => line.length > 0 && !line.startsWith('#')).join('\n');
} catch (e) {
return null;
}
Expand Down
104 changes: 68 additions & 36 deletions test/controllers/commit-controller.test.js
View file
Expand Up @@ -114,12 +114,12 @@ describe('CommitController', function() {
});

describe('committing', function() {
let workdirPath, repository;
let workdirPath, repository, commit;

beforeEach(async function() {
workdirPath = await cloneRepository('three-files');
repository = await buildRepositoryWithPipeline(workdirPath, {confirm, notificationManager, workspace});
const commit = message => repository.commit(message);
commit = sinon.stub().callsFake((...args) => repository.commit(...args));

app = React.cloneElement(app, {repository, commit});
});
Expand All @@ -136,6 +136,20 @@ describe('CommitController', function() {
assert.strictEqual(repository.getCommitMessage(), '');
});

it('sets the verbatim flag when committing from the mini editor', async function() {
await fs.writeFile(path.join(workdirPath, 'a.txt'), 'some changes', {encoding: 'utf8'});
await repository.git.exec(['add', '.']);

const wrapper = shallow(app, {disableLifecycleMethods: true});
await wrapper.instance().commit('message\n\n#123 do some things');

assert.isTrue(commit.calledWith('message\n\n#123 do some things', {
amend: false,
coAuthors: [],
verbatim: true,
}));
});

it('issues a notification on failure', async function() {
repository.setCommitMessage('some message');

Expand All @@ -156,44 +170,58 @@ describe('CommitController', function() {
});

describe('message formatting', function() {
let commitSpy;
let commitSpy, wrapper;

beforeEach(function() {
commitSpy = sinon.stub().returns(Promise.resolve());
app = React.cloneElement(app, {commit: commitSpy});
wrapper = shallow(app, {disableLifecycleMethods: true});
});

it('wraps the commit message body at 72 characters if github.automaticCommitMessageWrapping is true', async function() {
config.set('github.automaticCommitMessageWrapping', false);
describe('with automatic wrapping disabled', function() {
beforeEach(function() {
config.set('github.automaticCommitMessageWrapping', false);
});

const wrapper = shallow(app, {disableLifecycleMethods: true});
it('passes commit messages through unchanged', async function() {
await wrapper.instance().commit([
'Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor',
'',
'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.',
].join('\n'));

assert.strictEqual(commitSpy.args[0][0], [
'Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor',
'',
'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.',
].join('\n'));
});
});

await wrapper.instance().commit([
'Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor',
'',
'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.',
].join('\n'));

assert.deepEqual(commitSpy.args[0][0].split('\n'), [
'Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor',
'',
'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.',
]);

commitSpy.reset();
config.set('github.automaticCommitMessageWrapping', true);

await wrapper.instance().commit([
'Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor',
'',
'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.',
].join('\n'));

assert.deepEqual(commitSpy.args[0][0].split('\n'), [
'Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor',
'',
'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ',
'ut aliquip ex ea commodo consequat.',
]);
describe('with automatic wrapping enabled', function() {
beforeEach(function() {
config.set('github.automaticCommitMessageWrapping', true);
});

it('wraps lines within the commit body at 72 characters', async function() {
await wrapper.instance().commit([
'Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor',
'',
'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.',
].join('\n'));

assert.strictEqual(commitSpy.args[0][0], [
'Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor',
'',
'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ',
'ut aliquip ex ea commodo consequat.',
].join('\n'));
});

it('preserves existing line wraps within the commit body', async function() {
await wrapper.instance().commit('a\n\nb\n\nc');
assert.strictEqual(commitSpy.args[0][0], 'a\n\nb\n\nc');
});
});
});

Expand Down Expand Up @@ -289,10 +317,14 @@ describe('CommitController', function() {
const editor = workspace.getActiveTextEditor();
editor.setText('message in editor');
await editor.save();
wrapper.find('CommitView').prop('commit')('message in box');

await assert.async.strictEqual((await repository.getLastCommit()).getMessageSubject(), 'message in editor');
await assert.async.isFalse(fs.existsSync(wrapper.instance().getCommitMessagePath()));
await wrapper.find('CommitView').prop('commit')('message in box');

assert.strictEqual((await repository.getLastCommit()).getMessageSubject(), 'message in editor');
assert.isFalse(fs.existsSync(wrapper.instance().getCommitMessagePath()));
assert.isTrue(commit.calledWith('message in editor', {
amend: false, coAuthors: [], verbatim: false,
}));
});

it('asks user to confirm if commit editor has unsaved changes', async function() {
Expand Down
25 changes: 20 additions & 5 deletions test/controllers/git-tab-controller.test.js
View file
Expand Up @@ -572,7 +572,10 @@ describe('GitTabController', function() {
assert.strictEqual(wrapper.find('CommitView').instance().editor.getText(), '');

commandRegistry.dispatch(workspaceElement, 'github:amend-last-commit');
await assert.async.deepEqual(repository.commit.args[0][1], {amend: true, coAuthors: []});
await assert.async.deepEqual(
repository.commit.args[0][1],
{amend: true, coAuthors: [], verbatim: true},
);

// amending should commit all unstaged changes
await updateWrapper(repository, wrapper);
Expand All @@ -594,7 +597,10 @@ describe('GitTabController', function() {
assert.lengthOf(wrapper.find('GitTabView').prop('stagedChanges'), 0);

commandRegistry.dispatch(workspaceElement, 'github:amend-last-commit');
await assert.async.deepEqual(repository.commit.args[0][1], {amend: true, coAuthors: []});
await assert.async.deepEqual(
repository.commit.args[0][1],
{amend: true, coAuthors: [], verbatim: true},
);
await updateWrapper(repository, wrapper);

// new commit message is used
Expand All @@ -617,7 +623,10 @@ describe('GitTabController', function() {

commandRegistry.dispatch(workspaceElement, 'github:amend-last-commit');
// verify that coAuthor was passed
await assert.async.deepEqual(repository.commit.args[0][1], {amend: true, coAuthors: [author]});
await assert.async.deepEqual(
repository.commit.args[0][1],
{amend: true, coAuthors: [author], verbatim: true},
);
await repository.commit.returnValues[0];
await updateWrapper(repository, wrapper);

Expand All @@ -640,7 +649,10 @@ describe('GitTabController', function() {
commandRegistry.dispatch(workspaceElement, 'github:amend-last-commit');

// verify that coAuthor was passed
await assert.async.deepEqual(repository.commit.args[0][1], {amend: true, coAuthors: [author]});
await assert.async.deepEqual(
repository.commit.args[0][1],
{amend: true, coAuthors: [author], verbatim: true},
);
await repository.commit.returnValues[0];
await updateWrapper(repository, wrapper);

Expand Down Expand Up @@ -673,7 +685,10 @@ describe('GitTabController', function() {
// amend again
commandRegistry.dispatch(workspaceElement, 'github:amend-last-commit');
// verify that NO coAuthor was passed
await assert.async.deepEqual(repository.commit.args[0][1], {amend: true, coAuthors: []});
await assert.async.deepEqual(
repository.commit.args[0][1],
{amend: true, coAuthors: [], verbatim: true},
);
await repository.commit.returnValues[0];
await updateWrapper(repository, wrapper);

Expand Down

0 comments on commit 9b94879

Please sign in to comment.