Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Show dock items on first run #685

Merged
merged 30 commits into from
Apr 24, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
8b94876
Allow DockItem to be activated via prop
BinaryMuse Apr 19, 2017
f1fb49b
Show both panels when package is first activated
BinaryMuse Apr 19, 2017
0d38969
:white_check_mark: implement new tests for first run functionality
BinaryMuse Apr 19, 2017
c845606
Why is this failing on CI 😭
BinaryMuse Apr 19, 2017
5e96bc3
Activate with {firstRun: false} in tests
Apr 20, 2017
e9a2ee4
Merge remote-tracking branch 'origin/master' into mkt-show-panels-on-…
Apr 20, 2017
5a878d6
Update rootControllerTest to use new prop name
Apr 20, 2017
d219d47
Allow AsyncQueue to be disposed
Apr 20, 2017
166603b
Destroy GSOS and associated AsyncQueue when destroying Repository
Apr 20, 2017
6d1b62b
Never render or update the active context after destruction
smashwilson Apr 21, 2017
9ca2a3c
Delegate git operations in async state methods to the current state
smashwilson Apr 21, 2017
ae017aa
Make Loading.start() cancellable
smashwilson Apr 21, 2017
d2ce103
Only permit state transitions initiated by the current state
smashwilson Apr 21, 2017
fbbeb84
Consolidate .then callbacks
smashwilson Apr 21, 2017
4304f5b
Don't activate after close
smashwilson Apr 21, 2017
a0160d6
Allow DockItem to handle pane activation instead of Workspace.open()
smashwilson Apr 21, 2017
3120e65
Prevent Pane activation if the Workspace is destroyed
smashwilson Apr 21, 2017
511b9ac
Default useLegacyPanels correctly
smashwilson Apr 21, 2017
8e84ef5
Make init and clone cancellable
smashwilson Apr 21, 2017
cc99c80
Typo fix
smashwilson Apr 21, 2017
582bba9
Update sinon.stub(a, b, c) to sinon.stub(a, b).callsFake(c)
Apr 24, 2017
c7ec1d4
Don't render RootController in GithubPackage tests
Apr 24, 2017
8941e8f
Merge remote-tracking branch 'origin/master' into mkt-show-panels-on-…
Apr 24, 2017
d780083
Convert error log in GSOS child exit event to warning
Apr 24, 2017
57af78f
:fire: unnecessary try/catch
Apr 24, 2017
0a5bf85
Fixup tests
Apr 24, 2017
c9474d1
Make useLegacyPanels configurable in RootController tests
Apr 24, 2017
4b59e44
Test RootController with non-legacy panels if dock API exists
Apr 24, 2017
69cd523
Fix up conditional
Apr 24, 2017
f5c4df2
Aha
Apr 24, 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
23 changes: 18 additions & 5 deletions lib/async-queue.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,17 @@ export default class AsyncQueue {
}

push(fn, {parallel} = {parallel: true}) {
if (this.disposed) {
throw new Error('AsyncQueue is disposed');
}
const task = new Task(fn, parallel);
this.queue.push(task);
this.processQueue();
return task.getPromise();
}

processQueue() {
if (!this.queue.length || this.nonParallelizableOperation) { return; }
if (!this.queue.length || this.nonParallelizableOperation || this.disposed) { return; }

const task = this.queue[0];
const canRunParallelOp = task.runsInParallel() && this.tasksInProgress < this.parallelism;
Expand All @@ -59,14 +62,24 @@ export default class AsyncQueue {
}

async processTask(task, runsInParallel) {
if (this.disposed) { return; }

this.tasksInProgress++;
if (!runsInParallel) {
this.nonParallelizableOperation = true;
}

await task.execute();
this.tasksInProgress--;
this.nonParallelizableOperation = false;
this.processQueue();
try {
await task.execute();
} finally {
this.tasksInProgress--;
this.nonParallelizableOperation = false;
this.processQueue();
}
}

dispose() {
this.queue = [];
this.disposed = true;
}
}
9 changes: 6 additions & 3 deletions lib/controllers/root-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,14 @@ export default class RootController extends React.Component {
switchboard: PropTypes.instanceOf(Switchboard),
savedState: PropTypes.object,
useLegacyPanels: PropTypes.bool,
firstRun: React.PropTypes.bool,
}

static defaultProps = {
switchboard: new Switchboard(),
savedState: {},
useLegacyPanels: false,
firstRun: true,
}

serialize() {
Expand All @@ -77,8 +79,8 @@ export default class RootController extends React.Component {
this.state = {
...nullFilePatchState,
amending: false,
gitTabActive: !!props.savedState.gitTabActive,
githubPanelActive: !!props.savedState.githubPanelActive,
gitTabActive: props.firstRun || props.savedState.gitTabActive,
githubPanelActive: props.firstRun || props.savedState.githubPanelActive,
panelSize: props.savedState.panelSize || 400,
activeTab: props.savedState.activeTab || 0,
cloneDialogActive: false,
Expand Down Expand Up @@ -163,7 +165,8 @@ export default class RootController extends React.Component {
workspace={this.props.workspace}
getItem={({subtree}) => subtree.getWrappedComponent()}
onDidCloseItem={() => this.setState({gitTabActive: false})}
stubItemSelector="git-tab-controller">
stubItemSelector="git-tab-controller"
activate={this.props.firstRun}>
<EtchWrapper
ref={c => { this.gitTabController = c; }}
className="github-PanelEtchWrapper"
Expand Down
9 changes: 6 additions & 3 deletions lib/git-shell-out-strategy.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,8 @@ export default class GitShellOutStrategy {
env,
processCallback: child => {
child.on('error', err => {
console.error('Error executing: ' + formattedArgs);

console.error(err.stack);
console.warn('Error executing: ' + formattedArgs + ':');
console.warn(err.stack);
});
child.stdin.on('error', err => {
console.error('Error writing to process: ' + formattedArgs);
Expand Down Expand Up @@ -653,6 +652,10 @@ export default class GitShellOutStrategy {
return executable ? '100755' : '100644';
}
}

destroy() {
this.commandQueue.dispose();
}
}

function buildAddedFilePatch(filePath, contents, executable) {
Expand Down
11 changes: 11 additions & 0 deletions lib/github-package.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import GitTimingsView from './views/git-timings-view';
import AsyncQueue from './async-queue';

const defaultState = {
firstRun: true,
resolutionProgressByPath: {},
};

Expand Down Expand Up @@ -170,11 +171,16 @@ export default class GithubPackage {
return {
activeRepositoryPath,
gitController: this.controller.serialize(),
firstRun: false,
};
}

@autobind
rerender(callback) {
if (this.workspace.isDestroyed()) {
return;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@smashwilson 🤔 What if the workspace isn't destroyed but the package is deactivated? Something we should handle in these cases?

Copy link
Contributor

Choose a reason for hiding this comment

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

Good question. I'd hope that potential rerender() calls would be cleaned up by the CompositeDisposable disposal. But it's well worth a check to see if that's possible.


if (!this.element) {
this.element = document.createElement('div');
this.subscriptions.add(new Disposable(() => {
Expand All @@ -201,6 +207,7 @@ export default class GithubPackage {
cloneRepositoryForProjectPath={this.cloneRepositoryForProjectPath}
switchboard={this.switchboard}
useLegacyPanels={this.useLegacyPanels}
firstRun={this.savedState.firstRun}
/>, this.element, callback,
);
}
Expand Down Expand Up @@ -370,6 +377,10 @@ export default class GithubPackage {
}

async updateActiveContext(savedState = {}) {
if (this.workspace.isDestroyed()) {
return;
}

this.switchboard.didBeginActiveContextUpdate();

const nextActiveContext = await this.getNextContext(savedState);
Expand Down
6 changes: 5 additions & 1 deletion lib/models/repository-states/cloning.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,18 @@ export default class Cloning extends State {

async start() {
await mkdirs(this.workdir());
await this.git().clone(this.remoteUrl, {recursive: true});
await this.doClone(this.remoteUrl, {recursive: true});

await this.transitionTo('Loading');
}

showGitTabLoading() {
return true;
}

directClone(remoteUrl, options) {
return this.git().clone(remoteUrl, options);
}
}

State.register(Cloning);
1 change: 1 addition & 0 deletions lib/models/repository-states/destroyed.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import State from './state';
export default class Destroyed extends State {
start() {
this.didDestroy();
this.repository.git.destroy && this.repository.git.destroy();
this.repository.emitter.dispose();
}

Expand Down
6 changes: 5 additions & 1 deletion lib/models/repository-states/initializing.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,18 @@ import State from './state';
*/
export default class Initializing extends State {
async start() {
await this.git().init(this.workdir());
await this.doInit(this.workdir());

await this.transitionTo('Loading');
}

showGitTabLoading() {
return true;
}

directInit(workdir) {
return this.git().init(workdir);
}
}

State.register(Initializing);
14 changes: 13 additions & 1 deletion lib/models/repository-states/loading.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import State from './state';
*/
export default class Loading extends State {
async start() {
if (await this.git().isGitRepository()) {
if (await this.isGitRepository()) {
const history = await this.loadHistoryPayload();
return this.transitionTo('Present', history);
} else {
Expand All @@ -31,6 +31,18 @@ export default class Loading extends State {
showGitTabLoading() {
return true;
}

directIsGitRepository() {
return this.git().isGitRepository();
}

directGetConfig(key, options) {
return this.git().getConfig(key, options);
}

directGetBlobContents(sha) {
return this.git().getBlobContents(sha);
}
}

State.register(Loading);
49 changes: 47 additions & 2 deletions lib/models/repository-states/state.js
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,12 @@ export default class State {
return this.repository.getWorkingDirectoryPath();
}

// Call methods on the active Repository state, even if the state has transitioned beneath you.
// Use this to perform operations within `start()` methods to guard against interrupted state transitions.
current() {
return this.repository.state;
}

// Return a Promise that will resolve once the state transitions from Loading.
getLoadPromise() {
return this.repository.getLoadPromise();
Expand Down Expand Up @@ -439,16 +445,55 @@ export default class State {
return this.repository.emitter.emit('did-update');
}

// Direct git access
// Non-delegated git operations for internal use within states.

directIsGitRepository() {
return Promise.resolve(false);
}

directGetConfig(key, options = {}) {
return Promise.resolve(null);
}

directGetBlobContents() {
return Promise.reject(new Error('Not a valid object name'));
}

directInit() {
return Promise.resolve();
}

directClone(remoteUrl, options) {
return Promise.resolve();
}

// Deferred operations
// Direct raw git operations to the current state, even if the state has been changed. Use these methods within
// start() methods.

isGitRepository() {
return this.current().directIsGitRepository();
}

doInit(workdir) {
return this.current().directInit();
}

doClone(remoteUrl, options) {
return this.current().directClone(remoteUrl, options);
}

// Parse a DiscardHistory payload from the SHA recorded in config.
async loadHistoryPayload() {
const historySha = await this.git().getConfig('atomGithub.historySha');
const historySha = await this.current().directGetConfig('atomGithub.historySha');
if (!historySha) {
return {};
}

let blob;
try {
blob = await this.git().getBlobContents(historySha);
blob = await this.current().directGetBlobContents(historySha);
} catch (e) {
if (/Not a valid object name/.test(e.stdErr)) {
return {};
Expand Down
9 changes: 8 additions & 1 deletion lib/models/repository.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,18 @@ export default class Repository {
// State management //////////////////////////////////////////////////////////////////////////////////////////////////

transition(currentState, StateConstructor, ...payload) {
if (currentState !== this.state) {
// Attempted transition from a non-active state, most likely from an asynchronous start() method.
return Promise.resolve();
}

const nextState = new StateConstructor(this, ...payload);
this.state = nextState;

this.emitter.emit('did-change-state', {from: currentState, to: this.state});
this.emitter.emit('did-update');
if (!this.isDestroyed()) {
this.emitter.emit('did-update');
}

return this.state.start();
}
Expand Down
14 changes: 12 additions & 2 deletions lib/views/dock-item.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export default class DockItem extends React.Component {
getItem: PropTypes.func,
onDidCloseItem: PropTypes.func,
stubItemSelector: PropTypes.string,
activate: PropTypes.bool,
}

static defaultProps = {
Expand Down Expand Up @@ -65,8 +66,15 @@ export default class DockItem extends React.Component {
if (stub) {
stub.setRealItem(itemToAdd);
this.dockItem = stub;
if (this.props.activate) {
this.activate();
}
} else {
Promise.resolve(this.props.workspace.open(itemToAdd)).then(item => { this.dockItem = item; });
Promise.resolve(this.props.workspace.open(itemToAdd, {activatePane: false}))
.then(item => {
this.dockItem = item;
if (this.props.activate) { this.activate(); }
});
}

this.subscriptions = this.props.workspace.onDidDestroyPaneItem(({item}) => {
Expand Down Expand Up @@ -94,7 +102,9 @@ export default class DockItem extends React.Component {

activate() {
setTimeout(() => {
if (!this.dockItem) { return; }
if (!this.dockItem || this.didCloseItem || this.props.workspace.isDestroyed()) {
return;
}

const pane = this.props.workspace.paneForItem(this.dockItem);
if (pane) {
Expand Down
Loading