Merge pull request #1290 from github/aeisenberg/remote-history-label-editing

Allow remote query items to have their labels edited

Author: aeisenberg

extensions/ql-vscode/package.json

@@ -224,7 +224,7 @@
         },
         "codeQL.queryHistory.format": {
           "type": "string",
-          "default": "%q on %d - %s, %r result count [%t]",
+          "default": "%q on %d - %s, %r [%t]",
           "markdownDescription": "Default string for how to label query history items.\n* %t is the time of the query\n* %q is the human-readable query name\n* %f is the query file name\n* %d is the database name\n* %r is the number of results\n* %s is a status string"
         },
         "codeQL.queryHistory.ttl": {

extensions/ql-vscode/src/compare/compare-interface.ts

@@ -22,6 +22,7 @@ import { transformBqrsResultSet, RawResultSet, BQRSInfo } from '../pure/bqrs-cli
 import resultsDiff from './resultsDiff';
 import { CompletedLocalQueryInfo } from '../query-results';
 import { getErrorMessage } from '../pure/helpers-pure';
+import { HistoryItemLabelProvider } from '../history-item-label-provider';
 
 interface ComparePair {
   from: CompletedLocalQueryInfo;
@@ -39,6 +40,7 @@ export class CompareInterfaceManager extends DisposableObject {
     private databaseManager: DatabaseManager,
     private cliServer: CodeQLCliServer,
     private logger: Logger,
+    private labelProvider: HistoryItemLabelProvider,
     private showQueryResultsCallback: (
       item: CompletedLocalQueryInfo
     ) => Promise<void>
@@ -81,12 +83,12 @@ export class CompareInterfaceManager extends DisposableObject {
             // since we split the description into several rows
             // only run interpolation if the label is user-defined
             // otherwise we will wind up with duplicated rows
-            name: from.getShortLabel(),
+            name: this.labelProvider.getShortLabel(from),
             status: from.completedQuery.statusString,
             time: from.startTime,
           },
           toQuery: {
-            name: to.getShortLabel(),
+            name: this.labelProvider.getShortLabel(to),
             status: to.completedQuery.statusString,
             time: to.startTime,
           },

extensions/ql-vscode/src/extension.ts

@@ -96,6 +96,7 @@ import { RemoteQueryResult } from './remote-queries/remote-query-result';
 import { URLSearchParams } from 'url';
 import { handleDownloadPacks, handleInstallPackDependencies } from './packaging';
 import { RemoteQueryHistoryItem } from './remote-queries/remote-query-history-item';
+import { HistoryItemLabelProvider } from './history-item-label-provider';
 
 /**
  * extension.ts
@@ -447,6 +448,7 @@ async function activateWithInstalledDistribution(
     showResultsForCompletedQuery(item, WebviewReveal.Forced);
   const queryStorageDir = path.join(ctx.globalStorageUri.fsPath, 'queries');
   await fs.ensureDir(queryStorageDir);
+  const labelProvider = new HistoryItemLabelProvider(queryHistoryConfigurationListener);
 
   void logger.log('Initializing query history.');
   const qhm = new QueryHistoryManager(
@@ -455,6 +457,7 @@ async function activateWithInstalledDistribution(
     queryStorageDir,
     ctx,
     queryHistoryConfigurationListener,
+    labelProvider,
     async (from: CompletedLocalQueryInfo, to: CompletedLocalQueryInfo) =>
       showResultsForComparison(from, to),
   );
@@ -466,8 +469,9 @@ async function activateWithInstalledDistribution(
   });
 
   ctx.subscriptions.push(qhm);
+
   void logger.log('Initializing results panel interface.');
-  const intm = new InterfaceManager(ctx, dbm, cliServer, queryServerLogger);
+  const intm = new InterfaceManager(ctx, dbm, cliServer, queryServerLogger, labelProvider);
   ctx.subscriptions.push(intm);
 
   void logger.log('Initializing compare panel interface.');
@@ -476,6 +480,7 @@ async function activateWithInstalledDistribution(
     dbm,
     cliServer,
     queryServerLogger,
+    labelProvider,
     showResults
   );
   ctx.subscriptions.push(cmpm);
@@ -525,7 +530,7 @@ async function activateWithInstalledDistribution(
       token.onCancellationRequested(() => source.cancel());
 
       const initialInfo = await createInitialQueryInfo(selectedQuery, databaseInfo, quickEval, range);
-      const item = new LocalQueryInfo(initialInfo, queryHistoryConfigurationListener, source);
+      const item = new LocalQueryInfo(initialInfo, source);
       qhm.addQuery(item);
       try {
         const completedQueryInfo = await compileAndRunQueryAgainstDatabase(

extensions/ql-vscode/src/history-item-label-provider.ts

@@ -0,0 +1,82 @@
+import { env } from 'vscode';
+import * as path from 'path';
+import { QueryHistoryConfig } from './config';
+import { LocalQueryInfo, QueryHistoryInfo } from './query-results';
+import { RemoteQueryHistoryItem } from './remote-queries/remote-query-history-item';
+
+interface InterpolateReplacements {
+  t: string; // Start time
+  q: string; // Query name
+  d: string; // Database/Controller repo name
+  r: string; // Result count/Empty
+  s: string; // Status
+  f: string; // Query file name
+  '%': '%'; // Percent sign
+}
+
+export class HistoryItemLabelProvider {
+  constructor(private config: QueryHistoryConfig) {
+    /**/
+  }
+
+  getLabel(item: QueryHistoryInfo) {
+    const replacements = item.t === 'local'
+      ? this.getLocalInterpolateReplacements(item)
+      : this.getRemoteInterpolateReplacements(item);
+
+    const rawLabel = item.userSpecifiedLabel ?? (this.config.format || '%q');
+
+    return this.interpolate(rawLabel, replacements);
+  }
+
+  /**
+   * If there is a user-specified label for this query, interpolate and use that.
+   * Otherwise, use the raw name of this query.
+   *
+   * @returns the name of the query, unless there is a custom label for this query.
+   */
+  getShortLabel(item: QueryHistoryInfo): string {
+    return item.userSpecifiedLabel
+      ? this.getLabel(item)
+      : item.t === 'local'
+        ? item.getQueryName()
+        : item.remoteQuery.queryName;
+  }
+
+
+  private interpolate(rawLabel: string, replacements: InterpolateReplacements): string {
+    return rawLabel.replace(/%(.)/g, (match, key: keyof InterpolateReplacements) => {
+      const replacement = replacements[key];
+      return replacement !== undefined ? replacement : match;
+    });
+  }
+
+  private getLocalInterpolateReplacements(item: LocalQueryInfo): InterpolateReplacements {
+    const { resultCount = 0, statusString = 'in progress' } = item.completedQuery || {};
+    return {
+      t: item.startTime,
+      q: item.getQueryName(),
+      d: item.initialInfo.databaseInfo.name,
+      r: `${resultCount} results`,
+      s: statusString,
+      f: item.getQueryFileName(),
+      '%': '%',
+    };
+  }
+
+  private getRemoteInterpolateReplacements(item: RemoteQueryHistoryItem): InterpolateReplacements {
+    return {
+      t: new Date(item.remoteQuery.executionStartTime).toLocaleString(env.language),
+      q: item.remoteQuery.queryName,
+
+      // There is no database name for remote queries. Instead use the controller repository name.
+      d: `${item.remoteQuery.controllerRepository.owner}/${item.remoteQuery.controllerRepository.name}`,
+
+      // There is no synchronous way to get the results count.
+      r: '',
+      s: item.status,
+      f: path.basename(item.remoteQuery.queryFilePath),
+      '%': '%'
+    };
+  }
+}

extensions/ql-vscode/src/interface.ts

@@ -49,6 +49,7 @@ import { getDefaultResultSetName, ParsedResultSets } from './pure/interface-type
 import { RawResultSet, transformBqrsResultSet, ResultSetSchema } from './pure/bqrs-cli-types';
 import { PAGE_SIZE } from './config';
 import { CompletedLocalQueryInfo } from './query-results';
+import { HistoryItemLabelProvider } from './history-item-label-provider';
 
 /**
  * interface.ts
@@ -136,7 +137,8 @@ export class InterfaceManager extends DisposableObject {
     public ctx: vscode.ExtensionContext,
     private databaseManager: DatabaseManager,
     public cliServer: CodeQLCliServer,
-    public logger: Logger
+    public logger: Logger,
+    private labelProvider: HistoryItemLabelProvider
   ) {
     super();
     this.push(this._diagnosticCollection);
@@ -416,7 +418,7 @@ export class InterfaceManager extends DisposableObject {
         // more asynchronous message to not so abruptly interrupt
         // user's workflow by immediately revealing the panel.
         const showButton = 'View Results';
-        const queryName = fullQuery.getShortLabel();
+        const queryName = this.labelProvider.getShortLabel(fullQuery);
         const resultPromise = vscode.window.showInformationMessage(
           `Finished running query ${queryName.length > 0 ? ` "${queryName}"` : ''
           }.`,
@@ -483,7 +485,7 @@ export class InterfaceManager extends DisposableObject {
       database: fullQuery.initialInfo.databaseInfo,
       shouldKeepOldResultsWhileRendering,
       metadata: fullQuery.completedQuery.query.metadata,
-      queryName: fullQuery.label,
+      queryName: this.labelProvider.getLabel(fullQuery),
       queryPath: fullQuery.initialInfo.queryPath
     });
   }
@@ -516,7 +518,7 @@ export class InterfaceManager extends DisposableObject {
       resultSetNames,
       pageSize: interpretedPageSize(this._interpretation),
       numPages: numInterpretedPages(this._interpretation),
-      queryName: this._displayedQuery.label,
+      queryName: this.labelProvider.getLabel(this._displayedQuery),
       queryPath: this._displayedQuery.initialInfo.queryPath
     });
   }
@@ -601,7 +603,7 @@ export class InterfaceManager extends DisposableObject {
       database: results.initialInfo.databaseInfo,
       shouldKeepOldResultsWhileRendering: false,
       metadata: results.completedQuery.query.metadata,
-      queryName: results.label,
+      queryName: this.labelProvider.getLabel(results),
       queryPath: results.initialInfo.queryPath
     });
   }

extensions/ql-vscode/src/query-history.ts

@@ -36,6 +36,7 @@ import { QueryStatus } from './query-status';
 import { slurpQueryHistory, splatQueryHistory } from './query-serialization';
 import * as fs from 'fs-extra';
 import { CliVersionConstraint } from './cli';
+import { HistoryItemLabelProvider } from './history-item-label-provider';
 import { Credentials } from './authentication';
 import { cancelRemoteQuery } from './remote-queries/gh-actions-api-client';
 
@@ -123,7 +124,10 @@ export class HistoryTreeDataProvider extends DisposableObject {
 
   private current: QueryHistoryInfo | undefined;
 
-  constructor(extensionPath: string) {
+  constructor(
+    extensionPath: string,
+    private readonly labelProvider: HistoryItemLabelProvider,
+  ) {
     super();
     this.failedIconPath = path.join(
       extensionPath,
@@ -140,13 +144,13 @@ export class HistoryTreeDataProvider extends DisposableObject {
   }
 
   async getTreeItem(element: QueryHistoryInfo): Promise<TreeItem> {
-    const treeItem = new TreeItem(element.label);
+    const treeItem = new TreeItem(this.labelProvider.getLabel(element));
 
     treeItem.command = {
       title: 'Query History Item',
       command: 'codeQLQueryHistory.itemClicked',
       arguments: [element],
-      tooltip: element.failureReason || element.label
+      tooltip: element.failureReason || this.labelProvider.getLabel(element)
     };
 
     // Populate the icon and the context value. We use the context value to
@@ -185,8 +189,8 @@ export class HistoryTreeDataProvider extends DisposableObject {
   ): ProviderResult<QueryHistoryInfo[]> {
     return element ? [] : this.history.sort((h1, h2) => {
 
-      const h1Label = h1.label.toLowerCase();
-      const h2Label = h2.label.toLowerCase();
+      const h1Label = this.labelProvider.getLabel(h1).toLowerCase();
+      const h2Label = this.labelProvider.getLabel(h2).toLowerCase();
 
       const h1Date = h1.t === 'local'
         ? h1.initialInfo.start.getTime()
@@ -315,12 +319,13 @@ export class QueryHistoryManager extends DisposableObject {
     ._onWillOpenQueryItem.event;
 
   constructor(
-    private qs: QueryServerClient,
-    private dbm: DatabaseManager,
-    private queryStorageDir: string,
-    private ctx: ExtensionContext,
-    private queryHistoryConfigListener: QueryHistoryConfig,
-    private doCompareCallback: (
+    private readonly qs: QueryServerClient,
+    private readonly dbm: DatabaseManager,
+    private readonly queryStorageDir: string,
+    private readonly ctx: ExtensionContext,
+    private readonly queryHistoryConfigListener: QueryHistoryConfig,
+    private readonly labelProvider: HistoryItemLabelProvider,
+    private readonly doCompareCallback: (
       from: CompletedLocalQueryInfo,
       to: CompletedLocalQueryInfo
     ) => Promise<void>
@@ -334,7 +339,8 @@ export class QueryHistoryManager extends DisposableObject {
     this.queryMetadataStorageLocation = path.join((ctx.storageUri || ctx.globalStorageUri).fsPath, WORKSPACE_QUERY_HISTORY_FILE);
 
     this.treeDataProvider = this.push(new HistoryTreeDataProvider(
-      ctx.extensionPath
+      ctx.extensionPath,
+      this.labelProvider
     ));
     this.treeView = this.push(window.createTreeView('codeQLQueryHistory', {
       treeDataProvider: this.treeDataProvider,
@@ -537,7 +543,7 @@ export class QueryHistoryManager extends DisposableObject {
 
   async readQueryHistory(): Promise<void> {
     void logger.log(`Reading cached query history from '${this.queryMetadataStorageLocation}'.`);
-    const history = await slurpQueryHistory(this.queryMetadataStorageLocation, this.queryHistoryConfigListener);
+    const history = await slurpQueryHistory(this.queryMetadataStorageLocation);
     this.treeDataProvider.allHistory = history;
     this.treeDataProvider.allHistory.forEach((item) => {
       this._onDidAddQueryItem.fire(item);
@@ -605,7 +611,7 @@ export class QueryHistoryManager extends DisposableObject {
         // Remote queries can be removed locally, but not remotely.
         // The user must cancel the query on GitHub Actions explicitly.
         this.treeDataProvider.remove(item);
-        void logger.log(`Deleted ${item.label}.`);
+        void logger.log(`Deleted ${this.labelProvider.getLabel(item)}.`);
         if (item.status === QueryStatus.InProgress) {
           void logger.log('The variant analysis is still running on GitHub Actions. To cancel there, you must go to the workflow run in your browser.');
         }
@@ -652,20 +658,20 @@ export class QueryHistoryManager extends DisposableObject {
   ): Promise<void> {
     const { finalSingleItem, finalMultiSelect } = this.determineSelection(singleItem, multiSelect);
 
-    // TODO will support remote queries
-    if (!this.assertSingleQuery(finalMultiSelect) || finalSingleItem?.t !== 'local') {
+    if (!this.assertSingleQuery(finalMultiSelect)) {
       return;
     }
 
     const response = await window.showInputBox({
-      prompt: 'Label:',
-      placeHolder: '(use default)',
-      value: finalSingleItem.label,
+      placeHolder: `(use default: ${this.queryHistoryConfigListener.format})`,
+      value: finalSingleItem.userSpecifiedLabel ?? '',
+      title: 'Set query label',
+      prompt: 'Set the query history item label. See the description of the codeQL.queryHistory.format setting for more information.',
     });
     // undefined response means the user cancelled the dialog; don't change anything
     if (response !== undefined) {
       // Interpret empty string response as 'go back to using default'
-      finalSingleItem.initialInfo.userSpecifiedLabel = response === '' ? undefined : response;
+      finalSingleItem.userSpecifiedLabel = response === '' ? undefined : response;
       await this.refreshTreeView();
     }
   }
@@ -895,7 +901,7 @@ export class QueryHistoryManager extends DisposableObject {
         query.resultsPaths.interpretedResultsPath
       );
     } else {
-      const label = finalSingleItem.label;
+      const label = this.labelProvider.getLabel(finalSingleItem);
       void showAndLogInformationMessage(
         `Query ${label} has no interpreted results.`
       );
@@ -1084,7 +1090,7 @@ the file in the file explorer and dragging it into the workspace.`
           otherQuery.initialInfo.databaseInfo.name === dbName
       )
       .map((item) => ({
-        label: item.label,
+        label: this.labelProvider.getLabel(item),
         description: (item as CompletedLocalQueryInfo).initialInfo.databaseInfo.name,
         detail: (item as CompletedLocalQueryInfo).completedQuery.statusString,
         query: item as CompletedLocalQueryInfo,