Fix undo/redo when range len changes

Refactor pixeleditor.ts to use ranges
instead of brittle, manually tracked
start and end offset which weren't
surviving undo/redo when rewritten
words resulted in changed range len.

Author: fredsa

src/ide/pixeleditor.ts

@@ -1,7 +1,7 @@
 
-import { hex, tobin, rgb2bgr, rle_unpack } from "../common/util";
-import { ProjectWindows } from "./windows";
+import { hex, rgb2bgr, rle_unpack, tobin } from "../common/util";
 import { Toolbar } from "./toolbar";
+import { ProjectWindows } from "./windows";
 import Mousetrap = require('mousetrap');
 
 export type UintArray = number[] | Uint8Array | Uint16Array | Uint32Array; //{[i:number]:number};
@@ -496,43 +496,37 @@ export class FileDataNode extends CodeProjectDataNode {
 }
 
 export class TextDataNode extends CodeProjectDataNode {
-  text: string;
-  start: number;
-  end: number;
   bpw: number;
+  rangeId: string;
+
+  private static nextRangeId = 0;
 
-  // TODO: what if file size/layout changes?
   constructor(project: ProjectWindows, fileid: string, label: string, start: number, end: number, bpw?: number) {
     super();
     this.project = project;
     this.fileid = fileid;
     this.label = label;
-    this.start = start;
-    this.end = end;
     this.bpw = bpw || 8;
+    this.rangeId = `asset_${TextDataNode.nextRangeId++}`;
+    this.project.setAssetRange(this.fileid, this.rangeId, start, end);
   }
+
   updateLeft() {
     if (this.right.words.length != this.words.length)
       throw Error("Cannot put " + this.right.words.length + " image bytes into array of " + this.words.length + " bytes");
     this.words = this.right.words;
-    // TODO: reload editors?
-    var datastr = this.text.substring(this.start, this.end);
+    var datastr = this.project.getAssetText(this.fileid, this.rangeId);
     datastr = replaceHexWords(datastr, this.words, this.bpw);
-    if (this.project) {
-      this.project.replaceTextRange(this.fileid, this.start, this.end, datastr);
-    }
-    this.text = this.text.substring(0, this.start) + datastr + this.text.substring(this.end);
-    this.end = this.start + datastr.length;
+    // CM6 state field automatically remaps all tracked ranges.
+    this.project.replaceAssetText(this.fileid, this.rangeId, datastr);
     return true;
   }
+
   updateRight() {
-    if (this.project) {
-      this.text = this.project.project.getFile(this.fileid) as string;
-    }
-    var datastr = this.text.substring(this.start, this.end);
-    datastr = convertToHexStatements(datastr); // TODO?
+    var datastr = this.project.getAssetText(this.fileid, this.rangeId);
+    datastr = convertToHexStatements(datastr);
     var words = parseHexWords(datastr);
-    this.words = words; //new Uint8Array(words); // TODO: 16/32?
+    this.words = words;
     return true;
   }
 }

src/ide/views/asseteditor.ts

@@ -1,9 +1,9 @@
 
-import { newDiv, ProjectView } from "./baseviews";
-import { platform_id, current_project, projectWindows } from "../ui";
+import { hex, rgb2bgr, safeident } from "../../common/util";
 import { FileData } from "../../common/workertypes";
-import { hex, safeident, rgb2bgr } from "../../common/util";
 import * as pixed from "../pixeleditor";
+import { current_project, platform_id, projectWindows } from "../ui";
+import { newDiv, ProjectView } from "./baseviews";
 import Mousetrap = require('mousetrap');
 
 function getLineNumber(data: string, offset: number): number {
@@ -425,6 +425,8 @@ export class AssetEditorView implements ProjectView, pixed.EditorContext {
       this.clearAssets();
       current_project.iterateFiles((fileid, data) => {
         try {
+          // Clear stale tracked ranges before re-scanning this file.
+          projectWindows.clearAssetRanges(fileid);
           var nassets = this.refreshAssetsInFile(fileid, data);
         } catch (e) {
           console.log(e);

src/ide/views/baseviews.ts

@@ -22,7 +22,10 @@ export interface ProjectView {
     undoStep?(): void;
     redoStep?(): void;
     flushChanges?(): void;
-    replaceTextRange?(from: number, to: number, text: string): void;
+    setAssetRange?(id: string, from: number, to: number): void;
+    getAssetText?(id: string): string | null;
+    replaceAssetText?(id: string, text: string): void;
+    clearAssetRanges?(): void;
 };
 
 // detect mobile (https://stackoverflow.com/questions/3514784/what-is-the-best-way-to-detect-a-mobile-device)

src/ide/views/editors.ts

@@ -3,7 +3,7 @@ import { cpp } from "@codemirror/lang-cpp";
 import { markdown } from "@codemirror/lang-markdown";
 import { bracketMatching, foldGutter, indentOnInput, indentService, indentUnit } from "@codemirror/language";
 import { highlightSelectionMatches, search, searchKeymap } from "@codemirror/search";
-import { EditorState, Extension } from "@codemirror/state";
+import { EditorState, Extension, StateEffect, StateField } from "@codemirror/state";
 import { crosshairCursor, drawSelection, dropCursor, EditorView, highlightActiveLine, highlightActiveLineGutter, keymap, lineNumbers, rectangularSelection, ViewUpdate } from "@codemirror/view";
 import { CodeAnalyzer } from "../../common/analysis";
 import { hex, rpad } from "../../common/util";
@@ -31,6 +31,39 @@ import { currentPc, errorMessages, errorSpans, highlightLines, showValue } from
 // look ahead this many bytes when finding source lines for a PC
 export const PC_LINE_LOOKAHEAD = 64;
 
+// Asset range tracking. Positions are automatically remapped through
+// document changes (edits, undo, redo) by CodeMirror's transaction system.
+const setAssetRangesEffect = StateEffect.define<{id: string, from: number, to: number}[]>();
+const clearAssetRangesEffect = StateEffect.define<void>();
+
+const assetRangesField = StateField.define<Map<string, {from: number, to: number}>>({
+  create() { return new Map(); },
+  update(ranges, tr) {
+    let result = ranges;
+    for (let e of tr.effects) {
+      if (e.is(clearAssetRangesEffect)) {
+        result = new Map();
+      } else if (e.is(setAssetRangesEffect)) {
+        if (result === ranges) result = new Map(ranges);
+        for (let r of e.value) {
+          result.set(r.id, { from: r.from, to: r.to });
+        }
+      }
+    }
+    if (!tr.changes.empty) {
+      const mapped = new Map<string, {from: number, to: number}>();
+      for (const [id, r] of result) {
+        mapped.set(id, {
+          from: tr.changes.mapPos(r.from, -1),
+          to: tr.changes.mapPos(r.to, 1)
+        });
+      }
+      return mapped;
+    }
+    return result;
+  }
+});
+
 const MAX_ERRORS = 200;
 
 const MODEDEFS = {
@@ -249,6 +282,8 @@ export class SourceEditor implements ProjectView {
 
         highlightLines.field,
 
+        assetRangesField,
+
         createAssetHeaderPlugin((lineNumber: number) => {
           window.location.hash = 'asseteditor/' + encodeURIComponent(this.path) + '/' + lineNumber;
         }),
@@ -323,7 +358,6 @@ export class SourceEditor implements ProjectView {
 
   replaceTextRange(from: number, to: number, text: string) {
     const fromline = this.editor.state.doc.lineAt(from).number;
-    const toline = this.editor.state.doc.lineAt(to).number;
     this.editor.dispatch({
       changes: { from, to, insert: text },
       annotations: isolateHistory.of("full"),
@@ -334,6 +368,30 @@ export class SourceEditor implements ProjectView {
     });
   }
 
+  setAssetRange(id: string, from: number, to: number) {
+    this.editor.dispatch({
+      effects: setAssetRangesEffect.of([{ id, from, to }])
+    });
+  }
+
+  getAssetText(id: string): string | null {
+    var range = this.editor.state.field(assetRangesField).get(id);
+    if (!range) return null;
+    return this.editor.state.doc.sliceString(range.from, range.to);
+  }
+
+  replaceAssetText(id: string, text: string) {
+    var range = this.editor.state.field(assetRangesField).get(id);
+    if (!range) return;
+    this.replaceTextRange(range.from, range.to, text);
+  }
+
+  clearAssetRanges() {
+    this.editor.dispatch({
+      effects: clearAssetRangesEffect.of(undefined)
+    });
+  }
+
   insertLinesBefore(text: string) {
     const pos = this.editor.state.selection.main.from;
     const lineNum = this.editor.state.doc.lineAt(pos).number;

src/ide/windows.ts

@@ -1,8 +1,8 @@
 
 import $ = require("jquery");
+import { getFilenameForPath, getFilenamePrefix } from "../common/util";
+import { FileData, WorkerError } from "../common/workertypes";
 import { CodeProject } from "./project";
-import { WorkerError, FileData } from "../common/workertypes";
-import { getFilenamePrefix, getFilenameForPath } from "../common/util";
 import { ProjectView } from "./views/baseviews";
 
 type WindowCreateFunction = (id: string) => ProjectView;
@@ -176,13 +176,37 @@ export class ProjectWindows {
     this.redoStack = [];
   }
 
-  replaceTextRange(fileid: string, from: number, to: number, text: string) {
+  setAssetRange(fileid: string, id: string, from: number, to: number) {
     var wnd = this.id2window[fileid] || this.create(fileid);
-    wnd.replaceTextRange(from, to, text);
+    if (wnd.setAssetRange) {
+      wnd.setAssetRange(id, from, to);
+    }
+  }
+
+  getAssetText(fileid: string, id: string): string | null {
+    var wnd = this.id2window[fileid] || this.create(fileid);
+    if (wnd.getAssetText) {
+      return wnd.getAssetText(id);
+    }
+    return null;
+  }
+
+  replaceAssetText(fileid: string, id: string, text: string) {
+    var wnd = this.id2window[fileid] || this.create(fileid);
+    if (wnd.replaceAssetText) {
+      wnd.replaceAssetText(id, text);
+    }
     this.undoStack.push({ fileid });
     this.redoStack = [];
   }
 
+  clearAssetRanges(fileid: string) {
+    var wnd = this.id2window[fileid];
+    if (wnd && wnd.clearAssetRanges) {
+      wnd.clearAssetRanges();
+    }
+  }
+
   undoStep() {
     var entry = this.undoStack.pop();
     if (!entry) {

test/cli/testpixelconvert.js

@@ -11,15 +11,41 @@ function dumbEqual(a,b) {
   return assert.deepEqual(a,b);
 }
 
+// Minimal mock that satisfies TextDataNode's project interface.
+function mockProject(text) {
+  var fileText = text;
+  var ranges = {};
+  return {
+    setAssetRange: function(fileid, id, from, to) { ranges[id] = {from: from, to: to}; },
+    getAssetText: function(fileid, id) {
+      var r = ranges[id];
+      return r ? fileText.substring(r.from, r.to) : null;
+    },
+    replaceAssetText: function(fileid, id, newText) {
+      var r = ranges[id];
+      if (!r) return;
+      var delta = newText.length - (r.to - r.from);
+      fileText = fileText.substring(0, r.from) + newText + fileText.substring(r.to);
+      var replacedFrom = r.from, replacedTo = r.to;
+      r.to = r.from + newText.length;
+      for (var otherId in ranges) {
+        if (otherId === id) continue;
+        var other = ranges[otherId];
+        if (other.from >= replacedTo) other.from += delta;
+        if (other.to >= replacedTo) other.to += delta;
+      }
+    }
+  };
+}
+
 describe('Pixel editor', function() {
   it('Should decode', function() {
 
     var fmt = {w:14,h:16,bpp:4,brev:1};
     var palfmt = {pal:332,n:16};
 
     var paldatastr =  " 0x00, 0x03, 0x19, 0x50, 0x52, 0x07, 0x1f, 0x37, 0xe0, 0xa4, 0xfd, 0xff, 0x38, 0x70, 0x7f, 0x7f, "; // test two entries the same
-    var node4 = new pixed.TextDataNode(null, null, null, 0, paldatastr.length);
-    node4.text = paldatastr;
+    var node4 = new pixed.TextDataNode(mockProject(paldatastr), 'test', 'test', 0, paldatastr.length);
     var node5 = new pixed.PaletteFormatToRGB(palfmt);
     node4.addRight(node5);
     node4.refreshRight();
@@ -51,8 +77,7 @@ describe('Pixel editor', function() {
     };
 
     var datastr = "1,2, 0x00,0x00,0xef,0xef,0xe0,0x00,0x00, 0x00,0xee,0xee,0xfe,0xee,0xe0,0x00, 0x0e,0xed,0xef,0xef,0xed,0xee,0x00, 0x0e,0xee,0xdd,0xdd,0xde,0xee,0x00, 0x0e,0xee,0xed,0xde,0xee,0xee,0x00, 0x00,0xee,0xee,0xde,0xee,0xe0,0x00, 0x00,0xee,0xee,0xde,0xee,0xe0,0x00, 0x00,0x00,0xed,0xdd,0xe0,0x00,0x0d, 0xdd,0xdd,0xee,0xee,0xed,0xdd,0xd0, 0x0d,0xee,0xee,0xee,0xee,0xee,0x00, 0x0e,0xe0,0xee,0xee,0xe0,0xee,0x00, 0x0e,0xe0,0xee,0xee,0xe0,0xee,0x00, 0x0e,0xe0,0xdd,0xdd,0xd0,0xde,0x00, 0x0d,0x00,0xee,0x0e,0xe0,0x0d,0x00, 0x00,0x00,0xed,0x0e,0xe0,0x00,0x00, 0x00,0x0d,0xdd,0x0d,0xdd,0x00,0x18,";
-    var node1 = new pixed.TextDataNode(null, null, null, 0, datastr.length);
-    node1.text = datastr;
+    var node1 = new pixed.TextDataNode(mockProject(datastr), 'test', 'test', 0, datastr.length);
     var node2 = new pixed.Mapper(fmt);
     node1.addRight(node2);
     var node3 = new pixed.Palettizer(ctx, fmt);