Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue with CustomExecutions not working through tasks.executeTask #79132

Merged
merged 3 commits into from
Aug 15, 2019
+20 −5
Merged

Fix issue with CustomExecutions not working through tasks.executeTask #79132

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
406a5c9
Do not clear out the map, and track the provided execution during exe…
GabeDeBacker Aug 14, 2019
fb38674
Clear provided custom executions map on execution complete
GabeDeBacker Aug 14, 2019
df89de9
Save last custom execution
alexr00 Aug 15, 2019
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
25 changes: 20 additions & 5 deletions src/vs/workbench/api/node/extHostTask.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,14 @@ export class ExtHostTask implements ExtHostTaskShape {
if (dto === undefined) {
return Promise.reject(new Error('Task is not valid'));
}

// If this task is a custom execution, then we need to save it away
// in the provided custom execution map that is cleaned up after the
// task is executed.
if (CustomExecution2DTO.is(dto.execution)) {
await this.addCustomExecution2(dto, <vscode.Task2>task);
}

return this._proxy.$executeTask(dto).then(value => this.getTaskExecution(value, task));
}
}
Expand Down Expand Up @@ -529,11 +537,6 @@ export class ExtHostTask implements ExtHostTaskShape {
return Promise.reject(new Error('no handler found'));
}

// For custom execution tasks, we need to store the execution objects locally
// since we obviously cannot send callback functions through the proxy.
// So, clear out any existing ones.
this._providedCustomExecutions2.clear();

// Set up a list of task ID promises that we can wait on
// before returning the provided tasks. The ensures that
// our task IDs are calculated for any custom execution tasks.
Expand Down Expand Up @@ -692,5 +695,17 @@ export class ExtHostTask implements ExtHostTaskShape {
if (extensionCallback2) {
this._activeCustomExecutions2.delete(execution.id);
}

const lastCustomExecution = this._providedCustomExecutions2.get(execution.id);
// Technically we don't really need to do this, however, if an extension
// is executing a task through "executeTask" over and over again
// with different properties in the task definition, then this list
// could grow indefinitely, something we don't want.
this._providedCustomExecutions2.clear();
Copy link
Member

@alexr00 alexr00 Aug 15, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

As expected, we can't clear hear because this breaks the rerun task command for all custom execution tasks.

Copy link
Member

Choose a reason for hiding this comment

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

I added a simple fix for this.

// We do still need to hang on to the last custom execution so that the
// Rerun Task command doesn't choke when it tries to rerun a custom execution
if (lastCustomExecution) {
this._providedCustomExecutions2.set(execution.id, lastCustomExecution);
}
}
}