Display warning notification when pull results in merge conflicts

lib/controllers/root-controller.js

@@ -178,6 +178,7 @@ export default class RootController extends React.Component {
           tooltips={this.props.tooltips}
           confirm={this.props.confirm}
           toggleGitTab={this.gitTabTracker.toggle}
+          ensureGitTabVisible={this.gitTabTracker.ensureVisible}
         />
       </StatusBar>
     );

lib/controllers/status-bar-tile-controller.js

@@ -22,22 +22,7 @@ import {autobind} from 'core-decorators';
     return yubikiri({
       currentBranch: repository.getCurrentBranch(),
       branches: repository.getBranches(),
-      changedFilesCount: repository.getStatusesForChangedFiles().then(statuses => {
-        const {stagedFiles, unstagedFiles, mergeConflictFiles} = statuses;
-        const changedFiles = new Set();
-
-        for (const filePath in unstagedFiles) {
-          changedFiles.add(filePath);
-        }
-        for (const filePath in stagedFiles) {
-          changedFiles.add(filePath);
-        }
-        for (const filePath in mergeConflictFiles) {
-          changedFiles.add(filePath);
-        }
-
-        return changedFiles.size;
-      }),
+      statusesForChangedFiles: repository.getStatusesForChangedFiles(),
       currentRemote: async query => repository.getRemoteForBranch((await query.currentBranch).getName()),
       aheadCount: async query => repository.getAheadCount((await query.currentBranch).getName()),
       behindCount: async query => repository.getBehindCount((await query.currentBranch).getName()),
@@ -61,9 +46,10 @@ export default class StatusBarTileController extends React.Component {
     currentRemote: RemotePropType.isRequired,
     aheadCount: PropTypes.number,
     behindCount: PropTypes.number,
-    changedFilesCount: PropTypes.number,
+    statusesForChangedFiles: PropTypes.object,
     originExists: PropTypes.bool,
     toggleGitTab: PropTypes.func,
+    ensureGitTabVisible: PropTypes.func,
   }
 
   static defaultProps = {
@@ -83,15 +69,39 @@ export default class StatusBarTileController extends React.Component {
     };
   }
 
+  getChangedFilesCount() {
+    const {stagedFiles, unstagedFiles, mergeConflictFiles} = this.props.statusesForChangedFiles;
+    const changedFiles = new Set();
+
+    for (const filePath in unstagedFiles) {
+      changedFiles.add(filePath);
+    }
+    for (const filePath in stagedFiles) {
+      changedFiles.add(filePath);
+    }
+    for (const filePath in mergeConflictFiles) {
+      changedFiles.add(filePath);
+    }
+
+    return changedFiles.size;
+  }
+
   render() {
+    let changedFilesCount, mergeConflictsPresent;
+    if (this.props.statusesForChangedFiles) {
+      changedFilesCount = this.getChangedFilesCount();
+      mergeConflictsPresent = Object.keys(this.props.statusesForChangedFiles.mergeConflictFiles).length > 0;
+    }
+
     const repoProps = {
       repository: this.props.repository,
       currentBranch: this.props.currentBranch,
       branches: this.props.branches,
       currentRemote: this.props.currentRemote,
       aheadCount: this.props.aheadCount,
       behindCount: this.props.behindCount,
-      changedFilesCount: this.props.changedFilesCount,
+      changedFilesCount,
+      mergeConflictsPresent,
     };
 
     return (
@@ -194,14 +204,15 @@ export default class StatusBarTileController extends React.Component {
     });
   }
 
-  async attemptGitOperation(operation, errorTransform = message => ({message})) {
+  async attemptGitOperation(operation, errorTransform = error => ({message: error.stdErr})) {
     const operationPromise = operation();
     try {
       return await operationPromise;
     } catch (error) {
       if (!(error instanceof GitError)) { throw error; }
-      const {message, description} = errorTransform(error.stdErr);
-      this.props.notificationManager.addError(
+
+      const {notificationMethod = 'addError', message, description} = errorTransform(error);
+      this.props.notificationManager[notificationMethod](
         message || 'Cannot complete remote interaction',
         {description, dismissable: true},
       );
@@ -218,16 +229,16 @@ export default class StatusBarTileController extends React.Component {
   async push({force, setUpstream}) {
     await this.attemptGitOperation(
       () => this.doPush({force, setUpstream}),
-      description => {
-        if (/rejected[\s\S]*failed to push/.test(description)) {
+      error => {
+        if (/rejected[\s\S]*failed to push/.test(error.stdErr)) {
           return {
             message: 'Push rejected',
             description: 'The tip of your current branch is behind its remote counterpart.' +
               ' Try pulling before pushing again. Or, to force push, hold `cmd` or `ctrl` while clicking.',
           };
         }
 
-        return {message: 'Unable to push', description: `<pre>${description}</pre>`};
+        return {message: 'Unable to push', description: `<pre>${error.stdErr}</pre>`};
       },
     );
   }
@@ -251,18 +262,26 @@ export default class StatusBarTileController extends React.Component {
   async pull() {
     await this.attemptGitOperation(
       () => this.doPull(),
-      description => {
-        if (/error: Your local changes to the following files would be overwritten by merge/.test(description)) {
-          const lines = description.split('\n');
+      error => {
+        if (/error: Your local changes to the following files would be overwritten by merge/.test(error.stdErr)) {
+          const lines = error.stdErr.split('\n');
           const files = lines.slice(3, lines.length - 3).map(l => `\`${l.trim()}\``).join('<br>');
           return {
             message: 'Pull aborted',
             description: 'Local changes to the following would be overwritten by merge:<br>' + files +
               '<br>Please commit your changes or stash them before you merge.',
           };
+        } else if (/Automatic merge failed; fix conflicts and then commit the result./.test(error.stdOut)) {
+          this.props.ensureGitTabVisible();
+          return {
+            notificationMethod: 'addWarning',
+            message: 'Merge conflicts',
+            description: `Your local changes conflicted with changes made on the remote branch. Resolve the conflicts
+              with the Git panel and commit to continue.`,
+          };
         }
 
-        return {message: 'Unable to pull', description: `<pre>${description}</pre>`};
+        return {message: 'Unable to pull', description: `<pre>${error.stdErr}</pre>`};
       },
     );
 
@@ -276,10 +295,10 @@ export default class StatusBarTileController extends React.Component {
   async fetch() {
     await this.attemptGitOperation(
       () => this.doFetch(),
-      description => {
+      error => {
         return {
           message: 'Unable to fetch',
-          description: `<pre>${description}</pre>`,
+          description: `<pre>${error.stdErr}</pre>`,
         };
       },
     );

lib/views/changed-files-count-view.js

@@ -1,14 +1,17 @@
 import React from 'react';
 import PropTypes from 'prop-types';
+import Octicon from './octicon';
 
 export default class ChangedFilesCountView extends React.Component {
   static propTypes = {
     changedFilesCount: PropTypes.number.isRequired,
     didClick: PropTypes.func.isRequired,
+    mergeConflictsPresent: PropTypes.bool,
   }
 
   static defaultProps = {
     changedFilesCount: 0,
+    mergeConflictsPresent: false,
     didClick: () => {},
   }
 
@@ -21,7 +24,10 @@ export default class ChangedFilesCountView extends React.Component {
       <a
         ref="changedFiles"
         className="github-ChangedFilesCount inline-block icon icon-diff"
-        onClick={this.props.didClick}>{label}</a>
+        onClick={this.props.didClick}>
+        {label}
+        {this.props.mergeConflictsPresent && <Octicon icon="alert" />}
+      </a>
     );
   }
 }

test/controllers/status-bar-tile-controller.test.js

@@ -35,6 +35,7 @@ describe('StatusBarTileController', function() {
         notificationManager={notificationManager}
         tooltips={tooltips}
         confirm={confirm}
+        ensureGitTabVisible={() => {}}
       />
     );
   });
@@ -511,6 +512,33 @@ describe('StatusBarTileController', function() {
         assert.equal(fakeConfirm.callCount, 1);
         assert.isTrue(repository.push.calledWith('master', sinon.match({force: true, setUpstream: false})));
       });
+
+      it('displays a warning notification when pull results in merge conflicts', async function() {
+        const {localRepoPath} = await setUpLocalAndRemoteRepositories('multiple-commits', {remoteAhead: true});
+        fs.writeFileSync(path.join(localRepoPath, 'file.txt'), 'apple');
+
+        const repository = await buildRepository(localRepoPath);
+        await repository.git.exec(['commit', '-am', 'Add conflicting change']);
+
+        const wrapper = mount(React.cloneElement(component, {repository}));
+        await wrapper.instance().refreshModelData();
+
+        const tip = getTooltipNode(wrapper, PushPullView);
+
+        const pullButton = tip.querySelector('button.github-PushPullMenuView-pull');
+
+        sinon.stub(notificationManager, 'addWarning');
+
+        pullButton.click();
+        await wrapper.instance().refreshModelData();
+
+        await assert.async.isTrue(notificationManager.addWarning.called);
+        const notificationArgs = notificationManager.addWarning.args[0];
+        assert.equal(notificationArgs[0], 'Merge conflicts');
+        assert.match(notificationArgs[1].description, /Your local changes conflicted with changes made on the remote branch./);
+
+        assert.isTrue(await repository.isMerging());
+      });
     });
   });