Fix History Back, Push Bugs (#1447)

* Repro'd with a failing test

* Fixed history bug by splicing array at position

* Replaced 3 history implementations with one

* Copy the array when initializing

* Fix command history

* lint

* Remove comment

app/core/models/cmd-history.test.ts

@@ -0,0 +1,52 @@
+import CmdHistory from "./cmd-history"
+
+let h: CmdHistory
+beforeEach(() => {
+  h = new CmdHistory([], 0, 5)
+})
+
+test("push", () => {
+  h.push("a")
+
+  expect(h.all()).toEqual(["a"])
+})
+
+test("back", () => {
+  h.push("a")
+  h.push("b")
+
+  expect(h.back()).toEqual("a")
+  expect(h.all()).toEqual(["a", "b"])
+})
+
+test("fowrard", () => {
+  h.push("a")
+  h.push("b")
+  h.push("c")
+  h.back()
+  h.back()
+
+  expect(h.forward()).toEqual("b")
+  expect(h.all()).toEqual(["a", "b", "c"])
+})
+
+test("back when none", () => {
+  expect(h.back()).toEqual(null)
+  expect(h.all()).toEqual([])
+})
+
+test("forward when none", () => {
+  expect(h.forward()).toEqual(null)
+  expect(h.all()).toEqual([])
+})
+
+test("limit", () => {
+  h.push("a")
+  h.push("b")
+  h.push("c")
+  h.push("d")
+  h.push("e")
+  // This one is over the limit
+  h.push("f")
+  expect(h.all()).toEqual(["b", "c", "d", "e", "f"])
+})

app/core/models/cmd-history.ts

@@ -0,0 +1,36 @@
+export default class CmdHistory {
+  constructor(
+    private array: string[] = [],
+    private index: number = 0,
+    private limit: number
+  ) {}
+
+  push(cmd) {
+    if (this.array.length === this.limit) this.array.shift()
+    this.array.push(cmd)
+    this.index = this.array.length - 1
+  }
+
+  back() {
+    if (this.index > 0) this.index -= 1
+    return this.current()
+  }
+
+  forward() {
+    if (this.index < this.array.length - 1) this.index += 1
+    return this.current()
+  }
+
+  all() {
+    return this.array
+  }
+
+  empty() {
+    return this.array.length === 0
+  }
+
+  private current() {
+    if (this.empty()) return null
+    return this.array[this.index]
+  }
+}

app/core/models/history.test.ts

@@ -0,0 +1,141 @@
+import History from "./history"
+
+let history: History<string>
+beforeEach(() => {
+  history = new History<string>()
+})
+
+test("push new value", () => {
+  history.push("a")
+
+  expect(history.entries()).toEqual(["a"])
+})
+
+test("pushing the same value twice", () => {
+  history.push("a")
+  history.push("a")
+  expect(history.entries()).toEqual(["a"])
+})
+
+test("pushing the same value twice after going back", () => {
+  history.push("a")
+  history.push("b")
+  history.back()
+  history.push("a")
+  history.push("a")
+  expect(history.entries()).toEqual(["a", "b"])
+})
+
+test("replace", () => {
+  history.push("a")
+  history.push("b")
+  history.back()
+  history.replace("z")
+  expect(history.entries()).toEqual(["z", "b"])
+})
+
+test("update on object", () => {
+  type Person = {name: string; age: number}
+  const history = new History<Person>()
+  history.push({name: "A", age: 1})
+  history.push({name: "B", age: 2})
+  history.push({name: "C", age: 3})
+  history.update({name: "Z"})
+  expect(history.current()).toEqual({name: "Z", age: 3})
+})
+
+test("push, push, back, push, back", () => {
+  history.push("a")
+  history.push("b")
+  history.back()
+  history.push("c")
+  history.back()
+
+  expect(history.current()).toEqual("a")
+})
+
+test("current gets latest entry", () => {
+  history.push("a")
+  history.push("b")
+  history.push("c")
+
+  expect(history.current()).toEqual("c")
+})
+
+test("going back", () => {
+  history.push("a")
+  history.push("b")
+  history.push("c")
+
+  history.back()
+
+  expect(history.current()).toEqual("b")
+})
+
+test("going back twice", () => {
+  history.push("a")
+  history.push("b")
+  history.push("c")
+
+  history.back()
+  history.back()
+
+  expect(history.current()).toEqual("a")
+})
+
+test("going back twice then forward twice", () => {
+  history.push("a")
+  history.push("b")
+  history.push("c")
+
+  history.back()
+  history.back()
+  history.forward()
+  history.forward()
+
+  expect(history.current()).toEqual("c")
+})
+
+test("when going back too far", () => {
+  history.push("a")
+  history.push("b")
+  history.push("c")
+
+  history.back()
+  history.back()
+  history.back()
+
+  expect(history.current()).toEqual("a")
+})
+
+test("when going forward too far and then back", () => {
+  history.push("a")
+  history.push("b")
+  history.push("c")
+
+  history.forward()
+
+  expect(history.current()).toEqual("c")
+
+  history.back()
+
+  expect(history.current()).toEqual("b")
+})
+
+test("pushing duplicates", () => {
+  history.push("a")
+  history.push("b")
+  history.push("b")
+  history.push("b")
+
+  expect(history.entries()).toEqual(["a", "b"])
+})
+
+test("pushing duplicates after going back", () => {
+  history.push("a")
+  history.push("b")
+  history.back()
+  history.push("b")
+
+  expect(history.entries()).toEqual(["a", "b"])
+})

app/core/models/history.ts

@@ -0,0 +1,62 @@
+import {isEqual} from "lodash"
+
+export default class History<Entry> {
+  static parse<T>({entries, position}) {
+    return new History<T>([...entries], position)
+  }
+
+  constructor(private array: Entry[] = [], private index: number = 0) {}
+
+  push(entry: Entry) {
+    if (!isEqual(entry, this.current())) {
+      this.array.splice(this.index + 1, this.array.length, entry)
+      this.index = this.array.length - 1
+    }
+  }
+
+  update(updates: Partial<Entry>) {
+    const current = this.current()
+    if (typeof current == "object") {
+      this.array[this.index] = {...current, ...updates}
+    }
+  }
+
+  replace(entry: Entry) {
+    this.array[this.index] = entry
+  }
+
+  entries(): Entry[] {
+    return this.array
+  }
+
+  current(): Entry | undefined {
+    return this.array[this.index]
+  }
+
+  canGoBack() {
+    return this.index > 0
+  }
+
+  back() {
+    if (this.canGoBack()) {
+      this.index -= 1
+    }
+  }
+
+  canGoForward() {
+    return this.index < this.array.length - 1
+  }
+
+  forward() {
+    if (this.canGoForward()) {
+      this.index += 1
+    }
+  }
+
+  serialize() {
+    return {
+      entries: this.array,
+      position: this.index
+    }
+  }
+}