ignore: refactoring and tests

packages/app/src/components/titlebar-history.test.ts

@@ -0,0 +1,63 @@
+import { describe, expect, test } from "bun:test"
+import { applyPath, backPath, forwardPath, type TitlebarHistory } from "./titlebar-history"
+
+function history(): TitlebarHistory {
+  return { stack: [], index: 0, action: undefined }
+}
+
+describe("titlebar history", () => {
+  test("append and trim keeps max bounded", () => {
+    let state = history()
+    state = applyPath(state, "/", 3)
+    state = applyPath(state, "/a", 3)
+    state = applyPath(state, "/b", 3)
+    state = applyPath(state, "/c", 3)
+
+    expect(state.stack).toEqual(["/a", "/b", "/c"])
+    expect(state.stack.length).toBe(3)
+    expect(state.index).toBe(2)
+  })
+
+  test("back and forward indexes stay correct after trimming", () => {
+    let state = history()
+    state = applyPath(state, "/", 3)
+    state = applyPath(state, "/a", 3)
+    state = applyPath(state, "/b", 3)
+    state = applyPath(state, "/c", 3)
+
+    expect(state.stack).toEqual(["/a", "/b", "/c"])
+    expect(state.index).toBe(2)
+
+    const back = backPath(state)
+    expect(back?.to).toBe("/b")
+    expect(back?.state.index).toBe(1)
+
+    const afterBack = applyPath(back!.state, back!.to, 3)
+    expect(afterBack.stack).toEqual(["/a", "/b", "/c"])
+    expect(afterBack.index).toBe(1)
+
+    const forward = forwardPath(afterBack)
+    expect(forward?.to).toBe("/c")
+    expect(forward?.state.index).toBe(2)
+
+    const afterForward = applyPath(forward!.state, forward!.to, 3)
+    expect(afterForward.stack).toEqual(["/a", "/b", "/c"])
+    expect(afterForward.index).toBe(2)
+  })
+
+  test("action-driven navigation does not push duplicate history entries", () => {
+    const state: TitlebarHistory = {
+      stack: ["/", "/a", "/b"],
+      index: 2,
+      action: undefined,
+    }
+
+    const back = backPath(state)
+    expect(back?.to).toBe("/a")
+
+    const next = applyPath(back!.state, back!.to, 10)
+    expect(next.stack).toEqual(["/", "/a", "/b"])
+    expect(next.index).toBe(1)
+    expect(next.action).toBeUndefined()
+  })
+})

packages/app/src/components/titlebar-history.ts

@@ -0,0 +1,57 @@
+export const MAX_TITLEBAR_HISTORY = 100
+
+export type TitlebarAction = "back" | "forward" | undefined
+
+export type TitlebarHistory = {
+  stack: string[]
+  index: number
+  action: TitlebarAction
+}
+
+export function applyPath(state: TitlebarHistory, current: string, max = MAX_TITLEBAR_HISTORY): TitlebarHistory {
+  if (!state.stack.length) {
+    const stack = current === "/" ? ["/"] : ["/", current]
+    return { stack, index: stack.length - 1, action: undefined }
+  }
+
+  const active = state.stack[state.index]
+  if (current === active) {
+    if (!state.action) return state
+    return { ...state, action: undefined }
+  }
+
+  if (state.action) return { ...state, action: undefined }
+
+  return pushPath(state, current, max)
+}
+
+export function pushPath(state: TitlebarHistory, path: string, max = MAX_TITLEBAR_HISTORY): TitlebarHistory {
+  const stack = state.stack.slice(0, state.index + 1).concat(path)
+  const next = trimHistory(stack, stack.length - 1, max)
+  return { ...state, ...next, action: undefined }
+}
+
+export function trimHistory(stack: string[], index: number, max = MAX_TITLEBAR_HISTORY) {
+  if (stack.length <= max) return { stack, index }
+  const cut = stack.length - max
+  return {
+    stack: stack.slice(cut),
+    index: Math.max(0, index - cut),
+  }
+}
+
+export function backPath(state: TitlebarHistory) {
+  if (state.index <= 0) return
+  const index = state.index - 1
+  const to = state.stack[index]
+  if (!to) return
+  return { state: { ...state, index, action: "back" as const }, to }
+}
+
+export function forwardPath(state: TitlebarHistory) {
+  if (state.index >= state.stack.length - 1) return
+  const index = state.index + 1
+  const to = state.stack[index]
+  if (!to) return
+  return { state: { ...state, index, action: "forward" as const }, to }
+}

packages/app/src/components/titlebar.tsx

@@ -11,6 +11,7 @@ import { useLayout } from "@/context/layout"
 import { usePlatform } from "@/context/platform"
 import { useCommand } from "@/context/command"
 import { useLanguage } from "@/context/language"
+import { applyPath, backPath, forwardPath } from "./titlebar-history"
 
 export function Titlebar() {
   const layout = useLayout()
@@ -39,47 +40,27 @@ export function Titlebar() {
     const current = path()
 
     untrack(() => {
-      if (!history.stack.length) {
-        const stack = current === "/" ? ["/"] : ["/", current]
-        setHistory({ stack, index: stack.length - 1 })
-        return
-      }
-
-      const active = history.stack[history.index]
-      if (current === active) {
-        if (history.action) setHistory("action", undefined)
-        return
-      }
-
-      if (history.action) {
-        setHistory("action", undefined)
-        return
-      }
-
-      const next = history.stack.slice(0, history.index + 1).concat(current)
-      setHistory({ stack: next, index: next.length - 1 })
+      const next = applyPath(history, current)
+      if (next === history) return
+      setHistory(next)
     })
   })
 
   const canBack = createMemo(() => history.index > 0)
   const canForward = createMemo(() => history.index < history.stack.length - 1)
 
   const back = () => {
-    if (!canBack()) return
-    const index = history.index - 1
-    const to = history.stack[index]
-    if (!to) return
-    setHistory({ index, action: "back" })
-    navigate(to)
+    const next = backPath(history)
+    if (!next) return
+    setHistory(next.state)
+    navigate(next.to)
   }
 
   const forward = () => {
-    if (!canForward()) return
-    const index = history.index + 1
-    const to = history.stack[index]
-    if (!to) return
-    setHistory({ index, action: "forward" })
-    navigate(to)
+    const next = forwardPath(history)
+    if (!next) return
+    setHistory(next.state)
+    navigate(next.to)
   }
 
   command.register(() => [

packages/app/src/context/command.test.ts

@@ -0,0 +1,25 @@
+import { describe, expect, test } from "bun:test"
+import { upsertCommandRegistration } from "./command"
+
+describe("upsertCommandRegistration", () => {
+  test("replaces keyed registrations", () => {
+    const one = () => [{ id: "one", title: "One" }]
+    const two = () => [{ id: "two", title: "Two" }]
+
+    const next = upsertCommandRegistration([{ key: "layout", options: one }], { key: "layout", options: two })
+
+    expect(next).toHaveLength(1)
+    expect(next[0]?.options).toBe(two)
+  })
+
+  test("keeps unkeyed registrations additive", () => {
+    const one = () => [{ id: "one", title: "One" }]
+    const two = () => [{ id: "two", title: "Two" }]
+
+    const next = upsertCommandRegistration([{ options: one }], { options: two })
+
+    expect(next).toHaveLength(2)
+    expect(next[0]?.options).toBe(two)
+    expect(next[1]?.options).toBe(one)
+  })
+})

packages/app/src/context/command.tsx

@@ -64,6 +64,16 @@ export type CommandCatalogItem = {
   slash?: string
 }
 
+export type CommandRegistration = {
+  key?: string
+  options: Accessor<CommandOption[]>
+}
+
+export function upsertCommandRegistration(registrations: CommandRegistration[], entry: CommandRegistration) {
+  if (entry.key === undefined) return [entry, ...registrations]
+  return [entry, ...registrations.filter((x) => x.key !== entry.key)]
+}
+
 export function parseKeybind(config: string): Keybind[] {
   if (!config || config === "none") return []
 
@@ -166,9 +176,10 @@ export const { use: useCommand, provider: CommandProvider } = createSimpleContex
     const settings = useSettings()
     const language = useLanguage()
     const [store, setStore] = createStore({
-      registrations: [] as Accessor<CommandOption[]>[],
+      registrations: [] as CommandRegistration[],
       suspendCount: 0,
     })
+    const warnedDuplicates = new Set<string>()
 
     const [catalog, setCatalog, _, catalogReady] = persisted(
       Persist.global("command.catalog.v1"),
@@ -187,8 +198,14 @@ export const { use: useCommand, provider: CommandProvider } = createSimpleContex
       const all: CommandOption[] = []
 
       for (const reg of store.registrations) {
-        for (const opt of reg()) {
-          if (seen.has(opt.id)) continue
+        for (const opt of reg.options()) {
+          if (seen.has(opt.id)) {
+            if (import.meta.env.DEV && !warnedDuplicates.has(opt.id)) {
+              warnedDuplicates.add(opt.id)
+              console.warn(`[command] duplicate command id \"${opt.id}\" registered; keeping first entry`)
+            }
+            continue
+          }
           seen.add(opt.id)
           all.push(opt)
         }
@@ -296,14 +313,25 @@ export const { use: useCommand, provider: CommandProvider } = createSimpleContex
       document.removeEventListener("keydown", handleKeyDown)
     })
 
+    function register(cb: () => CommandOption[]): void
+    function register(key: string, cb: () => CommandOption[]): void
+    function register(key: string | (() => CommandOption[]), cb?: () => CommandOption[]) {
+      const id = typeof key === "string" ? key : undefined
+      const next = typeof key === "function" ? key : cb
+      if (!next) return
+      const options = createMemo(next)
+      const entry: CommandRegistration = {
+        key: id,
+        options,
+      }
+      setStore("registrations", (arr) => upsertCommandRegistration(arr, entry))
+      onCleanup(() => {
+        setStore("registrations", (arr) => arr.filter((x) => x !== entry))
+      })
+    }
+
     return {
-      register(cb: () => CommandOption[]) {
-        const results = createMemo(cb)
-        setStore("registrations", (arr) => [results, ...arr])
-        onCleanup(() => {
-          setStore("registrations", (arr) => arr.filter((x) => x !== results))
-        })
-      },
+      register,
       trigger(id: string, source?: "palette" | "keybind" | "slash") {
         run(id, source)
       },