Re-introduce commit message template

lib/controllers/commit-controller.js

@@ -27,7 +27,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,
@@ -73,10 +72,6 @@ export default class CommitController extends React.Component {
         }
       }),
     );
-
-    if (this.props.isMerging && !this.getCommitMessage()) {
-      this.setCommitMessage(this.props.mergeMessage || '');
-    }
   }
 
   render() {
@@ -110,13 +105,6 @@ export default class CommitController extends React.Component {
     );
   }
 
-  // eslint-disable-next-line camelcase
-  UNSAFE_componentWillReceiveProps(nextProps) {
-    if (!this.props.isMerging && nextProps.isMerging && !this.getCommitMessage()) {
-      this.setCommitMessage(nextProps.mergeMessage || '');
-    }
-  }
-
   componentWillUnmount() {
     this.subscriptions.dispose();
   }
@@ -135,10 +123,11 @@ 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; }
     const changed = this.props.repository.getCommitMessage() !== message;
-    this.props.repository.setCommitMessage(message);
+    this.props.repository.setCommitMessage(message, options);
+    // ask @smashwilson -- why do we need to do this??
     if (changed) { this.forceUpdate(); }
   }
 
@@ -154,7 +143,7 @@ export default class CommitController extends React.Component {
     if (!this.props.repository.isPresent()) {
       return;
     }
-    this.setCommitMessage(newMessage);
+    this.setCommitMessage(newMessage, {suppressUpdate: true});
   }
 
   getCommitMessageEditors() {

lib/controllers/git-tab-controller.js

@@ -275,13 +275,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() {

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.getCommitMessageTemplate();
+      repository.setCommitMessage(template || '');
       destroyFilePatchPaneItems({onlyStaged: true}, workspace);
       return result;
     } catch (error) {

lib/git-shell-out-strategy.js

@@ -415,6 +415,39 @@ export default class GitShellOutStrategy {
     return this.exec(args, {writeOperation: true});
   }
 
+  async getCommitMessageTemplate() {
+    let templatePath = await this.getConfig('commit.template');
+    if (!templatePath) {
+      return null;
+    }
+
+    /**
+     * Get absolute path from git path
+     * https://git-scm.com/docs/git-config#git-config-pathname
+     */
+    const homeDir = os.homedir();
+    // 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 regex = new RegExp('^~([^/]*)/');
+    templatePath = templatePath.trim().replace(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));

lib/models/repository-states/present.js

@@ -41,20 +41,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.getCommitMessageTemplate();
+    if (template) {
+      this.commitMessageTemplate = template;
+    }
+    if (mergeMessage) {
+      this.setCommitMessage(mergeMessage);
+    } else if (template) {
+      this.setCommitMessage(template);
+    }
   }
 
   getCommitMessage() {
     return this.commitMessage;
   }
 
+  getCommitMessageTemplate() {
+    return this.git().getCommitMessageTemplate();
+  }
+
   getOperationStates() {
     return this.operationStates;
   }
@@ -77,7 +103,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];
@@ -145,6 +172,49 @@ 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];
+      const endsWith = (...segments) => event.path.endsWith(path.join(...segments));
+
+      if (endsWith('.git', 'MERGE_HEAD')) {
+        if (event.action === 'created') {
+          if (this.isCommitMessageClean()) { // is it really necessary to check if commit message is clean?
+            this.setCommitMessage(await this.repository.getMergeMessage());
+            // this.didUpdate();
+          }
+        } else if (event.action === 'deleted') {
+          this.setCommitMessage(this.commitMessageTemplate || '');
+          // this.didUpdate();
+        }
+      }
+
+      if (endsWith('.git', 'config')) {
+        // this won't catch changes made to the template file itself...
+        const template = await this.getCommitMessageTemplate();
+        if (this.commitMessageTemplate !== template) {
+          this.setCommitMessageTemplate(template);
+          this.setCommitMessage(template);
+          // this.didUpdate();
+        }
+      }
+    }
+  }
+
+  observeFilesystemChange(events) {
+    this.invalidateCacheAfterFilesystemChange(events);
+    this.updateCommitMessageAfterFileSystemChange(events);
+  }
+
   refresh() {
     this.cache.clear();
     this.didUpdate();
@@ -281,7 +351,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 || '');
+      },
     );
   }
 

lib/models/repository-states/state.js

@@ -373,6 +373,10 @@ export default class State {
     return '';
   }
 
+  getCommitMessageTemplate() {
+    return unsupportedOperationPromise(this, 'getCommitMessageTemplate');
+  }
+
   // Cache
 
   getCache() {

lib/models/repository.js

@@ -358,6 +358,7 @@ const delegates = [
 
   'setCommitMessage',
   'getCommitMessage',
+  'getCommitMessageTemplate',
   'getCache',
 ];
 

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')));
 

lib/views/commit-view.js

@@ -468,13 +468,16 @@ export default class CommitView extends React.Component {
     }
   }
 
+  isValidMessage() {
+    return this.editor && this.editor.getText().replace(/^#.*$/gm, '').trim().length !== 0;
+  }
+
   commitIsEnabled(amend) {
-    const messageExists = this.editor && this.editor.getText().length !== 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() {
@@ -588,6 +591,11 @@ export default class CommitView extends React.Component {
 
     if (focus === CommitView.focus.EDITOR) {
       if (this.refEditor.map(focusElement).getOr(false)) {
+        if (this.editor && this.editor.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.editor.setCursorBufferPosition([0, 0]);
+        }
         return true;
       }
     }

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() {
@@ -100,21 +109,6 @@ describe('CommitController', function() {
       assert.strictEqual(wrapper.getCommitMessage(), 'new message');
       assert.isFalse(wrapper.props.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, {disableLifecycleMethods: true});
-        assert.strictEqual(wrapper.find('CommitView').prop('message'), '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('message'), 'some commit message');
-      });
-    });
   });
 
   describe('committing', function() {
@@ -173,6 +167,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;