From eb3a88a6a1a6e023606c002cf84c70483e8807d4 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 23 Sep 2016 11:28:21 -0700 Subject: [PATCH 01/10] Add Repository.getStagedChangesSincePreviousCommit This method does no caching for now. --- lib/git-shell-out-strategy.js | 1 + lib/models/repository.js | 8 ++++++-- test/models/repository.test.js | 27 +++++++++++++++++++++++++++ 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/lib/git-shell-out-strategy.js b/lib/git-shell-out-strategy.js index 9811afa976..5f97be4ada 100644 --- a/lib/git-shell-out-strategy.js +++ b/lib/git-shell-out-strategy.js @@ -98,6 +98,7 @@ export default class GitShellOutStrategy { async diff (options = {}) { let args = ['diff', '--no-prefix', '--no-renames', '--diff-filter=u'] if (options.staged) args.push('--staged') + if (options.baseCommit) args.push(options.baseCommit) let output = await this.exec(args) let rawDiffs = [] diff --git a/lib/models/repository.js b/lib/models/repository.js index d83ac36080..817b9c938b 100644 --- a/lib/models/repository.js +++ b/lib/models/repository.js @@ -91,6 +91,10 @@ export default class Repository { return this.stagedChangesPromise || this.refreshStagedChanges() } + getStagedChangesSincePreviousCommit () { + return this.fetchStagedChanges('HEAD~') + } + getMergeConflicts () { return this.mergeConflictsPromise || this.refreshMergeConflicts() } @@ -134,8 +138,8 @@ export default class Repository { .sort((a, b) => a.getPath().localeCompare(b.getPath())) } - async fetchStagedChanges () { - const rawDiffs = await this.git.diff({staged: true}) + async fetchStagedChanges (baseCommit) { + const rawDiffs = await this.git.diff({staged: true, baseCommit}) const validFilePatches = new Set() for (let newPatch of this.buildFilePatchesFromRawDiffs(rawDiffs)) { const path = newPatch.getPath() diff --git a/test/models/repository.test.js b/test/models/repository.test.js index c4ea3397b9..b6e2659c49 100644 --- a/test/models/repository.test.js +++ b/test/models/repository.test.js @@ -195,6 +195,33 @@ describe('Repository', function () { }) }) + describe('getStagedChangesSincePreviousCommit()', () => { + it('resolves to an array of FilePatches representing the diff between the index and HEAD~', async () => { + const workingDirPath = await cloneRepository('multiple-commits') + const repo = await buildRepository(workingDirPath) + + const newPath = path.join(workingDirPath, 'file2.txt') + fs.writeFileSync(newPath, 'four', 'utf8') + await repo.stageFiles([newPath]) + + assertDeepPropertyVals((await repo.getStagedChangesSincePreviousCommit()), [ + { + oldPath: 'file.txt', + newPath: 'file.txt', + status: 'modified', + hunks: [ + { + lines: [ + {status: 'deleted', text: 'two', oldLineNumber: 1, newLineNumber: -1}, + {status: 'added', text: 'three', oldLineNumber: -1, newLineNumber: 1}, + ] + } + ] + } + ].concat(await repo.getStagedChanges())) + }) + }) + describe('applyPatchToIndex', () => { it('can stage and unstage modified files', async () => { const workingDirPath = await cloneRepository('three-files') From ff21ed8a775b9cefc7e7415daad05c86e493efe5 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 23 Sep 2016 11:28:51 -0700 Subject: [PATCH 02/10] Show the staged changes since HEAD~ when amending --- lib/controllers/git-panel-controller.js | 18 +++++++++++++++++- lib/views/commit-view.js | 8 +++++++- lib/views/git-panel-view.js | 1 + test/controllers/git-panel-controller.test.js | 14 ++++++++++++++ test/views/commit-view.test.js | 15 ++++++++++++++- 5 files changed, 53 insertions(+), 3 deletions(-) diff --git a/lib/controllers/git-panel-controller.js b/lib/controllers/git-panel-controller.js index 424d71aba1..d3b679b027 100644 --- a/lib/controllers/git-panel-controller.js +++ b/lib/controllers/git-panel-controller.js @@ -15,6 +15,7 @@ export default class GitPanelController { this.pull = this.pull.bind(this) this.fetch = this.fetch.bind(this) this.commit = this.commit.bind(this) + this.setAmendMode = this.setAmendMode.bind(this) this.checkout = this.checkout.bind(this) this.abortMerge = this.abortMerge.bind(this) this.switchRepository(props.repository) @@ -47,6 +48,7 @@ export default class GitPanelController { stageFiles={this.stageFiles} unstageFiles={this.unstageFiles} commit={this.commit} + setAmendMode={this.setAmendMode} isMerging={this.isMerging} abortMerge={this.abortMerge} push={this.push} @@ -87,7 +89,7 @@ export default class GitPanelController { async _refreshModelData (repository) { if (repository) { this.unstagedChanges = await repository.getUnstagedChanges() - this.stagedChanges = await repository.getStagedChanges() + await this._refreshStagedChanges(repository) this.mergeConflicts = await repository.getMergeConflicts() this.lastCommit = await repository.getLastCommit() this.isMerging = await repository.isMerging() @@ -111,6 +113,14 @@ export default class GitPanelController { return etch.update(this) } + async _refreshStagedChanges (repository) { + if (this.isAmending) { + this.stagedChanges = await repository.getStagedChangesSincePreviousCommit() + } else { + this.stagedChanges = await repository.getStagedChanges() + } + } + unstageFilePatch (filePatch) { return this.repository.applyPatchToIndex(filePatch.getUnstagePatch()) } @@ -159,6 +169,12 @@ export default class GitPanelController { return this.repository.commit(message, options) } + async setAmendMode (isAmending) { + this.isAmending = isAmending + await this._refreshStagedChanges(this.repository) + return etch.update(this) + } + abortMerge () { return this.repository.abortMerge() } diff --git a/lib/views/commit-view.js b/lib/views/commit-view.js index 911a18b732..7953e9dbaa 100644 --- a/lib/views/commit-view.js +++ b/lib/views/commit-view.js @@ -92,8 +92,13 @@ export default class CommitView { return etch.update(this) } - handleAmendBoxClick () { + async handleAmendBoxClick () { const checked = this.refs.amend.checked + + if (this.props.setAmendMode) { + await this.props.setAmendMode(checked) + } + if (this.editor.getText()) { const choice = atom.confirm({ message: 'Would you like to discard the current commit message?', @@ -102,6 +107,7 @@ export default class CommitView { }) if (choice !== 0) return } + const text = (checked && this.props.lastCommit) ? this.props.lastCommit.message : '' this.editor.setText(text) return etch.update(this) diff --git a/lib/views/git-panel-view.js b/lib/views/git-panel-view.js index 833d6de544..4a5865e036 100644 --- a/lib/views/git-panel-view.js +++ b/lib/views/git-panel-view.js @@ -59,6 +59,7 @@ export default class GitPanelView { stagedChangesExist={this.props.stagedChanges.length > 0} mergeConflictsExist={this.props.mergeConflicts.length > 0} commit={this.props.commit} + setAmendMode={this.props.setAmendMode} disableCommitButton={this} abortMerge={this.props.abortMerge} branchName={this.props.branchName} diff --git a/test/controllers/git-panel-controller.test.js b/test/controllers/git-panel-controller.test.js index 12b09e0727..c743b06cd0 100644 --- a/test/controllers/git-panel-controller.test.js +++ b/test/controllers/git-panel-controller.test.js @@ -56,6 +56,20 @@ describe('GitPanelController', () => { assert.equal(controller.refs.gitPanel.props.unstagedChanges, await repository2.getUnstagedChanges()) }) + it('displays the staged changes since the *previous* commmit when amending', async function () { + const workdirPath = await cloneRepository('multiple-commits') + const repository = await buildRepository(workdirPath) + const controller = new GitPanelController({workspace, commandRegistry, repository}) + await controller.lastModelDataRefreshPromise + assert.deepEqual(controller.refs.gitPanel.props.stagedChanges, []) + + await controller.setAmendMode(true) + assert.deepEqual( + controller.refs.gitPanel.props.stagedChanges, + await controller.repository.getStagedChangesSincePreviousCommit() + ) + }) + describe('integration tests', () => { it('can stage and unstage files and commit', async () => { const workdirPath = await cloneRepository('three-files') diff --git a/test/views/commit-view.test.js b/test/views/commit-view.test.js index 2a7e579f5e..0081d85a4e 100644 --- a/test/views/commit-view.test.js +++ b/test/views/commit-view.test.js @@ -261,6 +261,19 @@ describe('CommitView', () => { await view.commit() assert.isFalse(amend.checked) }) - }) + it('calls props.setAmendMode() when the box is checked or unchecked', async function () { + const setAmendMode = sinon.spy() + const workdirPath = await cloneRepository('three-files') + const repository = await buildRepository(workdirPath) + const view = new CommitView({workspace, commandRegistry, stagedChangesExist: false, lastCommit: {message: 'previous commit\'s message'}, setAmendMode}) + const {editor, amend} = view.refs + + amend.click() + assert.deepEqual(setAmendMode.args, [[true]]) + + amend.click() + assert.deepEqual(setAmendMode.args, [[true], [false]]) + }) + }) }) From a61d0c56ef1ca977c2ca7aa1c8f1c531f7a41ff0 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 23 Sep 2016 12:28:53 -0700 Subject: [PATCH 03/10] Indicate that HEAD is included in staged changes list when amending --- lib/controllers/git-panel-controller.js | 1 + lib/helpers.js | 4 ++++ lib/views/git-panel-view.js | 2 ++ lib/views/staging-view.js | 10 +++++++++- 4 files changed, 16 insertions(+), 1 deletion(-) diff --git a/lib/controllers/git-panel-controller.js b/lib/controllers/git-panel-controller.js index d3b679b027..b8b83dd352 100644 --- a/lib/controllers/git-panel-controller.js +++ b/lib/controllers/git-panel-controller.js @@ -50,6 +50,7 @@ export default class GitPanelController { commit={this.commit} setAmendMode={this.setAmendMode} isMerging={this.isMerging} + isAmending={this.isAmending} abortMerge={this.abortMerge} push={this.push} pull={this.pull} diff --git a/lib/helpers.js b/lib/helpers.js index fc5bf690e5..7f6baa9921 100644 --- a/lib/helpers.js +++ b/lib/helpers.js @@ -38,6 +38,10 @@ export function fsStat (absoluteFilePath) { }) } +export function shortenSha (sha) { + return sha.slice(0, 8) +} + export const classNameForStatus = { added: 'added', deleted: 'removed', diff --git a/lib/views/git-panel-view.js b/lib/views/git-panel-view.js index 4a5865e036..d1edab75ff 100644 --- a/lib/views/git-panel-view.js +++ b/lib/views/git-panel-view.js @@ -52,6 +52,8 @@ export default class GitPanelView { didSelectMergeConflictFile={this.props.didSelectMergeConflictFile} stageFiles={this.props.stageFiles} unstageFiles={this.props.unstageFiles} + lastCommit={this.props.lastCommit} + isAmending={this.props.isAmending} />
- Staged Changes + + Staged Changes + { + this.props.isAmending ? + ` (including ${shortenSha(this.props.lastCommit.sha)})` : + '' + } +
this.selectList(ListTypes.STAGED, {suppressCallback: true})} From cf6859d366bc675b782ec21752f9f4e78bb0f512 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 23 Sep 2016 12:34:12 -0700 Subject: [PATCH 04/10] Make button indicate that action is different when amending --- lib/views/commit-view.js | 25 +++++++++++++++++++++---- lib/views/git-panel-view.js | 1 + 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/lib/views/commit-view.js b/lib/views/commit-view.js index 7953e9dbaa..448af03f59 100644 --- a/lib/views/commit-view.js +++ b/lib/views/commit-view.js @@ -3,6 +3,7 @@ import {CompositeDisposable} from 'atom' import etch from 'etch' +import {shortenSha} from '../helpers' export default class CommitView { constructor (props) { @@ -64,7 +65,7 @@ export default class CommitView { + disabled={!this.isCommitButtonEnabled()}>{this.commitButtonText()}
{this.getRemainingCharacters()}
@@ -99,17 +100,21 @@ export default class CommitView { await this.props.setAmendMode(checked) } + let updateMessage = true if (this.editor.getText()) { const choice = atom.confirm({ message: 'Would you like to discard the current commit message?', detailedMessage: checked ? 'It will be replaced with the most recent commit message' : '', buttons: ['Replace', 'Keep current'] }) - if (choice !== 0) return + if (choice !== 0) updateMessage = false + } + + if (updateMessage) { + const text = (checked && this.props.lastCommit) ? this.props.lastCommit.message : '' + this.editor.setText(text) } - const text = (checked && this.props.lastCommit) ? this.props.lastCommit.message : '' - this.editor.setText(text) return etch.update(this) } @@ -144,4 +149,16 @@ export default class CommitView { isCommitButtonEnabled () { return this.props.stagedChangesExist && !this.props.mergeConflictsExist && this.editor && this.editor.getText().length !== 0 } + + commitButtonText () { + if (this.props.isAmending) { + return `Amend commit ${shortenSha(this.props.lastCommit.sha)})` + } else { + if (this.props.branchName) { + return `Commit to ${this.props.branchName}` + } else { + return 'Commit' + } + } + } } diff --git a/lib/views/git-panel-view.js b/lib/views/git-panel-view.js index d1edab75ff..e87a8073b5 100644 --- a/lib/views/git-panel-view.js +++ b/lib/views/git-panel-view.js @@ -70,6 +70,7 @@ export default class GitPanelView { maximumCharacterLimit={72} message={this.props.mergeMessage} isMerging={this.props.isMerging} + isAmending={this.props.isAmending} lastCommit={this.props.lastCommit} /> From 95ddca96739f65c8872cb7cff9bfe7616ae8055c Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 23 Sep 2016 16:16:48 -0700 Subject: [PATCH 05/10] Add Repository.stageFilesFromParentCommit Signed-off-by: Nathan Sobo --- lib/controllers/git-panel-controller.js | 8 +- lib/git-shell-out-strategy.js | 4 +- lib/models/repository.js | 95 ++++++++++++++----- .../changed-files-count-controller.test.js | 8 +- test/controllers/git-panel-controller.test.js | 4 +- test/models/repository.test.js | 94 +++++++++++------- test/models/workspace-change-observer.test.js | 3 +- 7 files changed, 146 insertions(+), 70 deletions(-) diff --git a/lib/controllers/git-panel-controller.js b/lib/controllers/git-panel-controller.js index b8b83dd352..674cfe50c3 100644 --- a/lib/controllers/git-panel-controller.js +++ b/lib/controllers/git-panel-controller.js @@ -116,7 +116,7 @@ export default class GitPanelController { async _refreshStagedChanges (repository) { if (this.isAmending) { - this.stagedChanges = await repository.getStagedChangesSincePreviousCommit() + this.stagedChanges = await repository.getStagedChangesSinceParentCommit() } else { this.stagedChanges = await repository.getStagedChanges() } @@ -143,7 +143,11 @@ export default class GitPanelController { } unstageFiles (filePaths) { - return this.repository.unstageFiles(filePaths) + if (this.isAmending) { + return this.repository.stageFilesFromParentCommit(filePaths) + } else { + return this.repository.unstageFiles(filePaths) + } } push () { diff --git a/lib/git-shell-out-strategy.js b/lib/git-shell-out-strategy.js index 5f97be4ada..b042f7f5eb 100644 --- a/lib/git-shell-out-strategy.js +++ b/lib/git-shell-out-strategy.js @@ -45,9 +45,9 @@ export default class GitShellOutStrategy { return this.exec(args) } - unstageFiles (paths) { + unstageFiles (paths, commit = 'HEAD') { if (paths.length === 0) return - const args = ['reset', '--'].concat(paths) + const args = ['reset', commit, '--' ].concat(paths) return this.exec(args) } diff --git a/lib/models/repository.js b/lib/models/repository.js index 817b9c938b..5f09682221 100644 --- a/lib/models/repository.js +++ b/lib/models/repository.js @@ -45,6 +45,7 @@ export default class Repository { this.workingDirectory = workingDirectory this.emitter = new Emitter() this.stagedFilePatchesByPath = new Map() + this.stagedFilePatchesSinceParentCommitByPath = new Map() this.unstagedFilePatchesByPath = new Map() this.mergeConflictsByPath = new Map() this.git = gitStrategy @@ -76,41 +77,45 @@ export default class Repository { async refresh () { if (global.PRINT_GIT_TIMES) console.time('refresh') - await this.refreshStagedChanges() - await this.refreshUnstagedChanges() - await this.refreshMergeConflicts() + this.stagedChangesPromise = this.fetchStagedChanges() + this.stagedChangesSinceParentCommitPromise = this.fetchStagedChangesSinceParentCommit() + this.unstagedChangesPromise = this.fetchUnstagedChanges() + this.mergeConflictsPromise = this.fetchMergeConflicts() + await Promise.all([ + this.stagedChangesPromise, + this.stagedChangesSinceParentCommitPromise, + this.unstagedChangesPromise, + this.mergeConflictsPromise + ]) if (global.PRINT_GIT_TIMES) console.timeEnd('refresh') this.didUpdate() } getUnstagedChanges () { - return this.unstagedChangesPromise || this.refreshUnstagedChanges() + if (!this.unstagedChangesPromise) { + this.unstagedChangesPromise = this.fetchUnstagedChanges() + } + return this.unstagedChangesPromise } getStagedChanges () { - return this.stagedChangesPromise || this.refreshStagedChanges() + if (!this.stagedChangesPromise) { + this.stagedChangesPromise = this.fetchStagedChanges() + } + return this.stagedChangesPromise } - getStagedChangesSincePreviousCommit () { - return this.fetchStagedChanges('HEAD~') + getStagedChangesSinceParentCommit () { + if (!this.stagedChangesSinceParentCommitPromise) { + this.stagedChangesSinceParentCommitPromise = this.fetchStagedChangesSinceParentCommit() + } + return this.stagedChangesSinceParentCommitPromise } getMergeConflicts () { - return this.mergeConflictsPromise || this.refreshMergeConflicts() - } - - refreshUnstagedChanges () { - this.unstagedChangesPromise = this.fetchUnstagedChanges() - return this.unstagedChangesPromise - } - - refreshStagedChanges () { - this.stagedChangesPromise = this.fetchStagedChanges() - return this.stagedChangesPromise - } - - refreshMergeConflicts () { - this.mergeConflictsPromise = this.fetchMergeConflicts() + if (!this.mergeConflictsPromise) { + this.mergeConflictsPromise = this.fetchMergeConflicts() + } return this.mergeConflictsPromise } @@ -138,8 +143,8 @@ export default class Repository { .sort((a, b) => a.getPath().localeCompare(b.getPath())) } - async fetchStagedChanges (baseCommit) { - const rawDiffs = await this.git.diff({staged: true, baseCommit}) + async fetchStagedChanges () { + const rawDiffs = await this.git.diff({staged: true}) const validFilePatches = new Set() for (let newPatch of this.buildFilePatchesFromRawDiffs(rawDiffs)) { const path = newPatch.getPath() @@ -162,6 +167,40 @@ export default class Repository { .sort((a, b) => a.getPath().localeCompare(b.getPath())) } + async fetchStagedChangesSinceParentCommit () { + let rawDiffs + try { + rawDiffs = await this.git.diff({staged: true, baseCommit: 'HEAD~'}) + } catch (e) { + if (e.message.includes(`ambiguous argument 'HEAD~'`)) { + return [] + } else { + throw e + } + } + + const validFilePatches = new Set() + for (let newPatch of this.buildFilePatchesFromRawDiffs(rawDiffs)) { + const path = newPatch.getPath() + const existingPatch = this.stagedFilePatchesSinceParentCommitByPath.get(path) + if (existingPatch == null) { + this.stagedFilePatchesSinceParentCommitByPath.set(path, newPatch) + } else { + existingPatch.update(newPatch) + } + validFilePatches.add(path) + } + + for (let [path, patch] of this.stagedFilePatchesSinceParentCommitByPath) { + if (!validFilePatches.has(path)) { + this.stagedFilePatchesSinceParentCommitByPath.delete(path) + patch.destroy() + } + } + return Array.from(this.stagedFilePatchesSinceParentCommitByPath.values()) + .sort((a, b) => a.getPath().localeCompare(b.getPath())) + } + buildFilePatchesFromRawDiffs (rawDiffs) { const statusMap = { '+': 'added', @@ -232,6 +271,11 @@ export default class Repository { await this.refresh() } + async stageFilesFromParentCommit (paths) { + await this.git.unstageFiles(paths, 'HEAD~') + await this.refresh() + } + async applyPatchToIndex (filePatch) { const patchStr = filePatch.getHeaderString() + filePatch.toString() await this.git.applyPatchToIndex(patchStr) @@ -272,7 +316,8 @@ export default class Repository { async commit (message, options) { await this.git.commit(this.stripCommitComments(message), options) - this.refreshStagedChanges() + this.stagedChangesPromise = null + this.stagedChangesSinceParentCommitPromise = null this.didUpdate() } diff --git a/test/controllers/changed-files-count-controller.test.js b/test/controllers/changed-files-count-controller.test.js index baf94e7829..8a6f29005d 100644 --- a/test/controllers/changed-files-count-controller.test.js +++ b/test/controllers/changed-files-count-controller.test.js @@ -22,11 +22,13 @@ describe('ChangedFilesCountController', () => { fs.writeFileSync(path.join(workdirPath, 'a.txt'), 'a change\n') fs.unlinkSync(path.join(workdirPath, 'b.txt')) - const [patchToStage] = await repository.refreshUnstagedChanges() + repository.refresh() + + const [patchToStage] = await repository.getUnstagedChanges() await repository.applyPatchToIndex(patchToStage) await view.lastModelDataRefreshPromise - assert.deepEqual(view.refs.changedFilesCount.props.stagedChanges, await repository.refreshStagedChanges()) - assert.deepEqual(view.refs.changedFilesCount.props.unstagedChanges, await repository.refreshUnstagedChanges()) + assert.deepEqual(view.refs.changedFilesCount.props.stagedChanges, await repository.getStagedChanges()) + assert.deepEqual(view.refs.changedFilesCount.props.unstagedChanges, await repository.getUnstagedChanges()) view.refs.changedFilesCount.props.didClick() assert(didClick.calledOnce) diff --git a/test/controllers/git-panel-controller.test.js b/test/controllers/git-panel-controller.test.js index c743b06cd0..30f90e9f0e 100644 --- a/test/controllers/git-panel-controller.test.js +++ b/test/controllers/git-panel-controller.test.js @@ -56,7 +56,7 @@ describe('GitPanelController', () => { assert.equal(controller.refs.gitPanel.props.unstagedChanges, await repository2.getUnstagedChanges()) }) - it('displays the staged changes since the *previous* commmit when amending', async function () { + it('displays the staged changes since the parent commmit when amending', async function () { const workdirPath = await cloneRepository('multiple-commits') const repository = await buildRepository(workdirPath) const controller = new GitPanelController({workspace, commandRegistry, repository}) @@ -66,7 +66,7 @@ describe('GitPanelController', () => { await controller.setAmendMode(true) assert.deepEqual( controller.refs.gitPanel.props.stagedChanges, - await controller.repository.getStagedChangesSincePreviousCommit() + await controller.repository.getStagedChangesSinceParentCommit() ) }) diff --git a/test/models/repository.test.js b/test/models/repository.test.js index b6e2659c49..aaafa2556b 100644 --- a/test/models/repository.test.js +++ b/test/models/repository.test.js @@ -8,7 +8,7 @@ import sinon from 'sinon' import {cloneRepository, buildRepository, assertDeepPropertyVals, setUpLocalAndRemoteRepositories, getHeadCommitOnRemote} from '../helpers' describe('Repository', function () { - describe('refreshing staged and unstaged changes', () => { + describe('getting staged and unstaged changes', () => { it('returns a promise resolving to an array of FilePatch objects', async () => { const workingDirPath = await cloneRepository('three-files') fs.writeFileSync(path.join(workingDirPath, 'a.txt'), 'qux\nfoo\nbar\n', 'utf8') @@ -17,7 +17,7 @@ describe('Repository', function () { fs.writeFileSync(path.join(workingDirPath, 'e.txt'), 'qux', 'utf8') const repo = await buildRepository(workingDirPath) - const filePatches = await repo.refreshUnstagedChanges() + const filePatches = await repo.getUnstagedChanges() assertDeepPropertyVals(filePatches, [ { @@ -93,12 +93,14 @@ describe('Repository', function () { fs.unlinkSync(path.join(workingDirPath, 'b.txt')) fs.writeFileSync(path.join(workingDirPath, 'c.txt'), 'bar\nbaz') fs.writeFileSync(path.join(workingDirPath, 'd.txt'), 'qux', 'utf8') - const unstagedFilePatches1 = await repo.refreshUnstagedChanges() + await repo.refresh() + const unstagedFilePatches1 = await repo.getUnstagedChanges() fs.writeFileSync(path.join(workingDirPath, 'a.txt'), 'baz\nfoo\nqux', 'utf8') fs.writeFileSync(path.join(workingDirPath, 'c.txt'), 'baz') fs.unlinkSync(path.join(workingDirPath, 'd.txt')) - const unstagedFilePatches2 = await repo.refreshUnstagedChanges() + await repo.refresh() + const unstagedFilePatches2 = await repo.getUnstagedChanges() assert.equal(unstagedFilePatches1.length, 4) assert.equal(unstagedFilePatches2.length, 3) @@ -110,11 +112,11 @@ describe('Repository', function () { await repo.stageFiles([unstagedFilePatches2[0].getPath()]) await repo.stageFiles([unstagedFilePatches2[1].getPath()]) await repo.stageFiles([unstagedFilePatches2[2].getPath()]) - const stagedFilePatches1 = await repo.refreshStagedChanges() + const stagedFilePatches1 = await repo.getStagedChanges() await repo.unstageFiles([stagedFilePatches1[2].getPath()]) - const stagedFilePatches2 = await repo.refreshStagedChanges() - const unstagedFilePatches3 = await repo.refreshUnstagedChanges() + const stagedFilePatches2 = await repo.getStagedChanges() + const unstagedFilePatches3 = await repo.getUnstagedChanges() assert.equal(stagedFilePatches1.length, 3) assert.equal(stagedFilePatches2.length, 2) @@ -169,7 +171,7 @@ describe('Repository', function () { await repo.stageFiles([filePath1]) await repo.stageFiles([filePath2]) - assert.deepEqual(await repo.refreshStagedChanges(), patches) + assert.deepEqual(await repo.getStagedChanges(), patches) assert.deepEqual(await repo.getUnstagedChanges(), []) await repo.unstageFiles([filePath1]) @@ -193,18 +195,29 @@ describe('Repository', function () { assert.deepEqual(await repo.getUnstagedChanges(), [patch]) assert.deepEqual(await repo.getStagedChanges(), []) }) - }) - describe('getStagedChangesSincePreviousCommit()', () => { - it('resolves to an array of FilePatches representing the diff between the index and HEAD~', async () => { + it('can unstage and retrieve staged changes relative to HEAD~', async () => { const workingDirPath = await cloneRepository('multiple-commits') const repo = await buildRepository(workingDirPath) + fs.writeFileSync(path.join(workingDirPath, 'file.txt'), 'three\nfour\n', 'utf8') + assertDeepPropertyVals(await repo.getStagedChangesSinceParentCommit(), [ + { + oldPath: 'file.txt', + newPath: 'file.txt', + status: 'modified', + hunks: [ + { + lines: [ + {status: 'deleted', text: 'two', oldLineNumber: 1, newLineNumber: -1}, + {status: 'added', text: 'three', oldLineNumber: -1, newLineNumber: 1}, + ] + } + ] + } + ]) - const newPath = path.join(workingDirPath, 'file2.txt') - fs.writeFileSync(newPath, 'four', 'utf8') - await repo.stageFiles([newPath]) - - assertDeepPropertyVals((await repo.getStagedChangesSincePreviousCommit()), [ + await repo.stageFiles(['file.txt']) + assertDeepPropertyVals(await repo.getStagedChangesSinceParentCommit(), [ { oldPath: 'file.txt', newPath: 'file.txt', @@ -214,11 +227,15 @@ describe('Repository', function () { lines: [ {status: 'deleted', text: 'two', oldLineNumber: 1, newLineNumber: -1}, {status: 'added', text: 'three', oldLineNumber: -1, newLineNumber: 1}, + {status: 'added', text: 'four', oldLineNumber: -1, newLineNumber: 2}, ] } ] } - ].concat(await repo.getStagedChanges())) + ]) + + await repo.stageFilesFromParentCommit(['file.txt']) + assert.deepEqual(await repo.getStagedChangesSinceParentCommit(), []) }) }) @@ -230,7 +247,8 @@ describe('Repository', function () { const [unstagedPatch1] = (await repo.getUnstagedChanges()).map(p => p.copy()) fs.writeFileSync(path.join(workingDirPath, 'subdir-1', 'a.txt'), 'qux\nfoo\nbar\nbaz\n', 'utf8') - await repo.refreshUnstagedChanges() + await repo.refresh() + await repo.getUnstagedChanges() const [unstagedPatch2] = (await repo.getUnstagedChanges()).map(p => p.copy()) await repo.applyPatchToIndex(unstagedPatch1) @@ -256,6 +274,8 @@ describe('Repository', function () { ) unstagedLines.splice(11, 2, 'this is a modified line') fs.writeFileSync(filePath, unstagedLines.join('\n')) + await repository.refresh() + const [unstagedFilePatch] = await repository.getUnstagedChanges() const unstagedListener = sinon.spy() unstagedFilePatch.onDidUpdate(unstagedListener) @@ -450,7 +470,7 @@ describe('Repository', function () { }) describe('merge conflicts', () => { - describe('refreshMergeConflicts()', () => { + describe('getMergeConflicts()', () => { it('returns a promise resolving to an array of MergeConflict objects', async () => { const workingDirPath = await cloneRepository('merge-conflict') const repo = await buildRepository(workingDirPath) @@ -460,7 +480,8 @@ describe('Repository', function () { // expected } - let mergeConflicts = await repo.refreshMergeConflicts() + await repo.refresh() + let mergeConflicts = await repo.getMergeConflicts() const expected = [ { path: 'added-to-both.txt', @@ -497,7 +518,8 @@ describe('Repository', function () { assertDeepPropertyVals(mergeConflicts, expected) fs.unlinkSync(path.join(workingDirPath, 'removed-on-branch.txt')) - mergeConflicts = await repo.refreshMergeConflicts() + await repo.refresh() + mergeConflicts = await repo.getMergeConflicts() expected[3].fileStatus = 'deleted' assertDeepPropertyVals(mergeConflicts, expected) @@ -512,10 +534,11 @@ describe('Repository', function () { // expected } - const mergeConflicts1 = await repo.refreshMergeConflicts() + await repo.refresh() + const mergeConflicts1 = await repo.getMergeConflicts() await repo.stageFiles(['removed-on-master.txt']) - const mergeConflicts2 = await repo.refreshMergeConflicts() + const mergeConflicts2 = await repo.getMergeConflicts() assert.equal(mergeConflicts1.length, 5) assert.equal(mergeConflicts2.length, 4) @@ -545,39 +568,40 @@ describe('Repository', function () { // expected } + await repo.refresh() const mergeConflictPaths = (await repo.getMergeConflicts()).map(c => c.getPath()) assert.deepEqual(mergeConflictPaths, ['added-to-both.txt', 'modified-on-both-ours.txt', 'modified-on-both-theirs.txt', 'removed-on-branch.txt', 'removed-on-master.txt']) - let stagedFilePatches = await repo.refreshStagedChanges() + let stagedFilePatches = await repo.getStagedChanges() assert.deepEqual(stagedFilePatches.map(patch => patch.getPath()), []) await repo.stageFiles(['added-to-both.txt']) - stagedFilePatches = await repo.refreshStagedChanges() + stagedFilePatches = await repo.getStagedChanges() assert.deepEqual(stagedFilePatches.map(patch => patch.getPath()), ['added-to-both.txt']) // choose version of the file on head fs.writeFileSync(path.join(workingDirPath, 'modified-on-both-ours.txt'), 'master modification\n', 'utf8') await repo.stageFiles(['modified-on-both-ours.txt']) - stagedFilePatches = await repo.refreshStagedChanges() + stagedFilePatches = await repo.getStagedChanges() // nothing additional to stage assert.deepEqual(stagedFilePatches.map(patch => patch.getPath()), ['added-to-both.txt']) // choose version of the file on branch fs.writeFileSync(path.join(workingDirPath, 'modified-on-both-ours.txt'), 'branch modification\n', 'utf8') await repo.stageFiles(['modified-on-both-ours.txt']) - stagedFilePatches = await repo.refreshStagedChanges() + stagedFilePatches = await repo.getStagedChanges() assert.deepEqual(stagedFilePatches.map(patch => patch.getPath()), ['added-to-both.txt', 'modified-on-both-ours.txt']) // remove file that was deleted on branch fs.unlinkSync(path.join(workingDirPath, 'removed-on-branch.txt')) await repo.stageFiles(['removed-on-branch.txt']) - stagedFilePatches = await repo.refreshStagedChanges() + stagedFilePatches = await repo.getStagedChanges() assert.deepEqual(stagedFilePatches.map(patch => patch.getPath()), ['added-to-both.txt', 'modified-on-both-ours.txt', 'removed-on-branch.txt']) // remove file that was deleted on master fs.unlinkSync(path.join(workingDirPath, 'removed-on-master.txt')) await repo.stageFiles(['removed-on-master.txt']) - stagedFilePatches = await repo.refreshStagedChanges() + stagedFilePatches = await repo.getStagedChanges() // nothing additional to stage assert.deepEqual(stagedFilePatches.map(patch => patch.getPath()), ['added-to-both.txt', 'modified-on-both-ours.txt', 'removed-on-branch.txt']) }) @@ -654,8 +678,8 @@ describe('Repository', function () { await repo.abortMerge() assert.equal(await repo.isMerging(), false) - assert.equal((await repo.refreshStagedChanges()).length, 0) - assert.equal((await repo.refreshUnstagedChanges()).length, 1) + assert.equal((await repo.getStagedChanges()).length, 0) + assert.equal((await repo.getUnstagedChanges()).length, 1) assert.equal(fs.readFileSync(path.join(workingDirPath, 'fruit.txt')), 'a change\n') }) }) @@ -671,8 +695,8 @@ describe('Repository', function () { } fs.writeFileSync(path.join(workingDirPath, 'animal.txt'), 'a change\n') - const stagedChanges = await repo.refreshStagedChanges() - const unstagedChanges = await repo.refreshUnstagedChanges() + const stagedChanges = await repo.getStagedChanges() + const unstagedChanges = await repo.getUnstagedChanges() assert.equal(await repo.isMerging(), true) try { @@ -682,8 +706,8 @@ describe('Repository', function () { assert.match(e.command, /^git merge --abort/) } assert.equal(await repo.isMerging(), true) - assert.deepEqual(await repo.refreshStagedChanges(), stagedChanges) - assert.deepEqual(await repo.refreshUnstagedChanges(), unstagedChanges) + assert.deepEqual(await repo.getStagedChanges(), stagedChanges) + assert.deepEqual(await repo.getUnstagedChanges(), unstagedChanges) }) }) }) diff --git a/test/models/workspace-change-observer.test.js b/test/models/workspace-change-observer.test.js index f3111ac395..92d4fc9450 100644 --- a/test/models/workspace-change-observer.test.js +++ b/test/models/workspace-change-observer.test.js @@ -70,7 +70,8 @@ describe('WorkspaceChangeObserver', () => { assert.isTrue(changeSpy.calledOnce) changeSpy.reset() - const [unstagedChange] = await repository1.refreshUnstagedChanges() + repository1.refresh() + const [unstagedChange] = await repository1.getUnstagedChanges() await repository1.applyPatchToIndex(unstagedChange) await changeObserver.lastIndexChangePromise assert.isTrue(changeSpy.called) From 9718b120ae9d44be4ee6a825f17004e637876828 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 23 Sep 2016 17:08:01 -0700 Subject: [PATCH 06/10] Remove duplication between Repository fetch methods --- lib/models/repository.js | 64 ++++++++++------------------------------ 1 file changed, 15 insertions(+), 49 deletions(-) diff --git a/lib/models/repository.js b/lib/models/repository.js index 5f09682221..6d6bb32974 100644 --- a/lib/models/repository.js +++ b/lib/models/repository.js @@ -121,56 +121,18 @@ export default class Repository { async fetchUnstagedChanges () { const rawDiffs = await this.git.diff() - const validFilePatches = new Set() - for (let newPatch of this.buildFilePatchesFromRawDiffs(rawDiffs)) { - const path = newPatch.getPath() - const existingPatch = this.unstagedFilePatchesByPath.get(path) - if (existingPatch == null) { - this.unstagedFilePatchesByPath.set(path, newPatch) - } else { - existingPatch.update(newPatch) - } - validFilePatches.add(path) - } - - for (let [path, patch] of this.unstagedFilePatchesByPath) { - if (!validFilePatches.has(path)) { - this.unstagedFilePatchesByPath.delete(path) - patch.destroy() - } - } - return Array.from(this.unstagedFilePatchesByPath.values()) - .sort((a, b) => a.getPath().localeCompare(b.getPath())) + return this.updateFilePatches(this.unstagedFilePatchesByPath, rawDiffs) } async fetchStagedChanges () { const rawDiffs = await this.git.diff({staged: true}) - const validFilePatches = new Set() - for (let newPatch of this.buildFilePatchesFromRawDiffs(rawDiffs)) { - const path = newPatch.getPath() - const existingPatch = this.stagedFilePatchesByPath.get(path) - if (existingPatch == null) { - this.stagedFilePatchesByPath.set(path, newPatch) - } else { - existingPatch.update(newPatch) - } - validFilePatches.add(path) - } - - for (let [path, patch] of this.stagedFilePatchesByPath) { - if (!validFilePatches.has(path)) { - this.stagedFilePatchesByPath.delete(path) - patch.destroy() - } - } - return Array.from(this.stagedFilePatchesByPath.values()) - .sort((a, b) => a.getPath().localeCompare(b.getPath())) + return this.updateFilePatches(this.stagedFilePatchesByPath, rawDiffs) } async fetchStagedChangesSinceParentCommit () { - let rawDiffs try { - rawDiffs = await this.git.diff({staged: true, baseCommit: 'HEAD~'}) + const rawDiffs = await this.git.diff({staged: true, baseCommit: 'HEAD~'}) + return this.updateFilePatches(this.stagedFilePatchesSinceParentCommitByPath, rawDiffs) } catch (e) { if (e.message.includes(`ambiguous argument 'HEAD~'`)) { return [] @@ -178,27 +140,31 @@ export default class Repository { throw e } } + } + + updateFilePatches (patchesByPath, rawDiffs) { + const filePatches = this.buildFilePatchesFromRawDiffs(rawDiffs) const validFilePatches = new Set() - for (let newPatch of this.buildFilePatchesFromRawDiffs(rawDiffs)) { + for (let newPatch of filePatches) { const path = newPatch.getPath() - const existingPatch = this.stagedFilePatchesSinceParentCommitByPath.get(path) + const existingPatch = patchesByPath.get(path) if (existingPatch == null) { - this.stagedFilePatchesSinceParentCommitByPath.set(path, newPatch) + patchesByPath.set(path, newPatch) } else { existingPatch.update(newPatch) } validFilePatches.add(path) } - for (let [path, patch] of this.stagedFilePatchesSinceParentCommitByPath) { + for (let [path, patch] of patchesByPath) { if (!validFilePatches.has(path)) { - this.stagedFilePatchesSinceParentCommitByPath.delete(path) + patchesByPath.delete(path) patch.destroy() } } - return Array.from(this.stagedFilePatchesSinceParentCommitByPath.values()) - .sort((a, b) => a.getPath().localeCompare(b.getPath())) + + return Array.from(patchesByPath.values()).sort((a, b) => a.getPath().localeCompare(b.getPath())) } buildFilePatchesFromRawDiffs (rawDiffs) { From 93c72042962afe6875fdcf9ba584e1a1a5a91c7d Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 23 Sep 2016 17:12:58 -0700 Subject: [PATCH 07/10] Don't be amending after amending --- lib/controllers/git-panel-controller.js | 5 +++-- test/controllers/git-panel-controller.test.js | 4 ++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/controllers/git-panel-controller.js b/lib/controllers/git-panel-controller.js index 674cfe50c3..75c793e768 100644 --- a/lib/controllers/git-panel-controller.js +++ b/lib/controllers/git-panel-controller.js @@ -170,8 +170,9 @@ export default class GitPanelController { return this.stagedChanges.length === 0 && this.unstagedChanges.length === 0 } - commit (message, options) { - return this.repository.commit(message, options) + async commit (message, options) { + this.isAmending = false + await this.repository.commit(message, options) } async setAmendMode (isAmending) { diff --git a/test/controllers/git-panel-controller.test.js b/test/controllers/git-panel-controller.test.js index 30f90e9f0e..30ef319a8b 100644 --- a/test/controllers/git-panel-controller.test.js +++ b/test/controllers/git-panel-controller.test.js @@ -68,6 +68,10 @@ describe('GitPanelController', () => { controller.refs.gitPanel.props.stagedChanges, await controller.repository.getStagedChangesSinceParentCommit() ) + + await controller.commit('Delete most of the code', {amend: true}) + await controller.lastModelDataRefreshPromise + assert(!controller.refs.gitPanel.props.isAmending) }) describe('integration tests', () => { From 5d4d9d3641bd2896f6e2782a26b74e256c3adbde Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 23 Sep 2016 17:39:29 -0700 Subject: [PATCH 08/10] Rename setAmendMode -> setAmending --- lib/controllers/git-panel-controller.js | 6 +++--- lib/views/commit-view.js | 4 ++-- lib/views/git-panel-view.js | 2 +- lib/views/staging-view.js | 2 +- test/controllers/git-panel-controller.test.js | 2 +- test/views/commit-view.test.js | 10 +++++----- 6 files changed, 13 insertions(+), 13 deletions(-) diff --git a/lib/controllers/git-panel-controller.js b/lib/controllers/git-panel-controller.js index 75c793e768..5c447601fe 100644 --- a/lib/controllers/git-panel-controller.js +++ b/lib/controllers/git-panel-controller.js @@ -15,7 +15,7 @@ export default class GitPanelController { this.pull = this.pull.bind(this) this.fetch = this.fetch.bind(this) this.commit = this.commit.bind(this) - this.setAmendMode = this.setAmendMode.bind(this) + this.setAmending = this.setAmending.bind(this) this.checkout = this.checkout.bind(this) this.abortMerge = this.abortMerge.bind(this) this.switchRepository(props.repository) @@ -48,7 +48,7 @@ export default class GitPanelController { stageFiles={this.stageFiles} unstageFiles={this.unstageFiles} commit={this.commit} - setAmendMode={this.setAmendMode} + setAmending={this.setAmending} isMerging={this.isMerging} isAmending={this.isAmending} abortMerge={this.abortMerge} @@ -175,7 +175,7 @@ export default class GitPanelController { await this.repository.commit(message, options) } - async setAmendMode (isAmending) { + async setAmending (isAmending) { this.isAmending = isAmending await this._refreshStagedChanges(this.repository) return etch.update(this) diff --git a/lib/views/commit-view.js b/lib/views/commit-view.js index 448af03f59..0248e12833 100644 --- a/lib/views/commit-view.js +++ b/lib/views/commit-view.js @@ -96,8 +96,8 @@ export default class CommitView { async handleAmendBoxClick () { const checked = this.refs.amend.checked - if (this.props.setAmendMode) { - await this.props.setAmendMode(checked) + if (this.props.setAmending) { + await this.props.setAmending(checked) } let updateMessage = true diff --git a/lib/views/git-panel-view.js b/lib/views/git-panel-view.js index e87a8073b5..4569128189 100644 --- a/lib/views/git-panel-view.js +++ b/lib/views/git-panel-view.js @@ -61,7 +61,7 @@ export default class GitPanelView { stagedChangesExist={this.props.stagedChanges.length > 0} mergeConflictsExist={this.props.mergeConflicts.length > 0} commit={this.props.commit} - setAmendMode={this.props.setAmendMode} + setAmending={this.props.setAmending} disableCommitButton={this} abortMerge={this.props.abortMerge} branchName={this.props.branchName} diff --git a/lib/views/staging-view.js b/lib/views/staging-view.js index df37ecbfa3..2bda299b7c 100644 --- a/lib/views/staging-view.js +++ b/lib/views/staging-view.js @@ -225,7 +225,7 @@ export default class StagingView { Staged Changes { this.props.isAmending ? - ` (including ${shortenSha(this.props.lastCommit.sha)})` : + ` (amending ${shortenSha(this.props.lastCommit.sha)})` : '' } diff --git a/test/controllers/git-panel-controller.test.js b/test/controllers/git-panel-controller.test.js index 30ef319a8b..f725a89381 100644 --- a/test/controllers/git-panel-controller.test.js +++ b/test/controllers/git-panel-controller.test.js @@ -63,7 +63,7 @@ describe('GitPanelController', () => { await controller.lastModelDataRefreshPromise assert.deepEqual(controller.refs.gitPanel.props.stagedChanges, []) - await controller.setAmendMode(true) + await controller.setAmending(true) assert.deepEqual( controller.refs.gitPanel.props.stagedChanges, await controller.repository.getStagedChangesSinceParentCommit() diff --git a/test/views/commit-view.test.js b/test/views/commit-view.test.js index 0081d85a4e..0c8edcd3d1 100644 --- a/test/views/commit-view.test.js +++ b/test/views/commit-view.test.js @@ -262,18 +262,18 @@ describe('CommitView', () => { assert.isFalse(amend.checked) }) - it('calls props.setAmendMode() when the box is checked or unchecked', async function () { - const setAmendMode = sinon.spy() + it('calls props.setAmending() when the box is checked or unchecked', async function () { + const setAmending = sinon.spy() const workdirPath = await cloneRepository('three-files') const repository = await buildRepository(workdirPath) - const view = new CommitView({workspace, commandRegistry, stagedChangesExist: false, lastCommit: {message: 'previous commit\'s message'}, setAmendMode}) + const view = new CommitView({workspace, commandRegistry, stagedChangesExist: false, lastCommit: {message: 'previous commit\'s message'}, setAmending}) const {editor, amend} = view.refs amend.click() - assert.deepEqual(setAmendMode.args, [[true]]) + assert.deepEqual(setAmending.args, [[true]]) amend.click() - assert.deepEqual(setAmendMode.args, [[true], [false]]) + assert.deepEqual(setAmending.args, [[true], [false]]) }) }) }) From e272c6241b3adece7244addbb1d8316c8ccb1e87 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 23 Sep 2016 18:06:12 -0700 Subject: [PATCH 09/10] Only use confirm dialog when checking amend with custom message typed --- lib/views/commit-view.js | 38 ++++++++++++++++++---------------- test/views/commit-view.test.js | 30 +++++++++++++++------------ 2 files changed, 37 insertions(+), 31 deletions(-) diff --git a/lib/views/commit-view.js b/lib/views/commit-view.js index 0248e12833..9f3b1b2788 100644 --- a/lib/views/commit-view.js +++ b/lib/views/commit-view.js @@ -95,24 +95,26 @@ export default class CommitView { async handleAmendBoxClick () { const checked = this.refs.amend.checked - - if (this.props.setAmending) { - await this.props.setAmending(checked) - } - - let updateMessage = true - if (this.editor.getText()) { - const choice = atom.confirm({ - message: 'Would you like to discard the current commit message?', - detailedMessage: checked ? 'It will be replaced with the most recent commit message' : '', - buttons: ['Replace', 'Keep current'] - }) - if (choice !== 0) updateMessage = false - } - - if (updateMessage) { - const text = (checked && this.props.lastCommit) ? this.props.lastCommit.message : '' - this.editor.setText(text) + const currentMessage = this.editor.getText() + const lastCommitMessage = this.props.lastCommit ? this.props.lastCommit.message : '' + + if (this.props.setAmending) await this.props.setAmending(checked) + + if (checked) { + if (currentMessage && currentMessage !== lastCommitMessage) { + const choice = atom.confirm({ + message: 'Would you like to discard the current commit message?', + detailedMessage: checked ? 'It will be replaced with the most recent commit message' : '', + buttons: ['Keep current', 'Replace'] + }) + if (choice !== 0) { + this.editor.setText(lastCommitMessage) + } + } else { + this.editor.setText(lastCommitMessage) + } + } else if (currentMessage === lastCommitMessage) { + this.editor.setText('') } return etch.update(this) diff --git a/test/views/commit-view.test.js b/test/views/commit-view.test.js index 0c8edcd3d1..0808287e93 100644 --- a/test/views/commit-view.test.js +++ b/test/views/commit-view.test.js @@ -216,36 +216,40 @@ describe('CommitView', () => { const view = new CommitView({workspace, commandRegistry, stagedChangesExist: false, lastCommit: {message: 'previous commit\'s message'}}) const {editor, amend} = view.refs - // choose to keep current commit message when prompted - confirmChoice = 1 editor.setText('some commit message') - await view.update({repository, stagedChangesExist: true}) + + // When checking amend with a commit message typed, prompt for whether + // to keep the current message or replace it with the last commit's message. + confirmChoice = 0 assert.isFalse(amend.checked) + assert.equal(editor.getText(), 'some commit message') + amend.click() assert.isTrue(amend.checked) + assert.equal(atom.confirm.callCount, 1) assert.equal(editor.getText(), 'some commit message') - editor.setText('some commit message') - + // When unchecking amend with a different commit message typed, keep the + // current message without prompting. amend.click() assert.isFalse(amend.checked) + assert.equal(atom.confirm.callCount, 1) assert.equal(editor.getText(), 'some commit message') - // choose to discard current commit message when prompted - confirmChoice = 0 - editor.setText('some commit message') - - await view.update({repository, stagedChangesExist: true}) - assert.isFalse(amend.checked) + // When checking amend and choosing to discard the current message, replace + // the message with the last commit's message. + confirmChoice = 1 amend.click() assert.isTrue(amend.checked) + assert.equal(atom.confirm.callCount, 2) assert.equal(editor.getText(), 'previous commit\'s message') - editor.setText('some commit message') - + // When unchecking amend with the previous commit's message typed, clear + // the message box without prompting. amend.click() assert.isFalse(amend.checked) + assert.equal(atom.confirm.callCount, 2) assert.equal(editor.getText(), '') }) From 87c472fe4749640de399e790247dca1d7bac6a87 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 23 Sep 2016 20:48:20 -0700 Subject: [PATCH 10/10] Close staged diff views when switching to/from amend mode --- lib/controllers/git-panel-controller.js | 3 ++- lib/github-package.js | 10 +++++++++- test/controllers/git-panel-controller.test.js | 5 ++++- test/github-package.test.js | 18 ++++++++++++++++++ 4 files changed, 33 insertions(+), 3 deletions(-) diff --git a/lib/controllers/git-panel-controller.js b/lib/controllers/git-panel-controller.js index 5c447601fe..40c654ee61 100644 --- a/lib/controllers/git-panel-controller.js +++ b/lib/controllers/git-panel-controller.js @@ -171,11 +171,12 @@ export default class GitPanelController { } async commit (message, options) { - this.isAmending = false await this.repository.commit(message, options) + this.setAmending(false) } async setAmending (isAmending) { + if (this.props.didChangeAmending) this.props.didChangeAmending() this.isAmending = isAmending await this._refreshStagedChanges(this.repository) return etch.update(this) diff --git a/lib/github-package.js b/lib/github-package.js index 4836e77b10..01fdbf7846 100644 --- a/lib/github-package.js +++ b/lib/github-package.js @@ -28,7 +28,8 @@ export default class GithubPackage { commandRegistry, notificationManager, didSelectFilePatch: this.didSelectFilePatch.bind(this), - didSelectMergeConflictFile: this.didSelectMergeConflictFile.bind(this) + didSelectMergeConflictFile: this.didSelectMergeConflictFile.bind(this), + didChangeAmending: this.didChangeAmending.bind(this) }) this.changedFilesCountController = new ChangedFilesCountController({ repository: null, didClick: this.toggleGitPanel.bind(this) @@ -97,6 +98,13 @@ export default class GithubPackage { } } + didChangeAmending () { + if (this.filePatchController && this.filePatchController.props.stagingStatus === 'staged') { + this.filePatchController.destroy() + this.filePatchController = null + } + } + toggleGitPanel () { if (this.workspaceGitPanel) { this.workspaceGitPanel.destroy() diff --git a/test/controllers/git-panel-controller.test.js b/test/controllers/git-panel-controller.test.js index f725a89381..f340a3bea0 100644 --- a/test/controllers/git-panel-controller.test.js +++ b/test/controllers/git-panel-controller.test.js @@ -57,13 +57,16 @@ describe('GitPanelController', () => { }) it('displays the staged changes since the parent commmit when amending', async function () { + const didChangeAmending = sinon.spy() const workdirPath = await cloneRepository('multiple-commits') const repository = await buildRepository(workdirPath) - const controller = new GitPanelController({workspace, commandRegistry, repository}) + const controller = new GitPanelController({workspace, commandRegistry, repository, didChangeAmending}) await controller.lastModelDataRefreshPromise assert.deepEqual(controller.refs.gitPanel.props.stagedChanges, []) + assert.equal(didChangeAmending.callCount, 0) await controller.setAmending(true) + assert.equal(didChangeAmending.callCount, 1) assert.deepEqual( controller.refs.gitPanel.props.stagedChanges, await controller.repository.getStagedChangesSinceParentCommit() diff --git a/test/github-package.test.js b/test/github-package.test.js index a9554fa7ec..83e13a6b9b 100644 --- a/test/github-package.test.js +++ b/test/github-package.test.js @@ -214,6 +214,24 @@ describe('GithubPackage', () => { }) }) + describe('when amend mode is toggled in the staging panel while viewing a staged change', () => { + it('closes the file patch pane item', async () => { + const workdirPath = await cloneRepository('three-files') + const repository = await buildRepository(workdirPath) + + githubPackage.getActiveRepository = function () { return repository } + const hunk = new Hunk(1, 1, 1, 3, []) + const filePatch = new FilePatch('a.txt', 'a.txt', 'modified', [hunk]) + + githubPackage.gitPanelController.props.didSelectFilePatch(filePatch, 'staged') + assert.equal(workspace.getActivePaneItem(), githubPackage.filePatchController) + + githubPackage.gitPanelController.props.didChangeAmending() + assert.isNull(githubPackage.filePatchController) + assert.isUndefined(workspace.getActivePaneItem()) + }) + }) + describe('when the changed files label in the status bar is clicked', () => { it('toggles the git panel', async () => { const workdirPath = await cloneRepository('three-files')