Skip to content

Remove ProgressCallback / CancellationToken arguments where they aren't used #2525

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

Merged
robertbrignull merged 4 commits into from
Jun 20, 2023
Merged

Remove ProgressCallback / CancellationToken arguments where they aren't used #2525

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c93449a
Remove ProgressCallback / CancellationToken arguments where they aren…
robertbrignull Jun 19, 2023
4d8506b
Add back in manual progress update
robertbrignull Jun 20, 2023
f999574
Fix expected args in tests
robertbrignull Jun 20, 2023
3685575
Merge branch 'main' into robertbrignull/sendRequest_progress
robertbrignull Jun 20, 2023
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
17 changes: 6 additions & 11 deletions extensions/ql-vscode/src/data-extensions-editor/data-extensions-editor-view.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,6 @@ export class DataExtensionsEditorView extends AbstractWebview<
this.app.workspaceStoragePath ?? this.app.globalStoragePath,
this.app.credentials,
(update) => this.showProgress(update),
tokenSource.token,
this.cliServer,
);
if (!database) {
Expand Down Expand Up @@ -354,16 +353,12 @@ export class DataExtensionsEditorView extends AbstractWebview<

// After the flow model has been generated, we can remove the temporary database
// which we used for generating the flow model.
await this.databaseManager.removeDatabaseItem(
() =>
this.showProgress({
step: 3900,
maxStep: 4000,
message: "Removing temporary database",
}),
tokenSource.token,
database,
);
await this.showProgress({
Copy link
Copy Markdown
Contributor Author

@robertbrignull robertbrignull Jun 20, 2023

Choose a reason for hiding this comment

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

Upon re-reviewing my own PR, I think we should keep this one. I'm not 100% sure if it worked before, but the intention was to do a manual progress update to show that we're removing the database.

step: 3900,
maxStep: 4000,
message: "Removing temporary database",
});
await this.databaseManager.removeDatabaseItem(database);

await this.clearProgress();
}
Expand Down
16 changes: 1 addition & 15 deletions extensions/ql-vscode/src/databases/database-fetcher.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import fetch, { Response } from "node-fetch";
import { zip } from "zip-a-folder";
import { Open } from "unzipper";
import { Uri, CancellationToken, window, InputBoxOptions } from "vscode";
import { Uri, window, InputBoxOptions } from "vscode";
import { CodeQLCliServer } from "../codeql-cli/cli";
import {
ensureDir,
Expand Down Expand Up @@ -44,7 +44,6 @@ export async function promptImportInternetDatabase(
databaseManager: DatabaseManager,
storagePath: string,
progress: ProgressCallback,
token: CancellationToken,
cli?: CodeQLCliServer,
): Promise<DatabaseItem | undefined> {
const databaseUrl = await window.showInputBox({
Expand All @@ -63,7 +62,6 @@ export async function promptImportInternetDatabase(
storagePath,
undefined,
progress,
token,
cli,
);

Expand All @@ -86,7 +84,6 @@ export async function promptImportInternetDatabase(
* @param storagePath where to store the unzipped database.
* @param credentials the credentials to use to authenticate with GitHub
* @param progress the progress callback
* @param token the cancellation token
* @param cli the CodeQL CLI server
*/
export async function promptImportGithubDatabase(
Expand All @@ -95,7 +92,6 @@ export async function promptImportGithubDatabase(
storagePath: string,
credentials: Credentials | undefined,
progress: ProgressCallback,
token: CancellationToken,
cli?: CodeQLCliServer,
): Promise<DatabaseItem | undefined> {
const githubRepo = await askForGitHubRepo(progress);
Expand All @@ -109,7 +105,6 @@ export async function promptImportGithubDatabase(
storagePath,
credentials,
progress,
token,
cli,
);

Expand Down Expand Up @@ -157,7 +152,6 @@ export async function askForGitHubRepo(
* @param storagePath where to store the unzipped database.
* @param credentials the credentials to use to authenticate with GitHub
* @param progress the progress callback
* @param token the cancellation token
* @param cli the CodeQL CLI server
* @param language the language to download. If undefined, the user will be prompted to choose a language.
**/
Expand All @@ -167,7 +161,6 @@ export async function downloadGitHubDatabase(
storagePath: string,
credentials: Credentials | undefined,
progress: ProgressCallback,
token: CancellationToken,
cli?: CodeQLCliServer,
language?: string,
): Promise<DatabaseItem | undefined> {
Expand Down Expand Up @@ -213,7 +206,6 @@ export async function downloadGitHubDatabase(
storagePath,
`${owner}/${name}`,
progress,
token,
cli,
);
}
Expand All @@ -231,7 +223,6 @@ export async function importArchiveDatabase(
databaseManager: DatabaseManager,
storagePath: string,
progress: ProgressCallback,
token: CancellationToken,
cli?: CodeQLCliServer,
): Promise<DatabaseItem | undefined> {
try {
Expand All @@ -242,7 +233,6 @@ export async function importArchiveDatabase(
storagePath,
undefined,
progress,
token,
cli,
);
if (item) {
Expand Down Expand Up @@ -275,7 +265,6 @@ export async function importArchiveDatabase(
* @param storagePath where to store the unzipped database.
* @param nameOverride a name for the database that overrides the default
* @param progress callback to send progress messages to
* @param token cancellation token
*/
async function databaseArchiveFetcher(
databaseUrl: string,
Expand All @@ -284,7 +273,6 @@ async function databaseArchiveFetcher(
storagePath: string,
nameOverride: string | undefined,
progress: ProgressCallback,
token: CancellationToken,
cli?: CodeQLCliServer,
): Promise<DatabaseItem> {
progress({
Expand Down Expand Up @@ -327,8 +315,6 @@ async function databaseArchiveFetcher(
const makeSelected = true;

const item = await databaseManager.openDatabase(
progress,
token,
Uri.file(dbPath),
makeSelected,
nameOverride,
Expand Down
31 changes: 10 additions & 21 deletions extensions/ql-vscode/src/databases/local-databases-ui.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ export class DatabaseUI extends DisposableObject {

private async handleSetDefaultTourDatabase(): Promise<void> {
return withProgress(
async (progress, token) => {
async () => {
try {
if (!workspace.workspaceFolders?.length) {
throw new Error("No workspace folder is open.");
Expand All @@ -332,8 +332,6 @@ export class DatabaseUI extends DisposableObject {
const isTutorialDatabase = true;

await this.databaseManager.openDatabase(
progress,
token,
uri,
makeSelected,
nameOverride,
Expand Down Expand Up @@ -485,13 +483,12 @@ export class DatabaseUI extends DisposableObject {

private async handleChooseDatabaseInternet(): Promise<void> {
return withProgress(
async (progress, token) => {
async (progress) => {
await promptImportInternetDatabase(
this.app.commands,
this.databaseManager,
this.storagePath,
progress,
token,
this.queryServer?.cliServer,
);
},
Expand All @@ -503,7 +500,7 @@ export class DatabaseUI extends DisposableObject {

private async handleChooseDatabaseGithub(): Promise<void> {
return withProgress(
async (progress, token) => {
async (progress) => {
const credentials = isCanary() ? this.app.credentials : undefined;

await promptImportGithubDatabase(
Expand All @@ -512,7 +509,6 @@ export class DatabaseUI extends DisposableObject {
this.storagePath,
credentials,
progress,
token,
this.queryServer?.cliServer,
);
},
Expand Down Expand Up @@ -608,14 +604,13 @@ export class DatabaseUI extends DisposableObject {

private async handleClearCache(): Promise<void> {
return withProgress(
async (progress, token) => {
async (_progress, token) => {
if (
this.queryServer !== undefined &&
this.databaseManager.currentDatabaseItem !== undefined
) {
await this.queryServer.clearCacheInDatabase(
this.databaseManager.currentDatabaseItem,
progress,
token,
);
}
Expand All @@ -633,7 +628,7 @@ export class DatabaseUI extends DisposableObject {

private async handleSetCurrentDatabase(uri: Uri): Promise<void> {
return withProgress(
async (progress, token) => {
async (progress) => {
try {
// Assume user has selected an archive if the file has a .zip extension
if (uri.path.endsWith(".zip")) {
Expand All @@ -643,11 +638,10 @@ export class DatabaseUI extends DisposableObject {
this.databaseManager,
this.storagePath,
progress,
token,
this.queryServer?.cliServer,
);
} else {
await this.databaseManager.openDatabase(progress, token, uri);
await this.databaseManager.openDatabase(uri);
}
} catch (e) {
// rethrow and let this be handled by default error handling.
Expand All @@ -668,10 +662,10 @@ export class DatabaseUI extends DisposableObject {
databaseItems: DatabaseItem[],
): Promise<void> {
return withProgress(
async (progress, token) => {
async () => {
await Promise.all(
databaseItems.map((dbItem) =>
this.databaseManager.removeDatabaseItem(progress, token, dbItem),
this.databaseManager.removeDatabaseItem(dbItem),
),
);
},
Expand Down Expand Up @@ -758,15 +752,11 @@ export class DatabaseUI extends DisposableObject {

return await withInheritedProgress(
progress,
async (progress, token) => {
async (progress) => {
if (byFolder) {
const fixedUri = await this.fixDbUri(uri);
// we are selecting a database folder
return await this.databaseManager.openDatabase(
progress,
token,
fixedUri,
);
return await this.databaseManager.openDatabase(fixedUri);
} else {
// we are selecting a database archive. Must unzip into a workspace-controlled area
// before importing.
Expand All @@ -776,7 +766,6 @@ export class DatabaseUI extends DisposableObject {
this.databaseManager,
this.storagePath,
progress,
token,
this.queryServer?.cliServer,
);
}
Expand Down
Loading