refactor: Allow for curNode being null.

Author: gonfunko

src/actions/action_menu.ts

@@ -111,6 +111,7 @@ export class ActionMenu {
     const cursor = workspace.getCursor();
     if (!cursor) throw new Error('workspace has no cursor');
     const node = cursor.getCurNode();
+    if (!node) throw new Error('No node is currently selected');
     const nodeType = node.getType();
     switch (nodeType) {
       case ASTNode.types.BLOCK:
@@ -195,16 +196,16 @@ export class ActionMenu {
       connection,
     } as unknown as ContextMenuRegistry.Scope;
     for (const option of possibleOptions) {
-      const precondition = option.preconditionFn(scope);
+      const precondition = option.preconditionFn?.(scope);
       if (precondition === 'hidden') continue;
       const displayText =
-        typeof option.displayText === 'function'
+        (typeof option.displayText === 'function'
           ? option.displayText(scope)
-          : option.displayText;
+          : option.displayText) ?? '';
       menuOptions.push({
         text: displayText,
         enabled: precondition === 'enabled',
-        callback: option.callback,
+        callback: option.callback!,
         scope,
         weight: option.weight,
       });

src/actions/arrow_navigation.ts

@@ -40,7 +40,7 @@ export class ArrowNavigation {
       return false;
     }
     const curNode = cursor.getCurNode();
-    if (curNode.getType() === ASTNode.types.FIELD) {
+    if (curNode?.getType() === ASTNode.types.FIELD) {
       return (curNode.getLocation() as Field).onShortcut(shortcut);
     }
     return false;

src/actions/clipboard.ts

@@ -169,7 +169,9 @@ export class Clipboard {
   private cutCallback(workspace: WorkspaceSvg) {
     const cursor = workspace.getCursor();
     if (!cursor) throw new TypeError('no cursor');
-    const sourceBlock = cursor.getCurNode().getSourceBlock() as BlockSvg | null;
+    const sourceBlock = cursor
+      .getCurNode()
+      ?.getSourceBlock() as BlockSvg | null;
     if (!sourceBlock) throw new TypeError('no source block');
     this.copyData = sourceBlock.toCopyData();
     this.copyWorkspace = sourceBlock.workspace;
@@ -274,7 +276,9 @@ export class Clipboard {
     const sourceBlock = activeWorkspace
       ?.getCursor()
       ?.getCurNode()
-      .getSourceBlock() as BlockSvg;
+      ?.getSourceBlock() as BlockSvg;
+    if (!sourceBlock) return false;
+
     this.copyData = sourceBlock.toCopyData();
     this.copyWorkspace = sourceBlock.workspace;
     const copied = !!this.copyData;

src/actions/delete.ts

@@ -150,7 +150,7 @@ export class DeleteAction {
   private deletePrecondition(workspace: WorkspaceSvg) {
     if (!this.canCurrentlyEdit(workspace)) return false;
 
-    const sourceBlock = workspace.getCursor()?.getCurNode().getSourceBlock();
+    const sourceBlock = workspace.getCursor()?.getCurNode()?.getSourceBlock();
     return !!sourceBlock?.isDeletable();
   }
 
@@ -169,7 +169,10 @@ export class DeleteAction {
     const cursor = workspace.getCursor();
     if (!cursor) return false;
 
-    const sourceBlock = cursor.getCurNode().getSourceBlock() as BlockSvg;
+    const sourceBlock = cursor
+      .getCurNode()
+      ?.getSourceBlock() as BlockSvg | null;
+    if (!sourceBlock) return false;
     // Delete or backspace.
     // There is an event if this is triggered from a keyboard shortcut,
     // but not if it's triggered from a context menu.

src/actions/enter.ts

@@ -64,7 +64,7 @@ export class EnterAction {
               return false;
             }
             curNode = flyoutCursor.getCurNode();
-            nodeType = curNode.getType();
+            nodeType = curNode?.getType();
 
             switch (nodeType) {
               case ASTNode.types.STACK:
@@ -157,7 +157,8 @@ export class EnterAction {
     const button = this.navigation
       .getFlyoutCursor(workspace)!
       .getCurNode()
-      .getLocation() as FlyoutButton;
+      ?.getLocation() as FlyoutButton | undefined;
+    if (!button) return;
     const buttonCallback = (workspace as any).flyoutButtonCallbacks.get(
       (button as any).callbackKey,
     );
@@ -209,8 +210,8 @@ export class EnterAction {
     const curBlock = this.navigation
       .getFlyoutCursor(workspace)!
       .getCurNode()
-      .getLocation() as BlockSvg;
-    if (!curBlock.isEnabled()) {
+      ?.getLocation() as BlockSvg | undefined;
+    if (!curBlock?.isEnabled()) {
       console.warn("Can't insert a disabled block.");
       return null;
     }

src/actions/ws_movement.ts

@@ -108,7 +108,7 @@ export class WorkspaceMovement {
     const cursor = workspace.getCursor();
     if (!cursor) return false;
     const curNode = cursor?.getCurNode();
-    if (curNode.getType() !== ASTNode.types.WORKSPACE) return false;
+    if (!curNode || curNode.getType() !== ASTNode.types.WORKSPACE) return false;
 
     const wsCoord = curNode.getWsCoordinate();
     const newX = xDirection * this.WS_MOVE_DISTANCE + wsCoord.x;

src/line_cursor.ts

@@ -79,7 +79,8 @@ export class LineCursor extends Marker {
     const markerManager = this.workspace.getMarkerManager();
     this.oldCursor = markerManager.getCursor();
     markerManager.setCursor(this);
-    if (this.oldCursor) this.setCurNode(this.oldCursor.getCurNode());
+    const oldCursorNode = this.oldCursor?.getCurNode();
+    if (oldCursorNode) this.setCurNode(oldCursorNode);
     this.workspace.addChangeListener(this.selectListener);
     this.installed = true;
   }
@@ -423,7 +424,7 @@ export class LineCursor extends Marker {
   preDelete(deletedBlock: Blockly.Block) {
     const curNode = this.getCurNode();
 
-    const nodes: Blockly.ASTNode[] = [curNode];
+    const nodes: Blockly.ASTNode[] = curNode ? [curNode] : [];
     // The connection to which the deleted block is attached.
     const parentConnection =
       deletedBlock.previousConnection?.targetConnection ??
@@ -446,7 +447,7 @@ export class LineCursor extends Marker {
     }
     // A location on the workspace beneath the deleted block.
     // Move to the workspace.
-    const curBlock = curNode.getSourceBlock();
+    const curBlock = curNode?.getSourceBlock();
     if (curBlock) {
       const workspaceNode = Blockly.ASTNode.createWorkspaceNode(
         this.workspace,
@@ -474,6 +475,40 @@ export class LineCursor extends Marker {
     throw new Error('no valid nodes in this.potentialNodes');
   }
 
+  /**
+   * Sets the object in charge of drawing the marker.
+   *
+   * We want to customize drawing, so rather than directly setting the given
+   * object, we instead set a wrapper proxy object that passes through all
+   * method calls and property accesses except for draw(), which it delegates
+   * to the drawMarker() method in this class.
+   *
+   * @param drawer The object ~in charge of drawing the marker.
+   */
+  override setDrawer(drawer: Blockly.blockRendering.MarkerSvg) {
+    const altDraw = function (
+      this: LineCursor,
+      oldNode: ASTNode | null,
+      curNode: ASTNode | null,
+    ) {
+      // Pass the unproxied, raw drawer object so that drawMarker can call its
+      // `draw()` method without triggering infinite recursion.
+      this.drawMarker(oldNode, curNode, drawer);
+    }.bind(this);
+
+    super.setDrawer(
+      new Proxy(drawer, {
+        get(target: typeof drawer, prop: keyof typeof drawer) {
+          if (prop === 'draw') {
+            return altDraw;
+          }
+
+          return target[prop];
+        },
+      }),
+    );
+  }
+
   /**
    * Set the location of the cursor and draw it.
    *
@@ -482,7 +517,7 @@ export class LineCursor extends Marker {
    *
    * @param newNode The new location of the cursor.
    */
-  override setCurNode(newNode: ASTNode, selectionInSync = false) {
+  override setCurNode(newNode: ASTNode | null, selectionInSync = false) {
     if (newNode?.getLocation() === this.getCurNode()?.getLocation()) {
       return;
     }
@@ -505,18 +540,8 @@ export class LineCursor extends Marker {
       }
     }
 
-    const oldNode = super.getCurNode();
-    // Kludge: we can't set this.curNode directly, so we have to call
-    // super.setCurNode(...) to do it for us - but that would call
-    // this.drawer.draw(...), so prevent that by temporarily setting
-    // this.drawer to null (which we also can't do directly!)
-    const drawer = this.getDrawer();
-    this.setDrawer(null as any); // Cast required since param is not nullable.
     super.setCurNode(newNode);
-    this.setDrawer(drawer);
 
-    // Draw this marker the way we want to.
-    this.drawMarker(oldNode, newNode);
     // Try to scroll cursor into view.
     if (newNode?.getType() === ASTNode.types.BLOCK) {
       const block = newNode.getLocation() as Blockly.BlockSvg;
@@ -527,21 +552,6 @@ export class LineCursor extends Marker {
     }
   }
 
-  /**
-   * Redraw the current marker.
-   *
-   * Overrides normal Marker drawing logic to use this.drawMarker()
-   * instead of this.drawer.draw() directly.
-   *
-   * This hooks the method used by the renderer to draw the marker,
-   * preventing the marker drawer from showing a marker if we don't
-   * want it to.
-   */
-  override draw() {
-    const curNode = super.getCurNode();
-    this.drawMarker(curNode, curNode);
-  }
-
   /**
    * Draw this cursor's marker.
    *
@@ -566,7 +576,11 @@ export class LineCursor extends Marker {
    * @param oldNode The previous node.
    * @param curNode The current node.
    */
-  private drawMarker(oldNode: ASTNode, curNode: ASTNode) {
+  private drawMarker(
+    oldNode: ASTNode | null,
+    curNode: ASTNode | null,
+    realDrawer: Blockly.blockRendering.MarkerSvg,
+  ) {
     // If old node was a block, unselect it or remove fake selection.
     if (oldNode?.getType() === ASTNode.types.BLOCK) {
       const block = oldNode.getLocation() as Blockly.BlockSvg;
@@ -587,12 +601,12 @@ export class LineCursor extends Marker {
 
     // If drawing can't be handled locally, just use the drawer.
     if (curNodeType !== ASTNode.types.BLOCK && !isZelosInputConnection) {
-      this.getDrawer()?.draw(oldNode, curNode);
+      realDrawer.draw(oldNode, curNode);
       return;
     }
 
     // Hide any visible marker SVG and instead do some manual rendering.
-    super.hide(); // Calls this.drawer?.hide().
+    realDrawer.hide();
 
     if (isZelosInputConnection) {
       this.showAtInput(curNode);
@@ -607,7 +621,7 @@ export class LineCursor extends Marker {
 
     // Call MarkerSvg.prototype.fireMarkerEvent like
     // MarkerSvg.prototype.draw would (even though it's private).
-    (this.getDrawer() as any)?.fireMarkerEvent?.(oldNode, curNode);
+    (realDrawer as any)?.fireMarkerEvent?.(oldNode, curNode);
   }
 
   /**

src/navigation.ts

@@ -338,14 +338,14 @@ export class Navigation {
     if (
       !cursor ||
       !cursor.getCurNode() ||
-      !cursor.getCurNode().getSourceBlock()
+      !cursor.getCurNode()?.getSourceBlock()
     ) {
       return;
     }
 
     const curNode = cursor.getCurNode();
-    const sourceBlock = curNode.getSourceBlock()!;
-    if (sourceBlock.id === deletedBlockId || ids.includes(sourceBlock.id)) {
+    const sourceBlock = curNode?.getSourceBlock()!;
+    if (sourceBlock?.id === deletedBlockId || ids.includes(sourceBlock?.id)) {
       cursor.setCurNode(
         Blockly.ASTNode.createWorkspaceNode(
           workspace,