From ca3687e9b79c5365ab649264513996ba9886f450 Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Wed, 22 Feb 2017 18:57:14 -0800 Subject: [PATCH 01/30] Add test for GSOS#mergeFile --- test/git-shell-out-strategy.test.js | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/git-shell-out-strategy.test.js b/test/git-shell-out-strategy.test.js index 043c189bb9..3e791bccf5 100644 --- a/test/git-shell-out-strategy.test.js +++ b/test/git-shell-out-strategy.test.js @@ -697,4 +697,33 @@ describe('Git commands', function() { assert.equal(contents, 'foo\nbar\nbaz\n'); }); }); + + describe('merging files', () => { + describe('mergeFile(currentPath, basePath, otherPath, resultPath)', () => { + it('merges current/base/otherPaths and writes to resultPath, returning {filePath, resultPath, conflicts}', async () => { + const workingDirPath = await cloneRepository('three-files'); + const git = new GitShellOutStrategy(workingDirPath); + + // current and other paths are the same, so no conflicts + const resultsWithoutConflict = await git.mergeFile('a.txt', 'b.txt', 'a.txt', 'results-without-conflict.txt'); + assert.deepEqual(resultsWithoutConflict, { + filePath: 'a.txt', + resultPath: 'results-without-conflict.txt', + conflict: false, + }); + assert.equal(fs.readFileSync('results-without-conflict.txt', 'utf8'), fs.readFileSync(path.join(workingDirPath, 'a.txt'), 'utf8')); + + // contents of current and other paths conflict + const resultsWithConflict = await git.mergeFile('a.txt', 'b.txt', 'c.txt', 'results-with-conflict.txt'); + assert.deepEqual(resultsWithConflict, { + filePath: 'a.txt', + resultPath: 'results-with-conflict.txt', + conflict: true, + }); + const contents = fs.readFileSync('results-with-conflict.txt', 'utf8'); + assert.isTrue(contents.includes('<<<<<<<')); + assert.isTrue(contents.includes('>>>>>>>')); + }); + }); + }); }); From 0763353eda0d91796caa08d5fff7d8e75f0a011d Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Thu, 23 Feb 2017 18:41:12 -0800 Subject: [PATCH 02/30] Implement GSOS#getFileMode(fileName) --- lib/git-shell-out-strategy.js | 5 +++++ test/git-shell-out-strategy.test.js | 14 ++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/lib/git-shell-out-strategy.js b/lib/git-shell-out-strategy.js index b9c76724dd..b2dfdbf4b2 100644 --- a/lib/git-shell-out-strategy.js +++ b/lib/git-shell-out-strategy.js @@ -543,6 +543,11 @@ export default class GitShellOutStrategy { await writeFile(resultPath, output); return {filePath: currentPath, resultPath, conflict}; } + + async getFileMode(filePath) { + const output = await this.exec(['ls-files', '--stage', '--', filePath]); + return output.slice(0, 6); + } } function buildAddedFilePatch(filePath, contents, stats) { diff --git a/test/git-shell-out-strategy.test.js b/test/git-shell-out-strategy.test.js index 3e791bccf5..dec2923fcf 100644 --- a/test/git-shell-out-strategy.test.js +++ b/test/git-shell-out-strategy.test.js @@ -698,6 +698,20 @@ describe('Git commands', function() { }); }); + describe('getFileMode(filePath)', () => { + it('returns the file mode of the specified file', async () => { + const workingDirPath = await cloneRepository('three-files'); + const git = new GitShellOutStrategy(workingDirPath); + const absFilePath = path.join(workingDirPath, 'a.txt'); + fs.writeFileSync(absFilePath, 'qux\nfoo\nbar\n', 'utf8'); + + assert.equal(await git.getFileMode('a.txt'), '100644'); + + await git.exec(['update-index', '--chmod=+x', 'a.txt']); + assert.equal(await git.getFileMode('a.txt'), '100755'); + }); + }); + describe('merging files', () => { describe('mergeFile(currentPath, basePath, otherPath, resultPath)', () => { it('merges current/base/otherPaths and writes to resultPath, returning {filePath, resultPath, conflicts}', async () => { From c35db7b8d5d93660f724c20f67559bbe4e329921 Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Thu, 23 Feb 2017 18:57:11 -0800 Subject: [PATCH 03/30] Implement GSOS#updateIndex --- lib/git-shell-out-strategy.js | 12 ++++++++++++ test/git-shell-out-strategy.test.js | 23 +++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/lib/git-shell-out-strategy.js b/lib/git-shell-out-strategy.js index b2dfdbf4b2..c446ba16f9 100644 --- a/lib/git-shell-out-strategy.js +++ b/lib/git-shell-out-strategy.js @@ -5,6 +5,7 @@ import {CompositeDisposable} from 'atom'; import {GitProcess} from 'git-kitchen-sink'; import {parse as parseDiff} from 'what-the-diff'; +import dedent from 'dedent-js'; import GitPromptServer from './git-prompt-server'; import AsyncQueue from './async-queue'; @@ -544,6 +545,17 @@ export default class GitShellOutStrategy { return {filePath: currentPath, resultPath, conflict}; } + async updateIndex(filePath, commonSha, oursSha, theirsSha) { + const fileMode = await this.getFileMode(filePath); + const indexInfo = dedent` + 0 0000000000000000000000000000000000000000\t${filePath} + ${fileMode} ${commonSha} 1\t${filePath} + ${fileMode} ${oursSha} 2\t${filePath} + ${fileMode} ${theirsSha} 3\t${filePath} + `; + return this.exec(['update-index', '--index-info'], {stdin: indexInfo}); + } + async getFileMode(filePath) { const output = await this.exec(['ls-files', '--stage', '--', filePath]); return output.slice(0, 6); diff --git a/test/git-shell-out-strategy.test.js b/test/git-shell-out-strategy.test.js index dec2923fcf..af6b2163cc 100644 --- a/test/git-shell-out-strategy.test.js +++ b/test/git-shell-out-strategy.test.js @@ -2,6 +2,7 @@ import fs from 'fs-extra'; import path from 'path'; import mkdirp from 'mkdirp'; +import dedent from 'dedent-js'; import GitShellOutStrategy, {GitError} from '../lib/git-shell-out-strategy'; @@ -739,5 +740,27 @@ describe('Git commands', function() { assert.isTrue(contents.includes('>>>>>>>')); }); }); + + describe('udpateIndex(filePath, commonSha, oursSha, theirsSha)', () => { + it('updates the index to have the appropriate shas, retaining the original file mode', async () => { + const workingDirPath = await cloneRepository('three-files'); + const git = new GitShellOutStrategy(workingDirPath); + const absFilePath = path.join(workingDirPath, 'a.txt'); + fs.writeFileSync(absFilePath, 'qux\nfoo\nbar\n', 'utf8'); + await git.exec(['update-index', '--chmod=+x', 'a.txt']); + + const commonSha = '7f95a814cbd9b366c5dedb6d812536dfef2fffb7'; + const oursSha = '95d4c5b7b96b3eb0853f586576dc8b5ac54837e0'; + const theirsSha = '5da808cc8998a762ec2761f8be2338617f8f12d9'; + await git.updateIndex('a.txt', commonSha, oursSha, theirsSha); + + const index = await git.exec(['ls-files', '--stage', '--', 'a.txt']); + assert.equal(index.trim(), dedent` + 100755 ${commonSha} 1\ta.txt + 100755 ${oursSha} 2\ta.txt + 100755 ${theirsSha} 3\ta.txt + `); + }); + }); }); }); From 5daf3a5bbee888b01117261cc62eacde047931ec Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Thu, 23 Feb 2017 21:38:32 -0800 Subject: [PATCH 04/30] Handle null oursSha or theirSha in updateIndex --- lib/git-shell-out-strategy.js | 8 ++++---- test/git-shell-out-strategy.test.js | 27 +++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/lib/git-shell-out-strategy.js b/lib/git-shell-out-strategy.js index c446ba16f9..4aacc1ee3a 100644 --- a/lib/git-shell-out-strategy.js +++ b/lib/git-shell-out-strategy.js @@ -547,12 +547,12 @@ export default class GitShellOutStrategy { async updateIndex(filePath, commonSha, oursSha, theirsSha) { const fileMode = await this.getFileMode(filePath); - const indexInfo = dedent` + let indexInfo = dedent` 0 0000000000000000000000000000000000000000\t${filePath} - ${fileMode} ${commonSha} 1\t${filePath} - ${fileMode} ${oursSha} 2\t${filePath} - ${fileMode} ${theirsSha} 3\t${filePath} + ${fileMode} ${commonSha} 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}); } diff --git a/test/git-shell-out-strategy.test.js b/test/git-shell-out-strategy.test.js index af6b2163cc..09cd1f7015 100644 --- a/test/git-shell-out-strategy.test.js +++ b/test/git-shell-out-strategy.test.js @@ -761,6 +761,33 @@ describe('Git commands', function() { 100755 ${theirsSha} 3\ta.txt `); }); + + it('handles the case when oursSha or theirsSha is null', async () => { + const workingDirPath = await cloneRepository('three-files'); + const git = new GitShellOutStrategy(workingDirPath); + const absFilePath = path.join(workingDirPath, 'a.txt'); + fs.writeFileSync(absFilePath, 'qux\nfoo\nbar\n', 'utf8'); + await git.exec(['update-index', '--chmod=+x', 'a.txt']); + + const commonSha = '7f95a814cbd9b366c5dedb6d812536dfef2fffb7'; + const oursSha = '95d4c5b7b96b3eb0853f586576dc8b5ac54837e0'; + const theirsSha = '5da808cc8998a762ec2761f8be2338617f8f12d9'; + await git.updateIndex('a.txt', commonSha, null, theirsSha); + + let index = await git.exec(['ls-files', '--stage', '--', 'a.txt']); + assert.equal(index.trim(), dedent` + 100755 ${commonSha} 1\ta.txt + 100755 ${theirsSha} 3\ta.txt + `); + + await git.updateIndex('a.txt', commonSha, oursSha, null); + + index = await git.exec(['ls-files', '--stage', '--', 'a.txt']); + assert.equal(index.trim(), dedent` + 100755 ${commonSha} 1\ta.txt + 100755 ${oursSha} 2\ta.txt + `); + }); }); }); }); From 84d451bb604d437e595a3dd19b4a762d296dd067 Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Thu, 23 Feb 2017 21:44:57 -0800 Subject: [PATCH 05/30] Update index for conflicts on restoration of discarded changes --- lib/controllers/git-controller.js | 6 +++++- lib/models/discard-history.js | 7 ++++--- lib/models/repository.js | 4 ++++ test/controllers/git-controller.test.js | 4 +++- 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/lib/controllers/git-controller.js b/lib/controllers/git-controller.js index ef5d53631a..a5c650fbca 100644 --- a/lib/controllers/git-controller.js +++ b/lib/controllers/git-controller.js @@ -501,9 +501,13 @@ export default class GitController extends React.Component { async proceedWithLastDiscardUndo(results, partialDiscardFilePath = null) { const promises = results.map(async result => { - const {filePath, resultPath} = result; + const {filePath, resultPath, conflict, theirsSha, commonSha} = result; const absFilePath = path.join(this.props.repository.getWorkingDirectoryPath(), filePath); await copyFile(resultPath, absFilePath); + if (conflict) { + const currentSha = await this.props.repository.createBlob({filePath}); + await this.props.repository.updateIndex(filePath, commonSha, currentSha, theirsSha); + } }); await Promise.all(promises); await this.props.repository.popDiscardHistory(partialDiscardFilePath); diff --git a/lib/models/discard-history.js b/lib/models/discard-history.js index 546b326274..a1626a51b8 100644 --- a/lib/models/discard-history.js +++ b/lib/models/discard-history.js @@ -119,15 +119,16 @@ export default class DiscardHistory { const basePath = !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}; + return {filePath, basePath, otherPath, resultPath, theirsSha: beforeSha, commonSha: afterSha}; }); return await Promise.all(pathPromises); } async mergeFiles(filePaths) { - const mergeFilePromises = filePaths.map(({filePath, basePath, otherPath, resultPath}) => { + const mergeFilePromises = filePaths.map(async ({filePath, basePath, otherPath, resultPath, theirsSha, commonSha}) => { if (otherPath && basePath) { - return this.mergeFile(filePath, basePath, otherPath, resultPath); + const mergeResult = await this.mergeFile(filePath, basePath, otherPath, resultPath); + return Object.assign({}, mergeResult, {theirsSha, commonSha}); } else if (!otherPath && basePath) { // deleted file // TODO: handle this return null; diff --git a/lib/models/repository.js b/lib/models/repository.js index 70514c51ce..ec83fb37cb 100644 --- a/lib/models/repository.js +++ b/lib/models/repository.js @@ -432,6 +432,10 @@ export default class Repository { return this.git.mergeFile(currentPath, basePath, otherPath, resultPath); } + updateIndex(filePath, commonSha, oursSha, theirsSha) { + return this.git.updateIndex(filePath, commonSha, oursSha, theirsSha); + } + async createDiscardHistoryBlob() { return await this.discardHistory.createHistoryBlob(); } diff --git a/test/controllers/git-controller.test.js b/test/controllers/git-controller.test.js index 57519ed078..3bba8ef34d 100644 --- a/test/controllers/git-controller.test.js +++ b/test/controllers/git-controller.test.js @@ -693,7 +693,7 @@ describe('GitController', function() { }); }); - it('prompts user to continue if conflicts arise and proceeds based on user input', async () => { + it('prompts user to continue if conflicts arise and proceeds based on user input, updating index to reflect files under conflict', async () => { await repository.git.exec(['commit', '-am', 'commit files lengthy enough that changes don\'t conflict']); // add change to beginning of files @@ -758,6 +758,8 @@ describe('GitController', function() { await assert.async.isTrue(contentsAfterUndo.pathB.includes('>>>>>>>')); await assert.async.isFalse(contentsAfterUndo.pathC.includes('<<<<<<<')); await assert.async.isFalse(contentsAfterUndo.pathC.includes('>>>>>>>')); + const unmergedFiles = Object.keys(await repository.git.diffFileStatus({diffFilter: 'unmerged'})); + assert.deepEqual(unmergedFiles, ['a.txt', 'b.txt']); }); }); From bdc9de24b551aef61278f231d32d2250f8b6bbbc Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Fri, 24 Feb 2017 13:28:55 -0800 Subject: [PATCH 06/30] Use consistent ours/theirs/commonBase language --- lib/controllers/git-controller.js | 4 ++-- .../repository-conflict-controller.js | 4 ++-- lib/git-shell-out-strategy.js | 10 ++++----- lib/models/discard-history.js | 21 +++++++++--------- lib/models/repository.js | 8 +++---- test/git-strategies.test.js | 22 +++++++++---------- 6 files changed, 35 insertions(+), 34 deletions(-) diff --git a/lib/controllers/git-controller.js b/lib/controllers/git-controller.js index 1d1a90e38f..725a2e800a 100644 --- a/lib/controllers/git-controller.js +++ b/lib/controllers/git-controller.js @@ -501,12 +501,12 @@ export default class GitController extends React.Component { async proceedWithLastDiscardUndo(results, partialDiscardFilePath = null) { const promises = results.map(async result => { - const {filePath, resultPath, conflict, theirsSha, commonSha} = result; + const {filePath, resultPath, conflict, theirsSha, commonBaseSha} = result; const absFilePath = path.join(this.props.repository.getWorkingDirectoryPath(), filePath); await copyFile(resultPath, absFilePath); if (conflict) { const currentSha = await this.props.repository.createBlob({filePath}); - await this.props.repository.updateIndex(filePath, commonSha, currentSha, theirsSha); + await this.props.repository.updateIndex(filePath, commonBaseSha, currentSha, theirsSha); } }); await Promise.all(promises); diff --git a/lib/controllers/repository-conflict-controller.js b/lib/controllers/repository-conflict-controller.js index 228a653dc7..e4cb13a540 100644 --- a/lib/controllers/repository-conflict-controller.js +++ b/lib/controllers/repository-conflict-controller.js @@ -86,9 +86,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())); diff --git a/lib/git-shell-out-strategy.js b/lib/git-shell-out-strategy.js index 6c36070838..2f26fdbad8 100644 --- a/lib/git-shell-out-strategy.js +++ b/lib/git-shell-out-strategy.js @@ -529,9 +529,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; @@ -547,14 +547,14 @@ export default class GitShellOutStrategy { } } await writeFile(resultPath, output); - return {filePath: currentPath, resultPath, conflict}; + return {filePath: oursPath, resultPath, conflict}; } - async updateIndex(filePath, commonSha, oursSha, theirsSha) { + async updateIndex(filePath, commonBaseSha, oursSha, theirsSha) { const fileMode = await this.getFileMode(filePath); let indexInfo = dedent` 0 0000000000000000000000000000000000000000\t${filePath} - ${fileMode} ${commonSha} 1\t${filePath}\n + ${fileMode} ${commonBaseSha} 1\t${filePath}\n `; if (oursSha) { indexInfo += `${fileMode} ${oursSha} 2\t${filePath}\n`; } if (theirsSha) { indexInfo += `${fileMode} ${theirsSha} 3\t${filePath}\n`; } diff --git a/lib/models/discard-history.js b/lib/models/discard-history.js index a1626a51b8..5254d3fab3 100644 --- a/lib/models/discard-history.js +++ b/lib/models/discard-history.js @@ -3,13 +3,14 @@ import os from 'os'; import {PartialFileDiscardHistory, WholeFileDiscardHistory} from './discard-history-stores'; -import {getTempDir} from '../helpers'; +import {getTempDir, copyFile} from '../helpers'; 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,22 +115,22 @@ 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 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, theirsSha: beforeSha, commonSha: afterSha}; + return {filePath, commonBasePath, theirsPath, resultPath, theirsSha: beforeSha, commonBaseSha: afterSha}; }); return await Promise.all(pathPromises); } async mergeFiles(filePaths) { - const mergeFilePromises = filePaths.map(async ({filePath, basePath, otherPath, resultPath, theirsSha, commonSha}) => { - if (otherPath && basePath) { - const mergeResult = await this.mergeFile(filePath, basePath, otherPath, resultPath); - return Object.assign({}, mergeResult, {theirsSha, commonSha}); - } else if (!otherPath && basePath) { // deleted file + const mergeFilePromises = filePaths.map(async ({filePath, commonBasePath, theirsPath, resultPath, theirsSha, commonBaseSha}) => { + if (theirsPath && commonBasePath) { + const mergeResult = await this.mergeFile(filePath, commonBasePath, theirsPath, resultPath); + return Object.assign({}, mergeResult, {theirsSha, commonBaseSha}); + } else if (!theirsPath && commonBaseSha) { // deleted file // TODO: handle this return null; } diff --git a/lib/models/repository.js b/lib/models/repository.js index 25afc3e332..f254336741 100644 --- a/lib/models/repository.js +++ b/lib/models/repository.js @@ -428,12 +428,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); } - updateIndex(filePath, commonSha, oursSha, theirsSha) { - return this.git.updateIndex(filePath, commonSha, oursSha, theirsSha); + updateIndex(filePath, commonBaseSha, oursSha, theirsSha) { + return this.git.updateIndex(filePath, commonBaseSha, oursSha, theirsSha); } async createDiscardHistoryBlob() { diff --git a/test/git-strategies.test.js b/test/git-strategies.test.js index 245137e9f3..04c7209edf 100644 --- a/test/git-strategies.test.js +++ b/test/git-strategies.test.js @@ -724,8 +724,8 @@ import {cloneRepository, assertDeepPropertyVals, setUpLocalAndRemoteRepositories }); describe('merging files', () => { - describe('mergeFile(currentPath, basePath, otherPath, resultPath)', () => { - it('merges current/base/otherPaths and writes to resultPath, returning {filePath, resultPath, conflicts}', async () => { + describe('mergeFile(oursPath, commonBasePath, theirsPath, resultPath)', () => { + it('merges ours/base/theirsPaths and writes to resultPath, returning {filePath, resultPath, conflicts}', async () => { const workingDirPath = await cloneRepository('three-files'); const git = new GitShellOutStrategy(workingDirPath); @@ -751,7 +751,7 @@ import {cloneRepository, assertDeepPropertyVals, setUpLocalAndRemoteRepositories }); }); - describe('udpateIndex(filePath, commonSha, oursSha, theirsSha)', () => { + describe('udpateIndex(filePath, commonBaseSha, oursSha, theirsSha)', () => { it('updates the index to have the appropriate shas, retaining the original file mode', async () => { const workingDirPath = await cloneRepository('three-files'); const git = new GitShellOutStrategy(workingDirPath); @@ -759,14 +759,14 @@ import {cloneRepository, assertDeepPropertyVals, setUpLocalAndRemoteRepositories fs.writeFileSync(absFilePath, 'qux\nfoo\nbar\n', 'utf8'); await git.exec(['update-index', '--chmod=+x', 'a.txt']); - const commonSha = '7f95a814cbd9b366c5dedb6d812536dfef2fffb7'; + const commonBaseSha = '7f95a814cbd9b366c5dedb6d812536dfef2fffb7'; const oursSha = '95d4c5b7b96b3eb0853f586576dc8b5ac54837e0'; const theirsSha = '5da808cc8998a762ec2761f8be2338617f8f12d9'; - await git.updateIndex('a.txt', commonSha, oursSha, theirsSha); + await git.updateIndex('a.txt', commonBaseSha, oursSha, theirsSha); const index = await git.exec(['ls-files', '--stage', '--', 'a.txt']); assert.equal(index.trim(), dedent` - 100755 ${commonSha} 1\ta.txt + 100755 ${commonBaseSha} 1\ta.txt 100755 ${oursSha} 2\ta.txt 100755 ${theirsSha} 3\ta.txt `); @@ -779,22 +779,22 @@ import {cloneRepository, assertDeepPropertyVals, setUpLocalAndRemoteRepositories fs.writeFileSync(absFilePath, 'qux\nfoo\nbar\n', 'utf8'); await git.exec(['update-index', '--chmod=+x', 'a.txt']); - const commonSha = '7f95a814cbd9b366c5dedb6d812536dfef2fffb7'; + const commonBaseSha = '7f95a814cbd9b366c5dedb6d812536dfef2fffb7'; const oursSha = '95d4c5b7b96b3eb0853f586576dc8b5ac54837e0'; const theirsSha = '5da808cc8998a762ec2761f8be2338617f8f12d9'; - await git.updateIndex('a.txt', commonSha, null, theirsSha); + await git.updateIndex('a.txt', commonBaseSha, null, theirsSha); let index = await git.exec(['ls-files', '--stage', '--', 'a.txt']); assert.equal(index.trim(), dedent` - 100755 ${commonSha} 1\ta.txt + 100755 ${commonBaseSha} 1\ta.txt 100755 ${theirsSha} 3\ta.txt `); - await git.updateIndex('a.txt', commonSha, oursSha, null); + await git.updateIndex('a.txt', commonBaseSha, oursSha, null); index = await git.exec(['ls-files', '--stage', '--', 'a.txt']); assert.equal(index.trim(), dedent` - 100755 ${commonSha} 1\ta.txt + 100755 ${commonBaseSha} 1\ta.txt 100755 ${oursSha} 2\ta.txt `); }); From 61eaf736b8167a0ac989a8d19cfff51ddf08b91d Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Fri, 24 Feb 2017 14:30:10 -0800 Subject: [PATCH 07/30] Add test for GitController#discardWorkDirChangesForPaths --- lib/controllers/git-controller.js | 2 +- test/controllers/git-controller.test.js | 32 +++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/lib/controllers/git-controller.js b/lib/controllers/git-controller.js index 725a2e800a..8353a9d0d4 100644 --- a/lib/controllers/git-controller.js +++ b/lib/controllers/git-controller.js @@ -413,7 +413,7 @@ export default class GitController 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, ); } diff --git a/test/controllers/git-controller.test.js b/test/controllers/git-controller.test.js index c1ecb088cb..8ff5b1a46b 100644 --- a/test/controllers/git-controller.test.js +++ b/test/controllers/git-controller.test.js @@ -444,6 +444,38 @@ describe('GitController', function() { }); }); + describe('discardWorkDirChangesForPaths(filePaths)', () => { + it('only discards changes in files if all buffers are unmodified, otherwise notifies user', async () => { + const workdirPath = await cloneRepository('three-files'); + const repository = await buildRepository(workdirPath); + + fs.writeFileSync(path.join(workdirPath, 'a.txt'), 'do\n'); + fs.writeFileSync(path.join(workdirPath, 'b.txt'), 'ray\n'); + fs.writeFileSync(path.join(workdirPath, 'c.txt'), 'me\n'); + + const editor = await workspace.open(path.join(workdirPath, 'a.txt')); + + app = React.cloneElement(app, {repository}); + const wrapper = shallow(app); + + sinon.stub(repository, 'discardWorkDirChangesForPaths'); + sinon.stub(notificationManager, 'addError'); + // unmodified buffer + await wrapper.instance().discardWorkDirChangesForPaths(['a.txt', 'b.txt', 'c.txt']); + assert.isTrue(repository.discardWorkDirChangesForPaths.calledOnce); + assert.isFalse(notificationManager.addError.called); + + // modified buffer + repository.discardWorkDirChangesForPaths.reset(); + editor.setText('modify contents'); + await wrapper.instance().discardWorkDirChangesForPaths(['a.txt', 'b.txt', 'c.txt']); + assert.isFalse(repository.discardWorkDirChangesForPaths.called); + const notificationArgs = notificationManager.addError.args[0]; + assert.equal(notificationArgs[0], 'Cannot discard changes in selected files.'); + assert.match(notificationArgs[1].description, /You have unsaved changes in.*a\.txt/); + }); + }); + describe('undoLastDiscard(partialDiscardFilePath)', () => { describe('when partialDiscardFilePath is not null', () => { let unstagedFilePatch, repository, absFilePath, wrapper; From 1d5364a69130e67dbb45171fa7d62031e288761d Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Fri, 24 Feb 2017 14:31:47 -0800 Subject: [PATCH 08/30] Fix test description --- test/controllers/git-controller.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/controllers/git-controller.test.js b/test/controllers/git-controller.test.js index 8ff5b1a46b..7fa9cadd1b 100644 --- a/test/controllers/git-controller.test.js +++ b/test/controllers/git-controller.test.js @@ -617,7 +617,7 @@ describe('GitController', function() { }); }); - describe('when partial option is false', () => { + describe('when partialDiscardFilePath is falsey', () => { let repository, wrapper, pathA, pathB, pathC; beforeEach(async () => { const workdirPath = await cloneRepository('three-files'); From 79a6487d124b62b641aadc9f9956541d224bd4dd Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Fri, 24 Feb 2017 14:32:13 -0800 Subject: [PATCH 09/30] :fire: empty line --- test/controllers/git-controller.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/controllers/git-controller.test.js b/test/controllers/git-controller.test.js index 7fa9cadd1b..5e2ec348ac 100644 --- a/test/controllers/git-controller.test.js +++ b/test/controllers/git-controller.test.js @@ -814,6 +814,5 @@ describe('GitController', function() { }); }); }); - }); }); From b52b60278de8da39513b4b5d4e5416275bae439e Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Fri, 24 Feb 2017 14:42:03 -0800 Subject: [PATCH 10/30] Pass tooltips to GitController in tests --- test/controllers/git-controller.test.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/controllers/git-controller.test.js b/test/controllers/git-controller.test.js index 5e2ec348ac..2c6543bc26 100644 --- a/test/controllers/git-controller.test.js +++ b/test/controllers/git-controller.test.js @@ -9,7 +9,7 @@ import {cloneRepository, buildRepository} from '../helpers'; import GitController from '../../lib/controllers/git-controller'; describe('GitController', function() { - let atomEnv, workspace, commandRegistry, notificationManager, confirm, app; + let atomEnv, workspace, commandRegistry, notificationManager, confirm, tooltips, app; beforeEach(function() { atomEnv = global.buildAtomEnvironment(); @@ -17,13 +17,14 @@ describe('GitController', function() { commandRegistry = atomEnv.commands; notificationManager = atomEnv.notifications; confirm = sinon.stub(atomEnv, 'confirm'); - + tooltips = atomEnv.tooltips; app = ( ); }); From 65858eed09ea15609ad965966b436c2bd2da1261 Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Fri, 24 Feb 2017 14:50:39 -0800 Subject: [PATCH 11/30] :fire: unused option from GSOS#diffFileStatus --- lib/git-shell-out-strategy.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/git-shell-out-strategy.js b/lib/git-shell-out-strategy.js index 2f26fdbad8..cd2af5473b 100644 --- a/lib/git-shell-out-strategy.js +++ b/lib/git-shell-out-strategy.js @@ -244,7 +244,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 = { From 4cb9d0c82cbe47b0adcb610095bd07c7a1e58b96 Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Fri, 24 Feb 2017 14:55:33 -0800 Subject: [PATCH 12/30] :art: use git.exec rather than git.diffFileStatus --- test/controllers/git-controller.test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/controllers/git-controller.test.js b/test/controllers/git-controller.test.js index 2c6543bc26..278b3b0080 100644 --- a/test/controllers/git-controller.test.js +++ b/test/controllers/git-controller.test.js @@ -791,7 +791,8 @@ describe('GitController', function() { await assert.async.isTrue(contentsAfterUndo.pathB.includes('>>>>>>>')); await assert.async.isFalse(contentsAfterUndo.pathC.includes('<<<<<<<')); await assert.async.isFalse(contentsAfterUndo.pathC.includes('>>>>>>>')); - const unmergedFiles = Object.keys(await repository.git.diffFileStatus({diffFilter: 'unmerged'})); + let unmergedFiles = await repository.git.exec(['diff', '--name-status', '--diff-filter=U']); + unmergedFiles = unmergedFiles.trim().split('\n').map(line => line.split('\t')[1]); assert.deepEqual(unmergedFiles, ['a.txt', 'b.txt']); }); }); From 352b9af2b2194c5ff3d05bc0eb3867072c06ea54 Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Fri, 24 Feb 2017 15:40:44 -0800 Subject: [PATCH 13/30] :art: test to use getFileContents helper function --- test/controllers/git-controller.test.js | 76 ++++++++++++++----------- 1 file changed, 44 insertions(+), 32 deletions(-) diff --git a/test/controllers/git-controller.test.js b/test/controllers/git-controller.test.js index 278b3b0080..f9d2255603 100644 --- a/test/controllers/git-controller.test.js +++ b/test/controllers/git-controller.test.js @@ -619,11 +619,23 @@ describe('GitController', function() { }); describe('when partialDiscardFilePath is falsey', () => { - let repository, wrapper, pathA, pathB, pathC; + let repository, wrapper, pathA, pathB, pathC, getFileContents; beforeEach(async () => { const workdirPath = await cloneRepository('three-files'); repository = await buildRepository(workdirPath); + getFileContents = filePath => { + try { + return fs.readFileSync(filePath, 'utf8'); + } catch (e) { + if (e.code === 'ENOENT') { + return null; + } else { + throw e; + } + } + }; + pathA = path.join(workdirPath, 'a.txt'); pathB = path.join(workdirPath, 'b.txt'); pathC = path.join(workdirPath, 'c.txt'); @@ -635,39 +647,39 @@ describe('GitController', function() { wrapper = shallow(app); }); - it('reverses last discard', async () => { + it('reverses last discard if there are no conflicts', async () => { const contents1 = { - pathA: fs.readFileSync(pathA, 'utf8'), - pathB: fs.readFileSync(pathB, 'utf8'), - pathC: fs.readFileSync(pathC, 'utf8'), + pathA: getFileContents(pathA), + pathB: getFileContents(pathB), + pathC: getFileContents(pathC), }; await wrapper.instance().discardWorkDirChangesForPaths(['a.txt', 'b.txt']); const contents2 = { - pathA: fs.readFileSync(pathA, 'utf8'), - pathB: fs.readFileSync(pathB, 'utf8'), - pathC: fs.readFileSync(pathC, 'utf8'), + pathA: getFileContents(pathA), + pathB: getFileContents(pathB), + pathC: getFileContents(pathC), }; assert.notDeepEqual(contents1, contents2); await wrapper.instance().discardWorkDirChangesForPaths(['c.txt']); const contents3 = { - pathA: fs.readFileSync(pathA, 'utf8'), - pathB: fs.readFileSync(pathB, 'utf8'), - pathC: fs.readFileSync(pathC, 'utf8'), + pathA: getFileContents(pathA), + pathB: getFileContents(pathB), + pathC: getFileContents(pathC), }; assert.notDeepEqual(contents2, contents3); await wrapper.instance().undoLastDiscard(); await assert.async.deepEqual({ - pathA: fs.readFileSync(pathA, 'utf8'), - pathB: fs.readFileSync(pathB, 'utf8'), - pathC: fs.readFileSync(pathC, 'utf8'), + pathA: getFileContents(pathA), + pathB: getFileContents(pathB), + pathC: getFileContents(pathC), }, contents2); await wrapper.instance().undoLastDiscard(); await assert.async.deepEqual({ - pathA: fs.readFileSync(pathA, 'utf8'), - pathB: fs.readFileSync(pathB, 'utf8'), - pathC: fs.readFileSync(pathC, 'utf8'), + pathA: getFileContents(pathA), + pathB: getFileContents(pathB), + pathC: getFileContents(pathC), }, contents1); }); @@ -696,9 +708,9 @@ describe('GitController', function() { await repository.git.exec(['commit', '-am', 'commit files lengthy enough that changes don\'t conflict']); const originalContents = { - pathA: fs.readFileSync(pathA, 'utf8'), - pathB: fs.readFileSync(pathB, 'utf8'), - pathC: fs.readFileSync(pathC, 'utf8'), + pathA: getFileContents(pathA), + pathB: getFileContents(pathB), + pathC: getFileContents(pathC), }; // add change to beginning of files @@ -716,9 +728,9 @@ describe('GitController', function() { await wrapper.instance().undoLastDiscard(); await assert.async.deepEqual({ - pathA: fs.readFileSync(pathA, 'utf8'), - pathB: fs.readFileSync(pathB, 'utf8'), - pathC: fs.readFileSync(pathC, 'utf8'), + pathA: getFileContents(pathA), + pathB: getFileContents(pathB), + pathC: getFileContents(pathC), }, { pathA: 'change at beginning\n' + originalContents.pathA + 'change at end', pathB: 'change at beginning\n' + originalContents.pathB + 'change at end', @@ -742,9 +754,9 @@ describe('GitController', function() { fs.writeFileSync(pathC, fs.readFileSync(pathC, 'utf8') + 'change at end'); const contentsAfterConflictingChange = { - pathA: fs.readFileSync(pathA, 'utf8'), - pathB: fs.readFileSync(pathB, 'utf8'), - pathC: fs.readFileSync(pathC, 'utf8'), + pathA: getFileContents(pathA), + pathB: getFileContents(pathB), + pathC: getFileContents(pathC), }; // click 'Cancel' @@ -754,9 +766,9 @@ describe('GitController', function() { const confirmArg = confirm.args[0][0]; assert.match(confirmArg.message, /Undoing will result in conflicts/); await assert.async.deepEqual({ - pathA: fs.readFileSync(pathA, 'utf8'), - pathB: fs.readFileSync(pathB, 'utf8'), - pathC: fs.readFileSync(pathC, 'utf8'), + pathA: getFileContents(pathA), + pathB: getFileContents(pathB), + pathC: getFileContents(pathC), }, contentsAfterConflictingChange); // click 'Open in new editors' @@ -781,9 +793,9 @@ describe('GitController', function() { await wrapper.instance().undoLastDiscard(); assert.equal(confirm.callCount, 3); const contentsAfterUndo = { - pathA: fs.readFileSync(pathA, 'utf8'), - pathB: fs.readFileSync(pathB, 'utf8'), - pathC: fs.readFileSync(pathC, 'utf8'), + pathA: getFileContents(pathA), + pathB: getFileContents(pathB), + pathC: getFileContents(pathC), }; await assert.async.isTrue(contentsAfterUndo.pathA.includes('<<<<<<<')); await assert.async.isTrue(contentsAfterUndo.pathA.includes('>>>>>>>')); From a27ed978d23f8a443308014d8b524b7218a0f9c4 Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Fri, 24 Feb 2017 15:53:17 -0800 Subject: [PATCH 14/30] Add `~` to beginning of filename for merge results For quick identificatin of files opened in editor that are previews of conflicted files --- lib/models/discard-history.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/models/discard-history.js b/lib/models/discard-history.js index 5254d3fab3..ddee5c1d4e 100644 --- a/lib/models/discard-history.js +++ b/lib/models/discard-history.js @@ -119,7 +119,7 @@ export default class DiscardHistory { await this.expandBlobToFile(path.join(tempFolderPath, `${filePath}-before-discard`), beforeSha); const commonBasePath = !afterSha ? null : await this.expandBlobToFile(path.join(tempFolderPath, `${filePath}-after-discard`), afterSha); - const resultPath = path.join(tempFolderPath, `${filePath}-merge-result`); + const resultPath = path.join(tempFolderPath, `~${filePath}-merge-result`); return {filePath, commonBasePath, theirsPath, resultPath, theirsSha: beforeSha, commonBaseSha: afterSha}; }); return await Promise.all(pathPromises); From e9c46bf83c3bbae4abdfc5467e4fe530a142c547 Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Fri, 24 Feb 2017 17:24:00 -0800 Subject: [PATCH 15/30] More specific error catching in GSOS#createBlob --- lib/git-shell-out-strategy.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/git-shell-out-strategy.js b/lib/git-shell-out-strategy.js index cd2af5473b..83fa4e8a04 100644 --- a/lib/git-shell-out-strategy.js +++ b/lib/git-shell-out-strategy.js @@ -508,7 +508,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(); From 22458df70209de82c96fe15ea30e50ff5776ab89 Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Fri, 24 Feb 2017 17:32:37 -0800 Subject: [PATCH 16/30] Handle case of deleted files --- lib/controllers/git-controller.js | 10 ++-- lib/models/discard-history.js | 24 ++++++--- lib/models/repository.js | 1 + test/controllers/git-controller.test.js | 72 ++++++++++++++----------- 4 files changed, 65 insertions(+), 42 deletions(-) diff --git a/lib/controllers/git-controller.js b/lib/controllers/git-controller.js index 8353a9d0d4..d5822bc6f4 100644 --- a/lib/controllers/git-controller.js +++ b/lib/controllers/git-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 = { @@ -501,9 +501,13 @@ export default class GitController extends React.Component { async proceedWithLastDiscardUndo(results, partialDiscardFilePath = null) { const promises = results.map(async result => { - const {filePath, resultPath, conflict, theirsSha, commonBaseSha} = 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.updateIndex(filePath, commonBaseSha, currentSha, theirsSha); diff --git a/lib/models/discard-history.js b/lib/models/discard-history.js index ddee5c1d4e..d5e20c3d49 100644 --- a/lib/models/discard-history.js +++ b/lib/models/discard-history.js @@ -126,15 +126,25 @@ export default class DiscardHistory { } async mergeFiles(filePaths) { - const mergeFilePromises = filePaths.map(async ({filePath, commonBasePath, theirsPath, resultPath, theirsSha, commonBaseSha}) => { + const mergeFilePromises = filePaths.map(async ({filePath, commonBasePath, theirsPath, resultPath, theirsSha, commonBaseSha}, i) => { + let mergeResult; if (theirsPath && commonBasePath) { - const mergeResult = await this.mergeFile(filePath, commonBasePath, theirsPath, resultPath); - return Object.assign({}, mergeResult, {theirsSha, commonBaseSha}); - } else if (!theirsPath && commonBaseSha) { // deleted file - // TODO: handle this - return null; + 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 + // TODO + } else { + debugger; + 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); } diff --git a/lib/models/repository.js b/lib/models/repository.js index f254336741..4958be8a18 100644 --- a/lib/models/repository.js +++ b/lib/models/repository.js @@ -74,6 +74,7 @@ export default class Repository { this.createBlob, this.expandBlobToFile, this.mergeFile, + this.workingDirectoryPath, {maxHistoryLength: 60}, ); if (history) { this.discardHistory.updateHistory(history); } diff --git a/test/controllers/git-controller.test.js b/test/controllers/git-controller.test.js index f9d2255603..e53da16d05 100644 --- a/test/controllers/git-controller.test.js +++ b/test/controllers/git-controller.test.js @@ -619,9 +619,9 @@ describe('GitController', function() { }); describe('when partialDiscardFilePath is falsey', () => { - let repository, wrapper, pathA, pathB, pathC, getFileContents; + let repository, workdirPath, wrapper, pathA, pathB, pathDeleted, getFileContents; beforeEach(async () => { - const workdirPath = await cloneRepository('three-files'); + workdirPath = await cloneRepository('three-files'); repository = await buildRepository(workdirPath); getFileContents = filePath => { @@ -638,10 +638,10 @@ describe('GitController', function() { pathA = path.join(workdirPath, 'a.txt'); pathB = path.join(workdirPath, 'b.txt'); - pathC = path.join(workdirPath, 'c.txt'); + pathDeleted = path.join(workdirPath, 'c.txt'); fs.writeFileSync(pathA, [1, 2, 3, 4, 5, 6, 7, 8, 9].join('\n')); fs.writeFileSync(pathB, ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'].join('\n')); - fs.writeFileSync(pathC, ['A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J'].join('\n')); + fs.unlinkSync(pathDeleted); app = React.cloneElement(app, {repository}); wrapper = shallow(app); @@ -651,13 +651,13 @@ describe('GitController', function() { const contents1 = { pathA: getFileContents(pathA), pathB: getFileContents(pathB), - pathC: getFileContents(pathC), + pathDeleted: getFileContents(pathDeleted), }; await wrapper.instance().discardWorkDirChangesForPaths(['a.txt', 'b.txt']); const contents2 = { pathA: getFileContents(pathA), pathB: getFileContents(pathB), - pathC: getFileContents(pathC), + pathDeleted: getFileContents(pathDeleted), }; assert.notDeepEqual(contents1, contents2); @@ -665,7 +665,7 @@ describe('GitController', function() { const contents3 = { pathA: getFileContents(pathA), pathB: getFileContents(pathB), - pathC: getFileContents(pathC), + pathDeleted: getFileContents(pathDeleted), }; assert.notDeepEqual(contents2, contents3); @@ -673,13 +673,13 @@ describe('GitController', function() { await assert.async.deepEqual({ pathA: getFileContents(pathA), pathB: getFileContents(pathB), - pathC: getFileContents(pathC), + pathDeleted: getFileContents(pathDeleted), }, contents2); await wrapper.instance().undoLastDiscard(); await assert.async.deepEqual({ pathA: getFileContents(pathA), pathB: getFileContents(pathB), - pathC: getFileContents(pathC), + pathDeleted: getFileContents(pathDeleted), }, contents1); }); @@ -705,58 +705,62 @@ describe('GitController', function() { describe('when file content has changed since last discard', () => { it('successfully undoes discard if changes do not conflict', async () => { - await repository.git.exec(['commit', '-am', 'commit files lengthy enough that changes don\'t conflict']); + pathDeleted = path.join(workdirPath, 'deleted-file.txt'); + fs.writeFileSync(pathDeleted, 'this file will be deleted\n'); + await repository.git.exec(['add', '.']); + await repository.git.exec(['commit', '-m', 'commit files lengthy enough that changes don\'t conflict']); const originalContents = { pathA: getFileContents(pathA), pathB: getFileContents(pathB), - pathC: getFileContents(pathC), + pathDeleted: getFileContents(pathDeleted), }; // add change to beginning of files fs.writeFileSync(pathA, 'change at beginning\n' + fs.readFileSync(pathA, 'utf8')); fs.writeFileSync(pathB, 'change at beginning\n' + fs.readFileSync(pathB, 'utf8')); - fs.writeFileSync(pathC, 'change at beginning\n' + fs.readFileSync(pathC, 'utf8')); + fs.unlinkSync(pathDeleted); - await wrapper.instance().discardWorkDirChangesForPaths(['a.txt', 'b.txt', 'c.txt']); + await wrapper.instance().discardWorkDirChangesForPaths(['a.txt', 'b.txt']); // change file contents on disk in non-conflicting way fs.writeFileSync(pathA, fs.readFileSync(pathA, 'utf8') + 'change at end'); fs.writeFileSync(pathB, fs.readFileSync(pathB, 'utf8') + 'change at end'); - fs.writeFileSync(pathC, fs.readFileSync(pathC, 'utf8') + 'change at end'); await wrapper.instance().undoLastDiscard(); await assert.async.deepEqual({ pathA: getFileContents(pathA), pathB: getFileContents(pathB), - pathC: getFileContents(pathC), + pathDeleted: getFileContents(pathDeleted), }, { pathA: 'change at beginning\n' + originalContents.pathA + 'change at end', pathB: 'change at beginning\n' + originalContents.pathB + 'change at end', - pathC: 'change at beginning\n' + originalContents.pathC + 'change at end', + pathDeleted: null, }); }); it('prompts user to continue if conflicts arise and proceeds based on user input, updating index to reflect files under conflict', async () => { - await repository.git.exec(['commit', '-am', 'commit files lengthy enough that changes don\'t conflict']); + pathDeleted = path.join(workdirPath, 'deleted-file.txt'); + fs.writeFileSync(pathDeleted, 'this file will be deleted\n'); + await repository.git.exec(['add', '.']); + await repository.git.exec(['commit', '-m', 'commit files lengthy enough that changes don\'t conflict']); - // add change to beginning of files fs.writeFileSync(pathA, 'change at beginning\n' + fs.readFileSync(pathA, 'utf8')); fs.writeFileSync(pathB, 'change at beginning\n' + fs.readFileSync(pathB, 'utf8')); - fs.writeFileSync(pathC, 'change at beginning\n' + fs.readFileSync(pathC, 'utf8')); + fs.unlinkSync(pathDeleted); - await wrapper.instance().discardWorkDirChangesForPaths(['a.txt', 'b.txt', 'c.txt']); + await wrapper.instance().discardWorkDirChangesForPaths(['a.txt', 'b.txt', 'deleted-file.txt']); - // two files are changed in a conflicting way + // change files in a conflicting way fs.writeFileSync(pathA, 'conflicting change\n' + fs.readFileSync(pathA, 'utf8')); fs.writeFileSync(pathB, 'conflicting change\n' + fs.readFileSync(pathB, 'utf8')); - fs.writeFileSync(pathC, fs.readFileSync(pathC, 'utf8') + 'change at end'); + fs.writeFileSync(pathDeleted, 'conflicting change\n'); const contentsAfterConflictingChange = { pathA: getFileContents(pathA), pathB: getFileContents(pathB), - pathC: getFileContents(pathC), + pathDeleted: getFileContents(pathDeleted), }; // click 'Cancel' @@ -768,7 +772,7 @@ describe('GitController', function() { await assert.async.deepEqual({ pathA: getFileContents(pathA), pathB: getFileContents(pathB), - pathC: getFileContents(pathC), + pathDeleted: getFileContents(pathDeleted), }, contentsAfterConflictingChange); // click 'Open in new editors' @@ -780,13 +784,17 @@ describe('GitController', function() { const pB = b.getFileName(); if (pA < pB) { return -1; } else if (pA > pB) { return 1; } else { return 0; } }); - assert.equal(editors.length, 2); + assert.equal(editors.length, 3); assert.match(editors[0].getFileName(), /a.txt-/); assert.isTrue(editors[0].getText().includes('<<<<<<<')); assert.isTrue(editors[0].getText().includes('>>>>>>>')); assert.match(editors[1].getFileName(), /b.txt-/); assert.isTrue(editors[1].getText().includes('<<<<<<<')); assert.isTrue(editors[1].getText().includes('>>>>>>>')); + assert.match(editors[2].getFileName(), /deleted-file.txt-/); + // no merge markers since 'theirs' version is a deleted file + assert.isFalse(editors[2].getText().includes('<<<<<<<')); + assert.isFalse(editors[2].getText().includes('>>>>>>>')); // click 'Proceed and resolve conflicts' confirm.returns(0); @@ -795,25 +803,25 @@ describe('GitController', function() { const contentsAfterUndo = { pathA: getFileContents(pathA), pathB: getFileContents(pathB), - pathC: getFileContents(pathC), + pathDeleted: getFileContents(pathDeleted), }; await assert.async.isTrue(contentsAfterUndo.pathA.includes('<<<<<<<')); await assert.async.isTrue(contentsAfterUndo.pathA.includes('>>>>>>>')); await assert.async.isTrue(contentsAfterUndo.pathB.includes('<<<<<<<')); await assert.async.isTrue(contentsAfterUndo.pathB.includes('>>>>>>>')); - await assert.async.isFalse(contentsAfterUndo.pathC.includes('<<<<<<<')); - await assert.async.isFalse(contentsAfterUndo.pathC.includes('>>>>>>>')); + await assert.async.isFalse(contentsAfterUndo.pathDeleted.includes('<<<<<<<')); + await assert.async.isFalse(contentsAfterUndo.pathDeleted.includes('>>>>>>>')); let unmergedFiles = await repository.git.exec(['diff', '--name-status', '--diff-filter=U']); unmergedFiles = unmergedFiles.trim().split('\n').map(line => line.split('\t')[1]); - assert.deepEqual(unmergedFiles, ['a.txt', 'b.txt']); + assert.deepEqual(unmergedFiles, ['a.txt', 'b.txt', 'deleted-file.txt']); }); }); it('clears the discard history if the last blob is no longer valid', async () => { // this would occur in the case of garbage collection cleaning out the blob - await wrapper.instance().discardWorkDirChangesForPaths(['a.txt', 'b.txt']); - const snapshots = await wrapper.instance().discardWorkDirChangesForPaths(['c.txt']); - const {beforeSha} = snapshots['c.txt']; + await wrapper.instance().discardWorkDirChangesForPaths(['a.txt']); + const snapshots = await wrapper.instance().discardWorkDirChangesForPaths(['b.txt']); + const {beforeSha} = snapshots['b.txt']; // remove blob from git object store fs.unlinkSync(path.join(repository.getGitDirectoryPath(), 'objects', beforeSha.slice(0, 2), beforeSha.slice(2))); From 862c0a597ecf3621b441173259a549b5c812d2aa Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Sun, 26 Feb 2017 17:42:07 -0800 Subject: [PATCH 17/30] :shirt: --- lib/models/discard-history.js | 7 ++++--- lib/views/push-pull-menu-view.js | 4 ++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/models/discard-history.js b/lib/models/discard-history.js index d5e20c3d49..83f16a128c 100644 --- a/lib/models/discard-history.js +++ b/lib/models/discard-history.js @@ -126,7 +126,8 @@ export default class DiscardHistory { } async mergeFiles(filePaths) { - const mergeFilePromises = filePaths.map(async ({filePath, commonBasePath, theirsPath, resultPath, theirsSha, commonBaseSha}, i) => { + 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); @@ -141,8 +142,8 @@ export default class DiscardHistory { } else if (theirsPath && !commonBasePath) { // added file // TODO } else { - debugger; - throw new Error(`One of the following must be defined - theirsPath: ${theirsPath} or commonBasePath: ${commonBasePath}`); + throw new Error('One of the following must be defined - theirsPath:' + + `${theirsPath} or commonBasePath: ${commonBasePath}`); } return {...mergeResult, theirsSha, commonBaseSha}; }); diff --git a/lib/views/push-pull-menu-view.js b/lib/views/push-pull-menu-view.js index 4d3fc11d61..d0d5172250 100644 --- a/lib/views/push-pull-menu-view.js +++ b/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 = { From d8686e124e4cc00e6452288eda46be4a9a4c343a Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Sun, 26 Feb 2017 18:16:03 -0800 Subject: [PATCH 18/30] Add tests for undoing discard in an added file --- test/controllers/git-controller.test.js | 59 +++++++++++++++++++------ 1 file changed, 45 insertions(+), 14 deletions(-) diff --git a/test/controllers/git-controller.test.js b/test/controllers/git-controller.test.js index e53da16d05..399781f4c5 100644 --- a/test/controllers/git-controller.test.js +++ b/test/controllers/git-controller.test.js @@ -619,7 +619,7 @@ describe('GitController', function() { }); describe('when partialDiscardFilePath is falsey', () => { - let repository, workdirPath, wrapper, pathA, pathB, pathDeleted, getFileContents; + let repository, workdirPath, wrapper, pathA, pathB, pathDeleted, pathAdded, getFileContents; beforeEach(async () => { workdirPath = await cloneRepository('three-files'); repository = await buildRepository(workdirPath); @@ -639,8 +639,10 @@ describe('GitController', function() { pathA = path.join(workdirPath, 'a.txt'); pathB = path.join(workdirPath, 'b.txt'); pathDeleted = path.join(workdirPath, 'c.txt'); + pathAdded = path.join(workdirPath, 'added-file.txt'); fs.writeFileSync(pathA, [1, 2, 3, 4, 5, 6, 7, 8, 9].join('\n')); fs.writeFileSync(pathB, ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'].join('\n')); + fs.writeFileSync(pathAdded, ['!', '@', '#', '$', '%', '^', '&', '*', '(', ')'].join('\n')); fs.unlinkSync(pathDeleted); app = React.cloneElement(app, {repository}); @@ -652,20 +654,23 @@ describe('GitController', function() { pathA: getFileContents(pathA), pathB: getFileContents(pathB), pathDeleted: getFileContents(pathDeleted), + pathAdded: getFileContents(pathAdded), }; await wrapper.instance().discardWorkDirChangesForPaths(['a.txt', 'b.txt']); const contents2 = { pathA: getFileContents(pathA), pathB: getFileContents(pathB), pathDeleted: getFileContents(pathDeleted), + pathAdded: getFileContents(pathAdded), }; assert.notDeepEqual(contents1, contents2); - await wrapper.instance().discardWorkDirChangesForPaths(['c.txt']); + await wrapper.instance().discardWorkDirChangesForPaths(['c.txt', 'added-file.txt']); const contents3 = { pathA: getFileContents(pathA), pathB: getFileContents(pathB), pathDeleted: getFileContents(pathDeleted), + pathAdded: getFileContents(pathAdded), }; assert.notDeepEqual(contents2, contents3); @@ -674,21 +679,25 @@ describe('GitController', function() { pathA: getFileContents(pathA), pathB: getFileContents(pathB), pathDeleted: getFileContents(pathDeleted), + pathAdded: getFileContents(pathAdded), }, contents2); await wrapper.instance().undoLastDiscard(); await assert.async.deepEqual({ pathA: getFileContents(pathA), pathB: getFileContents(pathB), pathDeleted: getFileContents(pathDeleted), + pathAdded: getFileContents(pathAdded), }, contents1); }); it('does not undo if buffer is modified', async () => { - await wrapper.instance().discardWorkDirChangesForPaths(['a.txt', 'b.txt', 'c.txt']); + await wrapper.instance().discardWorkDirChangesForPaths(['a.txt', 'b.txt', 'c.txt', 'added-file.txt']); // modify buffers (await workspace.open(pathA)).getBuffer().append('stuff'); (await workspace.open(pathB)).getBuffer().append('other stuff'); + (await workspace.open(pathDeleted)).getBuffer().append('this stuff'); + (await workspace.open(pathAdded)).getBuffer().append('that stuff'); const expandBlobToFile = sinon.spy(repository, 'expandBlobToFile'); sinon.stub(notificationManager, 'addError'); @@ -699,7 +708,8 @@ describe('GitController', function() { assert.match(notificationArgs[1].description, /You have unsaved changes./); assert.match(notificationArgs[1].description, /a.txt/); assert.match(notificationArgs[1].description, /b.txt/); - assert.notMatch(notificationArgs[1].description, /c.txt/); + assert.match(notificationArgs[1].description, /c.txt/); + assert.match(notificationArgs[1].description, /added-file.txt/); assert.isFalse(expandBlobToFile.called); }); @@ -714,14 +724,17 @@ describe('GitController', function() { pathA: getFileContents(pathA), pathB: getFileContents(pathB), pathDeleted: getFileContents(pathDeleted), + pathAdded: getFileContents(pathAdded), }; + pathAdded = path.join(workdirPath, 'another-added-file.txt'); // add change to beginning of files fs.writeFileSync(pathA, 'change at beginning\n' + fs.readFileSync(pathA, 'utf8')); fs.writeFileSync(pathB, 'change at beginning\n' + fs.readFileSync(pathB, 'utf8')); fs.unlinkSync(pathDeleted); + fs.writeFileSync(pathAdded, 'foo\nbar\baz\n'); - await wrapper.instance().discardWorkDirChangesForPaths(['a.txt', 'b.txt']); + await wrapper.instance().discardWorkDirChangesForPaths(['a.txt', 'b.txt', 'deleted-file.txt', 'another-added-file.txt']); // change file contents on disk in non-conflicting way fs.writeFileSync(pathA, fs.readFileSync(pathA, 'utf8') + 'change at end'); @@ -733,10 +746,12 @@ describe('GitController', function() { pathA: getFileContents(pathA), pathB: getFileContents(pathB), pathDeleted: getFileContents(pathDeleted), + pathAdded: getFileContents(pathAdded), }, { pathA: 'change at beginning\n' + originalContents.pathA + 'change at end', pathB: 'change at beginning\n' + originalContents.pathB + 'change at end', pathDeleted: null, + pathAdded: originalContents.pathAdded, }); }); @@ -746,21 +761,25 @@ describe('GitController', function() { await repository.git.exec(['add', '.']); await repository.git.exec(['commit', '-m', 'commit files lengthy enough that changes don\'t conflict']); + pathAdded = path.join(workdirPath, 'another-added-file.txt'); fs.writeFileSync(pathA, 'change at beginning\n' + fs.readFileSync(pathA, 'utf8')); fs.writeFileSync(pathB, 'change at beginning\n' + fs.readFileSync(pathB, 'utf8')); fs.unlinkSync(pathDeleted); + fs.writeFileSync(pathAdded, 'foo\nbar\baz\n'); - await wrapper.instance().discardWorkDirChangesForPaths(['a.txt', 'b.txt', 'deleted-file.txt']); + await wrapper.instance().discardWorkDirChangesForPaths(['a.txt', 'b.txt', 'deleted-file.txt', 'another-added-file.txt']); // change files in a conflicting way fs.writeFileSync(pathA, 'conflicting change\n' + fs.readFileSync(pathA, 'utf8')); fs.writeFileSync(pathB, 'conflicting change\n' + fs.readFileSync(pathB, 'utf8')); fs.writeFileSync(pathDeleted, 'conflicting change\n'); + fs.writeFileSync(pathAdded, 'conflicting change\n'); const contentsAfterConflictingChange = { pathA: getFileContents(pathA), pathB: getFileContents(pathB), pathDeleted: getFileContents(pathDeleted), + pathAdded: getFileContents(pathAdded), }; // click 'Cancel' @@ -773,6 +792,7 @@ describe('GitController', function() { pathA: getFileContents(pathA), pathB: getFileContents(pathB), pathDeleted: getFileContents(pathDeleted), + pathAdded: getFileContents(pathAdded), }, contentsAfterConflictingChange); // click 'Open in new editors' @@ -785,16 +805,24 @@ describe('GitController', function() { if (pA < pB) { return -1; } else if (pA > pB) { return 1; } else { return 0; } }); assert.equal(editors.length, 3); + assert.match(editors[0].getFileName(), /a.txt-/); assert.isTrue(editors[0].getText().includes('<<<<<<<')); assert.isTrue(editors[0].getText().includes('>>>>>>>')); - assert.match(editors[1].getFileName(), /b.txt-/); - assert.isTrue(editors[1].getText().includes('<<<<<<<')); - assert.isTrue(editors[1].getText().includes('>>>>>>>')); - assert.match(editors[2].getFileName(), /deleted-file.txt-/); + + assert.match(editors[1].getFileName(), /another-added-file.txt-/); + // no merge markers since 'ours' version is a deleted file + assert.isFalse(editors[1].getText().includes('>>>>>>>')); + assert.isFalse(editors[1].getText().includes('>>>>>>>')); + + assert.match(editors[2].getFileName(), /b.txt-/); + assert.isTrue(editors[2].getText().includes('<<<<<<<')); + assert.isTrue(editors[2].getText().includes('>>>>>>>')); + + assert.match(editors[3].getFileName(), /deleted-file.txt-/); // no merge markers since 'theirs' version is a deleted file - assert.isFalse(editors[2].getText().includes('<<<<<<<')); - assert.isFalse(editors[2].getText().includes('>>>>>>>')); + assert.isFalse(editors[3].getText().includes('<<<<<<<')); + assert.isFalse(editors[3].getText().includes('>>>>>>>')); // click 'Proceed and resolve conflicts' confirm.returns(0); @@ -804,6 +832,7 @@ describe('GitController', function() { pathA: getFileContents(pathA), pathB: getFileContents(pathB), pathDeleted: getFileContents(pathDeleted), + pathAdded: getFileContents(pathAdded), }; await assert.async.isTrue(contentsAfterUndo.pathA.includes('<<<<<<<')); await assert.async.isTrue(contentsAfterUndo.pathA.includes('>>>>>>>')); @@ -811,9 +840,11 @@ describe('GitController', function() { await assert.async.isTrue(contentsAfterUndo.pathB.includes('>>>>>>>')); await assert.async.isFalse(contentsAfterUndo.pathDeleted.includes('<<<<<<<')); await assert.async.isFalse(contentsAfterUndo.pathDeleted.includes('>>>>>>>')); + await assert.async.isFalse(contentsAfterUndo.pathAdded.includes('<<<<<<<')); + await assert.async.isFalse(contentsAfterUndo.pathAdded.includes('>>>>>>>')); let unmergedFiles = await repository.git.exec(['diff', '--name-status', '--diff-filter=U']); - unmergedFiles = unmergedFiles.trim().split('\n').map(line => line.split('\t')[1]); - assert.deepEqual(unmergedFiles, ['a.txt', 'b.txt', 'deleted-file.txt']); + unmergedFiles = unmergedFiles.trim().split('\n').map(line => line.split('\t')[1]).sort(); + assert.deepEqual(unmergedFiles, ['a.txt', 'another-added-file.txt', 'b.txt', 'deleted-file.txt']); }); }); From 8ee9dd2776ba6aee6eeb25782ee604a208de8544 Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Thu, 2 Mar 2017 20:17:12 -0800 Subject: [PATCH 19/30] Make GSOS#checkoutFiles no longer delete untracked files This responsibility is a better fit for Repository#discardWorkDirChangesForPaths --- lib/git-shell-out-strategy.js | 23 ++++------------------- lib/models/repository.js | 24 ++++++++++++++++++++++-- test/models/repository.test.js | 3 ++- 3 files changed, 28 insertions(+), 22 deletions(-) diff --git a/lib/git-shell-out-strategy.js b/lib/git-shell-out-strategy.js index 83fa4e8a04..a804a72eda 100644 --- a/lib/git-shell-out-strategy.js +++ b/lib/git-shell-out-strategy.js @@ -431,26 +431,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() { diff --git a/lib/models/repository.js b/lib/models/repository.js index 4958be8a18..3f6e01a584 100644 --- a/lib/models/repository.js +++ b/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(); @@ -491,7 +504,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); + })); } } diff --git a/test/models/repository.test.js b/test/models/repository.test.js index fb8dd50093..cca1da0b1f 100644 --- a/test/models/repository.test.js +++ b/test/models/repository.test.js @@ -662,9 +662,10 @@ describe('Repository', function() { const repo = await buildRepository(workingDirPath); fs.writeFileSync(path.join(workingDirPath, 'subdir-1', 'a.txt'), 'qux\nfoo\nbar\n', 'utf8'); fs.writeFileSync(path.join(workingDirPath, 'subdir-1', 'b.txt'), 'qux\nfoo\nbar\n', 'utf8'); + fs.writeFileSync(path.join(workingDirPath, 'new-file.txt'), 'hello there', 'utf8'); const unstagedChanges = await repo.getUnstagedChanges(); - assert.equal(unstagedChanges.length, 2); + assert.equal(unstagedChanges.length, 3); await repo.discardWorkDirChangesForPaths(unstagedChanges.map(c => c.filePath)); repo.refresh(); assert.deepEqual(await repo.getUnstagedChanges(), []); From 79f901d0b6d5eec366e8322ed822ade17289ea74 Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Thu, 2 Mar 2017 21:51:12 -0800 Subject: [PATCH 20/30] Add isFileExecutable helper --- lib/helpers.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/helpers.js b/lib/helpers.js index fafcde1671..1e8e8d9471 100644 --- a/lib/helpers.js +++ b/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); } From b950c3838b4ee6fff54fc9271dd743ce065c9353 Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Thu, 2 Mar 2017 21:52:35 -0800 Subject: [PATCH 21/30] :art: GSOS#getDiffForFilePath to use isFileExecutable helper --- lib/git-shell-out-strategy.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/git-shell-out-strategy.js b/lib/git-shell-out-strategy.js index a804a72eda..1228a0f7cf 100644 --- a/lib/git-shell-out-strategy.js +++ b/lib/git-shell-out-strategy.js @@ -9,7 +9,7 @@ import dedent from 'dedent-js'; import GitPromptServer from './git-prompt-server'; import AsyncQueue from './async-queue'; -import {readFile, fileExists, writeFile, fsStat, deleteFileOrFolder} from './helpers'; +import {readFile, fileExists, writeFile, deleteFileOrFolder, isFileExecutable} from './helpers'; import GitTimingsView from './views/git-timings-view'; const LINE_ENDING_REGEX = /\r?\n/; @@ -284,9 +284,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]; @@ -555,7 +555,7 @@ export default class GitShellOutStrategy { } } -function buildAddedFilePatch(filePath, contents, stats) { +function buildAddedFilePatch(filePath, contents, executable) { const hunks = []; if (contents) { const noNewLine = contents[contents.length - 1] !== '\n'; @@ -570,7 +570,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, From b247137ceccece13ae649708c0ba82368b1fba08 Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Thu, 2 Mar 2017 22:06:28 -0800 Subject: [PATCH 22/30] Make GSOS#getFileMode handle untracked files --- lib/git-shell-out-strategy.js | 7 ++++++- test/git-strategies.test.js | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/lib/git-shell-out-strategy.js b/lib/git-shell-out-strategy.js index 1228a0f7cf..2e71457f0a 100644 --- a/lib/git-shell-out-strategy.js +++ b/lib/git-shell-out-strategy.js @@ -551,7 +551,12 @@ export default class GitShellOutStrategy { async getFileMode(filePath) { const output = await this.exec(['ls-files', '--stage', '--', filePath]); - return output.slice(0, 6); + if (output) { + return output.slice(0, 6); + } else { + const executable = await isFileExecutable(path.join(this.workingDir, filePath)); + return executable ? '100755' : '100644'; + } } } diff --git a/test/git-strategies.test.js b/test/git-strategies.test.js index 04c7209edf..32a1626552 100644 --- a/test/git-strategies.test.js +++ b/test/git-strategies.test.js @@ -8,6 +8,7 @@ import CompositeGitStrategy from '../lib/composite-git-strategy'; import GitShellOutStrategy, {GitError} from '../lib/git-shell-out-strategy'; import {cloneRepository, assertDeepPropertyVals, setUpLocalAndRemoteRepositories} from './helpers'; +import {fsStat} from '../lib/helpers'; /** * KU Thoughts: The GitShellOutStrategy methods are tested in Repository tests for the most part @@ -721,6 +722,20 @@ import {cloneRepository, assertDeepPropertyVals, setUpLocalAndRemoteRepositories await git.exec(['update-index', '--chmod=+x', 'a.txt']); assert.equal(await git.getFileMode('a.txt'), '100755'); }); + + it('returns the file mode for untracked files', async () => { + const workingDirPath = await cloneRepository('three-files'); + const git = new GitShellOutStrategy(workingDirPath); + const absFilePath = path.join(workingDirPath, 'new-file.txt'); + fs.writeFileSync(absFilePath, 'qux\nfoo\nbar\n', 'utf8'); + const regularMode = await fsStat(absFilePath).mode; + const executableMode = regularMode | fs.constants.S_IXUSR; + + assert.equal(await git.getFileMode('new-file.txt'), '100644'); + + fs.chmodSync(absFilePath, executableMode); + assert.equal(await git.getFileMode('new-file.txt'), '100755'); + }); }); describe('merging files', () => { From cd91dcd3119a125494cf401d621ab48b1054e3fd Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Thu, 2 Mar 2017 22:09:30 -0800 Subject: [PATCH 23/30] Make GSOS#updateIndex handle the case when commonBaseSha is null --- lib/git-shell-out-strategy.js | 6 ++---- test/git-strategies.test.js | 10 +++++++++- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/lib/git-shell-out-strategy.js b/lib/git-shell-out-strategy.js index 2e71457f0a..8c56581882 100644 --- a/lib/git-shell-out-strategy.js +++ b/lib/git-shell-out-strategy.js @@ -540,10 +540,8 @@ export default class GitShellOutStrategy { async updateIndex(filePath, commonBaseSha, oursSha, theirsSha) { const fileMode = await this.getFileMode(filePath); - let indexInfo = dedent` - 0 0000000000000000000000000000000000000000\t${filePath} - ${fileMode} ${commonBaseSha} 1\t${filePath}\n - `; + 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}); diff --git a/test/git-strategies.test.js b/test/git-strategies.test.js index 32a1626552..854fab82c5 100644 --- a/test/git-strategies.test.js +++ b/test/git-strategies.test.js @@ -787,7 +787,7 @@ import {fsStat} from '../lib/helpers'; `); }); - it('handles the case when oursSha or theirsSha is null', async () => { + it('handles the case when oursSha, commonBaseSha, or theirsSha is null', async () => { const workingDirPath = await cloneRepository('three-files'); const git = new GitShellOutStrategy(workingDirPath); const absFilePath = path.join(workingDirPath, 'a.txt'); @@ -812,6 +812,14 @@ import {fsStat} from '../lib/helpers'; 100755 ${commonBaseSha} 1\ta.txt 100755 ${oursSha} 2\ta.txt `); + + await git.updateIndex('a.txt', null, oursSha, theirsSha); + + index = await git.exec(['ls-files', '--stage', '--', 'a.txt']); + assert.equal(index.trim(), dedent` + 100755 ${oursSha} 2\ta.txt + 100755 ${theirsSha} 3\ta.txt + `); }); }); }); From 0440a0c1878218f2ff523db9da9b04098edccdc7 Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Thu, 2 Mar 2017 22:10:14 -0800 Subject: [PATCH 24/30] Make DiscardHistory#mergeFiles handle case of added files --- lib/models/discard-history.js | 14 ++++++++-- test/controllers/git-controller.test.js | 35 +++++++++++++------------ 2 files changed, 30 insertions(+), 19 deletions(-) diff --git a/lib/models/discard-history.js b/lib/models/discard-history.js index 83f16a128c..888a96f4f6 100644 --- a/lib/models/discard-history.js +++ b/lib/models/discard-history.js @@ -3,7 +3,10 @@ import os from 'os'; import {PartialFileDiscardHistory, WholeFileDiscardHistory} from './discard-history-stores'; -import {getTempDir, copyFile} 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, workdirPath, {maxHistoryLength} = {}) { @@ -140,7 +143,14 @@ export default class DiscardHistory { mergeResult = {filePath, resultPath, conflict: true}; } } else if (theirsPath && !commonBasePath) { // added file - // TODO + 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}`); diff --git a/test/controllers/git-controller.test.js b/test/controllers/git-controller.test.js index 399781f4c5..5ceadd8d71 100644 --- a/test/controllers/git-controller.test.js +++ b/test/controllers/git-controller.test.js @@ -720,20 +720,21 @@ describe('GitController', function() { await repository.git.exec(['add', '.']); await repository.git.exec(['commit', '-m', 'commit files lengthy enough that changes don\'t conflict']); - const originalContents = { - pathA: getFileContents(pathA), - pathB: getFileContents(pathB), - pathDeleted: getFileContents(pathDeleted), - pathAdded: getFileContents(pathAdded), - }; - pathAdded = path.join(workdirPath, 'another-added-file.txt'); - // add change to beginning of files + + // change files fs.writeFileSync(pathA, 'change at beginning\n' + fs.readFileSync(pathA, 'utf8')); fs.writeFileSync(pathB, 'change at beginning\n' + fs.readFileSync(pathB, 'utf8')); fs.unlinkSync(pathDeleted); fs.writeFileSync(pathAdded, 'foo\nbar\baz\n'); + const contentsBeforeDiscard = { + pathA: getFileContents(pathA), + pathB: getFileContents(pathB), + pathDeleted: getFileContents(pathDeleted), + pathAdded: getFileContents(pathAdded), + }; + await wrapper.instance().discardWorkDirChangesForPaths(['a.txt', 'b.txt', 'deleted-file.txt', 'another-added-file.txt']); // change file contents on disk in non-conflicting way @@ -748,10 +749,10 @@ describe('GitController', function() { pathDeleted: getFileContents(pathDeleted), pathAdded: getFileContents(pathAdded), }, { - pathA: 'change at beginning\n' + originalContents.pathA + 'change at end', - pathB: 'change at beginning\n' + originalContents.pathB + 'change at end', - pathDeleted: null, - pathAdded: originalContents.pathAdded, + pathA: contentsBeforeDiscard.pathA + 'change at end', + pathB: contentsBeforeDiscard.pathB + 'change at end', + pathDeleted: contentsBeforeDiscard.pathDeleted, + pathAdded: contentsBeforeDiscard.pathAdded, }); }); @@ -804,7 +805,7 @@ describe('GitController', function() { const pB = b.getFileName(); if (pA < pB) { return -1; } else if (pA > pB) { return 1; } else { return 0; } }); - assert.equal(editors.length, 3); + assert.equal(editors.length, 4); assert.match(editors[0].getFileName(), /a.txt-/); assert.isTrue(editors[0].getText().includes('<<<<<<<')); @@ -812,8 +813,8 @@ describe('GitController', function() { assert.match(editors[1].getFileName(), /another-added-file.txt-/); // no merge markers since 'ours' version is a deleted file - assert.isFalse(editors[1].getText().includes('>>>>>>>')); - assert.isFalse(editors[1].getText().includes('>>>>>>>')); + assert.isTrue(editors[1].getText().includes('<<<<<<<')); + assert.isTrue(editors[1].getText().includes('>>>>>>>')); assert.match(editors[2].getFileName(), /b.txt-/); assert.isTrue(editors[2].getText().includes('<<<<<<<')); @@ -840,8 +841,8 @@ describe('GitController', function() { await assert.async.isTrue(contentsAfterUndo.pathB.includes('>>>>>>>')); await assert.async.isFalse(contentsAfterUndo.pathDeleted.includes('<<<<<<<')); await assert.async.isFalse(contentsAfterUndo.pathDeleted.includes('>>>>>>>')); - await assert.async.isFalse(contentsAfterUndo.pathAdded.includes('<<<<<<<')); - await assert.async.isFalse(contentsAfterUndo.pathAdded.includes('>>>>>>>')); + await assert.async.isTrue(contentsAfterUndo.pathAdded.includes('<<<<<<<')); + await assert.async.isTrue(contentsAfterUndo.pathAdded.includes('>>>>>>>')); let unmergedFiles = await repository.git.exec(['diff', '--name-status', '--diff-filter=U']); unmergedFiles = unmergedFiles.trim().split('\n').map(line => line.split('\t')[1]).sort(); assert.deepEqual(unmergedFiles, ['a.txt', 'another-added-file.txt', 'b.txt', 'deleted-file.txt']); From d29cd8afce218abf03f85d3c3511c07904f43602 Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Thu, 2 Mar 2017 22:17:31 -0800 Subject: [PATCH 25/30] :shirt: --- lib/git-shell-out-strategy.js | 3 +-- test/git-strategies.test.js | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/git-shell-out-strategy.js b/lib/git-shell-out-strategy.js index 239e84a523..3699fe80b5 100644 --- a/lib/git-shell-out-strategy.js +++ b/lib/git-shell-out-strategy.js @@ -5,11 +5,10 @@ import {CompositeDisposable} from 'atom'; import {GitProcess} from 'git-kitchen-sink'; import {parse as parseDiff} from 'what-the-diff'; -import dedent from 'dedent-js'; import GitPromptServer from './git-prompt-server'; import AsyncQueue from './async-queue'; -import {readFile, fileExists, writeFile, deleteFileOrFolder, isFileExecutable} from './helpers'; +import {readFile, fileExists, writeFile, isFileExecutable} from './helpers'; import GitTimingsView from './views/git-timings-view'; const LINE_ENDING_REGEX = /\r?\n/; diff --git a/test/git-strategies.test.js b/test/git-strategies.test.js index fa9135b9df..e96981c614 100644 --- a/test/git-strategies.test.js +++ b/test/git-strategies.test.js @@ -764,7 +764,7 @@ import {fsStat} from '../lib/helpers'; const absFilePath = path.join(workingDirPath, 'new-file.txt'); fs.writeFileSync(absFilePath, 'qux\nfoo\nbar\n', 'utf8'); const regularMode = await fsStat(absFilePath).mode; - const executableMode = regularMode | fs.constants.S_IXUSR; + const executableMode = regularMode | fs.constants.S_IXUSR; // eslint-disable-line no-bitwise assert.equal(await git.getFileMode('new-file.txt'), '100644'); From 73941ff2b178f6e071a198c3c0da9d96fa768ff3 Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Thu, 2 Mar 2017 22:19:26 -0800 Subject: [PATCH 26/30] Make GSOS#updateIndex a write operation --- lib/git-shell-out-strategy.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/git-shell-out-strategy.js b/lib/git-shell-out-strategy.js index 3699fe80b5..17e733ab3c 100644 --- a/lib/git-shell-out-strategy.js +++ b/lib/git-shell-out-strategy.js @@ -568,7 +568,7 @@ export default class GitShellOutStrategy { 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}); + return this.exec(['update-index', '--index-info'], {stdin: indexInfo, writeOperation: true}); } async getFileMode(filePath) { From 65c38b567c507c858703a067d9ad42339cb45830 Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Thu, 2 Mar 2017 22:20:57 -0800 Subject: [PATCH 27/30] Rename `updateIndex` -> `writeMergeConflictToIndex` --- lib/controllers/root-controller.js | 2 +- lib/git-shell-out-strategy.js | 2 +- lib/models/repository.js | 4 ++-- test/git-strategies.test.js | 8 ++++---- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/controllers/root-controller.js b/lib/controllers/root-controller.js index 81fb5afe99..c8d9af8cee 100644 --- a/lib/controllers/root-controller.js +++ b/lib/controllers/root-controller.js @@ -518,7 +518,7 @@ export default class RootController extends React.Component { } if (conflict) { const currentSha = await this.props.repository.createBlob({filePath}); - await this.props.repository.updateIndex(filePath, commonBaseSha, currentSha, theirsSha); + await this.props.repository.writeMergeConflictToIndex(filePath, commonBaseSha, currentSha, theirsSha); } }); await Promise.all(promises); diff --git a/lib/git-shell-out-strategy.js b/lib/git-shell-out-strategy.js index 17e733ab3c..b18495ee4c 100644 --- a/lib/git-shell-out-strategy.js +++ b/lib/git-shell-out-strategy.js @@ -562,7 +562,7 @@ export default class GitShellOutStrategy { return {filePath: oursPath, resultPath, conflict}; } - async updateIndex(filePath, commonBaseSha, oursSha, theirsSha) { + 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`; } diff --git a/lib/models/repository.js b/lib/models/repository.js index d3479c6c85..6763656189 100644 --- a/lib/models/repository.js +++ b/lib/models/repository.js @@ -456,8 +456,8 @@ export default class Repository { return this.git.mergeFile(oursPath, commonBasePath, theirsPath, resultPath); } - updateIndex(filePath, commonBaseSha, oursSha, theirsSha) { - return this.git.updateIndex(filePath, commonBaseSha, oursSha, theirsSha); + writeMergeConflictToIndex(filePath, commonBaseSha, oursSha, theirsSha) { + return this.git.writeMergeConflictToIndex(filePath, commonBaseSha, oursSha, theirsSha); } async createDiscardHistoryBlob() { diff --git a/test/git-strategies.test.js b/test/git-strategies.test.js index e96981c614..669d805b51 100644 --- a/test/git-strategies.test.js +++ b/test/git-strategies.test.js @@ -812,7 +812,7 @@ import {fsStat} from '../lib/helpers'; const commonBaseSha = '7f95a814cbd9b366c5dedb6d812536dfef2fffb7'; const oursSha = '95d4c5b7b96b3eb0853f586576dc8b5ac54837e0'; const theirsSha = '5da808cc8998a762ec2761f8be2338617f8f12d9'; - await git.updateIndex('a.txt', commonBaseSha, oursSha, theirsSha); + await git.writeMergeConflictToIndex('a.txt', commonBaseSha, oursSha, theirsSha); const index = await git.exec(['ls-files', '--stage', '--', 'a.txt']); assert.equal(index.trim(), dedent` @@ -832,7 +832,7 @@ import {fsStat} from '../lib/helpers'; const commonBaseSha = '7f95a814cbd9b366c5dedb6d812536dfef2fffb7'; const oursSha = '95d4c5b7b96b3eb0853f586576dc8b5ac54837e0'; const theirsSha = '5da808cc8998a762ec2761f8be2338617f8f12d9'; - await git.updateIndex('a.txt', commonBaseSha, null, theirsSha); + await git.writeMergeConflictToIndex('a.txt', commonBaseSha, null, theirsSha); let index = await git.exec(['ls-files', '--stage', '--', 'a.txt']); assert.equal(index.trim(), dedent` @@ -840,7 +840,7 @@ import {fsStat} from '../lib/helpers'; 100755 ${theirsSha} 3\ta.txt `); - await git.updateIndex('a.txt', commonBaseSha, oursSha, null); + await git.writeMergeConflictToIndex('a.txt', commonBaseSha, oursSha, null); index = await git.exec(['ls-files', '--stage', '--', 'a.txt']); assert.equal(index.trim(), dedent` @@ -848,7 +848,7 @@ import {fsStat} from '../lib/helpers'; 100755 ${oursSha} 2\ta.txt `); - await git.updateIndex('a.txt', null, oursSha, theirsSha); + await git.writeMergeConflictToIndex('a.txt', null, oursSha, theirsSha); index = await git.exec(['ls-files', '--stage', '--', 'a.txt']); assert.equal(index.trim(), dedent` From 867f9b2ae2071500656f12d1e3eb8ce0c98a768c Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Thu, 2 Mar 2017 22:23:53 -0800 Subject: [PATCH 28/30] Use `createTestStrategy` instead of instantiating GitShellOutStrategy --- test/git-strategies.test.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/git-strategies.test.js b/test/git-strategies.test.js index 669d805b51..9f0c73229d 100644 --- a/test/git-strategies.test.js +++ b/test/git-strategies.test.js @@ -748,7 +748,7 @@ import {fsStat} from '../lib/helpers'; describe('getFileMode(filePath)', () => { it('returns the file mode of the specified file', async () => { const workingDirPath = await cloneRepository('three-files'); - const git = new GitShellOutStrategy(workingDirPath); + const git = createTestStrategy(workingDirPath); const absFilePath = path.join(workingDirPath, 'a.txt'); fs.writeFileSync(absFilePath, 'qux\nfoo\nbar\n', 'utf8'); @@ -760,7 +760,7 @@ import {fsStat} from '../lib/helpers'; it('returns the file mode for untracked files', async () => { const workingDirPath = await cloneRepository('three-files'); - const git = new GitShellOutStrategy(workingDirPath); + const git = createTestStrategy(workingDirPath); const absFilePath = path.join(workingDirPath, 'new-file.txt'); fs.writeFileSync(absFilePath, 'qux\nfoo\nbar\n', 'utf8'); const regularMode = await fsStat(absFilePath).mode; @@ -777,7 +777,7 @@ import {fsStat} from '../lib/helpers'; describe('mergeFile(oursPath, commonBasePath, theirsPath, resultPath)', () => { it('merges ours/base/theirsPaths and writes to resultPath, returning {filePath, resultPath, conflicts}', async () => { const workingDirPath = await cloneRepository('three-files'); - const git = new GitShellOutStrategy(workingDirPath); + const git = createTestStrategy(workingDirPath); // current and other paths are the same, so no conflicts const resultsWithoutConflict = await git.mergeFile('a.txt', 'b.txt', 'a.txt', 'results-without-conflict.txt'); @@ -804,7 +804,7 @@ import {fsStat} from '../lib/helpers'; describe('udpateIndex(filePath, commonBaseSha, oursSha, theirsSha)', () => { it('updates the index to have the appropriate shas, retaining the original file mode', async () => { const workingDirPath = await cloneRepository('three-files'); - const git = new GitShellOutStrategy(workingDirPath); + const git = createTestStrategy(workingDirPath); const absFilePath = path.join(workingDirPath, 'a.txt'); fs.writeFileSync(absFilePath, 'qux\nfoo\nbar\n', 'utf8'); await git.exec(['update-index', '--chmod=+x', 'a.txt']); @@ -824,7 +824,7 @@ import {fsStat} from '../lib/helpers'; it('handles the case when oursSha, commonBaseSha, or theirsSha is null', async () => { const workingDirPath = await cloneRepository('three-files'); - const git = new GitShellOutStrategy(workingDirPath); + const git = createTestStrategy(workingDirPath); const absFilePath = path.join(workingDirPath, 'a.txt'); fs.writeFileSync(absFilePath, 'qux\nfoo\nbar\n', 'utf8'); await git.exec(['update-index', '--chmod=+x', 'a.txt']); From 56f158c2478e8e96e844560c84c25a5f90944954 Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Thu, 2 Mar 2017 22:34:47 -0800 Subject: [PATCH 29/30] Make getFileMode test work on windows --- test/git-strategies.test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/git-strategies.test.js b/test/git-strategies.test.js index 9f0c73229d..89a539a0d8 100644 --- a/test/git-strategies.test.js +++ b/test/git-strategies.test.js @@ -769,7 +769,8 @@ import {fsStat} from '../lib/helpers'; assert.equal(await git.getFileMode('new-file.txt'), '100644'); fs.chmodSync(absFilePath, executableMode); - assert.equal(await git.getFileMode('new-file.txt'), '100755'); + const expectedFileMode = process.platform === 'win32' ? '100644' : '100755'; + assert.equal(await git.getFileMode('new-file.txt'), expectedFileMode); }); }); From 00034470c6ed74a288a9992b5125b18337bfa806 Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Thu, 2 Mar 2017 23:08:15 -0800 Subject: [PATCH 30/30] Fix bug undoing discard for file in subdirectory (fixes #588) --- lib/models/discard-history.js | 6 +++++- test/controllers/root-controller.test.js | 18 +++++++++--------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/lib/models/discard-history.js b/lib/models/discard-history.js index 888a96f4f6..54f59bb771 100644 --- a/lib/models/discard-history.js +++ b/lib/models/discard-history.js @@ -1,6 +1,8 @@ import path from 'path'; import os from 'os'; +import mkdirp from 'mkdirp'; + import {PartialFileDiscardHistory, WholeFileDiscardHistory} from './discard-history-stores'; import {getTempDir, copyFile, fileExists, writeFile} from '../helpers'; @@ -118,11 +120,13 @@ 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 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 commonBasePath = !afterSha ? null : await this.expandBlobToFile(path.join(tempFolderPath, `${filePath}-after-discard`), afterSha); - const resultPath = path.join(tempFolderPath, `~${filePath}-merge-result`); + const resultPath = path.join(tempFolderPath, `~${path.basename(filePath)}-merge-result`); return {filePath, commonBasePath, theirsPath, resultPath, theirsSha: beforeSha, commonBaseSha: afterSha}; }); return await Promise.all(pathPromises); diff --git a/test/controllers/root-controller.test.js b/test/controllers/root-controller.test.js index a7b59ee87e..2a92abd51e 100644 --- a/test/controllers/root-controller.test.js +++ b/test/controllers/root-controller.test.js @@ -638,7 +638,7 @@ describe('RootController', function() { }; pathA = path.join(workdirPath, 'a.txt'); - pathB = path.join(workdirPath, 'b.txt'); + pathB = path.join(workdirPath, 'subdir-1', 'b.txt'); pathDeleted = path.join(workdirPath, 'c.txt'); pathAdded = path.join(workdirPath, 'added-file.txt'); fs.writeFileSync(pathA, [1, 2, 3, 4, 5, 6, 7, 8, 9].join('\n')); @@ -657,7 +657,7 @@ describe('RootController', function() { pathDeleted: getFileContents(pathDeleted), pathAdded: getFileContents(pathAdded), }; - await wrapper.instance().discardWorkDirChangesForPaths(['a.txt', 'b.txt']); + await wrapper.instance().discardWorkDirChangesForPaths(['a.txt', 'subdir-1/b.txt']); const contents2 = { pathA: getFileContents(pathA), pathB: getFileContents(pathB), @@ -692,7 +692,7 @@ describe('RootController', function() { }); it('does not undo if buffer is modified', async () => { - await wrapper.instance().discardWorkDirChangesForPaths(['a.txt', 'b.txt', 'c.txt', 'added-file.txt']); + await wrapper.instance().discardWorkDirChangesForPaths(['a.txt', 'subdir-1/b.txt', 'c.txt', 'added-file.txt']); // modify buffers (await workspace.open(pathA)).getBuffer().append('stuff'); @@ -708,7 +708,7 @@ describe('RootController', function() { assert.equal(notificationArgs[0], 'Cannot undo last discard.'); assert.match(notificationArgs[1].description, /You have unsaved changes./); assert.match(notificationArgs[1].description, /a.txt/); - assert.match(notificationArgs[1].description, /b.txt/); + assert.match(notificationArgs[1].description, /subdir-1\/b.txt/); assert.match(notificationArgs[1].description, /c.txt/); assert.match(notificationArgs[1].description, /added-file.txt/); assert.isFalse(expandBlobToFile.called); @@ -736,7 +736,7 @@ describe('RootController', function() { pathAdded: getFileContents(pathAdded), }; - await wrapper.instance().discardWorkDirChangesForPaths(['a.txt', 'b.txt', 'deleted-file.txt', 'another-added-file.txt']); + await wrapper.instance().discardWorkDirChangesForPaths(['a.txt', 'subdir-1/b.txt', 'deleted-file.txt', 'another-added-file.txt']); // change file contents on disk in non-conflicting way fs.writeFileSync(pathA, fs.readFileSync(pathA, 'utf8') + 'change at end'); @@ -769,7 +769,7 @@ describe('RootController', function() { fs.unlinkSync(pathDeleted); fs.writeFileSync(pathAdded, 'foo\nbar\baz\n'); - await wrapper.instance().discardWorkDirChangesForPaths(['a.txt', 'b.txt', 'deleted-file.txt', 'another-added-file.txt']); + await wrapper.instance().discardWorkDirChangesForPaths(['a.txt', 'subdir-1/b.txt', 'deleted-file.txt', 'another-added-file.txt']); // change files in a conflicting way fs.writeFileSync(pathA, 'conflicting change\n' + fs.readFileSync(pathA, 'utf8')); @@ -846,15 +846,15 @@ describe('RootController', function() { await assert.async.isTrue(contentsAfterUndo.pathAdded.includes('>>>>>>>')); let unmergedFiles = await repository.git.exec(['diff', '--name-status', '--diff-filter=U']); unmergedFiles = unmergedFiles.trim().split('\n').map(line => line.split('\t')[1]).sort(); - assert.deepEqual(unmergedFiles, ['a.txt', 'another-added-file.txt', 'b.txt', 'deleted-file.txt']); + assert.deepEqual(unmergedFiles, ['a.txt', 'another-added-file.txt', 'deleted-file.txt', 'subdir-1/b.txt']); }); }); it('clears the discard history if the last blob is no longer valid', async () => { // this would occur in the case of garbage collection cleaning out the blob await wrapper.instance().discardWorkDirChangesForPaths(['a.txt']); - const snapshots = await wrapper.instance().discardWorkDirChangesForPaths(['b.txt']); - const {beforeSha} = snapshots['b.txt']; + const snapshots = await wrapper.instance().discardWorkDirChangesForPaths(['subdir-1/b.txt']); + const {beforeSha} = snapshots['subdir-1/b.txt']; // remove blob from git object store fs.unlinkSync(path.join(repository.getGitDirectoryPath(), 'objects', beforeSha.slice(0, 2), beforeSha.slice(2)));