Skip to content

Commit

Permalink
No more recursive loop when there are no targets
Browse files Browse the repository at this point in the history
Thanks to @sphaerophoria for correctly identifying the issue
and providing an initial solution in #503.

Fixes: #484, #447
  • Loading branch information
noseglid committed May 22, 2017
1 parent 0487f3f commit ad1edd4
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 7 deletions.
16 changes: 9 additions & 7 deletions lib/target-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ class TargetManager extends EventEmitter {

pathTarget.targets = settings;
pathTarget.loading = false;

return pathTarget;
}).catch(err => {
atom.notifications.addError('Ooops. Something went wrong.', {
detail: err.message,
Expand All @@ -131,12 +133,12 @@ class TargetManager extends EventEmitter {
});
});

return Promise.all(pathPromises).then(entries => {
this.fillTargets(require('./utils').activePath());
return Promise.all(pathPromises).then(pathTargets => {
this.fillTargets(require('./utils').activePath(), false);
this.emit('refresh-complete');
this.busyRegistry && this.busyRegistry.end('build.refresh-targets');

if (entries.length === 0) {
if (pathTargets.length === 0) {
return;
}

Expand All @@ -161,15 +163,15 @@ class TargetManager extends EventEmitter {
});
}

fillTargets(path) {
fillTargets(path, refreshOnEmpty = true) {
if (!this.targetsView) {
return;
}

const activeTarget = this.getActiveTarget(path);
activeTarget && this.targetsView.setActiveTarget(activeTarget.name);

this.getTargets(path)
this.getTargets(path, refreshOnEmpty)
.then(targets => targets.map(t => t.name))
.then(targetNames => this.targetsView && this.targetsView.setItems(targetNames));
}
Expand Down Expand Up @@ -206,13 +208,13 @@ class TargetManager extends EventEmitter {
});
}

getTargets(path) {
getTargets(path, refreshOnEmpty = true) {
const pathTarget = this.pathTargets.find(pt => pt.path === path);
if (!pathTarget) {
return Promise.resolve([]);
}

if (pathTarget.targets.length === 0) {
if (refreshOnEmpty && pathTarget.targets.length === 0) {
return this.refreshTargets([ pathTarget.path ]).then(() => pathTarget.targets);
}
return Promise.resolve(pathTarget.targets);
Expand Down
17 changes: 17 additions & 0 deletions spec/build-targets-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,23 @@ describe('Target', () => {
try { fs.removeSync(directory); } catch (e) { console.warn('Failed to clean up: ', e); }
});

describe('when no targets exists', () => {
it('should show a notification', () => {
runs(() => {
atom.commands.dispatch(workspaceElement, 'build:select-active-target');
});

waitsFor(() => {
return workspaceElement.querySelector('.select-list.build-target');
});

runs(() => {
const targets = [ ...workspaceElement.querySelectorAll('.select-list li.build-target') ].map(el => el.textContent);
expect(targets).toEqual([]);
});
});
});

describe('when multiple targets exists', () => {
it('should list those targets in a SelectListView (from .atom-build.json)', () => {
waitsForPromise(() => {
Expand Down

0 comments on commit ad1edd4

Please sign in to comment.