diff --git a/lib/controllers/commit-controller.js b/lib/controllers/commit-controller.js index c4d637c56b..db88bc8fe8 100644 --- a/lib/controllers/commit-controller.js +++ b/lib/controllers/commit-controller.js @@ -28,7 +28,6 @@ export default class CommitController extends React.Component { repository: PropTypes.object.isRequired, isMerging: PropTypes.bool.isRequired, - mergeMessage: PropTypes.string, mergeConflictsExist: PropTypes.bool.isRequired, stagedChangesExist: PropTypes.bool.isRequired, lastCommit: PropTypes.object.isRequired, @@ -78,10 +77,6 @@ export default class CommitController extends React.Component { } }), ); - - if (this.props.isMerging && !this.getCommitMessage()) { - this.commitMessageBuffer.setText(this.props.mergeMessage || ''); - } } render() { @@ -115,11 +110,7 @@ export default class CommitController extends React.Component { } componentDidUpdate(prevProps) { - if (!prevProps.isMerging && this.props.isMerging && !this.getCommitMessage()) { - this.commitMessageBuffer.setText(this.props.mergeMessage || ''); - } else { - this.commitMessageBuffer.setTextViaDiff(this.getCommitMessage()); - } + this.commitMessageBuffer.setTextViaDiff(this.getCommitMessage()); } componentWillUnmount() { @@ -140,9 +131,9 @@ export default class CommitController extends React.Component { return this.props.commit(msg.trim(), {amend, coAuthors, verbatim}); } - setCommitMessage(message) { + setCommitMessage(message, options) { if (!this.props.repository.isPresent()) { return; } - this.props.repository.setCommitMessage(message); + this.props.repository.setCommitMessage(message, options); } getCommitMessage() { @@ -157,7 +148,7 @@ export default class CommitController extends React.Component { if (!this.props.repository.isPresent()) { return; } - this.setCommitMessage(this.commitMessageBuffer.getText()); + this.setCommitMessage(this.commitMessageBuffer.getText(), {suppressUpdate: true}); } getCommitMessageEditors() { diff --git a/lib/controllers/git-tab-controller.js b/lib/controllers/git-tab-controller.js index 71cda86735..08c53355af 100644 --- a/lib/controllers/git-tab-controller.js +++ b/lib/controllers/git-tab-controller.js @@ -269,13 +269,15 @@ export default class GitTabController extends React.Component { const repo = this.props.repository; const lastCommit = await repo.getLastCommit(); if (lastCommit.isUnbornRef()) { return null; } + + await repo.undoLastCommit(); repo.setCommitMessage(lastCommit.getFullMessage()); const coAuthors = lastCommit.getCoAuthors().map(author => new Author(author.email, author.name)); this.updateSelectedCoAuthors(coAuthors); - return repo.undoLastCommit(); + return null; } async abortMerge() { diff --git a/lib/get-repo-pipeline-manager.js b/lib/get-repo-pipeline-manager.js index 489eea22a1..365fc8dfd1 100644 --- a/lib/get-repo-pipeline-manager.js +++ b/lib/get-repo-pipeline-manager.js @@ -210,7 +210,8 @@ export default function({confirm, notificationManager, workspace}) { commitPipeline.addMiddleware('failed-to-commit-error', async (next, repository) => { try { const result = await next(); - repository.setCommitMessage(''); + const template = await repository.fetchCommitMessageTemplate(); + repository.setCommitMessage(template || ''); destroyFilePatchPaneItems({onlyStaged: true}, workspace); return result; } catch (error) { diff --git a/lib/git-shell-out-strategy.js b/lib/git-shell-out-strategy.js index e6a4e6b697..7c8ceb5187 100644 --- a/lib/git-shell-out-strategy.js +++ b/lib/git-shell-out-strategy.js @@ -53,6 +53,17 @@ const DISABLE_COLOR_FLAGS = [ return acc; }, []); +/** + * Expand config path name per + * https://git-scm.com/docs/git-config#git-config-pathname + * this regex attempts to get the specified user's home directory + * Ex: on Mac ~kuychaco/ is expanded to the specified user’s home directory (/Users/kuychaco) + * Regex translation: + * ^~ line starts with tilde + * ([^/]*)/ captures non-forwardslash characters before first slash + */ +const EXPAND_TILDE_REGEX = new RegExp('^~([^/]*)/'); + export default class GitShellOutStrategy { static defaultExecArgs = { stdin: null, @@ -422,6 +433,29 @@ export default class GitShellOutStrategy { return this.exec(args, {writeOperation: true}); } + async fetchCommitMessageTemplate() { + let templatePath = await this.getConfig('commit.template'); + if (!templatePath) { + return null; + } + + const homeDir = os.homedir(); + + templatePath = templatePath.trim().replace(EXPAND_TILDE_REGEX, (_, user) => { + // if no user is specified, fall back to using the home directory. + return `${user ? path.join(path.dirname(homeDir), user) : homeDir}/`; + }); + + if (!path.isAbsolute(templatePath)) { + templatePath = path.join(this.workingDir, templatePath); + } + + if (!await fileExists(templatePath)) { + throw new Error(`Invalid commit template path set in Git config: ${templatePath}`); + } + return await fs.readFile(templatePath, {encoding: 'utf8'}); + } + unstageFiles(paths, commit = 'HEAD') { if (paths.length === 0) { return Promise.resolve(null); } const args = ['reset', commit, '--'].concat(paths.map(toGitPathSep)); @@ -889,7 +923,7 @@ export default class GitShellOutStrategy { let output; try { let args = ['config']; - if (local) { args.push('--local'); } + if (local || atom.inSpecMode()) { args.push('--local'); } args = args.concat(option); output = await this.exec(args); } catch (err) { diff --git a/lib/helpers.js b/lib/helpers.js index 37f1854de1..def4f8d70c 100644 --- a/lib/helpers.js +++ b/lib/helpers.js @@ -320,6 +320,9 @@ export function toGitPathSep(rawPath) { } } +export function filePathEndsWith(filePath, ...segments) { + return filePath.endsWith(path.join(...segments)); +} /** * Turns an array of things @kuychaco cannot eat diff --git a/lib/models/repository-states/present.js b/lib/models/repository-states/present.js index ed25c56e49..77c242c866 100644 --- a/lib/models/repository-states/present.js +++ b/lib/models/repository-states/present.js @@ -16,6 +16,7 @@ import RemoteSet from '../remote-set'; import Commit from '../commit'; import OperationStates from '../operation-states'; import {addEvent} from '../../reporter-proxy'; +import {filePathEndsWith} from '../../helpers'; /** * State used when the working directory contains a valid git repository and can be interacted with. Performs @@ -39,20 +40,46 @@ export default class Present extends State { this.operationStates = new OperationStates({didUpdate: this.didUpdate.bind(this)}); this.commitMessage = ''; + this.commitMessageTemplate = null; + this.fetchInitialMessage(); if (history) { this.discardHistory.updateHistory(history); } } - setCommitMessage(message) { + setCommitMessage(message, {suppressUpdate} = {suppressUpdate: false}) { this.commitMessage = message; + if (!suppressUpdate) { + this.didUpdate(); + } + } + + setCommitMessageTemplate(template) { + this.commitMessageTemplate = template; + } + + async fetchInitialMessage() { + const mergeMessage = await this.repository.getMergeMessage(); + const template = await this.fetchCommitMessageTemplate(); + if (template) { + this.commitMessageTemplate = template; + } + if (mergeMessage) { + this.setCommitMessage(mergeMessage); + } else if (template) { + this.setCommitMessage(template); + } } getCommitMessage() { return this.commitMessage; } + fetchCommitMessageTemplate() { + return this.git().fetchCommitMessageTemplate(); + } + getOperationStates() { return this.operationStates; } @@ -75,7 +102,8 @@ export default class Present extends State { this.didUpdate(); } - observeFilesystemChange(paths) { + invalidateCacheAfterFilesystemChange(events) { + const paths = events.map(e => e.special || e.path); const keys = new Set(); for (let i = 0; i < paths.length; i++) { const fullPath = paths[i]; @@ -88,10 +116,9 @@ export default class Present extends State { continue; } - const endsWith = (...segments) => fullPath.endsWith(path.join(...segments)); const includes = (...segments) => fullPath.includes(path.join(...segments)); - if (endsWith('.git', 'index')) { + if (filePathEndsWith(fullPath, '.git', 'index')) { keys.add(Keys.stagedChangesSinceParentCommit); keys.add(Keys.filePatch.all); keys.add(Keys.index.all); @@ -99,7 +126,7 @@ export default class Present extends State { continue; } - if (endsWith('.git', 'HEAD')) { + if (filePathEndsWith(fullPath, '.git', 'HEAD')) { keys.add(Keys.branches); keys.add(Keys.lastCommit); keys.add(Keys.recentCommits); @@ -125,7 +152,7 @@ export default class Present extends State { continue; } - if (endsWith('.git', 'config')) { + if (filePathEndsWith(fullPath, '.git', 'config')) { keys.add(Keys.remotes); keys.add(Keys.config.all); keys.add(Keys.statusBundle); @@ -144,6 +171,47 @@ export default class Present extends State { } } + isCommitMessageClean() { + if (this.commitMessage.trim() === '') { + return true; + } else if (this.commitMessageTemplate) { + return this.commitMessage === this.commitMessageTemplate; + } + return false; + } + + async updateCommitMessageAfterFileSystemChange(events) { + for (let i = 0; i < events.length; i++) { + const event = events[i]; + + if (filePathEndsWith(event.path, '.git', 'MERGE_HEAD')) { + if (event.action === 'created') { + if (this.isCommitMessageClean()) { + this.setCommitMessage(await this.repository.getMergeMessage()); + } + } else if (event.action === 'deleted') { + this.setCommitMessage(this.commitMessageTemplate || ''); + } + } + + if (filePathEndsWith(event.path, '.git', 'config')) { + // this won't catch changes made to the template file itself... + const template = await this.fetchCommitMessageTemplate(); + if (template === null) { + this.setCommitMessage(''); + } else if (this.commitMessageTemplate !== template) { + this.setCommitMessage(template); + } + this.setCommitMessageTemplate(template); + } + } + } + + observeFilesystemChange(events) { + this.invalidateCacheAfterFilesystemChange(events); + this.updateCommitMessageAfterFileSystemChange(events); + } + refresh() { this.cache.clear(); this.didUpdate(); @@ -280,7 +348,10 @@ export default class Present extends State { Keys.filePatch.all, Keys.index.all, ], - () => this.git().abortMerge(), + async () => { + await this.git().abortMerge(); + this.setCommitMessage(this.commitMessageTemplate || ''); + }, ); } diff --git a/lib/models/repository-states/state.js b/lib/models/repository-states/state.js index dc3b8e9bb4..7e3e88988f 100644 --- a/lib/models/repository-states/state.js +++ b/lib/models/repository-states/state.js @@ -374,6 +374,10 @@ export default class State { return ''; } + fetchCommitMessageTemplate() { + return unsupportedOperationPromise(this, 'fetchCommitMessageTemplate'); + } + // Cache getCache() { diff --git a/lib/models/repository.js b/lib/models/repository.js index ab50db5f52..8dc10b022a 100644 --- a/lib/models/repository.js +++ b/lib/models/repository.js @@ -358,6 +358,7 @@ const delegates = [ 'setCommitMessage', 'getCommitMessage', + 'fetchCommitMessageTemplate', 'getCache', ]; diff --git a/lib/models/workdir-context.js b/lib/models/workdir-context.js index b7a5ed749b..3c29a405ee 100644 --- a/lib/models/workdir-context.js +++ b/lib/models/workdir-context.js @@ -50,8 +50,7 @@ export default class WorkdirContext { // Wire up event forwarding among models this.subs.add(this.repository.onDidChangeState(this.repositoryChangedState)); this.subs.add(this.observer.onDidChange(events => { - const paths = events.map(e => e.special || e.path); - this.repository.observeFilesystemChange(paths); + this.repository.observeFilesystemChange(events); })); this.subs.add(this.observer.onDidChangeWorkdirOrHead(() => this.emitter.emit('did-change-workdir-or-head'))); diff --git a/lib/views/commit-view.js b/lib/views/commit-view.js index 7690da29d9..5b0560c589 100644 --- a/lib/views/commit-view.js +++ b/lib/views/commit-view.js @@ -460,13 +460,18 @@ export default class CommitView extends React.Component { } } + isValidMessage() { + // ensure that there are at least some non-comment lines in the commit message. + // Commented lines are stripped out of commit messages by git, by default configuration. + return this.props.messageBuffer.getText().replace(/^#.*$/gm, '').trim().length !== 0; + } + commitIsEnabled(amend) { - const messageExists = this.props.messageBuffer.getLength() > 0; return !this.props.isCommitting && (amend || this.props.stagedChangesExist) && !this.props.mergeConflictsExist && this.props.lastCommit.isPresent() && - (this.props.deactivateCommitBox || (amend || messageExists)); + (this.props.deactivateCommitBox || (amend || this.isValidMessage())); } commitButtonText() { @@ -580,6 +585,11 @@ export default class CommitView extends React.Component { if (focus === CommitView.focus.EDITOR) { if (this.refEditorComponent.map(focusElement).getOr(false)) { + if (this.props.messageBuffer.getText().length > 0 && !this.isValidMessage()) { + // there is likely a commit message template present + // we want the cursor to be at the beginning, not at the and of the template + this.refEditorComponent.get().getModel().setCursorBufferPosition([0, 0]); + } return true; } } diff --git a/test/controllers/commit-controller.test.js b/test/controllers/commit-controller.test.js index 3e716ea10d..b73caf5363 100644 --- a/test/controllers/commit-controller.test.js +++ b/test/controllers/commit-controller.test.js @@ -40,7 +40,6 @@ describe('CommitController', function() { isMerging={false} mergeConflictsExist={false} stagedChangesExist={false} - mergeMessage={''} lastCommit={lastCommit} currentBranch={nullBranch} userStore={store} @@ -61,19 +60,29 @@ describe('CommitController', function() { const workdirPath2 = await cloneRepository('three-files'); const repository2 = await buildRepository(workdirPath2); + // set commit template for repository2 + const templatePath = path.join(workdirPath2, 'a.txt'); + const templateText = fs.readFileSync(templatePath, 'utf8'); + await repository2.git.setConfig('commit.template', templatePath); + await assert.async.strictEqual(repository2.getCommitMessage(), templateText); + app = React.cloneElement(app, {repository: repository1}); const wrapper = shallow(app, {disableLifecycleMethods: true}); assert.strictEqual(wrapper.instance().getCommitMessage(), ''); wrapper.instance().setCommitMessage('message 1'); + assert.equal(wrapper.instance().getCommitMessage(), 'message 1'); wrapper.setProps({repository: repository2}); - - assert.strictEqual(wrapper.instance().getCommitMessage(), ''); + await assert.async.strictEqual(wrapper.instance().getCommitMessage(), templateText); + wrapper.instance().setCommitMessage('message 2'); wrapper.setProps({repository: repository1}); assert.equal(wrapper.instance().getCommitMessage(), 'message 1'); + + wrapper.setProps({repository: repository2}); + assert.equal(wrapper.instance().getCommitMessage(), 'message 2'); }); describe('the passed commit message', function() { @@ -101,21 +110,6 @@ describe('CommitController', function() { assert.strictEqual(instance.getCommitMessage(), 'new message'); assert.isFalse(repository.state.didUpdate.called); }); - - describe('when a merge message is defined', function() { - it('is set to the merge message when merging', function() { - app = React.cloneElement(app, {isMerging: true, mergeMessage: 'merge conflict!'}); - const wrapper = shallow(app); - assert.strictEqual(wrapper.find('CommitView').prop('messageBuffer').getText(), 'merge conflict!'); - }); - - it('is set to getCommitMessage() if it is set', function() { - repository.setCommitMessage('some commit message'); - app = React.cloneElement(app, {isMerging: true, mergeMessage: 'merge conflict!'}); - const wrapper = shallow(app, {disableLifecycleMethods: true}); - assert.strictEqual(wrapper.find('CommitView').prop('messageBuffer').getText(), 'some commit message'); - }); - }); }); describe('committing', function() { @@ -174,6 +168,26 @@ describe('CommitController', function() { assert.strictEqual(repository.getCommitMessage(), 'some message'); }); + it('restores template after committing', async function() { + const templatePath = path.join(workdirPath, 'a.txt'); + const templateText = fs.readFileSync(templatePath, 'utf8'); + await repository.git.setConfig('commit.template', templatePath); + await assert.async.strictEqual(repository.getCommitMessage(), templateText); + + const nonTemplateText = 'some new text...'; + repository.setCommitMessage(nonTemplateText); + + assert.strictEqual(repository.getCommitMessage(), nonTemplateText); + + app = React.cloneElement(app, {repository}); + const wrapper = shallow(app, {disableLifecycleMethods: true}); + await fs.writeFile(path.join(workdirPath, 'new-file.txt'), 'some changes', {encoding: 'utf8'}); + await repository.stageFiles(['new-file.txt']); + await wrapper.instance().commit(nonTemplateText); + + await assert.async.strictEqual(repository.getCommitMessage(), templateText); + }); + describe('message formatting', function() { let commitSpy, wrapper; diff --git a/test/git-strategies.test.js b/test/git-strategies.test.js index 635b09dafd..ccd3b6a806 100644 --- a/test/git-strategies.test.js +++ b/test/git-strategies.test.js @@ -85,6 +85,40 @@ import * as reporterProxy from '../lib/reporter-proxy'; }); }); + describe('fetchCommitMessageTemplate', function() { + it('gets commit message from template', async function() { + const workingDirPath = await cloneRepository('three-files'); + const git = createTestStrategy(workingDirPath); + const templateText = 'some commit message'; + + const commitMsgTemplatePath = path.join(workingDirPath, '.gitmessage'); + await fs.writeFile(commitMsgTemplatePath, templateText, {encoding: 'utf8'}); + + await git.setConfig('commit.template', commitMsgTemplatePath); + assert.equal(await git.fetchCommitMessageTemplate(), templateText); + }); + + it('if config is not set return null', async function() { + const workingDirPath = await cloneRepository('three-files'); + const git = createTestStrategy(workingDirPath); + + assert.isNotOk(await git.getConfig('commit.template')); // falsy value of null or '' + assert.isNull(await git.fetchCommitMessageTemplate()); + }); + + it('if config is set but file does not exist throw an error', async function() { + const workingDirPath = await cloneRepository('three-files'); + const git = createTestStrategy(workingDirPath); + + const nonExistentCommitTemplatePath = path.join(workingDirPath, 'file-that-doesnt-exist'); + await git.setConfig('commit.template', nonExistentCommitTemplatePath); + await assert.isRejected( + git.fetchCommitMessageTemplate(), + `Invalid commit template path set in Git config: ${nonExistentCommitTemplatePath}`, + ); + }); + }); + if (process.platform === 'win32') { describe('getStatusBundle()', function() { it('normalizes the path separator on Windows', async function() { diff --git a/test/helpers.js b/test/helpers.js index 4c58f0475a..4ed120522a 100644 --- a/test/helpers.js +++ b/test/helpers.js @@ -8,7 +8,7 @@ import React from 'react'; import ReactDom from 'react-dom'; import sinon from 'sinon'; import {Directory} from 'atom'; -import {Emitter} from 'event-kit'; +import {Emitter, CompositeDisposable, Disposable} from 'event-kit'; import Repository from '../lib/models/repository'; import GitShellOutStrategy from '../lib/git-shell-out-strategy'; @@ -16,6 +16,7 @@ import WorkerManager from '../lib/worker-manager'; import ContextMenuInterceptor from '../lib/context-menu-interceptor'; import getRepoPipelineManager from '../lib/get-repo-pipeline-manager'; import {clearRelayExpectations} from '../lib/relay-network-layer-manager'; +import FileSystemChangeObserver from '../lib/models/file-system-change-observer'; assert.autocrlfEqual = (actual, expected, ...args) => { const newActual = actual.replace(/\r\n/g, '\n'); @@ -382,3 +383,62 @@ export class ManualStateObserver { this.emitter.dispose(); } } + + +// File system event helpers +let observedEvents, eventCallback; + +export async function wireUpObserver(fixtureName = 'multi-commits-files', existingWorkdir = null) { + observedEvents = []; + eventCallback = () => {}; + + const workdir = existingWorkdir || await cloneRepository(fixtureName); + const repository = new Repository(workdir); + await repository.getLoadPromise(); + + const observer = new FileSystemChangeObserver(repository); + + const subscriptions = new CompositeDisposable( + new Disposable(async () => { + await observer.destroy(); + repository.destroy(); + }), + ); + + subscriptions.add(observer.onDidChange(events => { + observedEvents.push(...events); + eventCallback(); + })); + + return {repository, observer, subscriptions}; +} + +export function expectEvents(repository, ...suffixes) { + const pending = new Set(suffixes); + return new Promise((resolve, reject) => { + eventCallback = () => { + const matchingPaths = observedEvents + .filter(event => { + for (const suffix of pending) { + if (event.path.endsWith(suffix)) { + pending.delete(suffix); + return true; + } + } + return false; + }); + + if (matchingPaths.length > 0) { + repository.observeFilesystemChange(matchingPaths); + } + + if (pending.size === 0) { + resolve(); + } + }; + + if (observedEvents.length > 0) { + eventCallback(); + } + }); +} diff --git a/test/integration/file-patch.test.js b/test/integration/file-patch.test.js index 7341d3fcfb..9bfa828c05 100644 --- a/test/integration/file-patch.test.js +++ b/test/integration/file-patch.test.js @@ -275,6 +275,7 @@ describe('integration: file patches', function() { }); it('may be partially unstaged', async function() { + this.retries(5); // FLAKE getPatchEditor('staged', 'added-file.txt').setSelectedBufferRange([[3, 0], [4, 3]]); wrapper.find('.github-HunkHeaderView-stageButton').simulate('click'); @@ -299,6 +300,7 @@ describe('integration: file patches', function() { }); it('may be completely unstaged', async function() { + this.retries(5); // FLAKE getPatchEditor('staged', 'added-file.txt').selectAll(); wrapper.find('.github-HunkHeaderView-stageButton').simulate('click'); diff --git a/test/models/repository.test.js b/test/models/repository.test.js index d1ad3a84c4..39294c7857 100644 --- a/test/models/repository.test.js +++ b/test/models/repository.test.js @@ -4,18 +4,16 @@ import dedent from 'dedent-js'; import temp from 'temp'; import compareSets from 'compare-sets'; import isEqual from 'lodash.isequal'; -import {CompositeDisposable, Disposable} from 'event-kit'; import Repository from '../../lib/models/repository'; import {nullCommit} from '../../lib/models/commit'; import {nullOperationStates} from '../../lib/models/operation-states'; -import FileSystemChangeObserver from '../../lib/models/file-system-change-observer'; import Author from '../../lib/models/author'; import * as reporterProxy from '../../lib/reporter-proxy'; import { cloneRepository, setUpLocalAndRemoteRepositories, getHeadCommitOnRemote, - assertDeepPropertyVals, assertEqualSortedArraysByKey, FAKE_USER, + assertDeepPropertyVals, assertEqualSortedArraysByKey, FAKE_USER, wireUpObserver, expectEvents, } from '../helpers'; import {getPackageRoot, getTempDir} from '../../lib/helpers'; @@ -1833,73 +1831,17 @@ describe('Repository', function() { }); describe('from filesystem events', function() { - let workdir, sub; - let observedEvents, eventCallback; - - async function wireUpObserver(fixtureName = 'multi-commits-files', existingWorkdir = null) { - observedEvents = []; - eventCallback = () => {}; - - workdir = existingWorkdir || await cloneRepository(fixtureName); - const repository = new Repository(workdir); - await repository.getLoadPromise(); - - const observer = new FileSystemChangeObserver(repository); - - sub = new CompositeDisposable( - new Disposable(async () => { - await observer.destroy(); - repository.destroy(); - }), - ); - - sub.add(observer.onDidChange(events => { - observedEvents.push(...events); - eventCallback(); - })); - - return {repository, observer}; - } - - function expectEvents(repository, ...suffixes) { - const pending = new Set(suffixes); - return new Promise((resolve, reject) => { - eventCallback = () => { - const matchingPaths = observedEvents - .map(event => event.path) - .filter(eventPath => { - for (const suffix of pending) { - if (eventPath.endsWith(suffix)) { - pending.delete(suffix); - return true; - } - } - return false; - }); - - if (matchingPaths.length > 0) { - repository.observeFilesystemChange(matchingPaths); - } - - if (pending.size === 0) { - resolve(); - } - }; - - if (observedEvents.length > 0) { - eventCallback(); - } - }); - } + let sub; afterEach(function() { sub && sub.dispose(); }); it('when staging files', async function() { - const {repository, observer} = await wireUpObserver(); + const {repository, observer, subscriptions} = await wireUpObserver(); + sub = subscriptions; - await fs.writeFile(path.join(workdir, 'a.txt'), 'boop\n', {encoding: 'utf8'}); + await fs.writeFile(path.join(repository.getWorkingDirectoryPath(), 'a.txt'), 'boop\n', {encoding: 'utf8'}); await assertCorrectInvalidation({repository}, async () => { await observer.start(); @@ -1909,9 +1851,10 @@ describe('Repository', function() { }); it('when unstaging files', async function() { - const {repository, observer} = await wireUpObserver(); + const {repository, observer, subscriptions} = await wireUpObserver(); + sub = subscriptions; - await fs.writeFile(path.join(workdir, 'a.txt'), 'boop\n', {encoding: 'utf8'}); + await fs.writeFile(path.join(repository.getWorkingDirectoryPath(), 'a.txt'), 'boop\n', {encoding: 'utf8'}); await repository.git.stageFiles(['a.txt']); await assertCorrectInvalidation({repository}, async () => { @@ -1922,7 +1865,8 @@ describe('Repository', function() { }); it('when staging files from a parent commit', async function() { - const {repository, observer} = await wireUpObserver(); + const {repository, observer, subscriptions} = await wireUpObserver(); + sub = subscriptions; await assertCorrectInvalidation({repository}, async () => { await observer.start(); @@ -1932,9 +1876,10 @@ describe('Repository', function() { }); it('when applying a patch to the index', async function() { - const {repository, observer} = await wireUpObserver(); + const {repository, observer, subscriptions} = await wireUpObserver(); + sub = subscriptions; - await fs.writeFile(path.join(workdir, 'a.txt'), 'boop\n', {encoding: 'utf8'}); + await fs.writeFile(path.join(repository.getWorkingDirectoryPath(), 'a.txt'), 'boop\n', {encoding: 'utf8'}); const patch = await repository.getFilePatchForPath('a.txt'); await assertCorrectInvalidation({repository}, async () => { @@ -1948,9 +1893,10 @@ describe('Repository', function() { }); it('when applying a patch to the working directory', async function() { - const {repository, observer} = await wireUpObserver(); + const {repository, observer, subscriptions} = await wireUpObserver(); + sub = subscriptions; - await fs.writeFile(path.join(workdir, 'a.txt'), 'boop\n', {encoding: 'utf8'}); + await fs.writeFile(path.join(repository.getWorkingDirectoryPath(), 'a.txt'), 'boop\n', {encoding: 'utf8'}); const patch = (await repository.getFilePatchForPath('a.txt')).getUnstagePatchForLines(new Set([0])); await assertCorrectInvalidation({repository}, async () => { @@ -1964,9 +1910,10 @@ describe('Repository', function() { }); it('when committing', async function() { - const {repository, observer} = await wireUpObserver(); + const {repository, observer, subscriptions} = await wireUpObserver(); + sub = subscriptions; - await fs.writeFile(path.join(workdir, 'a.txt'), 'boop\n', {encoding: 'utf8'}); + await fs.writeFile(path.join(repository.getWorkingDirectoryPath(), 'a.txt'), 'boop\n', {encoding: 'utf8'}); await repository.stageFiles(['a.txt']); await assertCorrectInvalidation({repository}, async () => { @@ -1981,7 +1928,8 @@ describe('Repository', function() { }); it('when merging', async function() { - const {repository, observer} = await wireUpObserver('merge-conflict'); + const {repository, observer, subscriptions} = await wireUpObserver('merge-conflict'); + sub = subscriptions; await assertCorrectInvalidation({repository}, async () => { await observer.start(); @@ -1996,7 +1944,8 @@ describe('Repository', function() { }); it('when aborting a merge', async function() { - const {repository, observer} = await wireUpObserver('merge-conflict'); + const {repository, observer, subscriptions} = await wireUpObserver('merge-conflict'); + sub = subscriptions; await assert.isRejected(repository.merge('origin/branch')); await assertCorrectInvalidation({repository}, async () => { @@ -2012,7 +1961,8 @@ describe('Repository', function() { }); it('when checking out a revision', async function() { - const {repository, observer} = await wireUpObserver(); + const {repository, observer, subscriptions} = await wireUpObserver(); + sub = subscriptions; await assertCorrectInvalidation({repository}, async () => { await observer.start(); @@ -2028,7 +1978,8 @@ describe('Repository', function() { }); it('when checking out paths', async function() { - const {repository, observer} = await wireUpObserver(); + const {repository, observer, subscriptions} = await wireUpObserver(); + sub = subscriptions; await assertCorrectInvalidation({repository}, async () => { await observer.start(); @@ -2043,7 +1994,8 @@ describe('Repository', function() { it('when fetching', async function() { const {localRepoPath} = await setUpLocalAndRemoteRepositories({remoteAhead: true}); - const {repository, observer} = await wireUpObserver(null, localRepoPath); + const {repository, observer, subscriptions} = await wireUpObserver(null, localRepoPath); + sub = subscriptions; await repository.commit('wat', {allowEmpty: true}); await repository.commit('huh', {allowEmpty: true}); @@ -2060,7 +2012,8 @@ describe('Repository', function() { it('when pulling', async function() { const {localRepoPath} = await setUpLocalAndRemoteRepositories({remoteAhead: true}); - const {repository, observer} = await wireUpObserver(null, localRepoPath); + const {repository, observer, subscriptions} = await wireUpObserver(null, localRepoPath); + sub = subscriptions; await fs.writeFile(path.join(localRepoPath, 'file.txt'), 'one\n', {encoding: 'utf8'}); await repository.stageFiles(['file.txt']); @@ -2081,7 +2034,8 @@ describe('Repository', function() { it('when pushing', async function() { const {localRepoPath} = await setUpLocalAndRemoteRepositories(); - const {repository, observer} = await wireUpObserver(null, localRepoPath); + const {repository, observer, subscriptions} = await wireUpObserver(null, localRepoPath); + sub = subscriptions; await fs.writeFile(path.join(localRepoPath, 'new-file.txt'), 'one\n', {encoding: 'utf8'}); await repository.stageFiles(['new-file.txt']); @@ -2098,7 +2052,8 @@ describe('Repository', function() { }); it('when setting a config option', async function() { - const {repository, observer} = await wireUpObserver(); + const {repository, observer, subscriptions} = await wireUpObserver(); + sub = subscriptions; const optionNames = ['core.editor', 'color.ui']; await assertCorrectInvalidation({repository, optionNames}, async () => { @@ -2112,11 +2067,12 @@ describe('Repository', function() { }); it('when changing files in the working directory', async function() { - const {repository, observer} = await wireUpObserver(); + const {repository, observer, subscriptions} = await wireUpObserver(); + sub = subscriptions; await assertCorrectInvalidation({repository}, async () => { await observer.start(); - await fs.writeFile(path.join(workdir, 'b.txt'), 'new contents\n', {encoding: 'utf8'}); + await fs.writeFile(path.join(repository.getWorkingDirectoryPath(), 'b.txt'), 'new contents\n', {encoding: 'utf8'}); await expectEvents( repository, 'b.txt', @@ -2125,4 +2081,167 @@ describe('Repository', function() { }); }); }); + + describe('updating commit message', function() { + let sub; + + afterEach(function() { + sub && sub.dispose(); + }); + + describe('config commit.template change', function() { + it('updates commit messages to new template', async function() { + const {repository, observer, subscriptions} = await wireUpObserver(); + sub = subscriptions; + await observer.start(); + + assert.strictEqual(repository.getCommitMessage(), ''); + + const templatePath = path.join(repository.getWorkingDirectoryPath(), 'a.txt'); + await repository.git.setConfig('commit.template', templatePath); + await expectEvents( + repository, + path.join('.git', 'config'), + ); + await assert.async.strictEqual(repository.getCommitMessage(), fs.readFileSync(templatePath, 'utf8')); + }); + it('updates commit message to empty string if commit.template is unset', async function() { + const {repository, observer, subscriptions} = await wireUpObserver(); + sub = subscriptions; + await observer.start(); + + assert.strictEqual(repository.getCommitMessage(), ''); + + const templatePath = path.join(repository.getWorkingDirectoryPath(), 'a.txt'); + await repository.git.setConfig('commit.template', templatePath); + await expectEvents( + repository, + path.join('.git', 'config'), + ); + + await assert.async.strictEqual(repository.getCommitMessage(), fs.readFileSync(templatePath, 'utf8')); + + await repository.git.unsetConfig('commit.template'); + + await expectEvents( + repository, + path.join('.git', 'config'), + ); + await assert.async.strictEqual(repository.getCommitMessage(), ''); + }); + }); + + describe('merge events', function() { + describe('when commit message is empty', function() { + it('merge message is set as new commit message', async function() { + const {repository, observer, subscriptions} = await wireUpObserver('merge-conflict'); + sub = subscriptions; + await observer.start(); + + assert.strictEqual(repository.getCommitMessage(), ''); + await assert.isRejected(repository.git.merge('origin/branch')); + await expectEvents( + repository, + path.join('.git', 'MERGE_HEAD'), + ); + await assert.async.strictEqual(repository.getCommitMessage(), await repository.getMergeMessage()); + }); + }); + + describe('when commit message contains unmodified template', function() { + it('merge message is set as new commit message', async function() { + const {repository, observer, subscriptions} = await wireUpObserver('merge-conflict'); + sub = subscriptions; + await observer.start(); + + const templatePath = path.join(repository.getWorkingDirectoryPath(), 'added-to-both.txt'); + const templateText = fs.readFileSync(templatePath, 'utf8'); + await repository.git.setConfig('commit.template', templatePath); + await expectEvents( + repository, + path.join('.git', 'config'), + ); + + await assert.async.strictEqual(repository.getCommitMessage(), templateText); + + await assert.isRejected(repository.git.merge('origin/branch')); + await expectEvents( + repository, + path.join('.git', 'MERGE_HEAD'), + ); + await assert.async.strictEqual(repository.getCommitMessage(), await repository.getMergeMessage()); + }); + }); + + describe('when commit message is "dirty"', function() { + it('leaves commit message as is', async function() { + const {repository, observer, subscriptions} = await wireUpObserver('merge-conflict'); + sub = subscriptions; + await observer.start(); + + const dirtyMessage = 'foo bar baz'; + repository.setCommitMessage(dirtyMessage); + await assert.isRejected(repository.git.merge('origin/branch')); + await expectEvents( + repository, + path.join('.git', 'MERGE_HEAD'), + ); + assert.strictEqual(repository.getCommitMessage(), dirtyMessage); + }); + }); + + describe('when merge is aborted', function() { + it('merge message gets cleared', async function() { + const {repository, observer, subscriptions} = await wireUpObserver('merge-conflict'); + sub = subscriptions; + await observer.start(); + await assert.isRejected(repository.git.merge('origin/branch')); + await expectEvents( + repository, + path.join('.git', 'MERGE_HEAD'), + ); + await assert.async.strictEqual(repository.getCommitMessage(), await repository.getMergeMessage()); + + await repository.abortMerge(); + await expectEvents( + repository, + path.join('.git', 'MERGE_HEAD'), + ); + assert.strictEqual(repository.getCommitMessage(), ''); + + }); + + describe('when commit message template is present', function() { + it('sets template as commit message', async function() { + const {repository, observer, subscriptions} = await wireUpObserver('merge-conflict'); + sub = subscriptions; + await observer.start(); + + const templatePath = path.join(repository.getWorkingDirectoryPath(), 'added-to-both.txt'); + const templateText = fs.readFileSync(templatePath, 'utf8'); + await repository.git.setConfig('commit.template', templatePath); + await expectEvents( + repository, + path.join('.git', 'config'), + ); + + await assert.isRejected(repository.git.merge('origin/branch')); + await expectEvents( + repository, + path.join('.git', 'MERGE_HEAD'), + ); + await assert.async.strictEqual(repository.getCommitMessage(), await repository.getMergeMessage()); + + await repository.abortMerge(); + await expectEvents( + repository, + path.join('.git', 'MERGE_HEAD'), + ); + + await assert.async.strictEqual(repository.getCommitMessage(), templateText); + }); + }); + }); + }); + }); }); diff --git a/test/models/workdir-context.test.js b/test/models/workdir-context.test.js index c4d8df2371..a7732e9d09 100644 --- a/test/models/workdir-context.test.js +++ b/test/models/workdir-context.test.js @@ -64,9 +64,10 @@ describe('WorkdirContext', function() { sinon.spy(repo, 'observeFilesystemChange'); - context.getChangeObserver().didChange([{path: path.join('a', 'b', 'c')}]); + const events = [{path: path.join('a', 'b', 'c')}]; + context.getChangeObserver().didChange(events); - assert.isTrue(repo.observeFilesystemChange.calledWith([path.join('a', 'b', 'c')])); + assert.isTrue(repo.observeFilesystemChange.calledWith(events)); }); it('re-emits an event on workdir or head change', async function() {