Merge 3a3c68a into 26c39ee

.travis.yml

@@ -61,5 +61,5 @@ before_script:
 script:
 - ./script/cibuild
 
-after_success:
+after_script:
 - npm run coveralls

docs/react-component-classification.md

@@ -6,7 +6,7 @@ This is a high-level summary of the organization and implementation of our React
 
 **Items** are intended to be used as top-level components within subtrees that are rendered into some [Portal](https://reactjs.org/docs/portals.html) and passed to the Atom API, like pane items, dock items, or tooltips. They are mostly responsible for implementing the [Atom "item" contract](https://github.com/atom/atom/blob/a3631f0dafac146185289ac5e37eaff17b8b0209/src/workspace.js#L29-L174).
 
-These live within [`lib/items/`](/lib/items), are tested within [`test/items/`](/test/items), and are named with an `Item` suffix. Examples: `PullRequestDetailItem`, `FilePatchItem`.
+These live within [`lib/items/`](/lib/items), are tested within [`test/items/`](/test/items), and are named with an `Item` suffix. Examples: `PullRequestDetailItem`, `ChangedFileItem`.
 
 ## Containers
 

keymaps/git.cson

@@ -26,6 +26,11 @@
   'shift-tab': 'core:focus-previous'
   'o': 'github:open-file'
   'left': 'core:move-left'
+  'cmd-left': 'core:move-left'
+
+'.github-CommitView button':
+  'tab': 'core:focus-next'
+  'shift-tab': 'core:focus-previous'
 
 '.github-StagingView.unstaged-changes-focused':
   'cmd-backspace': 'github:discard-changes-in-selected-files'

lib/atom/uri-pattern.js

@@ -246,7 +246,7 @@ function dashEscape(raw) {
  * Reverse the escaping performed by `dashEscape` by un-doubling `-` characters.
  */
 function dashUnescape(escaped) {
-  return escaped.replace('--', '-');
+  return escaped.replace(/--/g, '-');
 }
 
 /**

lib/containers/file-patch-container.js → lib/containers/changed-file-container.js

@@ -5,9 +5,9 @@ import yubikiri from 'yubikiri';
 import {autobind} from '../helpers';
 import ObserveModel from '../views/observe-model';
 import LoadingView from '../views/loading-view';
-import FilePatchController from '../controllers/file-patch-controller';
+import MultiFilePatchController from '../controllers/multi-file-patch-controller';
 
-export default class FilePatchContainer extends React.Component {
+export default class ChangedFileContainer extends React.Component {
   static propTypes = {
     repository: PropTypes.object.isRequired,
     stagingStatus: PropTypes.oneOf(['staged', 'unstaged']),
@@ -30,8 +30,10 @@ export default class FilePatchContainer extends React.Component {
   }
 
   fetchData(repository) {
+    const staged = this.props.stagingStatus === 'staged';
+
     return yubikiri({
-      filePatch: repository.getFilePatchForPath(this.props.relPath, {staged: this.props.stagingStatus === 'staged'}),
+      multiFilePatch: repository.getFilePatchForPath(this.props.relPath, {staged}),
       isPartiallyStaged: repository.isPartiallyStaged(this.props.relPath),
       hasUndoHistory: repository.hasDiscardHistory(this.props.relPath),
     });
@@ -51,9 +53,8 @@ export default class FilePatchContainer extends React.Component {
     }
 
     return (
-      <FilePatchController
-        filePatch={data.filePatch}
-        isPartiallyStaged={data.isPartiallyStaged}
+      <MultiFilePatchController
+        multiFilePatch={data.multiFilePatch}
         hasUndoHistory={data.hasUndoHistory}
         {...this.props}
       />

lib/containers/commit-preview-container.js

@@ -0,0 +1,41 @@
+import React from 'react';
+import PropTypes from 'prop-types';
+import yubikiri from 'yubikiri';
+
+import ObserveModel from '../views/observe-model';
+import LoadingView from '../views/loading-view';
+import MultiFilePatchController from '../controllers/multi-file-patch-controller';
+
+export default class CommitPreviewContainer extends React.Component {
+  static propTypes = {
+    repository: PropTypes.object.isRequired,
+  }
+
+  fetchData = repository => {
+    return yubikiri({
+      multiFilePatch: repository.getStagedChangesPatch(),
+    });
+  }
+
+  render() {
+    return (
+      <ObserveModel model={this.props.repository} fetchData={this.fetchData}>
+        {this.renderResult}
+      </ObserveModel>
+    );
+  }
+
+  renderResult = data => {
+    if (this.props.repository.isLoading() || data === null) {
+      return <LoadingView />;
+    }
+
+    return (
+      <MultiFilePatchController
+        stagingStatus={'staged'}
+        {...data}
+        {...this.props}
+      />
+    );
+  }
+}

lib/controllers/commit-controller.js

@@ -8,7 +8,9 @@ import fs from 'fs-extra';
 
 import CommitView from '../views/commit-view';
 import RefHolder from '../models/ref-holder';
+import CommitPreviewItem from '../items/commit-preview-item';
 import {AuthorPropType, UserStorePropType} from '../prop-types';
+import {watchWorkspaceItem} from '../watch-workspace-item';
 import {autobind} from '../helpers';
 import {addEvent} from '../reporter-proxy';
 
@@ -42,7 +44,8 @@ export default class CommitController extends React.Component {
 
   constructor(props, context) {
     super(props, context);
-    autobind(this, 'commit', 'handleMessageChange', 'toggleExpandedCommitMessageEditor', 'grammarAdded');
+    autobind(this, 'commit', 'handleMessageChange', 'toggleExpandedCommitMessageEditor', 'grammarAdded',
+      'toggleCommitPreview');
 
     this.subscriptions = new CompositeDisposable();
     this.refCommitView = new RefHolder();
@@ -51,6 +54,15 @@ export default class CommitController extends React.Component {
     this.subscriptions.add(
       this.commitMessageBuffer.onDidChange(this.handleMessageChange),
     );
+
+    this.previewWatcher = watchWorkspaceItem(
+      this.props.workspace,
+      CommitPreviewItem.buildURI(this.props.repository.getWorkingDirectoryPath()),
+      this,
+      'commitPreviewActive',
+      {active: true},
+    );
+    this.subscriptions.add(this.previewWatcher);
   }
 
   componentDidMount() {
@@ -105,12 +117,20 @@ export default class CommitController extends React.Component {
         userStore={this.props.userStore}
         selectedCoAuthors={this.props.selectedCoAuthors}
         updateSelectedCoAuthors={this.props.updateSelectedCoAuthors}
+        toggleCommitPreview={this.toggleCommitPreview}
+        commitPreviewActive={this.state.commitPreviewActive}
       />
     );
   }
 
   componentDidUpdate(prevProps) {
     this.commitMessageBuffer.setTextViaDiff(this.getCommitMessage());
+
+    if (prevProps.repository !== this.props.repository) {
+      this.previewWatcher.setPattern(
+        CommitPreviewItem.buildURI(this.props.repository.getWorkingDirectoryPath()),
+      );
+    }
   }
 
   componentWillUnmount() {
@@ -240,12 +260,26 @@ export default class CommitController extends React.Component {
     return this.refCommitView.map(view => view.setFocus(focus)).getOr(false);
   }
 
-  hasFocus() {
-    return this.refCommitView.map(view => view.hasFocus()).getOr(false);
+  advanceFocus(...args) {
+    return this.refCommitView.map(view => view.advanceFocus(...args)).getOr(false);
+  }
+
+  retreatFocus(...args) {
+    return this.refCommitView.map(view => view.retreatFocus(...args)).getOr(false);
+  }
+
+  hasFocusAtBeginning() {
+    return this.refCommitView.map(view => view.hasFocusAtBeginning()).getOr(false);
   }
 
-  hasFocusEditor() {
-    return this.refCommitView.map(view => view.hasFocusEditor()).getOr(false);
+  toggleCommitPreview() {
+    addEvent('toggle-commit-preview', {package: 'github'});
+    const uri = CommitPreviewItem.buildURI(this.props.repository.getWorkingDirectoryPath());
+    if (this.props.workspace.hide(uri)) {
+      return Promise.resolve();
+    } else {
+      return this.props.workspace.open(uri, {searchAllPanes: true, pending: true});
+    }
   }
 }
 

lib/controllers/github-tab-controller.js

@@ -19,7 +19,7 @@ export default class GitHubTabController extends React.Component {
     allRemotes: RemoteSetPropType.isRequired,
     branches: BranchSetPropType.isRequired,
     selectedRemoteName: PropTypes.string,
-    aheadCount: PropTypes.number.isRequired,
+    aheadCount: PropTypes.number,
     pushInProgress: PropTypes.bool.isRequired,
     isLoading: PropTypes.bool.isRequired,
   }

lib/controllers/file-patch-controller.js → lib/controllers/multi-file-patch-controller.js

@@ -4,16 +4,16 @@ import path from 'path';
 
 import {autobind, equalSets} from '../helpers';
 import {addEvent} from '../reporter-proxy';
-import FilePatchItem from '../items/file-patch-item';
-import FilePatchView from '../views/file-patch-view';
+import {MultiFilePatchPropType} from '../prop-types';
+import ChangedFileItem from '../items/changed-file-item';
+import MultiFilePatchView from '../views/multi-file-patch-view';
 
-export default class FilePatchController extends React.Component {
+export default class MultiFilePatchController extends React.Component {
   static propTypes = {
     repository: PropTypes.object.isRequired,
     stagingStatus: PropTypes.oneOf(['staged', 'unstaged']),
-    relPath: PropTypes.string.isRequired,
-    filePatch: PropTypes.object.isRequired,
-    hasUndoHistory: PropTypes.bool.isRequired,
+    multiFilePatch: MultiFilePatchPropType.isRequired,
+    hasUndoHistory: PropTypes.bool,
 
     workspace: PropTypes.object.isRequired,
     commands: PropTypes.object.isRequired,
@@ -22,9 +22,10 @@ export default class FilePatchController extends React.Component {
     config: PropTypes.object.isRequired,
 
     destroy: PropTypes.func.isRequired,
-    discardLines: PropTypes.func.isRequired,
-    undoLastDiscard: PropTypes.func.isRequired,
-    surfaceFileAtPath: PropTypes.func.isRequired,
+    discardLines: PropTypes.func,
+    undoLastDiscard: PropTypes.func,
+    surfaceFileAtPath: PropTypes.func,
+    handleClick: PropTypes.func,
   }
 
   constructor(props) {
@@ -37,7 +38,7 @@ export default class FilePatchController extends React.Component {
     );
 
     this.state = {
-      lastFilePatch: this.props.filePatch,
+      lastMultiFilePatch: this.props.multiFilePatch,
       selectionMode: 'hunk',
       selectedRows: new Set(),
     };
@@ -51,7 +52,7 @@ export default class FilePatchController extends React.Component {
   }
 
   componentDidUpdate(prevProps) {
-    if (prevProps.filePatch !== this.props.filePatch) {
+    if (prevProps.multiFilePatch !== this.props.multiFilePatch) {
       this.resolvePatchChangePromise();
       this.patchChangePromise = new Promise(resolve => {
         this.resolvePatchChangePromise = resolve;
@@ -61,7 +62,7 @@ export default class FilePatchController extends React.Component {
 
   render() {
     return (
-      <FilePatchView
+      <MultiFilePatchView
         {...this.props}
 
         selectedRows={this.state.selectedRows}
@@ -83,31 +84,32 @@ export default class FilePatchController extends React.Component {
     );
   }
 
-  undoLastDiscard({eventSource} = {}) {
+  undoLastDiscard(filePatch, {eventSource} = {}) {
     addEvent('undo-last-discard', {
       package: 'github',
-      component: 'FilePatchController',
+      component: this.constructor.name,
       eventSource,
     });
 
-    return this.props.undoLastDiscard(this.props.relPath, this.props.repository);
+
+    return this.props.undoLastDiscard(filePatch.getPath(), this.props.repository);
   }
 
-  diveIntoMirrorPatch() {
+  diveIntoMirrorPatch(filePatch) {
     const mirrorStatus = this.withStagingStatus({staged: 'unstaged', unstaged: 'staged'});
     const workingDirectory = this.props.repository.getWorkingDirectoryPath();
-    const uri = FilePatchItem.buildURI(this.props.relPath, workingDirectory, mirrorStatus);
+    const uri = ChangedFileItem.buildURI(filePatch.getPath(), workingDirectory, mirrorStatus);
 
     this.props.destroy();
     return this.props.workspace.open(uri);
   }
 
-  surfaceFile() {
-    return this.props.surfaceFileAtPath(this.props.relPath, this.props.stagingStatus);
+  surfaceFile(filePatch) {
+    return this.props.surfaceFileAtPath(filePatch.getPath(), this.props.stagingStatus);
   }
 
-  async openFile(positions) {
-    const absolutePath = path.join(this.props.repository.getWorkingDirectoryPath(), this.props.relPath);
+  async openFile(filePatch, positions) {
+    const absolutePath = path.join(this.props.repository.getWorkingDirectoryPath(), filePatch.getPath());
     const editor = await this.props.workspace.open(absolutePath, {pending: true});
     if (positions.length > 0) {
       editor.setCursorBufferPosition(positions[0], {autoscroll: false});
@@ -119,10 +121,10 @@ export default class FilePatchController extends React.Component {
     return editor;
   }
 
-  toggleFile() {
+  toggleFile(filePatch) {
     return this.stagingOperation(() => {
       const methodName = this.withStagingStatus({staged: 'unstageFiles', unstaged: 'stageFiles'});
-      return this.props.repository[methodName]([this.props.relPath]);
+      return this.props.repository[methodName]([filePatch.getPath()]);
     });
   }
 
@@ -140,26 +142,27 @@ export default class FilePatchController extends React.Component {
 
     return this.stagingOperation(() => {
       const patch = this.withStagingStatus({
-        staged: () => this.props.filePatch.getUnstagePatchForLines(chosenRows),
-        unstaged: () => this.props.filePatch.getStagePatchForLines(chosenRows),
+        staged: () => this.props.multiFilePatch.getUnstagePatchForLines(chosenRows),
+        unstaged: () => this.props.multiFilePatch.getStagePatchForLines(chosenRows),
       });
       return this.props.repository.applyPatchToIndex(patch);
     });
   }
 
-  toggleModeChange() {
+  toggleModeChange(filePatch) {
     return this.stagingOperation(() => {
       const targetMode = this.withStagingStatus({
-        unstaged: this.props.filePatch.getNewMode(),
-        staged: this.props.filePatch.getOldMode(),
+        unstaged: filePatch.getNewMode(),
+        staged: filePatch.getOldMode(),
       });
-      return this.props.repository.stageFileModeChange(this.props.relPath, targetMode);
+      return this.props.repository.stageFileModeChange(filePatch.getPath(), targetMode);
     });
   }
 
-  toggleSymlinkChange() {
+  toggleSymlinkChange(filePatch) {
     return this.stagingOperation(() => {
-      const {filePatch, relPath, repository} = this.props;
+      const relPath = filePatch.getPath();
+      const repository = this.props.repository;
       return this.withStagingStatus({
         unstaged: () => {
           if (filePatch.hasTypechange() && filePatch.getStatus() === 'added') {
@@ -180,6 +183,14 @@ export default class FilePatchController extends React.Component {
   }
 
   async discardRows(rowSet, nextSelectionMode, {eventSource} = {}) {
+    // (kuychaco) For now we only support discarding rows for MultiFilePatches that contain a single file patch
+    // The only way to access this method from the UI is to be in a ChangedFileItem, which only has a single file patch
+    // This check is duplicated in RootController#discardLines. We also want it here to prevent us from sending metrics
+    // unnecessarily
+    if (this.props.multiFilePatch.getFilePatches().length !== 1) {
+      return Promise.resolve(null);
+    }
+
     let chosenRows = rowSet;
     if (chosenRows) {
       await this.selectedRowsChanged(chosenRows, nextSelectionMode);
@@ -189,12 +200,12 @@ export default class FilePatchController extends React.Component {
 
     addEvent('discard-unstaged-changes', {
       package: 'github',
-      component: 'FilePatchController',
+      component: this.constructor.name,
       lineCount: chosenRows.size,
       eventSource,
     });
 
-    return this.props.discardLines(this.props.filePatch, chosenRows, this.props.repository);
+    return this.props.discardLines(this.props.multiFilePatch, chosenRows, this.props.repository);
   }
 
   selectedRowsChanged(rows, nextSelectionMode) {