diff --git a/keymaps/git.cson b/keymaps/git.cson index 014ba5453f..2ee2bdcfcd 100644 --- a/keymaps/git.cson +++ b/keymaps/git.cson @@ -2,6 +2,10 @@ 'ctrl-9': 'github:toggle-git-panel' 'ctrl-(': 'github:toggle-git-panel-focus' # ctrl-shift-9 +'.github-Panel': + 'cmd-enter': 'github:commit' + 'ctrl-enter': 'github:commit' + '.github-StagingView': 'left': 'github:focus-diff-view' 'tab': 'core:focus-next' diff --git a/lib/controllers/commit-view-controller.js b/lib/controllers/commit-view-controller.js index 155a7dfbe8..4136ba8904 100644 --- a/lib/controllers/commit-view-controller.js +++ b/lib/controllers/commit-view-controller.js @@ -37,20 +37,14 @@ export default class CommitViewController { } render() { - let message = this.regularCommitMessage; - if (this.props.isAmending) { - message = this.amendingCommitMessage; - if (!message.length && this.props.lastCommit) { - message = this.props.lastCommit.message; - } - } else if (!message.length && this.props.mergeMessage) { - message = this.props.mergeMessage; - } + const message = this.getCommitMessage(); + return ( @@ -257,6 +259,18 @@ export default class GitController extends React.Component { return this.gitPanelController.getWrappedComponent().isFocused(); } + // Ensure that the Git panel is visible. Returns a Promise that resolves to `true` if the panel was initially + // hidden or `false` if it was already shown. + ensureGitPanel() { + if (!this.state.gitPanelActive) { + return new Promise((resolve, reject) => { + this.setState({gitPanelActive: true}, () => resolve(true)); + }); + } + + return Promise.resolve(false); + } + focusFilePatchView() { this.filePatchController.getWrappedComponent().focus(); } diff --git a/lib/controllers/git-panel-controller.js b/lib/controllers/git-panel-controller.js index 4324c118b9..50a1e3af38 100644 --- a/lib/controllers/git-panel-controller.js +++ b/lib/controllers/git-panel-controller.js @@ -14,6 +14,7 @@ export default class GitPanelController { this.unstageFilePatch = this.unstageFilePatch.bind(this); this.attemptFileStageOperation = this.attemptFileStageOperation.bind(this); + this.prepareToCommit = this.prepareToCommit.bind(this); this.commit = this.commit.bind(this); this.setAmending = this.setAmending.bind(this); this.checkout = this.checkout.bind(this); @@ -44,6 +45,7 @@ export default class GitPanelController { stageFilePatch={this.stageFilePatch} unstageFilePatch={this.unstageFilePatch} attemptFileStageOperation={this.attemptFileStageOperation} + prepareToCommit={this.prepareToCommit} commit={this.commit} setAmending={this.setAmending} isAmending={this.props.isAmending} @@ -172,6 +174,10 @@ export default class GitPanelController { } } + async prepareToCommit() { + return !await this.props.ensureGitPanel(); + } + async commit(message) { try { await this.getActiveRepository().commit(message, {amend: this.props.isAmending}); diff --git a/lib/views/commit-view.js b/lib/views/commit-view.js index d9d4712200..421abb5e55 100644 --- a/lib/views/commit-view.js +++ b/lib/views/commit-view.js @@ -12,11 +12,10 @@ export default class CommitView { constructor(props) { this.props = props; - this.commit = this.commit.bind(this); - this.abortMerge = this.abortMerge.bind(this); this.handleAmendBoxClick = this.handleAmendBoxClick.bind(this); this.commit = this.commit.bind(this); this.abortMerge = this.abortMerge.bind(this); + etch.initialize(this); this.editor = this.refs.editor; @@ -26,7 +25,7 @@ export default class CommitView { this.subscriptions = new CompositeDisposable( this.editor.onDidChange(() => this.props.onChangeMessage && this.props.onChangeMessage(this.editor.getText())), this.editor.onDidChangeCursorPosition(() => { etch.update(this); }), - props.commandRegistry.add(this.element, {'github:commit': this.commit}), + props.commandRegistry.add('atom-workspace', {'github:commit': this.commit}), ); const grammar = atom.grammars.grammarForScopeName(COMMIT_GRAMMAR_SCOPE); @@ -115,9 +114,11 @@ export default class CommitView { this.props.setAmending(this.refs.amend.checked); } - commit() { - if (this.isCommitButtonEnabled()) { + async commit() { + if (await this.props.prepareToCommit() && this.isCommitButtonEnabled()) { this.props.commit(this.editor.getText()); + } else { + this.focus(); } } diff --git a/lib/views/git-panel-view.js b/lib/views/git-panel-view.js index 64ec419be3..4e74a47d16 100644 --- a/lib/views/git-panel-view.js +++ b/lib/views/git-panel-view.js @@ -68,6 +68,7 @@ export default class GitPanelView { ref="commitViewController" stagedChangesExist={this.props.stagedChanges.length > 0} mergeConflictsExist={this.props.mergeConflicts.length > 0} + prepareToCommit={this.props.prepareToCommit} commit={this.props.commit} amending={this.props.amending} setAmending={this.props.setAmending} diff --git a/test/controllers/git-controller.test.js b/test/controllers/git-controller.test.js index 417a22e0cb..46cce443ff 100644 --- a/test/controllers/git-controller.test.js +++ b/test/controllers/git-controller.test.js @@ -18,6 +18,7 @@ describe('GitController', () => { workspace = atomEnv.workspace; commandRegistry = atomEnv.commands; notificationManager = atomEnv.notifications; + app = ( { }); }); + describe('ensureGitPanel()', () => { + let wrapper; + + beforeEach(async () => { + const workdirPath = await cloneRepository('multiple-commits'); + const repository = await buildRepository(workdirPath); + + app = React.cloneElement(app, {repository}); + wrapper = shallow(app); + }); + + it('opens the Git panel when it is initially closed', async () => { + assert.isFalse(wrapper.find('Panel').prop('visible')); + assert.isTrue(await wrapper.instance().ensureGitPanel()); + }); + + it('does nothing when the Git panel is already open', async () => { + wrapper.instance().toggleGitPanel(); + assert.isTrue(wrapper.find('Panel').prop('visible')); + assert.isFalse(await wrapper.instance().ensureGitPanel()); + assert.isTrue(wrapper.find('Panel').prop('visible')); + }); + }); + it('correctly updates state when switching repos', async () => { const workdirPath1 = await cloneRepository('three-files'); const repository1 = await buildRepository(workdirPath1); diff --git a/test/controllers/git-panel-controller.test.js b/test/controllers/git-panel-controller.test.js index eafa62d29e..54e587e61b 100644 --- a/test/controllers/git-panel-controller.test.js +++ b/test/controllers/git-panel-controller.test.js @@ -2,22 +2,25 @@ import fs from 'fs'; import path from 'path'; +import etch from 'etch'; import dedent from 'dedent-js'; import GitPanelController from '../../lib/controllers/git-panel-controller'; -import {cloneRepository, buildRepository} from '../helpers'; +import {cloneRepository, buildRepository, until} from '../helpers'; import {AbortMergeError, CommitError} from '../../lib/models/repository'; describe('GitPanelController', () => { - let atomEnvironment, workspace, commandRegistry, notificationManager; + let atomEnvironment, workspace, workspaceElement, commandRegistry, notificationManager; beforeEach(() => { atomEnvironment = global.buildAtomEnvironment(); workspace = atomEnvironment.workspace; commandRegistry = atomEnvironment.commands; notificationManager = atomEnvironment.notifications; + + workspaceElement = atomEnvironment.views.getView(workspace); }); afterEach(() => { @@ -76,11 +79,12 @@ describe('GitPanelController', () => { assert.deepEqual(controller.refs.gitPanel.props.unstagedChanges, await repository2.getUnstagedChanges()); }); - it('displays the staged changes since the parent commmit when amending', async () => { + it('displays the staged changes since the parent commit when amending', async () => { const didChangeAmending = sinon.spy(); const workdirPath = await cloneRepository('multiple-commits'); const repository = await buildRepository(workdirPath); - const controller = new GitPanelController({workspace, commandRegistry, repository, didChangeAmending, isAmending: false}); + const ensureGitPanel = () => Promise.resolve(false); + const controller = new GitPanelController({workspace, commandRegistry, repository, didChangeAmending, ensureGitPanel, isAmending: false}); await controller.getLastModelDataRefreshPromise(); assert.deepEqual(controller.refs.gitPanel.props.stagedChanges, []); assert.equal(didChangeAmending.callCount, 0); @@ -140,6 +144,28 @@ describe('GitPanelController', () => { }); }); + describe('prepareToCommit', () => { + it('shows the git panel and returns false if it was hidden', async () => { + const workdirPath = await cloneRepository('three-files'); + const repository = await buildRepository(workdirPath); + + const ensureGitPanel = () => Promise.resolve(true); + const controller = new GitPanelController({workspace, commandRegistry, repository, ensureGitPanel}); + + assert.isFalse(await controller.prepareToCommit()); + }); + + it('returns true if the git panel was already visible', async () => { + const workdirPath = await cloneRepository('three-files'); + const repository = await buildRepository(workdirPath); + + const ensureGitPanel = () => Promise.resolve(false); + const controller = new GitPanelController({workspace, commandRegistry, repository, ensureGitPanel}); + + assert.isTrue(await controller.prepareToCommit()); + }); + }); + describe('commit(message)', () => { it('shows an error notification when committing throws an ECONFLICT exception', async () => { const workdirPath = await cloneRepository('three-files'); @@ -196,29 +222,7 @@ describe('GitPanelController', () => { describe('keyboard navigation commands', () => { let controller, gitPanel, stagingView, commitView, commitViewController, focusElement; - beforeEach(async () => { - const workdirPath = await cloneRepository('each-staging-group'); - const repository = await buildRepository(workdirPath); - - // Merge with conflicts - assert.isRejected(repository.git.merge('origin/branch')); - - // Three unstaged files - fs.writeFileSync(path.join(workdirPath, 'unstaged-1.txt'), 'This is an unstaged file.'); - fs.writeFileSync(path.join(workdirPath, 'unstaged-2.txt'), 'This is an unstaged file.'); - fs.writeFileSync(path.join(workdirPath, 'unstaged-3.txt'), 'This is an unstaged file.'); - - // Three staged files - fs.writeFileSync(path.join(workdirPath, 'staged-1.txt'), 'This is a file with some changes staged for commit.'); - fs.writeFileSync(path.join(workdirPath, 'staged-2.txt'), 'This is another file staged for commit.'); - fs.writeFileSync(path.join(workdirPath, 'staged-3.txt'), 'This is a third file staged for commit.'); - await repository.stageFiles(['staged-1.txt', 'staged-2.txt', 'staged-3.txt']); - - await repository.refresh(); - - controller = new GitPanelController({workspace, commandRegistry, repository}); - await controller.getLastModelDataRefreshPromise(); - + const extractReferences = () => { gitPanel = controller.refs.gitPanel; stagingView = gitPanel.refs.stagingView; commitViewController = gitPanel.refs.commitViewController; @@ -228,55 +232,125 @@ describe('GitPanelController', () => { sinon.stub(commitView, 'focus', () => { focusElement = commitView; }); sinon.stub(commitView, 'isFocused', () => focusElement === commitView); sinon.stub(stagingView, 'focus', () => { focusElement = stagingView; }); - }); + }; const assertSelected = paths => { const selectionPaths = Array.from(stagingView.selection.getSelectedItems()).map(item => item.filePath); assert.deepEqual(selectionPaths, paths); }; - it('blurs on tool-panel:unfocus', () => { - sinon.spy(workspace.getActivePane(), 'activate'); + describe('with conflicts and staged files', () => { + beforeEach(async () => { + const workdirPath = await cloneRepository('each-staging-group'); + const repository = await buildRepository(workdirPath); - commandRegistry.dispatch(controller.element, 'tool-panel:unfocus'); + // Merge with conflicts + assert.isRejected(repository.git.merge('origin/branch')); - assert.isTrue(workspace.getActivePane().activate.called); - }); + fs.writeFileSync(path.join(workdirPath, 'unstaged-1.txt'), 'This is an unstaged file.'); + fs.writeFileSync(path.join(workdirPath, 'unstaged-2.txt'), 'This is an unstaged file.'); + fs.writeFileSync(path.join(workdirPath, 'unstaged-3.txt'), 'This is an unstaged file.'); - it('advances focus through StagingView groups and CommitView, but does not cycle', () => { - assertSelected(['unstaged-1.txt']); + // Three staged files + fs.writeFileSync(path.join(workdirPath, 'staged-1.txt'), 'This is a file with some changes staged for commit.'); + fs.writeFileSync(path.join(workdirPath, 'staged-2.txt'), 'This is another file staged for commit.'); + fs.writeFileSync(path.join(workdirPath, 'staged-3.txt'), 'This is a third file staged for commit.'); + await repository.stageFiles(['staged-1.txt', 'staged-2.txt', 'staged-3.txt']); + await repository.refresh(); - commandRegistry.dispatch(controller.element, 'core:focus-next'); - assertSelected(['conflict-1.txt']); + const didChangeAmending = () => {}; - commandRegistry.dispatch(controller.element, 'core:focus-next'); - assertSelected(['staged-1.txt']); + controller = new GitPanelController({workspace, commandRegistry, repository, didChangeAmending}); + await controller.getLastModelDataRefreshPromise(); - commandRegistry.dispatch(controller.element, 'core:focus-next'); - assertSelected(['staged-1.txt']); - assert.strictEqual(focusElement, commitView); + extractReferences(); + }); + + it('blurs on tool-panel:unfocus', () => { + sinon.spy(workspace.getActivePane(), 'activate'); + + commandRegistry.dispatch(controller.element, 'tool-panel:unfocus'); + + assert.isTrue(workspace.getActivePane().activate.called); + }); + + it('advances focus through StagingView groups and CommitView, but does not cycle', () => { + assertSelected(['unstaged-1.txt']); + + commandRegistry.dispatch(controller.element, 'core:focus-next'); + assertSelected(['conflict-1.txt']); + + commandRegistry.dispatch(controller.element, 'core:focus-next'); + assertSelected(['staged-1.txt']); + + commandRegistry.dispatch(controller.element, 'core:focus-next'); + assertSelected(['staged-1.txt']); + assert.strictEqual(focusElement, commitView); + + // This should be a no-op. (Actually, it'll insert a tab in the CommitView editor.) + commandRegistry.dispatch(controller.element, 'core:focus-next'); + assertSelected(['staged-1.txt']); + assert.strictEqual(focusElement, commitView); + }); + + it('retreats focus from the CommitView through StagingView groups, but does not cycle', () => { + commitView.focus(); + + commandRegistry.dispatch(controller.element, 'core:focus-previous'); + assertSelected(['staged-1.txt']); - // This should be a no-op. (Actually, it'll insert a tab in the CommitView editor.) - commandRegistry.dispatch(controller.element, 'core:focus-next'); - assertSelected(['staged-1.txt']); - assert.strictEqual(focusElement, commitView); + commandRegistry.dispatch(controller.element, 'core:focus-previous'); + assertSelected(['conflict-1.txt']); + + commandRegistry.dispatch(controller.element, 'core:focus-previous'); + assertSelected(['unstaged-1.txt']); + + // This should be a no-op. + commandRegistry.dispatch(controller.element, 'core:focus-previous'); + assertSelected(['unstaged-1.txt']); + }); }); - it('retreats focus from the CommitView through StagingView groups, but does not cycle', () => { - commitView.focus(); + describe('with staged changes', () => { + beforeEach(async () => { + const workdirPath = await cloneRepository('each-staging-group'); + const repository = await buildRepository(workdirPath); + + // A staged file + fs.writeFileSync(path.join(workdirPath, 'staged-1.txt'), 'This is a file with some changes staged for commit.'); + await repository.stageFiles(['staged-1.txt']); + await repository.refresh(); + + const didChangeAmending = () => {}; + const prepareToCommit = () => Promise.resolve(true); + const ensureGitPanel = () => Promise.resolve(false); + + controller = new GitPanelController({workspace, commandRegistry, repository, didChangeAmending, prepareToCommit, ensureGitPanel}); + await controller.getLastModelDataRefreshPromise(); - commandRegistry.dispatch(controller.element, 'core:focus-previous'); - assertSelected(['staged-1.txt']); + extractReferences(); + }); + + it('focuses the CommitView on github:commit with an empty commit message', async () => { + commitView.editor.setText(''); + sinon.spy(controller, 'commit'); + await etch.update(controller); // Ensure that the spy is passed to child components in props + + commandRegistry.dispatch(workspaceElement, 'github:commit'); - commandRegistry.dispatch(controller.element, 'core:focus-previous'); - assertSelected(['conflict-1.txt']); + await until('CommitView is focused', () => focusElement === commitView); + assert.isFalse(controller.commit.called); + }); + + it('creates a commit on github:commit with a nonempty commit message', async () => { + commitView.editor.setText('I fixed the things'); + sinon.spy(controller, 'commit'); + await etch.update(controller); // Ensure that the spy is passed to child components in props - commandRegistry.dispatch(controller.element, 'core:focus-previous'); - assertSelected(['unstaged-1.txt']); + commandRegistry.dispatch(workspaceElement, 'github:commit'); - // This should be a no-op. - commandRegistry.dispatch(controller.element, 'core:focus-previous'); - assertSelected(['unstaged-1.txt']); + await until('Commit method called', () => controller.commit.calledWith('I fixed the things')); + }); }); }); @@ -286,7 +360,8 @@ describe('GitPanelController', () => { const repository = await buildRepository(workdirPath); fs.writeFileSync(path.join(workdirPath, 'a.txt'), 'a change\n'); fs.unlinkSync(path.join(workdirPath, 'b.txt')); - const controller = new GitPanelController({workspace, commandRegistry, repository, didChangeAmending: sinon.stub()}); + const ensureGitPanel = () => Promise.resolve(false); + const controller = new GitPanelController({workspace, commandRegistry, repository, ensureGitPanel, didChangeAmending: sinon.stub()}); await controller.getLastModelDataRefreshPromise(); const stagingView = controller.refs.gitPanel.refs.stagingView; const commitView = controller.refs.gitPanel.refs.commitViewController.refs.commitView; diff --git a/test/views/commit-view.test.js b/test/views/commit-view.test.js index b4c9aa978b..d38a6f376d 100644 --- a/test/views/commit-view.test.js +++ b/test/views/commit-view.test.js @@ -1,6 +1,6 @@ /** @babel */ -import {cloneRepository, buildRepository} from '../helpers'; +import {cloneRepository, buildRepository, until} from '../helpers'; import etch from 'etch'; import CommitView from '../../lib/views/commit-view'; @@ -76,63 +76,104 @@ describe('CommitView', () => { assert.equal(view.editor.getGrammar().scopeName, 'text.git-commit'); }); - it('disables the commit button when no changes are staged, there are merge conflict files, or the commit message is empty', async () => { - const workdirPath = await cloneRepository('three-files'); - const repository = await buildRepository(workdirPath); - const viewState = {}; - const view = new CommitView({repository, commandRegistry, stagedChangesExist: false, viewState}); - const {editor, commitButton} = view.refs; - assert.isTrue(commitButton.disabled); + describe('the commit button', () => { + let view, editor, commitButton; - editor.setText('something'); - await etch.getScheduler().getNextUpdatePromise(); - assert.isTrue(commitButton.disabled); + beforeEach(async () => { + const workdirPath = await cloneRepository('three-files'); + const repository = await buildRepository(workdirPath); + const viewState = {}; + view = new CommitView({repository, commandRegistry, stagedChangesExist: true, mergeConflictsExist: false, viewState}); + editor = view.refs.editor; + commitButton = view.refs.commitButton; - await view.update({stagedChangesExist: true}); - assert.isFalse(commitButton.disabled); + editor.setText('something'); + await etch.getScheduler().getNextUpdatePromise(); + }); - await view.update({mergeConflictsExist: true}); - assert.isTrue(commitButton.disabled); + it('is disabled when no changes are staged', async () => { + await view.update({stagedChangesExist: false}); + assert.isTrue(commitButton.disabled); - await view.update({mergeConflictsExist: false}); - assert.isFalse(commitButton.disabled); + await view.update({stagedChangesExist: true}); + assert.isFalse(commitButton.disabled); + }); - editor.setText(''); - await etch.getScheduler().getNextUpdatePromise(); - assert.isTrue(commitButton.disabled); + it('is disabled when there are merge conflicts', async () => { + await view.update({mergeConflictsExist: false}); + assert.isFalse(commitButton.disabled); + + await view.update({mergeConflictsExist: true}); + assert.isTrue(commitButton.disabled); + }); + + it('is disabled when the commit message is empty', async () => { + editor.setText(''); + await etch.getScheduler().getNextUpdatePromise(); + assert.isTrue(commitButton.disabled); + + editor.setText('Not empty'); + await etch.getScheduler().getNextUpdatePromise(); + assert.isFalse(commitButton.disabled); + }); }); - it('calls props.commit(message) when the commit button is clicked or github:commit is dispatched', async () => { - const commit = sinon.spy(); - const view = new CommitView({commandRegistry, stagedChangesExist: false, commit, message: ''}); - const {editor, commitButton} = view.refs; - - // commit by clicking the commit button - await view.update({stagedChangesExist: true, message: 'Commit 1'}); - commitButton.dispatchEvent(new MouseEvent('click')); - assert.equal(commit.args[0][0], 'Commit 1'); - - // undo history is cleared - commandRegistry.dispatch(editor.element, 'core:undo'); - assert.equal(editor.getText(), ''); - - // commit via the github:commit command - commit.reset(); - await view.update({stagedChangesExist: true, message: 'Commit 2'}); - commandRegistry.dispatch(editor.element, 'github:commit'); - assert.equal(commit.args[0][0], 'Commit 2'); - - // disable github:commit when there are no staged changes... - commit.reset(); - await view.update({stagedChangesExist: false, message: 'Commit 4'}); - commandRegistry.dispatch(editor.element, 'github:commit'); - assert.equal(commit.callCount, 0); - - // ...or the commit message is empty - commit.reset(); - await view.update({stagedChangesExist: true, message: ''}); - commandRegistry.dispatch(editor.element, 'github:commit'); - assert.equal(commit.callCount, 0); + describe('committing', () => { + let view, commit, prepareToCommitResolution; + let editor, commitButton, workspaceElement; + + beforeEach(async () => { + const prepareToCommit = () => Promise.resolve(prepareToCommitResolution); + + commit = sinon.spy(); + view = new CommitView({commandRegistry, stagedChangesExist: true, prepareToCommit, commit, message: 'Something'}); + sinon.spy(view, 'focus'); + + editor = view.refs.editor; + commitButton = view.refs.commitButton; + + workspaceElement = atomEnv.views.getView(atomEnv.workspace); + + await view.update(); + }); + + describe('when props.prepareToCommit() resolves true', () => { + beforeEach(() => { prepareToCommitResolution = true; }); + + it('calls props.commit(message) when the commit button is clicked', async () => { + commitButton.dispatchEvent(new MouseEvent('click')); + + await until('props.commit() is called', () => commit.calledWith('Something')); + + // undo history is cleared + commandRegistry.dispatch(editor.element, 'core:undo'); + assert.equal(editor.getText(), ''); + }); + + it('calls props.commit(message) when github:commit is dispatched', async () => { + commandRegistry.dispatch(workspaceElement, 'github:commit'); + + await until('props.commit() is called', () => commit.calledWith('Something')); + }); + }); + + describe('when props.prepareToCommit() resolves false', () => { + beforeEach(() => { prepareToCommitResolution = false; }); + + it('takes no further action when the commit button is clicked', async () => { + commitButton.dispatchEvent(new MouseEvent('click')); + + await until('focus() is called', () => view.focus.called); + assert.isFalse(commit.called); + }); + + it('takes no further action when github:commit is dispatched', async () => { + commandRegistry.dispatch(workspaceElement, 'github:commit'); + + await until('focus() is called', () => view.focus.called); + assert.isFalse(commit.called); + }); + }); }); it('shows the "Abort Merge" button when props.isMerging is true', async () => {