Merge pull request #476 from microsoft/connor4312/support-delete

fix: improve range selection logic, support delete

media/editor/dataDisplay.tsx

@@ -4,14 +4,15 @@
 import React, { Suspense, useCallback, useEffect, useMemo, useRef, useState } from "react";
 import { useRecoilValue, useSetRecoilState } from "recoil";
 import { EditRangeOp, HexDocumentEditOp } from "../../shared/hexDocumentModel";
-import { InspectorLocation, MessageType } from "../../shared/protocol";
+import { DeleteAcceptedMessage, InspectorLocation, MessageType } from "../../shared/protocol";
+import { Range } from "../../shared/util/range";
 import { PastePopup } from "./copyPaste";
 import _style from "./dataDisplay.css";
 import { FocusedElement, dataCellCls, useDisplayContext, useIsFocused, useIsHovered, useIsSelected, useIsUnsaved } from "./dataDisplayContext";
 import { DataInspectorAside } from "./dataInspector";
 import { useGlobalHandler, useLastAsyncRecoilValue } from "./hooks";
 import * as select from "./state";
-import { Range, clamp, clsx, getAsciiCharacter, getScrollDimensions, throwOnUndefinedAccessInDev } from "./util";
+import { clamp, clsx, getAsciiCharacter, getScrollDimensions, throwOnUndefinedAccessInDev } from "./util";
 
 const style = throwOnUndefinedAccessInDev(_style);
 
@@ -457,6 +458,17 @@ const DataCell: React.FC<{
 			return;
 		}
 
+		if (e.key === "Backspace" || e.key === "Delete") {
+			// this is a bit of a hack, but this is kind of tricky: we got a delete
+			// for a range, and the edit must be undoable, but we aren't ensured to
+			// have the data paged in for the range. So make a separate request
+			// that will result in the extension host sending the edit to us.
+			select.messageHandler.sendRequest<DeleteAcceptedMessage>({
+				type: MessageType.RequestDeletes,
+				deletes: ctx.getSelectionRanges().map(r => ({ start: r.start, end: r.end })),
+			}).then(() => ctx.setSelectionRanges([]));
+		}
+
 		let newValue: number;
 		if (isChar && e.key.length === 1) {
 			newValue = e.key.charCodeAt(0);

media/editor/dataDisplayContext.tsx

@@ -3,9 +3,10 @@ import { createContext, useContext, useEffect, useState } from "react";
 import { SetterOrUpdater } from "recoil";
 import { HexDocumentEdit } from "../../shared/hexDocumentModel";
 import { MessageType } from "../../shared/protocol";
+import { Range, getRangeSelectionsFromStack } from "../../shared/util/range";
 import _style from "./dataDisplayContext.css";
 import { messageHandler, registerHandler } from "./state";
-import { Range, throwOnUndefinedAccessInDev } from "./util";
+import { throwOnUndefinedAccessInDev } from "./util";
 
 const style = throwOnUndefinedAccessInDev(_style);
 
@@ -253,24 +254,25 @@ export class DisplayContext {
 	}
 
 	private publishSelections() {
-		const selectedBytes = new Set();
-		for (const range of this._selection) {
-			for (let i = range.start; i < range.end; i++) {
-				if (selectedBytes.has(i)) {
-					selectedBytes.delete(i);
-				} else {
-					selectedBytes.add(i);
-				}
-			}
+		let selected = 0;
+		for (const range of this.getSelectionRanges()) {
+			selected += range.size;
 		}
 
 		messageHandler.sendEvent({
 			type: MessageType.SetSelectedCount,
-			selected: selectedBytes.size,
+			selected: selected,
 			focused: this._focusedByte?.byte
 		});
 	}
 
+	/**
+	 * Gets selected ranges.
+	 */
+	public getSelectionRanges(): Range[] {
+		return getRangeSelectionsFromStack(this._selection);
+	}
+
 	/**
 	 * Replaces the selection with the given ranges.
 	 */

media/editor/findWidget.tsx

@@ -13,11 +13,12 @@ import React, { useCallback, useEffect, useRef, useState } from "react";
 import { useRecoilState, useRecoilValue } from "recoil";
 import { HexDocumentEditOp, HexDocumentReplaceEdit } from "../../shared/hexDocumentModel";
 import { LiteralSearchQuery, MessageType, SearchRequestMessage, SearchResult, SearchResultsWithProgress } from "../../shared/protocol";
+import { Range } from "../../shared/util/range";
 import { FocusedElement, dataCellCls, useDisplayContext } from "./dataDisplayContext";
 import _style from "./findWidget.css";
 import { usePersistedState } from "./hooks";
 import * as select from "./state";
-import { Range, clsx, hexDecode, isHexString, parseHexDigit, throwOnUndefinedAccessInDev } from "./util";
+import { clsx, hexDecode, isHexString, parseHexDigit, throwOnUndefinedAccessInDev } from "./util";
 import { VsIconButton, VsIconCheckbox, VsProgressIndicator, VsTextFieldGroup } from "./vscodeUi";
 
 const style = throwOnUndefinedAccessInDev(_style);

media/editor/scrollContainer.tsx

@@ -1,9 +1,10 @@
 import React, { useCallback, useEffect, useRef, useState } from "react";
 import { useRecoilState, useRecoilValue } from "recoil";
+import { Range } from "../../shared/util/range";
 import { DataDisplay } from "./dataDisplay";
 import _style from "./scrollContainer.css";
 import * as select from "./state";
-import { Range, throwOnUndefinedAccessInDev } from "./util";
+import { throwOnUndefinedAccessInDev } from "./util";
 import { VirtualScrollContainer } from "./virtualScrollContainer";
 
 const style = throwOnUndefinedAccessInDev(_style);

media/editor/state.ts

@@ -6,7 +6,8 @@ import { atom, DefaultValue, selector, selectorFamily } from "recoil";
 import { buildEditTimeline, HexDocumentEdit, readUsingRanges } from "../../shared/hexDocumentModel";
 import { FromWebviewMessage, InspectorLocation, MessageHandler, MessageType, ReadRangeResponseMessage, ReadyResponseMessage, SearchResultsWithProgress, ToWebviewMessage } from "../../shared/protocol";
 import { deserializeEdits, serializeEdits } from "../../shared/serialization";
-import { clamp, Range } from "./util";
+import { Range } from "../../shared/util/range";
+import { clamp } from "./util";
 
 const acquireVsCodeApi: () => ({
 	postMessage(msg: unknown): void;
@@ -290,7 +291,8 @@ export const edits = atom<readonly HexDocumentEdit[]>({
 			});
 
 			registerHandler(MessageType.SetEdits, msg => {
-				fx.setSelf(deserializeEdits(msg.edits));
+				const edits = deserializeEdits(msg.edits);
+				fx.setSelf(prev => msg.appendOnly ? [...(prev instanceof DefaultValue ? [] : prev), ...edits] : edits);
 			});
 		}
 	]

media/editor/util.ts

@@ -3,6 +3,7 @@
 
 // Assorted helper functions
 
+import { Range } from "../../shared/util/range";
 import _style from "./util.css";
 
 /**
@@ -42,116 +43,6 @@ export const clsx = (...classes: (string | false | undefined | null)[]): string
 	return out;
 };
 
-/** Direction for the {@link Range} */
-export const enum RangeDirection {
-	/** When the range was constructed, end >= start */
-	Ascending,
-	/** When the range was constructed, start > end */
-	Descending,
-}
-
-/**
- * @description Class which represents a range of numbers. Ranges represent
- * a number range [start, end). They may be directional, as indicated by
- * the order of arguments in the constructor and reflected in the {@link direction}.
- */
-export class Range {
-	public readonly direction: RangeDirection;
-	/**
-	 * Gets the number of integers in the range [start, end)
-	 */
-	public get size(): number {
-		return this.end - this.start;
-	}
-
-	/**
-	 * Returns a range containing the single byte.
-	 */
-	public static single(byte: number): Range {
-		return new Range(byte, byte + 1);
-	}
-
-	/**
-	 * Creates a new range representing [start, end], inclusive.
-	 */
-	public static inclusive(start: number, end: number): Range {
-		return end >= start ? new Range(start, end + 1) : new Range(start + 1, end);
-	}
-
-	/**
-	 * @description Constructs a range object representing [start, end)
-	 * @param start Represents the start of the range
-	 * @param end Represents the end of the range
-	 * @param direction The direction of the range, inferred from
-	 * argument order if not provided.
-	 */
-	constructor(
-		public readonly start: number,
-		public readonly end: number = Number.MAX_SAFE_INTEGER,
-		direction?: RangeDirection,
-	) {
-		if (start < 0) {
-			throw new Error("Cannot construct a range with a negative start");
-		}
-
-		if (end < start) {
-			[this.start, this.end] = [end, start];
-			direction ??= RangeDirection.Descending;
-		} else {
-			direction ??= RangeDirection.Ascending;
-		}
-
-		this.direction = direction;
-	}
-	/**
-	 * @desciption Tests if the given number if within the range
-	 * @param {number} num The number to test
-	 * @returns {boolean} True if the number is in the range, false otherwise
-	 */
-	public includes(num: number): boolean {
-		return num >= this.start && num < this.end;
-	}
-
-	/**
-	 * Expands the range to include the given value, if it is not already
-	 * within the range.
-	 */
-	public expandToContain(value: number): Range {
-		if (value < this.start) {
-			return new Range(value, this.end, this.direction);
-		} else if (value >= this.end) {
-			return new Range(this.start, value + 1, this.direction);
-		} else {
-			return this;
-		}
-	}
-	/**
-	 * Returns whether this range overlaps the other one.
-	 */
-	public overlaps(other: Range): boolean {
-		return other.end > this.start && other.start < this.end;
-	}
-	/**
-	 * Returns one or more ranges representing ranges covered by exactly one of
-	 * this or the `otherRange`.
-	 */
-	public difference(otherRange: Range): Range[] {
-		if (!this.overlaps(otherRange)) {
-			return [this, otherRange];
-		}
-
-		const delta: Range[] = [];
-		if (this.start !== otherRange.start) {
-			delta.push(new Range(otherRange.start, this.start));
-		}
-		if (this.end !== otherRange.end) {
-			delta.push(new Range(otherRange.end, this.end));
-		}
-
-		return delta;
-	}
-}
-
 /**
  * @description Checks if the given number is in any of the ranges
  * @param {number} num The number to use when checking the ranges