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

Commit 11baafd

Browse files
smashwilsonsmashwilson
committed
Merge pull request #1500 from atom/aw/commit-message-wrapping
Revisit commit message processing
1 parent 24b4e90 commit 11baafd

File tree

7 files changed

+186
-61
lines changed

7 files changed

+186
-61
lines changed

‎lib/controllers/commit-controller.js‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,16 +122,18 @@ export default class CommitController extends React.Component {
122122
}
123123

124124
@autobind
125-
commit(message, coAuthors = [], amend) {
126-
let msg;
125+
commit(message, coAuthors = [], amend = false) {
126+
let msg, verbatim;
127127
if (this.isCommitMessageEditorExpanded()) {
128128
msg = this.getCommitMessageEditors()[0].getText();
129+
verbatim = false;
129130
} else {
130131
const wrapMessage = this.props.config.get('github.automaticCommitMessageWrapping');
131132
msg = wrapMessage ? wrapCommitMessage(message) : message;
133+
verbatim = true;
132134
}
133135

134-
return this.props.commit(msg.trim(), {amend, coAuthors});
136+
return this.props.commit(msg.trim(), {amend, coAuthors, verbatim});
135137
}
136138

137139
setCommitMessage(message) {

‎lib/git-shell-out-strategy.js‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -398,19 +398,33 @@ export default class GitShellOutStrategy {
398398
return this.exec(args, {stdin: patch, writeOperation: true});
399399
}
400400

401-
async commit(rawMessage, {allowEmpty, amend, coAuthors} = {}) {
402-
const args = ['commit', '--cleanup=strip'];
403-
401+
async commit(rawMessage, {allowEmpty, amend, coAuthors, verbatim} = {}) {
402+
const args = ['commit'];
404403
let msg;
405404

406-
// if amending and no new message is passed, use last commit's message
405+
// if amending and no new message is passed, use last commit's message. Ensure that we don't
406+
// mangle it in the process.
407407
if (amend && rawMessage.length === 0) {
408408
const {unbornRef, messageBody, messageSubject} = await this.getHeadCommit();
409-
msg = unbornRef ? rawMessage : `${messageSubject}\n\n${messageBody}`.trim();
409+
if (unbornRef) {
410+
msg = rawMessage;
411+
} else {
412+
msg = `${messageSubject}\n\n${messageBody}`.trim();
413+
verbatim = true;
414+
}
410415
} else {
411416
msg = rawMessage;
412417
}
413418

419+
// Determine the cleanup mode.
420+
if (verbatim) {
421+
args.push('--cleanup=verbatim');
422+
} else {
423+
const configured = await this.getConfig('commit.cleanup');
424+
const mode = (configured && configured !== 'default') ? configured : 'strip';
425+
args.push(`--cleanup=${mode}`);
426+
}
427+
414428
// add co-author commit trailers if necessary
415429
if (coAuthors && coAuthors.length > 0) {
416430
msg = await this.addCoAuthorsToMessage(msg, coAuthors);

‎lib/helpers.js‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -317,17 +317,19 @@ export function destroyEmptyFilePatchPaneItems(workspace) {
317317
}
318318

319319
export function extractCoAuthorsAndRawCommitMessage(commitMessage) {
320-
let rawMessage = '';
321-
const coAuthors = commitMessage.split(LINE_ENDING_REGEX).reduce((authors, line) => {
320+
const messageLines = [];
321+
const coAuthors = [];
322+
323+
for (const line of commitMessage.split(LINE_ENDING_REGEX)) {
322324
const match = line.match(CO_AUTHOR_REGEX);
323325
if (match) {
324326
// eslint-disable-next-line no-unused-vars
325327
const [_, name, email] = match;
326-
authors.push({name, email});
328+
coAuthors.push({name, email});
327329
} else {
328-
rawMessage += line;
330+
messageLines.push(line);
329331
}
330-
return authors;
331-
}, []);
332-
return {message: rawMessage, coAuthors};
332+
}
333+
334+
return {message: messageLines.join('\n'), coAuthors};
333335
}

‎lib/models/repository.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ export default class Repository {
131131
async getMergeMessage() {
132132
try {
133133
const contents = await fs.readFile(path.join(this.getGitDirectoryPath(), 'MERGE_MSG'), {encoding: 'utf8'});
134-
return contents;
134+
return contents.split(/\n/).filter(line => line.length > 0 && !line.startsWith('#')).join('\n');
135135
} catch (e) {
136136
return null;
137137
}

‎test/controllers/commit-controller.test.js‎

Lines changed: 68 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -104,12 +104,12 @@ describe('CommitController', function() {
104104
});
105105

106106
describe('committing', function() {
107-
let workdirPath, repository;
107+
let workdirPath, repository, commit;
108108

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

114114
app = React.cloneElement(app, {repository, commit});
115115
});
@@ -126,6 +126,20 @@ describe('CommitController', function() {
126126
assert.strictEqual(repository.getCommitMessage(), '');
127127
});
128128

129+
it('sets the verbatim flag when committing from the mini editor', async function() {
130+
await fs.writeFile(path.join(workdirPath, 'a.txt'), 'some changes', {encoding: 'utf8'});
131+
await repository.git.exec(['add', '.']);
132+
133+
const wrapper = shallow(app, {disableLifecycleMethods: true});
134+
await wrapper.instance().commit('message\n\n#123 do some things');
135+
136+
assert.isTrue(commit.calledWith('message\n\n#123 do some things', {
137+
amend: false,
138+
coAuthors: [],
139+
verbatim: true,
140+
}));
141+
});
142+
129143
it('issues a notification on failure', async function() {
130144
repository.setCommitMessage('some message');
131145

@@ -146,44 +160,58 @@ describe('CommitController', function() {
146160
});
147161

148162
describe('message formatting', function() {
149-
let commitSpy;
163+
let commitSpy, wrapper;
164+
150165
beforeEach(function() {
151166
commitSpy = sinon.stub().returns(Promise.resolve());
152167
app = React.cloneElement(app, {commit: commitSpy});
168+
wrapper = shallow(app, {disableLifecycleMethods: true});
153169
});
154170

155-
it('wraps the commit message body at 72 characters if github.automaticCommitMessageWrapping is true', async function() {
156-
config.set('github.automaticCommitMessageWrapping', false);
171+
describe('with automatic wrapping disabled', function() {
172+
beforeEach(function() {
173+
config.set('github.automaticCommitMessageWrapping', false);
174+
});
157175

158-
const wrapper = shallow(app);
176+
it('passes commit messages through unchanged', async function() {
177+
await wrapper.instance().commit([
178+
'Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor',
179+
'',
180+
'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.',
181+
].join('\n'));
182+
183+
assert.strictEqual(commitSpy.args[0][0], [
184+
'Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor',
185+
'',
186+
'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.',
187+
].join('\n'));
188+
});
189+
});
159190

160-
await wrapper.instance().commit([
161-
'Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor',
162-
'',
163-
'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.',
164-
].join('\n'));
165-
166-
assert.deepEqual(commitSpy.args[0][0].split('\n'), [
167-
'Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor',
168-
'',
169-
'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.',
170-
]);
171-
172-
commitSpy.reset();
173-
config.set('github.automaticCommitMessageWrapping', true);
174-
175-
await wrapper.instance().commit([
176-
'Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor',
177-
'',
178-
'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.',
179-
].join('\n'));
180-
181-
assert.deepEqual(commitSpy.args[0][0].split('\n'), [
182-
'Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor',
183-
'',
184-
'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ',
185-
'ut aliquip ex ea commodo consequat.',
186-
]);
191+
describe('with automatic wrapping enabled', function() {
192+
beforeEach(function() {
193+
config.set('github.automaticCommitMessageWrapping', true);
194+
});
195+
196+
it('wraps lines within the commit body at 72 characters', async function() {
197+
await wrapper.instance().commit([
198+
'Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor',
199+
'',
200+
'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.',
201+
].join('\n'));
202+
203+
assert.strictEqual(commitSpy.args[0][0], [
204+
'Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor',
205+
'',
206+
'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ',
207+
'ut aliquip ex ea commodo consequat.',
208+
].join('\n'));
209+
});
210+
211+
it('preserves existing line wraps within the commit body', async function() {
212+
await wrapper.instance().commit('a\n\nb\n\nc');
213+
assert.strictEqual(commitSpy.args[0][0], 'a\n\nb\n\nc');
214+
});
187215
});
188216
});
189217

@@ -278,10 +306,14 @@ describe('CommitController', function() {
278306
const editor = workspace.getActiveTextEditor();
279307
editor.setText('message in editor');
280308
await editor.save();
281-
wrapper.find('CommitView').prop('commit')('message in box');
282309

283-
await assert.async.strictEqual((await repository.getLastCommit()).getMessageSubject(), 'message in editor');
284-
await assert.async.isFalse(fs.existsSync(wrapper.instance().getCommitMessagePath()));
310+
await wrapper.find('CommitView').prop('commit')('message in box');
311+
312+
assert.strictEqual((await repository.getLastCommit()).getMessageSubject(), 'message in editor');
313+
assert.isFalse(fs.existsSync(wrapper.instance().getCommitMessagePath()));
314+
assert.isTrue(commit.calledWith('message in editor', {
315+
amend: false, coAuthors: [], verbatim: false,
316+
}));
285317
});
286318

287319
it('asks user to confirm if commit editor has unsaved changes', async function() {

‎test/controllers/git-tab-controller.test.js‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -626,7 +626,10 @@ describe('GitTabController', function() {
626626
assert.strictEqual(wrapper.find('CommitView').getNode().editor.getText(), '');
627627

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

631634
// amending should commit all unstaged changes
632635
await assert.async.lengthOf(wrapper.find('GitTabView').prop('stagedChanges'), 0);
@@ -648,7 +651,10 @@ describe('GitTabController', function() {
648651
await assert.async.lengthOf(wrapper.find('GitTabView').prop('stagedChanges'), 0);
649652

650653
commandRegistry.dispatch(workspaceElement, 'github:amend-last-commit');
651-
await assert.async.deepEqual(repository.commit.args[0][1], {amend: true, coAuthors: []});
654+
await assert.async.deepEqual(
655+
repository.commit.args[0][1],
656+
{amend: true, coAuthors: [], verbatim: true},
657+
);
652658

653659
// new commit message is used
654660
await assert.async.equal(getLastCommit().getMessageSubject(), newMessage);
@@ -669,7 +675,11 @@ describe('GitTabController', function() {
669675

670676
commandRegistry.dispatch(workspaceElement, 'github:amend-last-commit');
671677
// verify that coAuthor was passed
672-
await assert.async.deepEqual(repository.commit.args[0][1], {amend: true, coAuthors: [author]});
678+
await assert.async.deepEqual(
679+
repository.commit.args[0][1],
680+
{amend: true, coAuthors: [author], verbatim: true},
681+
);
682+
await repository.commit.returnValues[0];
673683

674684
await assert.async.deepEqual(getLastCommit().coAuthors, [author]);
675685
assert.strictEqual(getLastCommit().getMessageSubject(), commitBeforeAmend.getMessageSubject());
@@ -690,7 +700,11 @@ describe('GitTabController', function() {
690700
commandRegistry.dispatch(workspaceElement, 'github:amend-last-commit');
691701

692702
// verify that coAuthor was passed
693-
await assert.async.deepEqual(repository.commit.args[0][1], {amend: true, coAuthors: [author]});
703+
await assert.async.deepEqual(
704+
repository.commit.args[0][1],
705+
{amend: true, coAuthors: [author], verbatim: true},
706+
);
707+
await repository.commit.returnValues[0];
694708

695709
// verify that commit message has coauthor
696710
await assert.async.deepEqual(getLastCommit().coAuthors, [author]);
@@ -721,7 +735,11 @@ describe('GitTabController', function() {
721735
// amend again
722736
commandRegistry.dispatch(workspaceElement, 'github:amend-last-commit');
723737
// verify that NO coAuthor was passed
724-
await assert.async.deepEqual(repository.commit.args[0][1], {amend: true, coAuthors: []});
738+
await assert.async.deepEqual(
739+
repository.commit.args[0][1],
740+
{amend: true, coAuthors: [], verbatim: true},
741+
);
742+
await repository.commit.returnValues[0];
725743

726744
// assert that no co-authors are in last commit
727745
await assert.async.deepEqual(getLastCommit().coAuthors, []);

0 commit comments

Comments
 (0)