202 lines
8.7 KiB
Markdown
202 lines
8.7 KiB
Markdown
# Editable and deletable library nodes
|
|
|
|
## Context and problem statement
|
|
|
|
The search feature (`architecture/search.md`) introduced *creatable* nodes:
|
|
`%` under `/tidal/search` turns a typed term into a tree node holding search
|
|
results. Those nodes are currently immutable — a typo'd term can only be
|
|
abandoned, and stale terms accumulate for the lifetime of the server process.
|
|
|
|
This feature makes such nodes *modifiable*: in the library pane, `e` renames
|
|
the selected node (for a search term: re-runs the search under the new term)
|
|
and `d` deletes it. Modifiability is a per-node capability advertised by the
|
|
provider, exactly like `is_creatable` — the TUI never hardcodes which paths
|
|
support what.
|
|
|
|
Run autonomously per standing user instruction; every decision below records
|
|
the options considered and the rationale.
|
|
|
|
## Assumptions
|
|
|
|
- "Nodes that got created via `%`" are today exactly the search-term nodes
|
|
under `/tidal/search`; the design must not special-case them (playlists are
|
|
the obvious future candidate), but they are the only provider implementation
|
|
in this iteration.
|
|
- Editing means **renaming** (the title is the only user-supplied property a
|
|
node has). For a search term, the title *is* the query, so a rename re-runs
|
|
the search.
|
|
- `e`/`d` act on the **selected item in the library list** (the same selection
|
|
model as queueing), not on the currently-open node.
|
|
- Proto changes must stay wire-compatible (additive fields/rpcs only), as with
|
|
the search feature.
|
|
|
|
## Decisions
|
|
|
|
### D1 — Two capability flags, on the child only
|
|
|
|
Options considered:
|
|
|
|
1. One `is_modifiable` flag implying both rename and delete.
|
|
2. Two flags `is_editable` / `is_deletable`.
|
|
3. Flags on both `LibraryNodeChild` and `LibraryNode` (mirroring
|
|
`is_creatable`).
|
|
|
|
**Decision: (2), child-only** — `LibraryNodeChild.is_editable = 5`,
|
|
`LibraryNodeChild.is_deletable = 6`. Future node kinds plausibly support only
|
|
one of the two (a favorites entry may be deletable but not renamable), and an
|
|
extra bool costs nothing on the wire. Unlike `is_creatable` (consumed for the
|
|
*open* node: pane hint + `%` target), edit/delete are only ever checked
|
|
against the selected **child**, so node-level copies would have no consumer —
|
|
they are left out until something reads them.
|
|
|
|
### D2 — RPC shapes
|
|
|
|
Options considered:
|
|
|
|
1. One generic `UpdateLibraryNode` with optional fields + a separate delete.
|
|
2. `RenameLibraryNode(path, new_title) → node` and
|
|
`DeleteLibraryNode(path) → parent node`.
|
|
|
|
**Decision: (2).** Rename is the only edit that exists; a generic update
|
|
message would be speculative surface area. Both responses carry the node the
|
|
TUI should display next, following `CreateLibraryNode`:
|
|
|
|
- **Rename returns the renamed node** and the TUI navigates into it — the
|
|
same "show me the result" behavior as `%` create. The path changes on
|
|
rename (term is percent-encoded into the path), so returning the node is
|
|
also what tells the client the new path.
|
|
- **Delete returns the refreshed parent node** — the user is looking at the
|
|
parent listing when they press `d`; returning it saves a follow-up
|
|
`GetLibraryNode` and can never serve a stale cached listing.
|
|
|
|
### D3 — Provider semantics (tidaldy)
|
|
|
|
- `rename_lib_node(path, new_title)`: `path` must parse to
|
|
`TidalPath::SearchTerm`, else `NotSupported`. Title is trimmed; empty →
|
|
`InvalidInput`. The old term is **replaced in place** (keeps its position in
|
|
the terms list). Renaming to an already-existing term merges: the old entry
|
|
is removed, the existing one wins — the list never holds duplicates.
|
|
Renaming a term the server doesn't know (stale client cache, restart)
|
|
registers the new term — same forgiveness as `get_lib_node` on unknown
|
|
terms. Returns `get_lib_node(new_path)`.
|
|
- `delete_lib_node(path)`: `path` must parse to `TidalPath::SearchTerm`, else
|
|
`NotSupported`. Removing an unknown term succeeds silently — delete is
|
|
idempotent. Returns the refreshed parent node (`get_lib_node(/tidal/search)`).
|
|
- Both reuse the `search_terms` `RwLock` discipline: poison-tolerant, never
|
|
held across an await.
|
|
- **Queued search tracks survive rename/delete**: a queue entry
|
|
`/tidal/search/<enc-term>/<track-id>` resolves URLs/metadata from the
|
|
embedded track id alone; term registration is irrelevant to playback.
|
|
|
|
### D4 — No delete confirmation (for now)
|
|
|
|
**Options**: confirm prompt (`y`/`n` mini-mode) vs immediate delete.
|
|
|
|
**Decision: immediate.** The only deletable nodes are search terms, which are
|
|
free to recreate (`%` + retype); a confirmation mode adds a third input state
|
|
for no protected value. **Open question**: when higher-value nodes (user
|
|
playlists) become deletable, a confirmation step must be revisited — noted
|
|
here so the decision is rediscovered.
|
|
|
|
### D5 — TUI input overlay grows a purpose
|
|
|
|
`InputState` today hardcodes creation (`parent_path` + buffer). It becomes
|
|
|
|
```rust
|
|
InputState { purpose: InputPurpose, buffer: String }
|
|
enum InputPurpose { Create { parent_path }, Rename { path } }
|
|
```
|
|
|
|
- `e` opens the overlay **prefilled with the current title** (append-only
|
|
editing as before: chars push, Backspace pops); Enter sends
|
|
`MessageFromUi::RenameNode`, Esc cancels. Overlay label: `rename: <buffer>▏`
|
|
vs `new node: <buffer>▏`.
|
|
- Prefilling means "rename" degrades gracefully to "retype" — no cursor
|
|
movement is introduced in this iteration (matches the existing overlay).
|
|
- Submitting an unchanged title is sent anyway; the provider treats it as a
|
|
no-op rename and returns the node (harmless refresh).
|
|
- `d` sends `MessageFromUi::DeleteNode { path }` directly (D4).
|
|
- Both keys are silently ignored when the selected item lacks the flag,
|
|
mirroring `%` on non-creatable nodes.
|
|
- List marker: editable/deletable children render a `[e]`, `[d]` or `[ed]`
|
|
suffix in `COLOR_SECONDARY`, alongside the existing `[%]` for creatable
|
|
ones. The bindings table gains `e`/`d` in the Library scope (plain `e` and
|
|
`d` are unbound there today; `d` only exists in Queue scope), so the help
|
|
modal picks them up automatically.
|
|
|
|
### D6 — Client cache handling
|
|
|
|
`RpcClient` keeps `library_node_cache`. On rename: evict the **old path** and
|
|
the **parent**, insert the returned node under its new path. On delete: evict
|
|
the deleted path and the parent, insert the returned parent. (Same reasoning
|
|
as create's eviction — a stale `/tidal/search` listing would resurrect the
|
|
old term in the UI.)
|
|
|
|
### D7 — Server plumbing
|
|
|
|
Two new `ProviderCommand`s (`RenameLibraryNode`, `DeleteLibraryNode`) with the
|
|
established bounded(1) reply rendezvous; orchestrator routes `/tidal`-prefixed
|
|
paths to the tidal client and answers anything else `NotSupported`. gRPC error
|
|
mapping is identical to create: `NotSupported` → `failed_precondition`,
|
|
`InvalidInput` → `invalid_argument`, rest → `internal`; no internals leak into
|
|
`Status` messages.
|
|
|
|
## Flows
|
|
|
|
```d2
|
|
shape: sequence_diagram
|
|
user: { shape: person }
|
|
tui: cbd-tui
|
|
server: crabidy-server
|
|
tidal: tidaldy
|
|
|
|
user -> tui: "e on selected [ed] node"
|
|
tui -> tui: open overlay prefilled with title
|
|
user -> tui: edit text, Enter
|
|
tui -> server: RenameLibraryNode(path, new_title)
|
|
server -> tidal: rename_lib_node
|
|
tidal -> tidal: replace term in list (merge on collision)
|
|
tidal -> server: node at new path (fresh search)
|
|
server -> tui: renamed node
|
|
tui -> tui: evict old path + parent, show renamed node
|
|
|
|
user -> tui: "d on selected [ed] node"
|
|
tui -> server: DeleteLibraryNode(path)
|
|
server -> tidal: delete_lib_node
|
|
tidal -> tidal: remove term (idempotent)
|
|
tidal -> server: refreshed parent node
|
|
server -> tui: parent node
|
|
tui -> tui: evict path + parent, show parent listing
|
|
```
|
|
|
|
Capability flags travel with every listing:
|
|
|
|
```d2
|
|
direction: right
|
|
tidaldy: {
|
|
search_arm: "get_lib_node(/tidal/search)"
|
|
}
|
|
proto: "LibraryNodeChild { is_editable=5, is_deletable=6 }"
|
|
tui: {
|
|
list: "library list: title [ed]"
|
|
keys: "e -> rename overlay\nd -> DeleteNode"
|
|
}
|
|
tidaldy.search_arm -> proto: term children flagged
|
|
proto -> tui.list: render marker
|
|
proto -> tui.keys: gate actions
|
|
```
|
|
|
|
## Boundaries and risks
|
|
|
|
- **Proto**: additive only — two rpcs, two child fields (5, 6). Old clients
|
|
ignore the flags and never call the rpcs; old servers reject unknown rpcs
|
|
with `unimplemented` (tonic default), which the TUI logs without crashing.
|
|
- **Rename-to-collision** merges terms; the response node is the *existing*
|
|
term's node. The user sees the results they asked for either way.
|
|
- **Concurrent clients**: two TUIs editing the same term list race benignly —
|
|
the list is a `RwLock`-guarded Vec, every operation is atomic under the
|
|
write lock, and stale views self-heal on the next listing fetch.
|
|
- **Not in scope**: editing anything but the title; deleting non-search
|
|
nodes; confirmation UX (D4); persistent search terms (still per-process,
|
|
as shipped by the search feature).
|