Merge pull request #1235 from github/aeisenberg/history-sort
Add query history sorting for remote queries
This commit is contained in:
@@ -3,6 +3,8 @@
|
||||
## [UNRELEASED]
|
||||
|
||||
- Fix a bug where the AST viewer was not synchronizing its selected node when the editor selection changes. [#1230](https://github.com/github/vscode-codeql/pull/1230)
|
||||
- Open the directory in the finder/explorer (instead of just highlighting it) when running the "Open query directory" command from the query history view. [#1235](https://github.com/github/vscode-codeql/pull/1235)
|
||||
- Ensure query label in the query history view changes are persisted across restarts. [#1235](https://github.com/github/vscode-codeql/pull/1235)
|
||||
|
||||
## 1.6.1 - 17 March 2022
|
||||
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import * as path from 'path';
|
||||
import * as fs from 'fs-extra';
|
||||
import {
|
||||
commands,
|
||||
Disposable,
|
||||
@@ -28,7 +29,7 @@ import { URLSearchParams } from 'url';
|
||||
import { QueryServerClient } from './queryserver-client';
|
||||
import { DisposableObject } from './pure/disposable-object';
|
||||
import { commandRunner } from './commandRunner';
|
||||
import { assertNever, ONE_HOUR_IN_MS, TWO_HOURS_IN_MS, getErrorMessage, getErrorStack } from './pure/helpers-pure';
|
||||
import { assertNever, ONE_HOUR_IN_MS, TWO_HOURS_IN_MS, getErrorMessage, getErrorStack } from './pure/helpers-pure';
|
||||
import { CompletedLocalQueryInfo, LocalQueryInfo as LocalQueryInfo, QueryHistoryInfo } from './query-results';
|
||||
import { DatabaseManager } from './databases';
|
||||
import { registerQueryHistoryScubber } from './query-history-scrubber';
|
||||
@@ -183,38 +184,48 @@ export class HistoryTreeDataProvider extends DisposableObject {
|
||||
): ProviderResult<QueryHistoryInfo[]> {
|
||||
return element ? [] : this.history.sort((h1, h2) => {
|
||||
|
||||
// TODO remote queries are not implemented yet.
|
||||
if (h1.t !== 'local' && h2.t !== 'local') {
|
||||
return 0;
|
||||
}
|
||||
if (h1.t !== 'local') {
|
||||
return -1;
|
||||
}
|
||||
if (h2.t !== 'local') {
|
||||
return 1;
|
||||
}
|
||||
const h1Label = h1.label.toLowerCase();
|
||||
const h2Label = h2.label.toLowerCase();
|
||||
|
||||
const resultCount1 = h1.completedQuery?.resultCount ?? -1;
|
||||
const resultCount2 = h2.completedQuery?.resultCount ?? -1;
|
||||
const h1Date = h1.t === 'local'
|
||||
? h1.initialInfo.start.getTime()
|
||||
: h1.remoteQuery?.executionStartTime;
|
||||
|
||||
const h2Date = h2.t === 'local'
|
||||
? h2.initialInfo.start.getTime()
|
||||
: h2.remoteQuery?.executionStartTime;
|
||||
|
||||
// result count for remote queries is not available here.
|
||||
const resultCount1 = h1.t === 'local'
|
||||
? h1.completedQuery?.resultCount ?? -1
|
||||
: -1;
|
||||
const resultCount2 = h2.t === 'local'
|
||||
? h2.completedQuery?.resultCount ?? -1
|
||||
: -1;
|
||||
|
||||
switch (this.sortOrder) {
|
||||
case SortOrder.NameAsc:
|
||||
return h1.label.localeCompare(h2.label, env.language);
|
||||
return h1Label.localeCompare(h2Label, env.language);
|
||||
|
||||
case SortOrder.NameDesc:
|
||||
return h2.label.localeCompare(h1.label, env.language);
|
||||
return h2Label.localeCompare(h1Label, env.language);
|
||||
|
||||
case SortOrder.DateAsc:
|
||||
return h1.initialInfo.start.getTime() - h2.initialInfo.start.getTime();
|
||||
return h1Date - h2Date;
|
||||
|
||||
case SortOrder.DateDesc:
|
||||
return h2.initialInfo.start.getTime() - h1.initialInfo.start.getTime();
|
||||
return h2Date - h1Date;
|
||||
|
||||
case SortOrder.CountAsc:
|
||||
// If the result counts are equal, sort by name.
|
||||
return resultCount1 - resultCount2 === 0
|
||||
? h1.label.localeCompare(h2.label, env.language)
|
||||
? h1Label.localeCompare(h2Label, env.language)
|
||||
: resultCount1 - resultCount2;
|
||||
|
||||
case SortOrder.CountDesc:
|
||||
// If the result counts are equal, sort by name.
|
||||
return resultCount2 - resultCount1 === 0
|
||||
? h2.label.localeCompare(h1.label, env.language)
|
||||
? h2Label.localeCompare(h1Label, env.language)
|
||||
: resultCount2 - resultCount1;
|
||||
default:
|
||||
assertNever(this.sortOrder);
|
||||
@@ -650,7 +661,7 @@ export class QueryHistoryManager extends DisposableObject {
|
||||
if (response !== undefined) {
|
||||
// Interpret empty string response as 'go back to using default'
|
||||
finalSingleItem.initialInfo.userSpecifiedLabel = response === '' ? undefined : response;
|
||||
this.treeDataProvider.refresh();
|
||||
await this.refreshTreeView();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -741,20 +752,28 @@ export class QueryHistoryManager extends DisposableObject {
|
||||
return;
|
||||
}
|
||||
|
||||
let p: string | undefined;
|
||||
let externalFilePath: string | undefined;
|
||||
if (finalSingleItem.t === 'local') {
|
||||
if (finalSingleItem.completedQuery) {
|
||||
p = finalSingleItem.completedQuery.query.querySaveDir;
|
||||
externalFilePath = path.join(finalSingleItem.completedQuery.query.querySaveDir, 'timestamp');
|
||||
}
|
||||
} else if (finalSingleItem.t === 'remote') {
|
||||
p = path.join(this.queryStorageDir, finalSingleItem.queryId);
|
||||
externalFilePath = path.join(this.queryStorageDir, finalSingleItem.queryId, 'timestamp');
|
||||
}
|
||||
|
||||
if (p) {
|
||||
if (externalFilePath) {
|
||||
if (!(await fs.pathExists(externalFilePath))) {
|
||||
// timestamp file is missing (manually deleted?) try selecting the parent folder.
|
||||
// It's less nice, but at least it will work.
|
||||
externalFilePath = path.dirname(externalFilePath);
|
||||
if (!(await fs.pathExists(externalFilePath))) {
|
||||
throw new Error(`Query directory does not exist: ${externalFilePath}`);
|
||||
}
|
||||
}
|
||||
try {
|
||||
await commands.executeCommand('revealFileInOS', Uri.file(p));
|
||||
await commands.executeCommand('revealFileInOS', Uri.file(externalFilePath));
|
||||
} catch (e) {
|
||||
throw new Error(`Failed to open ${p}: ${getErrorMessage(e)}`);
|
||||
throw new Error(`Failed to open ${externalFilePath}: ${getErrorMessage(e)}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -427,9 +427,11 @@ describe('query-history', () => {
|
||||
|
||||
describe('getChildren', () => {
|
||||
const history = [
|
||||
item('a', 10, 20),
|
||||
item('b', 5, 30),
|
||||
item('c', 1, 25),
|
||||
item('a', 2, 'remote'),
|
||||
item('b', 10, 'local', 20),
|
||||
item('c', 5, 'local', 30),
|
||||
item('d', 1, 'local', 25),
|
||||
item('e', 6, 'remote'),
|
||||
];
|
||||
let treeDataProvider: HistoryTreeDataProvider;
|
||||
|
||||
@@ -456,7 +458,7 @@ describe('query-history', () => {
|
||||
});
|
||||
|
||||
it('should get children for date ascending', async () => {
|
||||
const expected = [history[2], history[1], history[0]];
|
||||
const expected = [history[3], history[0], history[2], history[4], history[1]];
|
||||
treeDataProvider.sortOrder = SortOrder.DateAsc;
|
||||
|
||||
const children = await treeDataProvider.getChildren();
|
||||
@@ -464,7 +466,7 @@ describe('query-history', () => {
|
||||
});
|
||||
|
||||
it('should get children for date descending', async () => {
|
||||
const expected = [history[0], history[1], history[2]];
|
||||
const expected = [history[3], history[0], history[2], history[4], history[1]].reverse();
|
||||
treeDataProvider.sortOrder = SortOrder.DateDesc;
|
||||
|
||||
const children = await treeDataProvider.getChildren();
|
||||
@@ -472,7 +474,7 @@ describe('query-history', () => {
|
||||
});
|
||||
|
||||
it('should get children for result count ascending', async () => {
|
||||
const expected = [history[0], history[2], history[1]];
|
||||
const expected = [history[0], history[4], history[1], history[3], history[2]];
|
||||
treeDataProvider.sortOrder = SortOrder.CountAsc;
|
||||
|
||||
const children = await treeDataProvider.getChildren();
|
||||
@@ -480,7 +482,7 @@ describe('query-history', () => {
|
||||
});
|
||||
|
||||
it('should get children for result count descending', async () => {
|
||||
const expected = [history[1], history[2], history[0]];
|
||||
const expected = [history[0], history[4], history[1], history[3], history[2]].reverse();
|
||||
treeDataProvider.sortOrder = SortOrder.CountDesc;
|
||||
|
||||
const children = await treeDataProvider.getChildren();
|
||||
@@ -509,17 +511,27 @@ describe('query-history', () => {
|
||||
expect(children).to.deep.eq(expected);
|
||||
});
|
||||
|
||||
function item(label: string, start: number, resultCount?: number) {
|
||||
return {
|
||||
label,
|
||||
initialInfo: {
|
||||
start: new Date(start),
|
||||
},
|
||||
completedQuery: {
|
||||
resultCount,
|
||||
},
|
||||
t: 'local'
|
||||
};
|
||||
function item(label: string, start: number, t = 'local', resultCount?: number) {
|
||||
if (t === 'local') {
|
||||
return {
|
||||
label,
|
||||
initialInfo: {
|
||||
start: new Date(start),
|
||||
},
|
||||
completedQuery: {
|
||||
resultCount,
|
||||
},
|
||||
t
|
||||
};
|
||||
} else {
|
||||
return {
|
||||
label,
|
||||
remoteQuery: {
|
||||
executionStartTime: start,
|
||||
},
|
||||
t
|
||||
};
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user