-
Notifications
You must be signed in to change notification settings - Fork 226
Query history: Add new VariantAnalysisHistoryItem type
#1590
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.
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
Changes from all commits
64a0f54
e993d9e
f5d3e91
0612385
127a23e
a5795df
c5db0e1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -206,13 +206,9 @@ export class HistoryTreeDataProvider extends DisposableObject implements TreeDat | |
| const h1Label = this.labelProvider.getLabel(h1).toLowerCase(); | ||
| const h2Label = this.labelProvider.getLabel(h2).toLowerCase(); | ||
|
|
||
| const h1Date = h1.t === 'local' | ||
| ? h1.initialInfo.start.getTime() | ||
| : h1.remoteQuery?.executionStartTime; | ||
| const h1Date = this.getItemDate(h1); | ||
|
|
||
| const h2Date = h2.t === 'local' | ||
| ? h2.initialInfo.start.getTime() | ||
| : h2.remoteQuery?.executionStartTime; | ||
| const h2Date = this.getItemDate(h2); | ||
|
|
||
| const resultCount1 = h1.t === 'local' | ||
| ? h1.completedQuery?.resultCount ?? -1 | ||
|
|
@@ -311,6 +307,19 @@ export class HistoryTreeDataProvider extends DisposableObject implements TreeDat | |
| this._sortOrder = newSortOrder; | ||
| this._onDidChangeTreeData.fire(undefined); | ||
| } | ||
|
|
||
| private getItemDate(item: QueryHistoryInfo) { | ||
| switch (item.t) { | ||
| case 'local': | ||
| return item.initialInfo.start.getTime(); | ||
| case 'remote': | ||
| return item.remoteQuery.executionStartTime; | ||
| case 'variant-analysis': | ||
| return item.variantAnalysis.executionStartTime; | ||
| default: | ||
| assertNever(item); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| export class QueryHistoryManager extends DisposableObject { | ||
|
|
@@ -649,10 +658,20 @@ export class QueryHistoryManager extends DisposableObject { | |
| return; | ||
| } | ||
|
|
||
| const queryPath = finalSingleItem.t === 'local' | ||
| ? finalSingleItem.initialInfo.queryPath | ||
| : finalSingleItem.remoteQuery.queryFilePath; | ||
|
|
||
| let queryPath: string; | ||
| switch (finalSingleItem.t) { | ||
| case 'local': | ||
| queryPath = finalSingleItem.initialInfo.queryPath; | ||
| break; | ||
| case 'remote': | ||
| queryPath = finalSingleItem.remoteQuery.queryFilePath; | ||
| break; | ||
| case 'variant-analysis': | ||
| queryPath = finalSingleItem.variantAnalysis.query.filePath; | ||
| break; | ||
| default: | ||
| assertNever(finalSingleItem); | ||
| } | ||
| const textDocument = await workspace.openTextDocument( | ||
| Uri.file(queryPath) | ||
| ); | ||
|
|
@@ -710,8 +729,12 @@ export class QueryHistoryManager extends DisposableObject { | |
| // We need to delete it from disk as well. | ||
| await item.completedQuery?.query.deleteQuery(); | ||
| } | ||
| } else { | ||
| } else if (item.t === 'remote') { | ||
| await this.removeRemoteQuery(item); | ||
| } else if (item.t === 'variant-analysis') { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add an |
||
| // TODO | ||
| } else { | ||
| assertNever(item); | ||
| } | ||
| })); | ||
|
|
||
|
|
@@ -1025,15 +1048,20 @@ export class QueryHistoryManager extends DisposableObject { | |
| isQuickEval: String(!!(finalSingleItem.t === 'local' && finalSingleItem.initialInfo.quickEvalPosition)), | ||
| queryText: encodeURIComponent(await this.getQueryText(finalSingleItem)), | ||
| }); | ||
| const queryId = finalSingleItem.t === 'local' | ||
| ? finalSingleItem.initialInfo.id | ||
| : finalSingleItem.queryId; | ||
|
|
||
| const uri = Uri.parse( | ||
| `codeql:${queryId}?${params.toString()}`, true | ||
| ); | ||
| const doc = await workspace.openTextDocument(uri); | ||
| await window.showTextDocument(doc, { preview: false }); | ||
| if (finalSingleItem.t === 'variant-analysis') { | ||
| // TODO | ||
| } else { | ||
| const queryId = finalSingleItem.t === 'local' | ||
| ? finalSingleItem.initialInfo.id | ||
| : finalSingleItem.queryId; | ||
|
|
||
| const uri = Uri.parse( | ||
| `codeql:${queryId}?${params.toString()}`, true | ||
| ); | ||
| const doc = await workspace.openTextDocument(uri); | ||
| await window.showTextDocument(doc, { preview: false }); | ||
| } | ||
| } | ||
|
|
||
| async handleViewSarifAlerts( | ||
|
|
@@ -1149,9 +1177,16 @@ export class QueryHistoryManager extends DisposableObject { | |
| } | ||
|
|
||
| async getQueryText(item: QueryHistoryInfo): Promise<string> { | ||
| return item.t === 'local' | ||
| ? item.initialInfo.queryText | ||
| : item.remoteQuery.queryText; | ||
| switch (item.t) { | ||
| case 'local': | ||
| return item.initialInfo.queryText; | ||
| case 'remote': | ||
| return item.remoteQuery.queryText; | ||
| case 'variant-analysis': | ||
| return 'TODO'; | ||
| default: | ||
| assertNever(item); | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| } | ||
|
|
||
| async handleExportResults(): Promise<void> { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,8 +15,8 @@ export async function slurpQueryHistory(fsPath: string): Promise<QueryHistoryInf | |
|
|
||
| const data = await fs.readFile(fsPath, 'utf8'); | ||
| const obj = JSON.parse(data); | ||
| if (obj.version !== 1) { | ||
| void showAndLogErrorMessage(`Unsupported query history format: v${obj.version}. `); | ||
| if (![1, 2].includes(obj.version)) { | ||
| void showAndLogErrorMessage(`Can't parse query history. Unsupported query history format: v${obj.version}. `); | ||
| return []; | ||
| } | ||
|
|
||
|
|
@@ -54,7 +54,7 @@ export async function slurpQueryHistory(fsPath: string): Promise<QueryHistoryInf | |
| // most likely another workspace has deleted them because the | ||
| // queries aged out. | ||
| return asyncFilter(parsedQueries, async (q) => { | ||
| if (q.t === 'remote') { | ||
| if (q.t === 'remote' || q.t === 'variant-analysis') { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On line 18, there is a |
||
| // the slurper doesn't know where the remote queries are stored | ||
| // so we need to assume here that they exist. Later, we check to | ||
| // see if they exist on disk. | ||
|
|
@@ -90,7 +90,7 @@ export async function splatQueryHistory(queries: QueryHistoryInfo[], fsPath: str | |
| // remove incomplete local queries since they cannot be recreated on restart | ||
| const filteredQueries = queries.filter(q => q.t === 'local' ? q.completedQuery !== undefined : true); | ||
| const data = JSON.stringify({ | ||
| version: 1, | ||
| version: 2, // version 2 adds the `variant-analysis` type. | ||
| queries: filteredQueries | ||
| }, null, 2); | ||
| await fs.writeFile(fsPath, data); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| import { QueryStatus } from '../query-status'; | ||
| import { VariantAnalysis } from './shared/variant-analysis'; | ||
|
|
||
| /** | ||
| * Information about a variant analysis. | ||
| */ | ||
| export interface VariantAnalysisHistoryItem { | ||
| readonly t: 'variant-analysis'; | ||
| failureReason?: string; | ||
| resultCount?: number; | ||
| status: QueryStatus; | ||
| completed: boolean; | ||
| variantAnalysis: VariantAnalysis; | ||
| userSpecifiedLabel?: string; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should probably add a
defaultcase.