Skip to content

Commit bf02494

Browse files
authored
Merge pull request #2794 from github/koesie10/improve-model-editor
Improve responsiveness of model editor
2 parents 567cb10 + 72ab83f commit bf02494

File tree

8 files changed

+81
-36
lines changed

8 files changed

+81
-36
lines changed

extensions/ql-vscode/src/local-queries/run-query.ts

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
11
import { CancellationToken } from "vscode";
2-
import { CodeQLCliServer } from "../codeql-cli/cli";
32
import { ProgressCallback } from "../common/vscode/progress";
43
import { DatabaseItem } from "../databases/local-databases";
54
import { CoreCompletedQuery, QueryRunner } from "../query-server";
6-
import { createLockFileForStandardQuery } from "./standard-queries";
75
import { TeeLogger, showAndLogExceptionWithTelemetry } from "../common/logging";
86
import { QueryResultType } from "../query-server/new-messages";
97
import { extLogger } from "../common/logging/vscode";
@@ -12,7 +10,6 @@ import { redactableError } from "../common/errors";
1210
import { basename } from "path";
1311

1412
type RunQueryOptions = {
15-
cliServer: CodeQLCliServer;
1613
queryRunner: QueryRunner;
1714
databaseItem: DatabaseItem;
1815
queryPath: string;
@@ -21,11 +18,9 @@ type RunQueryOptions = {
2118
extensionPacks: string[] | undefined;
2219
progress: ProgressCallback;
2320
token: CancellationToken;
24-
createLockFile: boolean;
2521
};
2622

2723
export async function runQuery({
28-
cliServer,
2924
queryRunner,
3025
databaseItem,
3126
queryPath,
@@ -34,18 +29,7 @@ export async function runQuery({
3429
extensionPacks,
3530
progress,
3631
token,
37-
createLockFile,
3832
}: RunQueryOptions): Promise<CoreCompletedQuery | undefined> {
39-
let cleanupLockFile: (() => Promise<void>) | undefined = undefined;
40-
if (createLockFile) {
41-
// Create a lock file for the query. This is required to resolve dependencies and library path for the query.
42-
const { cleanup } = await createLockFileForStandardQuery(
43-
cliServer,
44-
queryPath,
45-
);
46-
cleanupLockFile = cleanup;
47-
}
48-
4933
// Create a query run to execute
5034
const queryRun = queryRunner.createQueryRun(
5135
databaseItem.databaseUri.fsPath,
@@ -68,8 +52,6 @@ export async function runQuery({
6852
new TeeLogger(queryRunner.logger, queryRun.outputDir.logPath),
6953
);
7054

71-
await cleanupLockFile?.();
72-
7355
if (completedQuery.resultType !== QueryResultType.SUCCESS) {
7456
void showAndLogExceptionWithTelemetry(
7557
extLogger,

extensions/ql-vscode/src/model-editor/auto-model-codeml-queries.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,6 @@ export async function runAutoModelQueries({
7575

7676
// Run the actual query
7777
const completedQuery = await runQuery({
78-
cliServer,
7978
queryRunner,
8079
databaseItem,
8180
queryPath,
@@ -84,7 +83,6 @@ export async function runAutoModelQueries({
8483
extensionPacks,
8584
progress,
8685
token: cancellationTokenSource.token,
87-
createLockFile: false,
8886
});
8987

9088
if (!completedQuery) {

extensions/ql-vscode/src/model-editor/extension-pack-picker.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,12 @@ import {
1818
} from "./extension-pack-name";
1919
import { autoPickExtensionsDirectory } from "./extensions-workspace-folder";
2020

21-
const maxStep = 3;
22-
2321
export async function pickExtensionPack(
2422
cliServer: Pick<CodeQLCliServer, "resolveQlpacks">,
2523
databaseItem: Pick<DatabaseItem, "name" | "language">,
2624
logger: NotificationLogger,
2725
progress: ProgressCallback,
26+
maxStep: number,
2827
): Promise<ExtensionPack | undefined> {
2928
progress({
3029
message: "Resolving extension packs...",

extensions/ql-vscode/src/model-editor/external-api-usage-queries.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,6 @@ export async function runExternalApiQueries(
9292

9393
// Run the actual query
9494
const completedQuery = await runQuery({
95-
cliServer,
9695
queryRunner,
9796
databaseItem,
9897
queryPath,
@@ -106,8 +105,6 @@ export async function runExternalApiQueries(
106105
message: update.message,
107106
}),
108107
token,
109-
// We need to create a lock file, because the query is inside our own pack
110-
createLockFile: true,
111108
});
112109

113110
if (!completedQuery) {

extensions/ql-vscode/src/model-editor/flow-model-queries.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,6 @@ async function runSingleFlowQuery(
120120

121121
// Run the query
122122
const completedQuery = await runQuery({
123-
cliServer,
124123
queryRunner,
125124
databaseItem,
126125
queryPath,
@@ -134,7 +133,6 @@ async function runSingleFlowQuery(
134133
maxStep: 4000,
135134
}),
136135
token,
137-
createLockFile: false,
138136
});
139137

140138
if (!completedQuery) {

extensions/ql-vscode/src/model-editor/model-editor-module.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,8 @@ export class ModelEditorModule extends DisposableObject {
102102

103103
return withProgress(
104104
async (progress) => {
105+
const maxStep = 4;
106+
105107
if (!(await this.cliServer.cliConstraints.supportsQlpacksKind())) {
106108
void showAndLogErrorMessage(
107109
this.app.logger,
@@ -125,18 +127,31 @@ export class ModelEditorModule extends DisposableObject {
125127
db,
126128
this.app.logger,
127129
progress,
130+
maxStep,
128131
);
129132
if (!modelFile) {
130133
return;
131134
}
132135

136+
progress({
137+
message: "Installing dependencies...",
138+
step: 3,
139+
maxStep,
140+
});
141+
133142
// Create new temporary directory for query files and pack dependencies
134143
const queryDir = (await dir({ unsafeCleanup: true })).path;
135144
const success = await setUpPack(this.cliServer, queryDir, language);
136145
if (!success) {
137146
return;
138147
}
139148

149+
progress({
150+
message: "Opening editor...",
151+
step: 4,
152+
maxStep,
153+
});
154+
140155
const view = new ModelEditorView(
141156
this.ctx,
142157
this.app,

extensions/ql-vscode/src/model-editor/model-editor-view.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,7 @@ export class ModelEditorView extends AbstractWebview<
484484
addedDatabase,
485485
this.app.logger,
486486
progress,
487+
3,
487488
);
488489
if (!modelFile) {
489490
return;

extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/extension-pack-picker.test.ts

Lines changed: 64 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ describe("pickExtensionPack", () => {
3434
let workspaceFolder: WorkspaceFolder;
3535

3636
const logger = createMockLogger();
37+
const maxStep = 4;
3738

3839
beforeEach(async () => {
3940
tmpDir = (
@@ -98,7 +99,13 @@ describe("pickExtensionPack", () => {
9899
const cliServer = mockCliServer(qlPacks);
99100

100101
expect(
101-
await pickExtensionPack(cliServer, databaseItem, logger, progress),
102+
await pickExtensionPack(
103+
cliServer,
104+
databaseItem,
105+
logger,
106+
progress,
107+
maxStep,
108+
),
102109
).toEqual(autoExtensionPack);
103110
expect(cliServer.resolveQlpacks).toHaveBeenCalledTimes(1);
104111
expect(cliServer.resolveQlpacks).toHaveBeenCalledWith(
@@ -173,7 +180,13 @@ describe("pickExtensionPack", () => {
173180
const cliServer = mockCliServer({});
174181

175182
expect(
176-
await pickExtensionPack(cliServer, databaseItem, logger, progress),
183+
await pickExtensionPack(
184+
cliServer,
185+
databaseItem,
186+
logger,
187+
progress,
188+
maxStep,
189+
),
177190
).toEqual({
178191
path: newPackDir,
179192
yamlPath: join(newPackDir, "codeql-pack.yml"),
@@ -241,7 +254,13 @@ describe("pickExtensionPack", () => {
241254
const cliServer = mockCliServer({});
242255

243256
expect(
244-
await pickExtensionPack(cliServer, databaseItem, logger, progress),
257+
await pickExtensionPack(
258+
cliServer,
259+
databaseItem,
260+
logger,
261+
progress,
262+
maxStep,
263+
),
245264
).toEqual({
246265
path: newPackDir,
247266
yamlPath: join(newPackDir, "codeql-pack.yml"),
@@ -277,7 +296,13 @@ describe("pickExtensionPack", () => {
277296
});
278297

279298
expect(
280-
await pickExtensionPack(cliServer, databaseItem, logger, progress),
299+
await pickExtensionPack(
300+
cliServer,
301+
databaseItem,
302+
logger,
303+
progress,
304+
maxStep,
305+
),
281306
).toEqual(undefined);
282307
expect(logger.showErrorMessage).toHaveBeenCalledTimes(1);
283308
expect(logger.showErrorMessage).toHaveBeenCalledWith(
@@ -296,7 +321,13 @@ describe("pickExtensionPack", () => {
296321
});
297322

298323
expect(
299-
await pickExtensionPack(cliServer, databaseItem, logger, progress),
324+
await pickExtensionPack(
325+
cliServer,
326+
databaseItem,
327+
logger,
328+
progress,
329+
maxStep,
330+
),
300331
).toEqual(undefined);
301332
expect(logger.showErrorMessage).toHaveBeenCalledTimes(1);
302333
expect(logger.showErrorMessage).toHaveBeenCalledWith(
@@ -317,7 +348,13 @@ describe("pickExtensionPack", () => {
317348
await outputFile(join(tmpDir.path, "codeql-pack.yml"), dumpYaml("java"));
318349

319350
expect(
320-
await pickExtensionPack(cliServer, databaseItem, logger, progress),
351+
await pickExtensionPack(
352+
cliServer,
353+
databaseItem,
354+
logger,
355+
progress,
356+
maxStep,
357+
),
321358
).toEqual(undefined);
322359
expect(logger.showErrorMessage).toHaveBeenCalledTimes(1);
323360
expect(logger.showErrorMessage).toHaveBeenCalledWith(
@@ -348,7 +385,13 @@ describe("pickExtensionPack", () => {
348385
);
349386

350387
expect(
351-
await pickExtensionPack(cliServer, databaseItem, logger, progress),
388+
await pickExtensionPack(
389+
cliServer,
390+
databaseItem,
391+
logger,
392+
progress,
393+
maxStep,
394+
),
352395
).toEqual(undefined);
353396
expect(logger.showErrorMessage).toHaveBeenCalledTimes(1);
354397
expect(logger.showErrorMessage).toHaveBeenCalledWith(
@@ -382,7 +425,13 @@ describe("pickExtensionPack", () => {
382425
);
383426

384427
expect(
385-
await pickExtensionPack(cliServer, databaseItem, logger, progress),
428+
await pickExtensionPack(
429+
cliServer,
430+
databaseItem,
431+
logger,
432+
progress,
433+
maxStep,
434+
),
386435
).toEqual(undefined);
387436
expect(logger.showErrorMessage).toHaveBeenCalledTimes(1);
388437
expect(logger.showErrorMessage).toHaveBeenCalledWith(
@@ -433,7 +482,13 @@ describe("pickExtensionPack", () => {
433482
};
434483

435484
expect(
436-
await pickExtensionPack(cliServer, databaseItem, logger, progress),
485+
await pickExtensionPack(
486+
cliServer,
487+
databaseItem,
488+
logger,
489+
progress,
490+
maxStep,
491+
),
437492
).toEqual(extensionPack);
438493
});
439494
});

0 commit comments

Comments
 (0)