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:
@@ -154,16 +155,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;
+
     workspace.hideChaff();
     this.copyData = sourceBlock.toCopyData();
     this.copyWorkspace = sourceBlock.workspace;

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

@@ -59,7 +59,7 @@ export class EnterAction {
               return false;
             }
             curNode = flyoutCursor.getCurNode();
-            nodeType = curNode.getType();
+            nodeType = curNode?.getType();
 
             switch (nodeType) {
               case ASTNode.types.STACK:
@@ -88,22 +88,25 @@ export class EnterAction {
     const cursor = workspace.getCursor();
     if (!cursor) return;
     const curNode = cursor.getCurNode();
-    const nodeType = curNode.getType();
+    const nodeType = curNode?.getType();
     if (nodeType === ASTNode.types.FIELD) {
-      (curNode.getLocation() as Field).showEditor();
+      (curNode!.getLocation() as Field).showEditor();
     } else if (nodeType === ASTNode.types.BLOCK) {
-      const block = curNode.getLocation() as Block;
-      if (!this.tryShowFullBlockFieldEditor(block)) {
+      const block = curNode?.getLocation() as Block | undefined;
+      if (block && !this.tryShowFullBlockFieldEditor(block)) {
         const metaKey = navigator.platform.startsWith('Mac') ? 'Cmd' : 'Ctrl';
         const canMoveInHint = `Press right arrow to move in or ${metaKey} + Enter for more options`;
         const genericHint = `Press ${metaKey} + Enter for options`;
         const hint =
-          curNode.in()?.getSourceBlock() === block
+          curNode?.in()?.getSourceBlock() === block
             ? canMoveInHint
             : genericHint;
         alert(hint);
       }
-    } else if (curNode.isConnection() || nodeType === ASTNode.types.WORKSPACE) {
+    } else if (
+      curNode?.isConnection() ||
+      nodeType === ASTNode.types.WORKSPACE
+    ) {
       this.navigation.openToolboxOrFlyout(workspace);
     } else if (nodeType === ASTNode.types.STACK) {
       console.warn('Cannot mark a stack.');
@@ -152,7 +155,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,
     );
@@ -204,8 +208,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

@@ -55,7 +55,7 @@ export class WorkspaceMovement {
       callback: (workspace) => this.moveWSCursor(workspace, 0, -1),
       keyCodes: [createSerializedKey(KeyCodes.W, [KeyCodes.SHIFT])],
     },
-    
+
     /** Move the cursor on the workspace down. */
     {
       name: Constants.SHORTCUT_NAMES.MOVE_WS_CURSOR_DOWN,
@@ -100,7 +100,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

@@ -74,7 +74,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;
   }
@@ -402,7 +403,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 ??
@@ -425,7 +426,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,
@@ -475,7 +476,7 @@ export class LineCursor extends Marker {
    *
    * @returns The current field, connection, or block the cursor is on.
    */
-  override getCurNode(): ASTNode {
+  override getCurNode(): ASTNode | null {
     const curNode = super.getCurNode();
     const selected = Blockly.common.getSelected();
     if (selected?.workspace !== this.workspace) return curNode;
@@ -494,6 +495,40 @@ export class LineCursor extends Marker {
     return newNode;
   }
 
+  /**
+   * 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.
    *
@@ -503,17 +538,7 @@ export class LineCursor extends Marker {
    * @param newNode The new location of the cursor.
    */
   override setCurNode(newNode: ASTNode) {
-    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;
@@ -540,21 +565,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.
    *
@@ -579,7 +589,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;
@@ -592,13 +606,13 @@ export class LineCursor extends Marker {
 
     // If curNode node is not block, just use the drawer.
     if (curNode?.getType() !== ASTNode.types.BLOCK) {
-      this.getDrawer()?.draw(oldNode, curNode);
+      realDrawer.draw(oldNode, curNode);
       return;
     }
 
     // curNode is a block.  Hide any visible marker SVG and instead
     // select the block or make it look selected.
-    super.hide(); // Calls this.drawer?.hide().
+    realDrawer.hide();
     const block = curNode.getLocation() as Blockly.BlockSvg;
     if (!block.isShadow()) {
       Blockly.common.setSelected(block);
@@ -608,7 +622,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

@@ -362,14 +362,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,