Assembler UI fixes (#260)

* Display filename in page title instead of panel title
* Change compare panel title
* Change comparison failure message
* Fix jump-on-highlight behavior
* Lock and recolor highlight on comparison error
* Default rom format to bin when opening file from assembler
* Make symbol table always visible

components/src/stores/asm.store.ts

@@ -180,6 +180,7 @@ export interface AsmPageState {
   compareName: string | undefined;
   lineNumbers: number[];
   error?: CompilationError;
+  compareError: boolean;
 }
 
 export type AsmStoreDispatch = Dispatch<{
@@ -193,7 +194,7 @@ export function makeAsmStore(
   dispatch: MutableRefObject<AsmStoreDispatch>
 ) {
   const translator = new Translator();
-  const highlightInfo = {
+  const highlightInfo: HighlightInfo = {
     resultHighlight: undefined,
     sourceHighlight: undefined,
     highlightMap: new Map(),
@@ -202,6 +203,7 @@ export function makeAsmStore(
   let animate = true;
   let compiled = false;
   let translating = false;
+  let failure = false;
 
   const reducers = {
     setAsm(
@@ -236,6 +238,7 @@ export function makeAsmStore(
       state.lineNumbers = Array.from(translator.lineNumbers);
       state.sourceHighlight = highlightInfo.sourceHighlight;
       state.resultHighlight = highlightInfo.resultHighlight;
+      state.compareError = failure;
     },
 
     compare(state: AsmPageState) {
@@ -245,15 +248,18 @@ export function makeAsmStore(
         .filter((line) => line.trim() != "");
 
       if (resultLines.length != compareLines.length) {
+        failure = true;
         setStatus("Comparison failed - different lengths");
         return;
       }
 
       for (let i = 0; i < compareLines.length; i++) {
         for (let j = 0; j < compareLines[i].length; j++) {
           if (resultLines[i][j] !== compareLines[i][j]) {
-            setStatus(`Comparison failed at ${i}:${j}`);
-            state.resultHighlight = {
+            setStatus(`Comparison failure: Line ${i}`);
+
+            failure = true;
+            highlightInfo.resultHighlight = {
               start: i * 17,
               end: (i + 1) * 17,
               line: -1,
@@ -341,7 +347,16 @@ export function makeAsmStore(
       return translator.done;
     },
 
+    compare() {
+      dispatch.current({ action: "compare" });
+      this.updateHighlight(highlightInfo.resultHighlight?.start ?? 0, false);
+      dispatch.current({ action: "update" });
+    },
+
     updateHighlight(index: number, fromSource: boolean) {
+      if (failure) {
+        return;
+      }
       for (const [sourceSpan, resultSpan] of highlightInfo.highlightMap) {
         if (
           (fromSource &&
@@ -363,6 +378,7 @@ export function makeAsmStore(
     },
 
     reset() {
+      failure = false;
       setStatus("Reset");
       translator.reset();
       this.resetHighlightInfo();
@@ -399,6 +415,7 @@ export function makeAsmStore(
     compare: "",
     compareName: undefined,
     lineNumbers: [],
+    compareError: false,
   };
 
   return { initialState, reducers, actions };

web/src/App.context.ts

@@ -1,6 +1,6 @@
 import { FileSystem } from "@davidsouther/jiffies/lib/esm/fs.js";
 import { AsmPageState } from "@nand2tetris/components/stores/asm.store";
-import { MemoryAdapter } from "@nand2tetris/simulator/cpu/memory";
+import { Format, MemoryAdapter } from "@nand2tetris/simulator/cpu/memory";
 import { createContext, useCallback, useState } from "react";
 import { useDialog } from "./shell/dialog";
 import { useFilePicker } from "./shell/file_select";
@@ -32,19 +32,22 @@ export function useMonaco() {
 export function useToolStates() {
   const [rom, setRom] = useState<MemoryAdapter>();
   const [cpuPath, setCpuPath] = useState<string>();
+  const [cpuFormat, setCpuFormat] = useState<Format>("asm");
 
   const [asmState, setAsmState] = useState<AsmPageState>();
 
   const setCpuState = (
     path: string | undefined,
-    rom: MemoryAdapter | undefined
+    rom: MemoryAdapter | undefined,
+    format: Format
   ) => {
     setCpuPath(path);
     setRom(rom);
+    setCpuFormat(format);
   };
 
   return {
-    cpuState: { rom: rom, path: cpuPath },
+    cpuState: { rom: rom, path: cpuPath, format: cpuFormat },
     setCpuState,
     asmState,
     setAsmState,
@@ -123,7 +126,7 @@ export const AppContext = createContext<ReturnType<typeof useAppContext>>({
     return undefined;
   },
   toolStates: {
-    cpuState: { rom: undefined, path: undefined },
+    cpuState: { rom: undefined, path: undefined, format: "asm" },
     setCpuState() {
       return undefined;
     },
@@ -139,6 +142,7 @@ export const AppContext = createContext<ReturnType<typeof useAppContext>>({
       compare: "",
       compareName: undefined,
       lineNumbers: [],
+      compareError: false,
     },
     setAsmState() {
       return undefined;

web/src/pages/asm.scss

@@ -7,13 +7,12 @@
   margin: 0px;
   gap: 0;
 
-  grid-template-areas: "source result compare";
+  grid-template-areas: "source result compare" "source sym compare";
   grid-template-columns: 2.5fr 1fr 1fr;
-  grid-template-rows: 1fr;
+  grid-template-rows: 2fr 1fr;
 
   .source {
     grid-area: source;
-    // min-height: calc(var(--line-height) * 10rem);
   }
 
   .result {
@@ -23,4 +22,8 @@
   .compare {
     grid-area: compare;
   }
+
+  .sym {
+    grid-area: sym;
+  }
 }

web/src/pages/asm.tsx

@@ -19,7 +19,7 @@ import "./asm.scss";
 
 export const Asm = () => {
   const { state, actions, dispatch } = useAsmPageStore();
-  const { toolStates, filePicker } = useContext(AppContext);
+  const { toolStates, filePicker, setTitle } = useContext(AppContext);
 
   const sourceCursorPos = useRef(0);
   const resultCursorPos = useRef(0);
@@ -30,6 +30,9 @@ export const Asm = () => {
   useEffect(() => {
     if (toolStates.asmState) {
       actions.overrideState(toolStates.asmState);
+      if (toolStates.asmState.path) {
+        setTitle(toolStates.asmState.path.split("/").pop() ?? "");
+      }
     }
   }, []);
 
@@ -67,6 +70,7 @@ export const Asm = () => {
     requestAnimationFrame(async () => {
       await actions.loadAsm(path);
       setStatus("");
+      setTitle(path.split("/").pop() ?? "");
     });
   };
 
@@ -95,7 +99,7 @@ export const Asm = () => {
   };
 
   const compare = () => {
-    dispatch.current({ action: "compare" });
+    actions.compare();
   };
 
   const onSpeedChange = (speed: number) => {
@@ -104,7 +108,11 @@ export const Asm = () => {
 
   const loadToCpu = async () => {
     const bytes = await loadHack(state.result);
-    toolStates.setCpuState(state.path, new ROM(new Int16Array(bytes)));
+    toolStates.setCpuState(
+      state.path?.replace(".asm", ".hack"),
+      new ROM(new Int16Array(bytes)),
+      "bin"
+    );
     redirectRef.current?.click();
   };
 
@@ -116,7 +124,6 @@ export const Asm = () => {
           <>
             <div>
               <Trans>Source</Trans>
-              {state.path && `: ${state.path.split("/").pop()}`}
             </div>
             <div className="flex-1">
               {runnerAssigned && runner.current && (
@@ -153,6 +160,7 @@ export const Asm = () => {
         <Editor
           value={state.asm}
           error={state.error}
+          alwaysRecenter={false}
           onChange={(source: string) => {
             actions.setAsm(source);
           }}
@@ -230,18 +238,20 @@ export const Asm = () => {
           }}
           grammar={undefined}
           language={""}
-          dynamicHeight={true}
+          alwaysRecenter={false}
           lineNumberTransform={(n) => (n - 1).toString()}
         />
-        {state.symbols.length > 0 && state.translating && "Symbol Table"}
+      </Panel>
+      <Panel className="sym" header={<Trans>Symbol Table</Trans>}>
+        {/* {state.symbols.length > 0 && state.translating && "Symbol Table"} */}
         {state.translating && <Table values={state.symbols} />}
       </Panel>
       <Panel
         className="compare"
         header={
           <>
             <div>
-              <Trans>Compare</Trans>
+              <Trans>Compare Code</Trans>
               {state.compareName && `: ${state.compareName}`}
             </div>
             <div>
@@ -255,6 +265,8 @@ export const Asm = () => {
         <Editor
           value={state.compare}
           highlight={state.translating ? state.resultHighlight : undefined}
+          highlightType={state.compareError ? "error" : "highlight"}
+          alwaysRecenter={false}
           onChange={function (source: string): void {
             dispatch.current({
               action: "setCmp",

web/src/pages/cpu.tsx

@@ -32,18 +32,18 @@ export const CPU = () => {
         action: "replaceROM",
         payload: toolStates.cpuState.rom,
       });
+      setRomFormat(toolStates.cpuState.format);
       if (toolStates.cpuState.path) {
         const name = toolStates.cpuState.path.split("/").pop() ?? "";
         setTitle(name);
         setFileName(name);
-        if (toolStates.cpuState.path.endsWith(".hack")) setRomFormat("bin");
         onUpload(toolStates.cpuState.path);
       }
     }
   }, []);
 
   useEffect(() => {
-    toolStates.setCpuState(fileName, state.sim.ROM);
+    toolStates.setCpuState(fileName, state.sim.ROM, romFormat);
   });
 
   useEffect(() => {

web/src/pico/pico.scss

@@ -29,6 +29,7 @@
     --card-border-color: black;
     --text-color: black;
     --mark-background-color: rgb(255, 230, 121);
+    --mark-error-color: rgb(252, 169, 154);
     --light-grey: rgb(170, 170, 170);
     --file-picker-width: 400px;
   }

web/src/shell/editor.scss

@@ -14,6 +14,10 @@
     background-color: var(--mark-background-color);
   }
 
+  .error-highlight {
+    background-color: var(--mark-error-color);
+  }
+
   .red {
     color: rgb(190, 16, 16);
   }