Show files as conflicted in git panel when discard undo creates merge conflicts, handle various edge cases

lib/controllers/repository-conflict-controller.js

@@ -82,9 +82,9 @@ export default class RepositoryConflictController extends React.Component {
       return [];
     }
 
-    const basePath = this.props.workingDirectoryPath;
+    const commonBasePath = this.props.workingDirectoryPath;
     const fullMergeConflictPaths = new Set(
-      this.props.mergeConflictPaths.map(relativePath => path.join(basePath, relativePath)),
+      this.props.mergeConflictPaths.map(relativePath => path.join(commonBasePath, relativePath)),
     );
 
     return this.state.openEditors.filter(editor => fullMergeConflictPaths.has(editor.getPath()));

lib/controllers/root-controller.js

@@ -21,7 +21,7 @@ import RepositoryConflictController from './repository-conflict-controller';
 import ModelObserver from '../models/model-observer';
 import ModelStateRegistry from '../models/model-state-registry';
 import Conflict from '../models/conflicts/conflict';
-import {copyFile} from '../helpers';
+import {copyFile, deleteFileOrFolder} from '../helpers';
 import {GitError} from '../git-shell-out-strategy';
 
 const nullFilePatchState = {
@@ -421,7 +421,7 @@ export default class RootController extends React.Component {
     };
     return await this.props.repository.storeBeforeAndAfterBlobs(
       filePaths,
-      () => this.ensureNoUnsavedFiles(filePaths, 'Cannot discard lines.'),
+      () => this.ensureNoUnsavedFiles(filePaths, 'Cannot discard changes in selected files.'),
       destructiveAction,
     );
   }
@@ -509,9 +509,17 @@ export default class RootController extends React.Component {
 
   async proceedWithLastDiscardUndo(results, partialDiscardFilePath = null) {
     const promises = results.map(async result => {
-      const {filePath, resultPath} = result;
+      const {filePath, resultPath, deleted, conflict, theirsSha, commonBaseSha} = result;
       const absFilePath = path.join(this.props.repository.getWorkingDirectoryPath(), filePath);
-      await copyFile(resultPath, absFilePath);
+      if (deleted && resultPath === null) {
+        await deleteFileOrFolder(absFilePath);
+      } else {
+        await copyFile(resultPath, absFilePath);
+      }
+      if (conflict) {
+        const currentSha = await this.props.repository.createBlob({filePath});
+        await this.props.repository.writeMergeConflictToIndex(filePath, commonBaseSha, currentSha, theirsSha);
+      }
     });
     await Promise.all(promises);
     await this.props.repository.popDiscardHistory(partialDiscardFilePath);

lib/git-shell-out-strategy.js

@@ -8,7 +8,7 @@ import {parse as parseDiff} from 'what-the-diff';
 
 import GitPromptServer from './git-prompt-server';
 import AsyncQueue from './async-queue';
-import {readFile, fileExists, writeFile, fsStat, deleteFileOrFolder} from './helpers';
+import {readFile, fileExists, writeFile, isFileExecutable} from './helpers';
 import GitTimingsView from './views/git-timings-view';
 
 const LINE_ENDING_REGEX = /\r?\n/;
@@ -247,7 +247,6 @@ export default class GitShellOutStrategy {
     const args = ['diff', '--name-status', '--no-renames'];
     if (options.staged) { args.push('--staged'); }
     if (options.target) { args.push(options.target); }
-    if (options.diffFilter === 'unmerged') { args.push('--diff-filter=U'); }
     const output = await this.exec(args);
 
     const statusMap = {
@@ -288,9 +287,9 @@ export default class GitShellOutStrategy {
     if (!staged && (await this.getUntrackedFiles()).includes(filePath)) {
       // add untracked file
       const absPath = path.join(this.workingDir, filePath);
-      const stats = await fsStat(absPath);
+      const executable = await isFileExecutable(absPath);
       const contents = await readFile(absPath);
-      rawDiffs.push(buildAddedFilePatch(filePath, contents, stats));
+      rawDiffs.push(buildAddedFilePatch(filePath, contents, executable));
     }
     if (rawDiffs.length > 1) { throw new Error(`Expected 0 or 1 diffs for ${filePath} but got ${rawDiffs.length}`); }
     return rawDiffs[0];
@@ -457,26 +456,11 @@ export default class GitShellOutStrategy {
     return this.exec(args.concat(branchName), {writeOperation: true});
   }
 
-  async checkoutFiles(paths, revision) {
+  checkoutFiles(paths, revision) {
     if (paths.length === 0) { return null; }
-    try {
-      const args = ['checkout'];
-      if (revision) { args.push(revision); }
-      return await this.exec(args.concat('--', paths), {writeOperation: true});
-    } catch (error) {
-      const matches = error.stdErr.match(/error: pathspec .* did not match any file\(s\) known to git\./g);
-      if (matches.length) {
-        const filePaths = matches.map(line => {
-          return line.match(/error: pathspec '(.*)' did not match any file\(s\) known to git\./)[1];
-        });
-        return Promise.all(filePaths.map(filePath => {
-          const absPath = path.join(this.workingDir, filePath);
-          return deleteFileOrFolder(absPath);
-        }));
-      } else {
-        return Promise.resolve();
-      }
-    }
+    const args = ['checkout'];
+    if (revision) { args.push(revision); }
+    return this.exec(args.concat('--', paths), {writeOperation: true});
   }
 
   async getBranches() {
@@ -533,7 +517,11 @@ export default class GitShellOutStrategy {
       try {
         output = (await this.exec(['hash-object', '-w', filePath], {writeOperation: true})).trim();
       } catch (e) {
-        output = null;
+        if (e.stdErr.match(/fatal: Cannot open .*: No such file or directory/)) {
+          output = null;
+        } else {
+          throw e;
+        }
       }
     } else if (stdin) {
       output = (await this.exec(['hash-object', '-w', '--stdin'], {stdin, writeOperation: true})).trim();
@@ -553,9 +541,9 @@ export default class GitShellOutStrategy {
     return await this.exec(['cat-file', '-p', sha]);
   }
 
-  async mergeFile(currentPath, basePath, otherPath, resultPath) {
+  async mergeFile(oursPath, commonBasePath, theirsPath, resultPath) {
     const args = [
-      'merge-file', '-p', currentPath, basePath, otherPath,
+      'merge-file', '-p', oursPath, commonBasePath, theirsPath,
       '-L', 'current', '-L', 'after discard', '-L', 'before discard',
     ];
     let output;
@@ -571,11 +559,30 @@ export default class GitShellOutStrategy {
       }
     }
     await writeFile(resultPath, output);
-    return {filePath: currentPath, resultPath, conflict};
+    return {filePath: oursPath, resultPath, conflict};
+  }
+
+  async writeMergeConflictToIndex(filePath, commonBaseSha, oursSha, theirsSha) {
+    const fileMode = await this.getFileMode(filePath);
+    let indexInfo = `0 0000000000000000000000000000000000000000\t${filePath}\n`;
+    if (commonBaseSha) { indexInfo += `${fileMode} ${commonBaseSha} 1\t${filePath}\n`; }
+    if (oursSha) { indexInfo += `${fileMode} ${oursSha} 2\t${filePath}\n`; }
+    if (theirsSha) { indexInfo += `${fileMode} ${theirsSha} 3\t${filePath}\n`; }
+    return this.exec(['update-index', '--index-info'], {stdin: indexInfo, writeOperation: true});
+  }
+
+  async getFileMode(filePath) {
+    const output = await this.exec(['ls-files', '--stage', '--', filePath]);
+    if (output) {
+      return output.slice(0, 6);
+    } else {
+      const executable = await isFileExecutable(path.join(this.workingDir, filePath));
+      return executable ? '100755' : '100644';
+    }
   }
 }
 
-function buildAddedFilePatch(filePath, contents, stats) {
+function buildAddedFilePatch(filePath, contents, executable) {
   const hunks = [];
   if (contents) {
     const noNewLine = contents[contents.length - 1] !== '\n';
@@ -590,7 +597,6 @@ function buildAddedFilePatch(filePath, contents, stats) {
       newLineCount: noNewLine ? lines.length - 1 : lines.length,
     });
   }
-  const executable = Boolean((stats >> 6) && 1); // eslint-disable-line no-bitwise
   return {
     oldPath: null,
     newPath: filePath,

lib/helpers.js

@@ -127,6 +127,11 @@ export function fsStat(absoluteFilePath) {
   });
 }
 
+export async function isFileExecutable(absoluteFilePath) {
+  const stat = await fsStat(absoluteFilePath);
+  return stat.mode & fs.constants.S_IXUSR; // eslint-disable-line no-bitwise
+}
+
 export function shortenSha(sha) {
   return sha.slice(0, 8);
 }

lib/models/discard-history.js

@@ -1,15 +1,21 @@
 import path from 'path';
 import os from 'os';
 
+import mkdirp from 'mkdirp';
+
 import {PartialFileDiscardHistory, WholeFileDiscardHistory} from './discard-history-stores';
 
-import {getTempDir} from '../helpers';
+import {getTempDir, copyFile, fileExists, writeFile} from '../helpers';
+
+const emptyFilePath = path.join(os.tmpdir(), 'empty-file.txt');
+const emptyFilePromise = writeFile(emptyFilePath, '');
 
 export default class DiscardHistory {
-  constructor(createBlob, expandBlobToFile, mergeFile, {maxHistoryLength} = {}) {
+  constructor(createBlob, expandBlobToFile, mergeFile, workdirPath, {maxHistoryLength} = {}) {
     this.createBlob = createBlob;
     this.expandBlobToFile = expandBlobToFile;
     this.mergeFile = mergeFile;
+    this.workdirPath = workdirPath;
     this.partialFileHistory = new PartialFileDiscardHistory(maxHistoryLength);
     this.wholeFileHistory = new WholeFileDiscardHistory(maxHistoryLength);
   }
@@ -114,25 +120,46 @@ export default class DiscardHistory {
   async expandBlobsToFilesInTempFolder(snapshots) {
     const tempFolderPath = await getTempDir(path.join(os.tmpdir(), 'github-discard-history-'));
     const pathPromises = snapshots.map(async ({filePath, beforeSha, afterSha}) => {
-      const otherPath = !beforeSha ? null :
+      const dir = path.dirname(path.join(tempFolderPath, filePath));
+      await mkdirp(dir);
+      const theirsPath = !beforeSha ? null :
         await this.expandBlobToFile(path.join(tempFolderPath, `${filePath}-before-discard`), beforeSha);
-      const basePath = !afterSha ? null :
+      const commonBasePath = !afterSha ? null :
         await this.expandBlobToFile(path.join(tempFolderPath, `${filePath}-after-discard`), afterSha);
-      const resultPath = path.join(tempFolderPath, `${filePath}-merge-result`);
-      return {filePath, basePath, otherPath, resultPath};
+      const resultPath = path.join(tempFolderPath, `~${path.basename(filePath)}-merge-result`);
+      return {filePath, commonBasePath, theirsPath, resultPath, theirsSha: beforeSha, commonBaseSha: afterSha};
     });
     return await Promise.all(pathPromises);
   }
 
   async mergeFiles(filePaths) {
-    const mergeFilePromises = filePaths.map(({filePath, basePath, otherPath, resultPath}) => {
-      if (otherPath && basePath) {
-        return this.mergeFile(filePath, basePath, otherPath, resultPath);
-      } else if (!otherPath && basePath) { // deleted file
-        // TODO: handle this
-        return null;
+    const mergeFilePromises = filePaths.map(async (filePathInfo, i) => {
+      const {filePath, commonBasePath, theirsPath, resultPath, theirsSha, commonBaseSha} = filePathInfo;
+      let mergeResult;
+      if (theirsPath && commonBasePath) {
+        mergeResult = await this.mergeFile(filePath, commonBasePath, theirsPath, resultPath);
+      } else if (!theirsPath && commonBasePath) { // deleted file
+        const oursSha = await this.createBlob({filePath});
+        if (oursSha === commonBaseSha) { // no changes since discard, mark file to be deleted
+          mergeResult = {filePath, resultPath: null, deleted: true, conflict: false};
+        } else { // changes since discard result in conflict
+          await copyFile(path.join(this.workdirPath, filePath), resultPath);
+          mergeResult = {filePath, resultPath, conflict: true};
+        }
+      } else if (theirsPath && !commonBasePath) { // added file
+        const fileDoesExist = await fileExists(path.join(this.workdirPath, filePath));
+        if (!fileDoesExist) {
+          await copyFile(theirsPath, resultPath);
+          mergeResult = {filePath, resultPath, conflict: false};
+        } else {
+          await emptyFilePromise;
+          mergeResult = await this.mergeFile(filePath, emptyFilePath, theirsPath, resultPath);
+        }
+      } else {
+        throw new Error('One of the following must be defined - theirsPath:' +
+          `${theirsPath} or commonBasePath: ${commonBasePath}`);
       }
-      return null;
+      return {...mergeResult, theirsSha, commonBaseSha};
     });
     return await Promise.all(mergeFilePromises);
   }

lib/models/repository.js

@@ -8,10 +8,23 @@ import FilePatch from './file-patch';
 import Hunk from './hunk';
 import HunkLine from './hunk-line';
 import DiscardHistory from './discard-history';
-import {readFile} from '../helpers';
+import {readFile, deleteFileOrFolder} from '../helpers';
 
 const MERGE_MARKER_REGEX = /^(>|<){7} \S+$/m;
 
+function partition(array, predicate) {
+  const matches = [];
+  const nonmatches = [];
+  array.forEach(item => {
+    if (predicate(item)) {
+      matches.push(item);
+    } else {
+      nonmatches.push(item);
+    }
+  });
+  return [matches, nonmatches];
+}
+
 export class AbortMergeError extends Error {
   constructor(code, filePath) {
     super();
@@ -80,6 +93,7 @@ export default class Repository {
       this.createBlob,
       this.expandBlobToFile,
       this.mergeFile,
+      this.workingDirectoryPath,
       {maxHistoryLength: 60},
     );
     if (history) { this.discardHistory.updateHistory(history); }
@@ -438,8 +452,12 @@ export default class Repository {
   }
 
   @autobind
-  mergeFile(currentPath, basePath, otherPath, resultPath) {
-    return this.git.mergeFile(currentPath, basePath, otherPath, resultPath);
+  mergeFile(oursPath, commonBasePath, theirsPath, resultPath) {
+    return this.git.mergeFile(oursPath, commonBasePath, theirsPath, resultPath);
+  }
+
+  writeMergeConflictToIndex(filePath, commonBaseSha, oursSha, theirsSha) {
+    return this.git.writeMergeConflictToIndex(filePath, commonBaseSha, oursSha, theirsSha);
   }
 
   async createDiscardHistoryBlob() {
@@ -496,7 +514,14 @@ export default class Repository {
     return this.discardHistory.getLastSnapshots(partialDiscardFilePath);
   }
 
+  @autobind
   async discardWorkDirChangesForPaths(paths) {
-    await this.git.checkoutFiles(paths);
+    const untrackedFiles = await this.git.getUntrackedFiles();
+    const [filesToRemove, filesToCheckout] = partition(paths, f => untrackedFiles.includes(f));
+    await this.git.checkoutFiles(filesToCheckout);
+    await Promise.all(filesToRemove.map(filePath => {
+      const absPath = path.join(this.getWorkingDirectoryPath(), filePath);
+      return deleteFileOrFolder(absPath);
+    }));
   }
 }

lib/views/push-pull-menu-view.js

@@ -11,6 +11,10 @@ export default class PushPullMenuView extends React.Component {
     remoteName: React.PropTypes.string,
     aheadCount: React.PropTypes.number,
     behindCount: React.PropTypes.number,
+    notificationManager: React.PropTypes.object,
+    fetch: React.PropTypes.func,
+    push: React.PropTypes.func,
+    pull: React.PropTypes.func,
   }
 
   static defaultProps = {