Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
b32fc85
Rename "EventWatcher" to Switchboard
smashwilson Apr 4, 2017
6bf2f49
Rename resolve* methods
smashwilson Apr 4, 2017
c97b0da
Test subscription method triples too
smashwilson Apr 4, 2017
6a68386
:candle: :skull: _metaprogramming_ :skull: :candle:
smashwilson Apr 4, 2017
acf60de
:fire: `.only(`
smashwilson Apr 4, 2017
371404d
Measure successive durations from a starting point
smashwilson Apr 4, 2017
a07b231
Rename some Switchboard variables and add a few events
smashwilson Apr 4, 2017
c4aa4e6
Fuss with the Yardstick API
smashwilson Apr 4, 2017
7135830
Accept a Switchboard in RootController
smashwilson Apr 4, 2017
aa6ba37
Use the Switchboard to measure stuff with Yardstick
smashwilson Apr 4, 2017
1b62b02
Configuration options to tune what and how to measure
smashwilson Apr 5, 2017
76966ef
Capture profiles or log to console
smashwilson Apr 5, 2017
22920db
Completing a render is a pretty common finish point
smashwilson Apr 5, 2017
75c86da
Flush recorded DurationSets to disk at regular intervals
smashwilson Apr 5, 2017
38329fb
Flush final DurationSets to disk during package deactivation
smashwilson Apr 5, 2017
0221e22
Provide some context for didFinishRender events
smashwilson Apr 5, 2017
ef109a7
Only subscribe to onDidUpdate with an actual Repository
smashwilson Apr 5, 2017
e921b38
Apparently that can be `undefined` in tests?
smashwilson Apr 5, 2017
152f8bd
Accept Arrays of series names in .mark() and .finish()
smashwilson Apr 5, 2017
06d18d2
Capture (un)?stage hunk operations and staging operations
smashwilson Apr 5, 2017
79e3be3
Merge branch 'master' into aw-yardstick
smashwilson Apr 5, 2017
dc5a282
Merge branch 'master' into aw-yardstick
smashwilson Apr 5, 2017
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 19 additions & 10 deletions lib/controllers/file-patch-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import React from 'react';
import {Point, Emitter} from 'atom';
import {autobind} from 'core-decorators';

import EventWatcher from '../event-watcher';
import Switchboard from '../switchboard';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Names are hard ¯\(ツ)

import FilePatchView from '../views/file-patch-view';

export default class FilePatchController extends React.Component {
Expand All @@ -20,11 +20,11 @@ export default class FilePatchController extends React.Component {
quietlySelectItem: React.PropTypes.func.isRequired,
undoLastDiscard: React.PropTypes.func.isRequired,
openFiles: React.PropTypes.func.isRequired,
eventWatcher: React.PropTypes.instanceOf(EventWatcher),
switchboard: React.PropTypes.instanceOf(Switchboard),
}

static defaultProps = {
eventWatcher: new EventWatcher(),
switchboard: new Switchboard(),
}

constructor(props, context) {
Expand Down Expand Up @@ -68,7 +68,7 @@ export default class FilePatchController extends React.Component {
attemptHunkStageOperation={this.attemptHunkStageOperation}
didSurfaceFile={this.didSurfaceFile}
didDiveIntoCorrespondingFilePatch={this.diveIntoCorrespondingFilePatch}
eventWatcher={this.props.eventWatcher}
switchboard={this.props.switchboard}
openCurrentFile={this.openCurrentFile}
discardLines={this.props.discardLines}
undoLastDiscard={this.undoLastDiscard}
Expand All @@ -88,17 +88,22 @@ export default class FilePatchController extends React.Component {
}

async stageHunk(hunk) {
this.props.switchboard.didBeginStageOperation({stage: true, hunk: true});

await this.props.repository.applyPatchToIndex(
this.props.filePatch.getStagePatchForHunk(hunk),
);
this.props.eventWatcher.resolveStageOperationPromise();
this.props.switchboard.didFinishStageOperation({stage: true, hunk: true});
}

async unstageHunk(hunk) {
this.props.switchboard.didBeginStageOperation({unstage: true, hunk: true});
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little conflicted about following this pattern. Specifically, there are two alternatives:

  1. Separate Switchboard methods for every operation. Aggregation of "related" operations is the subscribers' responsibility.
  2. Individual Switchboard methods can be called in multiple related situations. Distinguishing context from the event payloads is the subscribers' responsibility.

On one hand, the metaprogramming I've set up in Switchboard makes it easy to quickly define a large number of events, which makes (1) more tenable. On the other, I feel like that could get out of hand?


await this.props.repository.applyPatchToIndex(
this.props.filePatch.getUnstagePatchForHunk(hunk),
);
this.props.eventWatcher.resolveStageOperationPromise();

this.props.switchboard.didFinishStageOperation({unstage: true, hunk: true});
}

stageOrUnstageHunk(hunk) {
Expand All @@ -118,27 +123,31 @@ export default class FilePatchController extends React.Component {
}

this.stagingOperationInProgress = true;
this.props.eventWatcher.getPatchChangedPromise().then(() => {
this.props.switchboard.getChangePatchPromise().then(() => {
this.stagingOperationInProgress = false;
});

this.stageOrUnstageHunk(hunk);
}

async stageLines(lines) {
this.props.switchboard.didBeginStageOperation({stage: true, line: true});

await this.props.repository.applyPatchToIndex(
this.props.filePatch.getStagePatchForLines(lines),
);

this.props.eventWatcher.resolveStageOperationPromise();
this.props.switchboard.didFinishStageOperation({stage: true, line: true});
}

async unstageLines(lines) {
this.props.switchboard.didBeginStageOperation({unstage: true, line: true});

await this.props.repository.applyPatchToIndex(
this.props.filePatch.getUnstagePatchForLines(lines),
);

this.props.eventWatcher.resolveStageOperationPromise();
this.props.switchboard.didFinishStageOperation({unstage: true, line: true});
}

stageOrUnstageLines(lines) {
Expand All @@ -158,7 +167,7 @@ export default class FilePatchController extends React.Component {
}

this.stagingOperationInProgress = true;
this.props.eventWatcher.getPatchChangedPromise().then(() => {
this.props.switchboard.getChangePatchPromise().then(() => {
this.stagingOperationInProgress = false;
});

Expand Down
10 changes: 9 additions & 1 deletion lib/controllers/root-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import RepositoryConflictController from './repository-conflict-controller';
import ModelObserver from '../models/model-observer';
import ModelStateRegistry from '../models/model-state-registry';
import Conflict from '../models/conflicts/conflict';
import Switchboard from '../switchboard';
import {copyFile, deleteFileOrFolder} from '../helpers';
import {GitError} from '../git-shell-out-strategy';

Expand All @@ -47,10 +48,12 @@ export default class RootController extends React.Component {
repository: React.PropTypes.object,
resolutionProgress: React.PropTypes.object,
statusBar: React.PropTypes.object,
switchboard: React.PropTypes.instanceOf(Switchboard),
savedState: React.PropTypes.object,
}

static defaultProps = {
switchboard: new Switchboard(),
savedState: {},
}

Expand Down Expand Up @@ -211,6 +214,7 @@ export default class RootController extends React.Component {
openFiles={this.openFiles}
discardLines={this.discardLines}
undoLastDiscard={this.undoLastDiscard}
switchboard={this.props.switchboard}
/>
</PaneItem>
</div>
Expand Down Expand Up @@ -316,10 +320,14 @@ export default class RootController extends React.Component {
if (activate && this.filePatchControllerPane) {
this.filePatchControllerPane.activate();
}
this.props.switchboard.didFinishRender('RootController.showFilePatchForPath');
resolve();
});
} else {
this.setState({...nullFilePatchState}, resolve);
this.setState({...nullFilePatchState}, () => {
this.props.switchboard.didFinishRender('RootController.showFilePatchForPath');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like we'll eventually want didFinishRender() calls in every setState(), forceUpdate(), or ReactDom.render() callback. Is there a better way to hook those without muddying up the codebase?

resolve();
});
}
});
}
Expand Down
9 changes: 8 additions & 1 deletion lib/decorators/observe-model.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,14 @@ export default function ObserveModelDecorator(spec) {
fetchData: model => fetchData(model, this.props),
didUpdate: () => {
if (this.mounted) {
this.setState({modelData: this.modelObserver.getActiveModelData()}, this.resolve);
this.setState({modelData: this.modelObserver.getActiveModelData()}, () => {
/* eslint-disable react/prop-types */
if (this.props.switchboard) {
this.props.switchboard.didFinishRender('ObserveModel.didUpdate');
}
/* eslint-enable react/prop-types */
this.resolve();
});
}
},
});
Expand Down
87 changes: 0 additions & 87 deletions lib/event-watcher.js

This file was deleted.

46 changes: 45 additions & 1 deletion lib/github-package.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import StyleCalculator from './models/style-calculator';
import FilePatchController from './controllers/file-patch-controller';
import RootController from './controllers/root-controller';
import IssueishPaneItem from './atom-items/issueish-pane-item';
import Switchboard from './switchboard';
import yardstick from './yardstick';
import GitTimingsView from './views/git-timings-view';

const defaultState = {
Expand All @@ -40,6 +42,35 @@ export default class GithubPackage {

this.subscriptions = new CompositeDisposable();
this.savedState = {};

this.switchboard = new Switchboard();
this.setupYardstick();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dropped this into a separate method to make it easy to comment out if it causes any trouble. Might just be a relic of me developing the mechanism, though.

}

setupYardstick() {
const stagingSeries = ['stageLine', 'stageHunk', 'unstageLine', 'unstageHunk'];

this.subscriptions.add(
this.switchboard.onDidBeginStageOperation(payload => {
if (payload.stage && payload.line) {
yardstick.begin('stageLine');
} else if (payload.stage && payload.hunk) {
yardstick.begin('stageHunk');
} else if (payload.unstage && payload.line) {
yardstick.begin('unstageLine');
} else if (payload.unstage && payload.hunk) {
yardstick.begin('unstageHunk');
}
}),
this.switchboard.onDidUpdateRepository(() => {
yardstick.mark(stagingSeries, 'update-repository');
}),
this.switchboard.onDidFinishRender(context => {
if (context === 'RootController.showFilePatchForPath') {
yardstick.finish(stagingSeries);
}
}),
);
}

activate(state = {}) {
Expand Down Expand Up @@ -149,14 +180,18 @@ export default class GithubPackage {
savedState={this.savedState.gitController}
createRepositoryForProjectPath={this.createRepositoryForProjectPath}
cloneRepositoryForProjectPath={this.cloneRepositoryForProjectPath}
switchboard={this.switchboard}
/>, this.element,
);
}

async deactivate() {
this.subscriptions.dispose();
if (this.destroyedRepositorySubscription) { this.destroyedRepositorySubscription.dispose(); }
await this.destroyModelsForPaths(Array.from(this.modelPromisesByProjectPath.keys()));
await Promise.all([
this.destroyModelsForPaths(Array.from(this.modelPromisesByProjectPath.keys())),
yardstick.flush(),
]);
}

consumeStatusBar(statusBar) {
Expand Down Expand Up @@ -278,6 +313,15 @@ export default class GithubPackage {
}

setActiveModels(repository, resolutionProgress) {
if (this.activeRepository !== repository) {
if (this.changedRepositorySubscription) { this.changedRepositorySubscription.dispose(); }
if (repository) {
this.changedRepositorySubscription = repository.onDidUpdate(() => {
this.switchboard.didUpdateRepository();
});
}
}

this.activeRepository = repository;
this.activeResolutionProgress = resolutionProgress;
this.rerender();
Expand Down
Loading