Merge pull request #1568 from asgerf/asgerf/navigate-alerts

Add commands for navigation of alerts

Author: aeisenberg

extensions/ql-vscode/CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## [UNRELEASED]
 
+- Add commands for navigating up, down, left, or right in the result viewer. Previously there were only commands for moving up and down the currently-selected path. We suggest binding keyboard shortcuts to these commands, for navigating the result viewer using the keyboard. [#1568](https://github.com/github/vscode-codeql/pull/1568)
+
 ## 1.7.2 - 14 October 2022
 
 - Fix a bug where results created in older versions were thought to be unsuccessful. [#1605](https://github.com/github/vscode-codeql/pull/1605)

extensions/ql-vscode/README.md

@@ -99,6 +99,10 @@ When the results are ready, they're displayed in the CodeQL Query Results view.
 
 If there are any problems running a query, a notification is displayed in the bottom right corner of the application. In addition to the error message, the notification includes details of how to fix the problem.
 
+### Keyboad navigation
+
+If you wish to navigate the query results from your keyboard, you can bind shortcuts to the **CodeQL: Navigate Up/Down/Left/Right in Result Viewer** commands.
+
 ## What next?
 
 For more information about the CodeQL extension, [see the documentation](https://codeql.github.com/docs/codeql-for-visual-studio-code/). Otherwise, you could:

extensions/ql-vscode/package.json

@@ -602,12 +602,20 @@
         "title": "Copy Repository List"
       },
       {
-        "command": "codeQLQueryResults.nextPathStep",
-        "title": "CodeQL: Show Next Step on Path"
+        "command": "codeQLQueryResults.down",
+        "title": "CodeQL: Navigate Down in Local Result Viewer"
       },
       {
-        "command": "codeQLQueryResults.previousPathStep",
-        "title": "CodeQL: Show Previous Step on Path"
+        "command": "codeQLQueryResults.up",
+        "title": "CodeQL: Navigate Up in Local Result Viewer"
+      },
+      {
+        "command": "codeQLQueryResults.right",
+        "title": "CodeQL: Navigate Right in Local Result Viewer"
+      },
+      {
+        "command": "codeQLQueryResults.left",
+        "title": "CodeQL: Navigate Left in Local Result Viewer"
       },
       {
         "command": "codeQL.restartQueryServer",

extensions/ql-vscode/src/interface.ts

@@ -27,6 +27,7 @@ import {
   ALERTS_TABLE_NAME,
   GRAPH_TABLE_NAME,
   RawResultsSortState,
+  NavigationDirection,
 } from './pure/interface-types';
 import { Logger } from './logging';
 import { commandRunner } from './commandRunner';
@@ -141,19 +142,24 @@ export class ResultsView extends AbstractWebview<IntoResultsViewMsg, FromResults
         this.handleSelectionChange.bind(this)
       )
     );
-    void logger.log('Registering path-step navigation commands.');
-    this.push(
-      commandRunner(
-        'codeQLQueryResults.nextPathStep',
-        this.navigatePathStep.bind(this, 1)
-      )
-    );
-    this.push(
-      commandRunner(
-        'codeQLQueryResults.previousPathStep',
-        this.navigatePathStep.bind(this, -1)
-      )
-    );
+    const navigationCommands = {
+      'codeQLQueryResults.up': NavigationDirection.up,
+      'codeQLQueryResults.down': NavigationDirection.down,
+      'codeQLQueryResults.left': NavigationDirection.left,
+      'codeQLQueryResults.right': NavigationDirection.right,
+      // For backwards compatibility with keybindings set using an earlier version of the extension.
+      'codeQLQueryResults.nextPathStep': NavigationDirection.down,
+      'codeQLQueryResults.previousPathStep': NavigationDirection.up,
+    };
+    void logger.log('Registering result view navigation commands.');
+    for (const [commandId, direction] of Object.entries(navigationCommands)) {
+      this.push(
+        commandRunner(
+          commandId,
+          this.navigateResultView.bind(this, direction)
+        )
+      );
+    }
 
     this.push(
       this.databaseManager.onDidChangeDatabaseItem(({ kind }) => {
@@ -169,8 +175,13 @@ export class ResultsView extends AbstractWebview<IntoResultsViewMsg, FromResults
     );
   }
 
-  async navigatePathStep(direction: number): Promise<void> {
-    await this.postMessage({ t: 'navigatePath', direction });
+  async navigateResultView(direction: NavigationDirection): Promise<void> {
+    if (!this.panel?.visible) {
+      return;
+    }
+    // Reveal the panel now as the subsequent call to 'Window.showTextEditor' in 'showLocation' may destroy the webview otherwise.
+    this.panel.reveal();
+    await this.postMessage({ t: 'navigate', direction });
   }
 
   protected getPanelConfig(): WebviewPanelConfig {

extensions/ql-vscode/src/pure/interface-types.ts

@@ -145,12 +145,17 @@ export interface ShowInterpretedPageMsg {
   queryPath: string;
 }
 
-/** Advance to the next or previous path no in the path viewer */
-export interface NavigatePathMsg {
-  t: 'navigatePath';
+export const enum NavigationDirection {
+  up = 'up',
+  down = 'down',
+  left = 'left',
+  right = 'right',
+}
 
-  /** 1 for next, -1 for previous */
-  direction: number;
+/** Move up, down, left, or right in the result viewer. */
+export interface NavigateMsg {
+  t: 'navigate';
+  direction: NavigationDirection;
 }
 
 /**
@@ -168,7 +173,7 @@ export type IntoResultsViewMsg =
   | ResultsUpdatingMsg
   | SetStateMsg
   | ShowInterpretedPageMsg
-  | NavigatePathMsg
+  | NavigateMsg
   | UntoggleShowProblemsMsg;
 
 /**

extensions/ql-vscode/src/pure/result-keys.ts

@@ -1,43 +1,52 @@
 import * as sarif from 'sarif';
 
+/**
+ * Identifies a result, a path, or one of the nodes on a path.
+ */
+interface ResultKeyBase {
+  resultIndex: number;
+  pathIndex?: number;
+  pathNodeIndex?: number;
+}
+
 /**
  * Identifies one of the results in a result set by its index in the result list.
  */
-export interface Result {
+export interface Result extends ResultKeyBase {
   resultIndex: number;
+  pathIndex?: undefined;
+  pathNodeIndex?: undefined;
 }
 
 /**
  * Identifies one of the paths associated with a result.
  */
-export interface Path extends Result {
+export interface Path extends ResultKeyBase {
   pathIndex: number;
+  pathNodeIndex?: undefined;
 }
 
 /**
  * Identifies one of the nodes in a path.
  */
-export interface PathNode extends Path {
+export interface PathNode extends ResultKeyBase {
+  pathIndex: number;
   pathNodeIndex: number;
 }
 
-/** Alias for `undefined` but more readable in some cases */
-export const none: PathNode | undefined = undefined;
+export type ResultKey = Result | Path | PathNode;
 
 /**
  * Looks up a specific result in a result set.
  */
-export function getResult(sarif: sarif.Log, key: Result): sarif.Result | undefined {
-  if (sarif.runs.length === 0) return undefined;
-  if (sarif.runs[0].results === undefined) return undefined;
-  const results = sarif.runs[0].results;
-  return results[key.resultIndex];
+export function getResult(sarif: sarif.Log, key: Result | Path | PathNode): sarif.Result | undefined {
+  return sarif.runs[0]?.results?.[key.resultIndex];
 }
 
 /**
  * Looks up a specific path in a result set.
  */
-export function getPath(sarif: sarif.Log, key: Path): sarif.ThreadFlow | undefined {
+export function getPath(sarif: sarif.Log, key: Path | PathNode): sarif.ThreadFlow | undefined {
   const result = getResult(sarif, key);
   if (result === undefined) return undefined;
   let index = -1;
@@ -58,22 +67,13 @@ export function getPath(sarif: sarif.Log, key: Path): sarif.ThreadFlow | undefin
 export function getPathNode(sarif: sarif.Log, key: PathNode): sarif.Location | undefined {
   const path = getPath(sarif, key);
   if (path === undefined) return undefined;
-  return path.locations[key.pathNodeIndex];
-}
-
-/**
- * Returns true if the two keys are both `undefined` or contain the same set of indices.
- */
-export function equals(key1: PathNode | undefined, key2: PathNode | undefined): boolean {
-  if (key1 === key2) return true;
-  if (key1 === undefined || key2 === undefined) return false;
-  return key1.resultIndex === key2.resultIndex && key1.pathIndex === key2.pathIndex && key1.pathNodeIndex === key2.pathNodeIndex;
+  return path.locations[key.pathNodeIndex]?.location;
 }
 
 /**
  * Returns true if the two keys contain the same set of indices and neither are `undefined`.
  */
-export function equalsNotUndefined(key1: PathNode | undefined, key2: PathNode | undefined): boolean {
+export function equalsNotUndefined(key1: Partial<PathNode> | undefined, key2: Partial<PathNode> | undefined): boolean {
   if (key1 === undefined || key2 === undefined) return false;
   return key1.resultIndex === key2.resultIndex && key1.pathIndex === key2.pathIndex && key1.pathNodeIndex === key2.pathNodeIndex;
 }
@@ -93,3 +93,11 @@ export function getAllPaths(result: sarif.Result): sarif.ThreadFlow[] {
   }
   return paths;
 }
+
+/**
+ * Creates a unique string representation of the given key, suitable for use
+ * as the key in a map or set.
+ */
+export function keyToString(key: ResultKey) {
+  return key.resultIndex + '-' + (key.pathIndex ?? '') + '-' + (key.pathNodeIndex ?? '');
+}

extensions/ql-vscode/src/view/results/RawTableRow.tsx

@@ -1,28 +1,36 @@
 import * as React from 'react';
 import { ResultRow } from '../../pure/bqrs-cli-types';
-import { zebraStripe } from './result-table-utils';
+import { selectedRowClassName, zebraStripe } from './result-table-utils';
 import RawTableValue from './RawTableValue';
+import { ScrollIntoViewHelper } from './scroll-into-view-helper';
 
 interface Props {
   rowIndex: number;
   row: ResultRow;
   databaseUri: string;
   className?: string;
+  selectedColumn?: number;
+  onSelected?: (row: number, column: number) => void;
+  scroller?: ScrollIntoViewHelper;
 }
 
 export default function RawTableRow(props: Props) {
   return (
     <tr key={props.rowIndex} {...zebraStripe(props.rowIndex, props.className || '')}>
       <td key={-1}>{props.rowIndex + 1}</td>
 
-      {props.row.map((value, columnIndex) => (
-        <td key={columnIndex}>
-          <RawTableValue
-            value={value}
-            databaseUri={props.databaseUri}
-          />
-        </td>
-      ))}
+      {props.row.map((value, columnIndex) => {
+        const isSelected = props.selectedColumn === columnIndex;
+        return (
+          <td ref={props.scroller?.ref(isSelected)} key={columnIndex} {...isSelected ? { className: selectedRowClassName } : {}}>
+            <RawTableValue
+              value={value}
+              databaseUri={props.databaseUri}
+              onSelected={() => props.onSelected?.(props.rowIndex, columnIndex)}
+            />
+          </td>
+        );
+      })}
     </tr>
   );
 }

extensions/ql-vscode/src/view/results/RawTableValue.tsx

@@ -6,6 +6,7 @@ import { CellValue } from '../../pure/bqrs-cli-types';
 interface Props {
   value: CellValue;
   databaseUri: string;
+  onSelected?: () => void;
 }
 
 export default function RawTableValue(props: Props): JSX.Element {
@@ -18,5 +19,5 @@ export default function RawTableValue(props: Props): JSX.Element {
     return <span>{renderLocation(undefined, rawValue.toString())}</span>;
   }
 
-  return renderLocation(rawValue.url, rawValue.label, props.databaseUri);
+  return renderLocation(rawValue.url, rawValue.label, props.databaseUri, undefined, props.onSelected);
 }