Skip to content

Commit 64d97aa

Browse files
koesie10koesie10
authored
Merge pull request #2165 from github/koesie10/simpler-progress-task
Simpler `withProgress` calls
2 parents 3d354e1 + 964640c commit 64d97aa

4 files changed

Lines changed: 115 additions & 106 deletions

File tree

‎extensions/ql-vscode/src/commandRunner.ts‎

Lines changed: 44 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import {
22
CancellationToken,
3-
ProgressOptions,
3+
ProgressOptions as VSCodeProgressOptions,
44
window as Window,
55
commands,
66
Disposable,
@@ -42,22 +42,40 @@ export interface ProgressUpdate {
4242

4343
export type ProgressCallback = (p: ProgressUpdate) => void;
4444

45+
// Make certain properties within a type optional
46+
type Optional<T, K extends keyof T> = Pick<Partial<T>, K> & Omit<T, K>;
47+
48+
export type ProgressOptions = Optional<VSCodeProgressOptions, "location">;
49+
50+
/**
51+
* A task that reports progress.
52+
*
53+
* @param progress a progress handler function. Call this
54+
* function with a `ProgressUpdate` instance in order to
55+
* denote some progress being achieved on this task.
56+
* @param token a cancellation token
57+
*/
58+
export type ProgressTask<R> = (
59+
progress: ProgressCallback,
60+
token: CancellationToken,
61+
) => Thenable<R>;
62+
4563
/**
4664
* A task that handles command invocations from `commandRunner`
4765
* and includes a progress monitor.
4866
*
4967
*
5068
* Arguments passed to the command handler are passed along,
51-
* untouched to this `ProgressTask` instance.
69+
* untouched to this `ProgressTaskWithArgs` instance.
5270
*
5371
* @param progress a progress handler function. Call this
5472
* function with a `ProgressUpdate` instance in order to
5573
* denote some progress being achieved on this task.
56-
* @param token a cencellation token
74+
* @param token a cancellation token
5775
* @param args arguments passed to this task passed on from
5876
* `commands.registerCommand`.
5977
*/
60-
export type ProgressTask<R> = (
78+
export type ProgressTaskWithArgs<R> = (
6179
progress: ProgressCallback,
6280
token: CancellationToken,
6381
...args: any[]
@@ -90,23 +108,29 @@ type NoProgressTask = (...args: any[]) => Promise<any>;
90108
* request).
91109
*/
92110
export function withProgress<R>(
93-
options: ProgressOptions,
94111
task: ProgressTask<R>,
95-
...args: any[]
112+
{
113+
location = ProgressLocation.Notification,
114+
title,
115+
cancellable,
116+
}: ProgressOptions = {},
96117
): Thenable<R> {
97118
let progressAchieved = 0;
98-
return Window.withProgress(options, (progress, token) => {
99-
return task(
100-
(p) => {
119+
return Window.withProgress(
120+
{
121+
location,
122+
title,
123+
cancellable,
124+
},
125+
(progress, token) => {
126+
return task((p) => {
101127
const { message, step, maxStep } = p;
102128
const increment = (100 * (step - progressAchieved)) / maxStep;
103129
progressAchieved = step;
104130
progress.report({ message, increment });
105-
},
106-
token,
107-
...args,
108-
);
109-
});
131+
}, token);
132+
},
133+
);
110134
}
111135

112136
/**
@@ -177,19 +201,17 @@ export function commandRunner(
177201
*/
178202
export function commandRunnerWithProgress<R>(
179203
commandId: string,
180-
task: ProgressTask<R>,
181-
progressOptions: Partial<ProgressOptions>,
204+
task: ProgressTaskWithArgs<R>,
205+
progressOptions: ProgressOptions,
182206
outputLogger = extLogger,
183207
): Disposable {
184208
return commandRunner(
185209
commandId,
186210
async (...args: any[]) => {
187-
const progressOptionsWithDefaults = {
188-
location: ProgressLocation.Notification,
189-
...progressOptions,
190-
};
191-
192-
return withProgress(progressOptionsWithDefaults, task, ...args);
211+
return withProgress(
212+
(progress, token) => task(progress, token, ...args),
213+
progressOptions,
214+
);
193215
},
194216
outputLogger,
195217
);

‎extensions/ql-vscode/src/contextual/templateProvider.ts‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import {
44
Location,
55
LocationLink,
66
Position,
7-
ProgressLocation,
87
ReferenceContext,
98
ReferenceProvider,
109
TextDocument,
@@ -73,11 +72,6 @@ export class TemplateQueryDefinitionProvider implements DefinitionProvider {
7372

7473
private async getDefinitions(uriString: string): Promise<LocationLink[]> {
7574
return withProgress(
76-
{
77-
location: ProgressLocation.Notification,
78-
cancellable: true,
79-
title: "Finding definitions",
80-
},
8175
async (progress, token) => {
8276
return getLocationsForUriString(
8377
this.cli,
@@ -91,6 +85,10 @@ export class TemplateQueryDefinitionProvider implements DefinitionProvider {
9185
(src, _dest) => src === uriString,
9286
);
9387
},
88+
{
89+
cancellable: true,
90+
title: "Finding definitions",
91+
},
9492
);
9593
}
9694
}
@@ -136,11 +134,6 @@ export class TemplateQueryReferenceProvider implements ReferenceProvider {
136134

137135
private async getReferences(uriString: string): Promise<FullLocationLink[]> {
138136
return withProgress(
139-
{
140-
location: ProgressLocation.Notification,
141-
cancellable: true,
142-
title: "Finding references",
143-
},
144137
async (progress, token) => {
145138
return getLocationsForUriString(
146139
this.cli,
@@ -154,6 +147,10 @@ export class TemplateQueryReferenceProvider implements ReferenceProvider {
154147
(src, _dest) => src === uriString,
155148
);
156149
},
150+
{
151+
cancellable: true,
152+
title: "Finding references",
153+
},
157154
);
158155
}
159156
}

‎extensions/ql-vscode/src/extension.ts‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import {
99
extensions,
1010
languages,
1111
ProgressLocation,
12-
ProgressOptions,
1312
QuickPickItem,
1413
Range,
1514
Uri,
@@ -322,16 +321,15 @@ export async function activate(
322321
await commands.executeCommand("workbench.action.reloadWindow");
323322
}
324323
} else {
325-
const progressOptions: ProgressOptions = {
326-
title: progressTitle,
327-
location: ProgressLocation.Notification,
328-
};
329-
330-
await withProgress(progressOptions, (progress) =>
331-
distributionManager.installExtensionManagedDistributionRelease(
332-
result.updatedRelease,
333-
progress,
334-
),
324+
await withProgress(
325+
(progress) =>
326+
distributionManager.installExtensionManagedDistributionRelease(
327+
result.updatedRelease,
328+
progress,
329+
),
330+
{
331+
title: progressTitle,
332+
},
335333
);
336334

337335
await ctx.globalState.update(shouldUpdateOnNextActivationKey, false);

‎extensions/ql-vscode/src/local-databases.ts‎

Lines changed: 54 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -794,74 +794,66 @@ export class DatabaseManager extends DisposableObject {
794794
}
795795

796796
public async loadPersistedState(): Promise<void> {
797-
return withProgress(
798-
{
799-
location: vscode.ProgressLocation.Notification,
800-
},
801-
async (progress, token) => {
802-
const currentDatabaseUri =
803-
this.ctx.workspaceState.get<string>(CURRENT_DB);
804-
const databases = this.ctx.workspaceState.get<PersistedDatabaseItem[]>(
805-
DB_LIST,
806-
[],
797+
return withProgress(async (progress, token) => {
798+
const currentDatabaseUri =
799+
this.ctx.workspaceState.get<string>(CURRENT_DB);
800+
const databases = this.ctx.workspaceState.get<PersistedDatabaseItem[]>(
801+
DB_LIST,
802+
[],
803+
);
804+
let step = 0;
805+
progress({
806+
maxStep: databases.length,
807+
message: "Loading persisted databases",
808+
step,
809+
});
810+
try {
811+
void this.logger.log(
812+
`Found ${databases.length} persisted databases: ${databases
813+
.map((db) => db.uri)
814+
.join(", ")}`,
807815
);
808-
let step = 0;
809-
progress({
810-
maxStep: databases.length,
811-
message: "Loading persisted databases",
812-
step,
813-
});
814-
try {
815-
void this.logger.log(
816-
`Found ${databases.length} persisted databases: ${databases
817-
.map((db) => db.uri)
818-
.join(", ")}`,
816+
for (const database of databases) {
817+
progress({
818+
maxStep: databases.length,
819+
message: `Loading ${database.options?.displayName || "databases"}`,
820+
step: ++step,
821+
});
822+
823+
const databaseItem = await this.createDatabaseItemFromPersistedState(
824+
progress,
825+
token,
826+
database,
819827
);
820-
for (const database of databases) {
821-
progress({
822-
maxStep: databases.length,
823-
message: `Loading ${
824-
database.options?.displayName || "databases"
825-
}`,
826-
step: ++step,
827-
});
828-
829-
const databaseItem =
830-
await this.createDatabaseItemFromPersistedState(
831-
progress,
832-
token,
833-
database,
834-
);
835-
try {
836-
await databaseItem.refresh();
837-
await this.registerDatabase(progress, token, databaseItem);
838-
if (currentDatabaseUri === database.uri) {
839-
await this.setCurrentDatabaseItem(databaseItem, true);
840-
}
841-
void this.logger.log(
842-
`Loaded database ${databaseItem.name} at URI ${database.uri}.`,
843-
);
844-
} catch (e) {
845-
// When loading from persisted state, leave invalid databases in the list. They will be
846-
// marked as invalid, and cannot be set as the current database.
847-
void this.logger.log(
848-
`Error loading database ${database.uri}: ${e}.`,
849-
);
828+
try {
829+
await databaseItem.refresh();
830+
await this.registerDatabase(progress, token, databaseItem);
831+
if (currentDatabaseUri === database.uri) {
832+
await this.setCurrentDatabaseItem(databaseItem, true);
850833
}
834+
void this.logger.log(
835+
`Loaded database ${databaseItem.name} at URI ${database.uri}.`,
836+
);
837+
} catch (e) {
838+
// When loading from persisted state, leave invalid databases in the list. They will be
839+
// marked as invalid, and cannot be set as the current database.
840+
void this.logger.log(
841+
`Error loading database ${database.uri}: ${e}.`,
842+
);
851843
}
852-
await this.updatePersistedDatabaseList();
853-
} catch (e) {
854-
// database list had an unexpected type - nothing to be done?
855-
void showAndLogExceptionWithTelemetry(
856-
redactableError(
857-
asError(e),
858-
)`Database list loading failed: ${getErrorMessage(e)}`,
859-
);
860844
}
845+
await this.updatePersistedDatabaseList();
846+
} catch (e) {
847+
// database list had an unexpected type - nothing to be done?
848+
void showAndLogExceptionWithTelemetry(
849+
redactableError(
850+
asError(e),
851+
)`Database list loading failed: ${getErrorMessage(e)}`,
852+
);
853+
}
861854

862-
void this.logger.log("Finished loading persisted databases.");
863-
},
864-
);
855+
void this.logger.log("Finished loading persisted databases.");
856+
});
865857
}
866858

867859
public get databaseItems(): readonly DatabaseItem[] {

0 commit comments

Comments
 (0)