Refactor the commandRunner

Split commandRunner into two functions: commandRunner and
commandRunnerWithProgress.

Also, take advantage of default arguments for ProgressOptions.

And updates changelog.
This commit is contained in:
Andrew Eisenberg
2020-10-08 10:43:40 -07:00
parent e5d439ae89
commit 8e817ee01a
7 changed files with 124 additions and 102 deletions

View File

@@ -4,11 +4,9 @@
- Add friendly welcome message when the databases view is empty.
- Add open query, open results, and remove query commands in the query history view title bar.
- Max number of simultaneous queries launchable by runQueries command is now configurable by changing the `codeQL.runningQueries.maxQueries` setting.
- The maximum number of simultaneous queries launchable by the `CodeQL: Run Queries in Selected Files` command is now configurable by changing the `codeQL.runningQueries.maxQueries` setting.
- Allow simultaneously run queries to be canceled in a single-click.
- Prevent multiple upgrade dialogs from appearing when running simultaneous queries on upgradeable databases.
- Allow simultaneously run queries to be canceled in a single-click.
- Max number of simultaneous queries launchable by runQueries command is now configurable by changing the codeQL.runningQueries.maxQueries setting.
- Fix sorting of results. Some pages of results would have the wrong sort order and columns.
- Remember previous sort order when reloading query results.
- Fix proper escaping of backslashes in SARIF message strings.

View File

@@ -20,7 +20,6 @@ import { showLocation } from './interface-utils';
import { isStringLoc, isWholeFileLoc, isLineColumnLoc } from './bqrs-utils';
import { commandRunner } from './helpers';
export interface AstItem {
id: BqrsId;
label?: string;

View File

@@ -44,7 +44,7 @@ export class TemplateQueryDefinitionProvider implements vscode.DefinitionProvide
}
private async getDefinitions(uriString: string): Promise<vscode.LocationLink[]> {
return await withProgress({
return withProgress({
location: vscode.ProgressLocation.Notification,
cancellable: true,
title: 'Finding definitions'
@@ -91,7 +91,7 @@ export class TemplateQueryReferenceProvider implements vscode.ReferenceProvider
}
private async getReferences(uriString: string): Promise<FullLocationLink[]> {
return await withProgress({
return withProgress({
location: vscode.ProgressLocation.Notification,
cancellable: true,
title: 'Finding references'

View File

@@ -9,8 +9,7 @@ import {
TreeItem,
Uri,
window,
env,
ProgressLocation
env
} from 'vscode';
import * as fs from 'fs-extra';
@@ -23,6 +22,7 @@ import {
} from './databases';
import {
commandRunner,
commandRunnerWithProgress,
getOnDiskWorkspaceFolders,
ProgressCallback,
showAndLogErrorMessage
@@ -233,79 +233,66 @@ export class DatabaseUI extends DisposableObject {
logger.log('Registering database panel commands.');
ctx.subscriptions.push(
commandRunner(
commandRunnerWithProgress(
'codeQL.setCurrentDatabase',
this.handleSetCurrentDatabase,
{
location: ProgressLocation.Notification,
title: 'Importing database from archive',
cancellable: false,
}
)
);
ctx.subscriptions.push(
commandRunner(
commandRunnerWithProgress(
'codeQL.upgradeCurrentDatabase',
this.handleUpgradeCurrentDatabase,
{
location: ProgressLocation.Notification,
title: 'Upgrading current database',
cancellable: true,
}
)
);
ctx.subscriptions.push(
commandRunner(
commandRunnerWithProgress(
'codeQL.clearCache',
this.handleClearCache,
{
location: ProgressLocation.Notification,
title: 'Clearing Cache',
cancellable: false,
})
);
ctx.subscriptions.push(
commandRunner(
commandRunnerWithProgress(
'codeQLDatabases.chooseDatabaseFolder',
this.handleChooseDatabaseFolder,
{
location: ProgressLocation.Notification,
title: 'Adding database from folder',
cancellable: false,
}
)
);
ctx.subscriptions.push(
commandRunner(
commandRunnerWithProgress(
'codeQLDatabases.chooseDatabaseArchive',
this.handleChooseDatabaseArchive,
{
location: ProgressLocation.Notification,
title: 'Adding database from archive',
cancellable: false,
}
)
);
ctx.subscriptions.push(
commandRunner(
commandRunnerWithProgress(
'codeQLDatabases.chooseDatabaseInternet',
this.handleChooseDatabaseInternet,
{
location: ProgressLocation.Notification,
title: 'Adding database from URL',
cancellable: false,
}
)
);
ctx.subscriptions.push(
commandRunner(
commandRunnerWithProgress(
'codeQLDatabases.chooseDatabaseLgtm',
this.handleChooseDatabaseLgtm,
{
location: ProgressLocation.Notification,
title: 'Adding database from LGTM',
cancellable: false,
})
);
ctx.subscriptions.push(
@@ -333,11 +320,10 @@ export class DatabaseUI extends DisposableObject {
)
);
ctx.subscriptions.push(
commandRunner(
commandRunnerWithProgress(
'codeQLDatabases.upgradeDatabase',
this.handleUpgradeDatabase,
{
location: ProgressLocation.Notification,
title: 'Upgrading database',
cancellable: true,
}
@@ -522,9 +508,9 @@ export class DatabaseUI extends DisposableObject {
};
private handleSetCurrentDatabase = async (
uri: Uri,
progress: ProgressCallback,
token: CancellationToken,
uri: Uri,
): Promise<void> => {
try {
// Assume user has selected an archive if the file has a .zip extension

View File

@@ -165,12 +165,10 @@ export async function activate(ctx: ExtensionContext): Promise<void> {
}
} else {
const progressOptions: ProgressOptions = {
location: ProgressLocation.Notification,
title: progressTitle,
cancellable: false,
location: ProgressLocation.Notification,
};
// Avoid using commandRunner here because this function is called upon extension activation
await helpers.withProgress(progressOptions, progress =>
distributionManager.installExtensionManagedDistributionRelease(result.updatedRelease, progress));
@@ -459,7 +457,7 @@ async function activateWithInstalledDistribution(
logger.log('Registering top-level command palette commands.');
ctx.subscriptions.push(
helpers.commandRunner(
helpers.commandRunnerWithProgress(
'codeQL.runQuery',
async (
progress: helpers.ProgressCallback,
@@ -467,14 +465,13 @@ async function activateWithInstalledDistribution(
uri: Uri | undefined
) => await compileAndRunQuery(false, uri, progress, token),
{
location: ProgressLocation.Notification,
title: 'Running query',
cancellable: true
}
)
);
ctx.subscriptions.push(
helpers.commandRunner(
helpers.commandRunnerWithProgress(
'codeQL.runQueries',
async (
progress: helpers.ProgressCallback,
@@ -533,13 +530,12 @@ async function activateWithInstalledDistribution(
));
},
{
location: ProgressLocation.Notification,
title: 'Running queries',
cancellable: true
})
);
ctx.subscriptions.push(
helpers.commandRunner(
helpers.commandRunnerWithProgress(
'codeQL.quickEval',
async (
progress: helpers.ProgressCallback,
@@ -547,17 +543,19 @@ async function activateWithInstalledDistribution(
uri: Uri | undefined
) => await compileAndRunQuery(true, uri, progress, token),
{
location: ProgressLocation.Notification,
title: 'Running query',
cancellable: true
})
);
ctx.subscriptions.push(
helpers.commandRunner('codeQL.quickQuery', async (
helpers.commandRunnerWithProgress('codeQL.quickQuery', async (
progress: helpers.ProgressCallback,
token: CancellationToken
) =>
displayQuickQuery(ctx, cliServer, databaseUI, progress, token)
displayQuickQuery(ctx, cliServer, databaseUI, progress, token),
{
title: 'Run Quick Query'
}
)
);
@@ -586,28 +584,24 @@ async function activateWithInstalledDistribution(
)
);
ctx.subscriptions.push(
helpers.commandRunner('codeQL.chooseDatabaseLgtm', (
helpers.commandRunnerWithProgress('codeQL.chooseDatabaseLgtm', (
progress: helpers.ProgressCallback,
token: CancellationToken
) =>
databaseUI.handleChooseDatabaseLgtm(progress, token),
{
location: ProgressLocation.Notification,
title: 'Adding database from LGTM',
cancellable: false,
})
);
ctx.subscriptions.push(
helpers.commandRunner('codeQL.chooseDatabaseInternet', (
helpers.commandRunnerWithProgress('codeQL.chooseDatabaseInternet', (
progress: helpers.ProgressCallback,
token: CancellationToken
) =>
databaseUI.handleChooseDatabaseInternet(progress, token),
{
location: ProgressLocation.Notification,
title: 'Adding database from URL',
cancellable: false,
})
);
@@ -627,7 +621,7 @@ async function activateWithInstalledDistribution(
);
const astViewer = new AstViewer(ctx);
ctx.subscriptions.push(helpers.commandRunner('codeQL.viewAst', async (
ctx.subscriptions.push(helpers.commandRunnerWithProgress('codeQL.viewAst', async (
progress: helpers.ProgressCallback,
token: CancellationToken
) => {
@@ -637,7 +631,6 @@ async function activateWithInstalledDistribution(
astViewer.updateRoots(await ast.getRoots(), ast.db, ast.fileName);
}
}, {
location: ProgressLocation.Notification,
cancellable: true,
title: 'Calculate AST'
}));

View File

@@ -9,7 +9,8 @@ import {
window as Window,
workspace,
commands,
Disposable
Disposable,
ProgressLocation
} from 'vscode';
import { CodeQLCliServer } from './cli';
import { logger } from './logging';
@@ -41,12 +42,35 @@ export interface ProgressUpdate {
export type ProgressCallback = (p: ProgressUpdate) => void;
/**
* A task that handles command invocations from `commandRunner`
* and includes a progress monitor.
*
*
* Arguments passed to the command handler are passed along,
* untouched to this `ProgressTask` instance.
*
* @param progress a progress handler function. Call this
* function with a `ProgressUpdate` instance in order to
* denote some progress being achieved on this task.
* @param token a cencellation token
* @param args arguments passed to this task passed on from
* `commands.registerCommand`.
*/
export type ProgressTask<R> = (
progress: ProgressCallback,
token: CancellationToken,
...args: any[]
) => Thenable<R>;
/**
* A task that handles command invocations from `commandRunner`.
* Arguments passed to the command handler are passed along,
* untouched to this `NoProgressTask` instance.
*
* @param args arguments passed to this task passed on from
* `commands.registerCommand`.
*/
type NoProgressTask = ((...args: any[]) => Promise<any>);
/**
@@ -83,36 +107,21 @@ export function withProgress<R>(
}
/**
* A generic wrapper for commands. This wrapper adds error handling and progress monitoring
* for any command.
* A generic wrapper for command registration. This wrapper adds uniform error handling for commands.
*
* There are two ways to invoke the command task: with or without a progress monitor
* If progressOptions are passed in, then the command task will run with a progress monitor
* Otherwise, no progress monitor will be used.
*
* If a task is run with a progress monitor, the first two arguments to the task are always
* the progress callback, and the cancellation token. And this is followed by any extra command arguments
* (eg- selection, multiselection, ...).
*
* If there is no progress monitor, then only extra command arguments are passed in.
* In this variant of the command runner, no progress monitor is used.
*
* @param commandId The ID of the command to register.
* @param task The task to run. If passing taskOptions, then this task must be a `ProgressTask`.
* @param progressOptions Optional argument. If present, then the task is run with a progress monitor
* and cancellation token, otherwise it is run with no arguments.
* @param task The task to run. It is passed directly to `commands.registerCommand`. Any
* arguments to the command handler are passed on to the task.
*/
export function commandRunner<R>(
export function commandRunner(
commandId: string,
task: ProgressTask<R> | NoProgressTask,
progressOptions?: ProgressOptions
task: NoProgressTask,
): Disposable {
return commands.registerCommand(commandId, async (...args: any[]) => {
try {
if (progressOptions) {
await withProgress(progressOptions, task as ProgressTask<R>, ...args);
} else {
await (task as ((...args: any[]) => Promise<any>))(...args);
}
await task(...args);
} catch (e) {
if (e instanceof UserCancellationException) {
// User has cancelled this action manually
@@ -121,10 +130,45 @@ export function commandRunner<R>(
} else {
showAndLogWarningMessage(e.message);
}
} else if (e instanceof Error) {
showAndLogErrorMessage(e.message);
} else {
throw e;
showAndLogErrorMessage(e.message || e);
}
}
});
}
/**
* A generic wrapper for command registration. This wrapper adds uniform error handling,
* progress monitoring, and cancellation for commands.
*
* @param commandId The ID of the command to register.
* @param task The task to run. It is passed directly to `commands.registerCommand`. Any
* arguments to the command handler are passed on to the task after the progress callback
* and cancellation token.
* @param progressOptions Progress options to be sent to the progress monitor.
*/
export function commandRunnerWithProgress<R>(
commandId: string,
task: ProgressTask<R>,
progressOptions: Partial<ProgressOptions>
): Disposable {
return commands.registerCommand(commandId, async (...args: any[]) => {
const progressOptionsWithDefaults = {
location: ProgressLocation.Notification,
...progressOptions
};
try {
await withProgress(progressOptionsWithDefaults, task, ...args);
} catch (e) {
if (e instanceof UserCancellationException) {
// User has cancelled this action manually
if (e.silent) {
logger.log(e.message);
} else {
showAndLogWarningMessage(e.message);
}
} else {
showAndLogErrorMessage(e.message || e);
}
}
});

View File

@@ -151,33 +151,35 @@ export async function showResolvableLocation(
}
export async function showLocation(location?: Location) {
if (location) {
const doc = await workspace.openTextDocument(location.uri);
const editorsWithDoc = Window.visibleTextEditors.filter(
(e) => e.document === doc
);
const editor =
editorsWithDoc.length > 0
? editorsWithDoc[0]
: await Window.showTextDocument(doc, ViewColumn.One);
const range = location.range;
// When highlighting the range, vscode's occurrence-match and bracket-match highlighting will
// trigger based on where we place the cursor/selection, and will compete for the user's attention.
// For reference:
// - Occurences are highlighted when the cursor is next to or inside a word or a whole word is selected.
// - Brackets are highlighted when the cursor is next to a bracket and there is an empty selection.
// - Multi-line selections explicitly highlight line-break characters, but multi-line decorators do not.
//
// For single-line ranges, select the whole range, mainly to disable bracket highlighting.
// For multi-line ranges, place the cursor at the beginning to avoid visual artifacts from selected line-breaks.
// Multi-line ranges are usually large enough to overshadow the noise from bracket highlighting.
const selectionEnd =
range.start.line === range.end.line ? range.end : range.start;
editor.selection = new Selection(range.start, selectionEnd);
editor.revealRange(range, TextEditorRevealType.InCenter);
editor.setDecorations(shownLocationDecoration, [range]);
editor.setDecorations(shownLocationLineDecoration, [range]);
if (!location) {
return;
}
const doc = await workspace.openTextDocument(location.uri);
const editorsWithDoc = Window.visibleTextEditors.filter(
(e) => e.document === doc
);
const editor =
editorsWithDoc.length > 0
? editorsWithDoc[0]
: await Window.showTextDocument(doc, ViewColumn.One);
const range = location.range;
// When highlighting the range, vscode's occurrence-match and bracket-match highlighting will
// trigger based on where we place the cursor/selection, and will compete for the user's attention.
// For reference:
// - Occurences are highlighted when the cursor is next to or inside a word or a whole word is selected.
// - Brackets are highlighted when the cursor is next to a bracket and there is an empty selection.
// - Multi-line selections explicitly highlight line-break characters, but multi-line decorators do not.
//
// For single-line ranges, select the whole range, mainly to disable bracket highlighting.
// For multi-line ranges, place the cursor at the beginning to avoid visual artifacts from selected line-breaks.
// Multi-line ranges are usually large enough to overshadow the noise from bracket highlighting.
const selectionEnd =
range.start.line === range.end.line ? range.end : range.start;
editor.selection = new Selection(range.start, selectionEnd);
editor.revealRange(range, TextEditorRevealType.InCenter);
editor.setDecorations(shownLocationDecoration, [range]);
editor.setDecorations(shownLocationLineDecoration, [range]);
}
const findMatchBackground = new ThemeColor('editor.findMatchBackground');