Fix set label command on history items

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 github#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.
aeisenberg · Dec 16, 2020 · 1f7d0fa · 1f7d0fa
1 parent 3ea3eda
commit 1f7d0fa
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 36 deletions.
diff --git a/extensions/ql-vscode/CHANGELOG.md b/extensions/ql-vscode/CHANGELOG.md
@@ -11,6 +11,7 @@
 - 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)
 - Add descriptive text and a link in the results view. [#711](https://github.com/github/vscode-codeql/pull/711)
+- 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
 

diff --git a/extensions/ql-vscode/src/extension.ts b/extensions/ql-vscode/src/extension.ts
@@ -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);
     }
   }
 

diff --git a/extensions/ql-vscode/src/query-history.ts b/extensions/ql-vscode/src/query-history.ts
@@ -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
@@ -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(
@@ -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 {
@@ -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();
     }
   }
@@ -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);
   }
 }
diff --git a/extensions/ql-vscode/src/query-results.ts b/extensions/ql-vscode/src/query-results.ts
@@ -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';
@@ -17,7 +17,6 @@ export class CompletedQuery implements QueryWithResults {
   readonly database: DatabaseInfo;
   readonly logFileLocation?: string;
   options: QueryHistoryItemOptions;
-  treeItem?: TreeItem;
   dispose: () => void;
 
   /**