Merge pull request #2864 from github/koesie10/right-align-format-numbers

Right align and format raw result numbers

extensions/ql-vscode/src/view/common/RawNumberValue.tsx

@@ -0,0 +1,17 @@
+import * as React from "react";
+import { styled } from "styled-components";
+import { formatDecimal } from "../../common/number";
+
+const RightAlignedSpan = styled.span`
+  display: inline-block;
+  text-align: right;
+  width: 100%;
+`;
+
+type Props = {
+  value: number;
+};
+
+export const RawNumberValue = ({ value }: Props) => {
+  return <RightAlignedSpan>{formatDecimal(value)}</RightAlignedSpan>;
+};

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

@@ -2,6 +2,7 @@ import * as React from "react";
 
 import { Location } from "./locations/Location";
 import { CellValue } from "../../common/bqrs-cli-types";
+import { RawNumberValue } from "../common/RawNumberValue";
 
 interface Props {
   value: CellValue;
@@ -9,22 +10,26 @@ interface Props {
   onSelected?: () => void;
 }
 
-export default function RawTableValue(props: Props): JSX.Element {
-  const rawValue = props.value;
-  if (
-    typeof rawValue === "string" ||
-    typeof rawValue === "number" ||
-    typeof rawValue === "boolean"
-  ) {
-    return <Location label={rawValue.toString()} />;
+export default function RawTableValue({
+  value,
+  databaseUri,
+  onSelected,
+}: Props): JSX.Element {
+  switch (typeof value) {
+    case "boolean":
+      return <span>{value.toString()}</span>;
+    case "number":
+      return <RawNumberValue value={value} />;
+    case "string":
+      return <Location label={value.toString()} />;
+    default:
+      return (
+        <Location
+          loc={value.url}
+          label={value.label}
+          databaseUri={databaseUri}
+          onClick={onSelected}
+        />
+      );
   }
-
-  return (
-    <Location
-      loc={rawValue.url}
-      label={rawValue.label}
-      databaseUri={props.databaseUri}
-      onClick={props.onSelected}
-    />
-  );
 }

extensions/ql-vscode/src/view/variant-analysis/RawResultCell.tsx

@@ -5,6 +5,7 @@ import { CellValue } from "../../common/bqrs-cli-types";
 import { sendTelemetry } from "../common/telemetry";
 import { convertNonPrintableChars } from "../../common/text-utils";
 import { tryGetRemoteLocation } from "../../common/bqrs-utils";
+import { RawNumberValue } from "../common/RawNumberValue";
 
 type CellProps = {
   value: CellValue;
@@ -20,9 +21,11 @@ export const RawResultCell = ({
   sourceLocationPrefix,
 }: CellProps) => {
   switch (typeof value) {
-    case "string":
-    case "number":
     case "boolean":
+      return <span>{value.toString()}</span>;
+    case "number":
+      return <RawNumberValue value={value} />;
+    case "string":
       return <span>{convertNonPrintableChars(value.toString())}</span>;
     case "object": {
       const url = tryGetRemoteLocation(

extensions/ql-vscode/src/view/variant-analysis/__tests__/AnalyzedRepoItemContent.spec.tsx

@@ -71,6 +71,12 @@ describe(AnalyzedRepoItemContent.name, () => {
             {
               kind: "i",
             },
+            {
+              kind: "s",
+            },
+            {
+              kind: "b",
+            },
           ],
         },
         resultSet: {
@@ -81,9 +87,18 @@ describe(AnalyzedRepoItemContent.name, () => {
               {
                 kind: "i",
               },
+              {
+                kind: "s",
+              },
+              {
+                kind: "b",
+              },
             ],
           },
-          rows: [[60688]],
+          rows: [
+            [60688, "foo", true],
+            [5, "bar", false],
+          ],
         },
         fileLinkPrefix:
           "https://github.com/octodemo/hello-world-1/blob/59a2a6c7d9dde7a6ecb77c2f7e8197d6925c143b",
@@ -92,7 +107,12 @@ describe(AnalyzedRepoItemContent.name, () => {
       },
     });
 
-    expect(screen.getByText("60688")).toBeInTheDocument();
+    expect(screen.getByText("60,688")).toBeInTheDocument();
+    expect(screen.getByText("foo")).toBeInTheDocument();
+    expect(screen.getByText("true")).toBeInTheDocument();
+    expect(screen.getByText("5")).toBeInTheDocument();
+    expect(screen.getByText("bar")).toBeInTheDocument();
+    expect(screen.getByText("false")).toBeInTheDocument();
   });
 
   it("renders the failed state", () => {