Skip to content

Commit

Permalink
Fix set label command on history items
Browse files Browse the repository at this point in the history
This removes the cached treeItem that is a property of the
completedQuery. We should not be caching them since they are cached by
the vscode api itself. Instead, we should recreate whenever requested.

Also, this change fixes #598 in a new way. Instead of adding the
context to the cached treeItem, we simply refresh only the item that has
changed. This is a fast operation.
  • Loading branch information
aeisenberg committed Dec 15, 2020
1 parent ca9510c commit 4462cf0
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 36 deletions.
1 change: 1 addition & 0 deletions extensions/ql-vscode/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
- Add clearer error message when trying to run a command from the query history view if no item in the history is selected. [#702](https://github.com/github/vscode-codeql/pull/702)
- Fix a bug where it is not possible to download some database archives. This fix specifically addresses large archives and archives whose central directories do not align with file headers. [#700](https://github.com/github/vscode-codeql/pull/700)
- Avoid error dialogs when QL test discovery or database cleanup encounters a missing directory. [#706](https://github.com/github/vscode-codeql/pull/706)
- Fix the _Set Label_ command in the query history view. [#710](https://github.com/github/vscode-codeql/pull/710)

## 1.3.7 - 24 November 2020

Expand Down
2 changes: 1 addition & 1 deletion extensions/ql-vscode/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ async function activateWithInstalledDistribution(
// The call to showResults potentially creates SARIF file;
// Update the tree item context value to allow viewing that
// SARIF file from context menu.
await qhm.updateTreeItemContextValue(item);
await qhm.refreshTreeView(item);
}
}

Expand Down
51 changes: 18 additions & 33 deletions extensions/ql-vscode/src/query-history.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,10 @@ const SHOW_QUERY_TEXT_QUICK_EVAL_MSG = `\
*/
const FAILED_QUERY_HISTORY_ITEM_ICON = 'media/red-x.svg';

interface QueryHistoryDataProvider extends vscode.TreeDataProvider<CompletedQuery> {
updateTreeItemContextValue(element: CompletedQuery): Promise<void>;
}

/**
* Tree data provider for the query history view.
*/
class HistoryTreeDataProvider extends DisposableObject implements QueryHistoryDataProvider {
class HistoryTreeDataProvider extends DisposableObject {
private _onDidChangeTreeData = super.push(new vscode.EventEmitter<CompletedQuery | undefined>());

readonly onDidChangeTreeData: vscode.Event<CompletedQuery | undefined> = this
Expand All @@ -82,37 +78,28 @@ class HistoryTreeDataProvider extends DisposableObject implements QueryHistoryDa
);
}

async updateTreeItemContextValue(element: CompletedQuery): Promise<void> {
// Mark this query history item according to whether it has a
// SARIF file so that we can make context menu items conditionally
// available.
const hasResults = await element.query.hasInterpretedResults();
element.treeItem!.contextValue = hasResults
? 'interpretedResultsItem'
: 'rawResultsItem';
this.refresh();
}

async getTreeItem(element: CompletedQuery): Promise<vscode.TreeItem> {
if (element.treeItem !== undefined)
return element.treeItem;

const it = new vscode.TreeItem(element.toString());
const treeItem = new vscode.TreeItem(element.toString());

it.command = {
treeItem.command = {
title: 'Query History Item',
command: 'codeQLQueryHistory.itemClicked',
arguments: [element],
};

element.treeItem = it;
this.updateTreeItemContextValue(element);
// Mark this query history item according to whether it has a
// SARIF file so that we can make context menu items conditionally
// available.
const hasResults = await element.query.hasInterpretedResults();
treeItem.contextValue = hasResults
? 'interpretedResultsItem'
: 'rawResultsItem';

if (!element.didRunSuccessfully) {
it.iconPath = this.failedIconPath;
treeItem.iconPath = this.failedIconPath;
}

return it;
return treeItem;
}

getChildren(
Expand Down Expand Up @@ -157,8 +144,8 @@ class HistoryTreeDataProvider extends DisposableObject implements QueryHistoryDa
return this.history;
}

refresh() {
this._onDidChangeTreeData.fire(undefined);
refresh(CompletedQuery?: CompletedQuery) {
this._onDidChangeTreeData.fire(CompletedQuery);
}

find(queryId: number): CompletedQuery | undefined {
Expand Down Expand Up @@ -367,10 +354,8 @@ export class QueryHistoryManager extends DisposableObject {
});
// undefined response means the user cancelled the dialog; don't change anything
if (response !== undefined) {
if (response === '')
// Interpret empty string response as 'go back to using default'
singleItem.options.label = undefined;
else singleItem.options.label = response;
// Interpret empty string response as 'go back to using default'
singleItem.options.label = response === '' ? undefined : response;
this.treeDataProvider.refresh();
}
}
Expand Down Expand Up @@ -695,7 +680,7 @@ the file in the file explorer and dragging it into the workspace.`
};
}

async updateTreeItemContextValue(element: CompletedQuery): Promise<void> {
this.treeDataProvider.updateTreeItemContextValue(element);
async refreshTreeView(completedQuery: CompletedQuery): Promise<void> {
this.treeDataProvider.refresh(completedQuery);
}
}
3 changes: 1 addition & 2 deletions extensions/ql-vscode/src/query-results.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { env, TreeItem } from 'vscode';
import { env } from 'vscode';

import { QueryWithResults, tmpDir, QueryInfo } from './run-queries';
import * as messages from './pure/messages';
Expand All @@ -17,7 +17,6 @@ export class CompletedQuery implements QueryWithResults {
readonly database: DatabaseInfo;
readonly logFileLocation?: string;
options: QueryHistoryItemOptions;
treeItem?: TreeItem;
dispose: () => void;

/**
Expand Down

0 comments on commit 4462cf0

Please sign in to comment.