U5: row context menu on right-click #15

Merged
yuris merged 1 commit from upgrade/u5-ctxmenu into main 2026-09-01 18:34:19 +00:00
Owner

Right-clicking a history row opens a menu of actions for that interval. Three
things that needed a terminal before.

31/08 12:00 · admin · 0:30      (header)
Continue admin
────────────────────────────    (separator)
Delete this interval
Undo last change                (disabled unless this panel changed something)

Where it can be bound is not a choice

pointerContext has exactly one producer in the shell — ui_tree_reconciler.cpp:1385,
inside setOnRightClickWithPointer — and panel.openContextMenu returns false
without it (plugin_bindings.cpp:760-765). onRightClick is in turn absent from the
kFlex and kBox whitelists, so no container can host a menu. That leaves the row's
tag button, which is a ui.button only because onClick needed one; the dot beside it
is a ui.box and cannot share this the way it shares the click.

onActivate must be a named global string, not a closure
(plugin_bindings.cpp:650, max 128 bytes). So this PR reintroduces exactly one named
global — the thing #13 deleted 27 of — deliberately, and routes every row through it
using the request's context. There is no per-row name and no numbering, which is what
keeps the list uncapped.

Ruling: stable identifier, not a row index

timew's ids move. @N counts backwards from the newest interval, so every insert
renumbers every id. Verified live rather than reasoned about:

$ timew summary :ids            @2 = alpha "deep work",  @1 = beta
$ timew start gamma             Tracking gamma
$ timew continue @2             Tracking beta          ← not the interval the user saw

An interval's start is the identifier that does not move: timew forbids overlap, so
no two intervals share one, and nothing in this panel modifies a start.
Parse.interval now keeps the whole timestamp beside the two clipped halves the list
displays, and the menu carries it in context.

Positional is safe at open time and wrong at activate time, which is why the index
is spent in one place and the identifier crosses into the other:

  • Open. openRowMenu(i) indexes state.shown, exactly as filterFromRow(i) does.
    Safe by the same invariant: the shell resolves a closure out of the handler table the
    newest render installed (luau_host.cpp:3084-3090) and commit publishes
    state.shown off the very model it renders, so the closure and the list it indexes
    are always from one commit.
  • Activate. The menu is open across renders. The panel re-fetches on its own second
    tick and on all four state watches, so by the time an item is chosen state.shown may
    be shorter, reordered, or a different day's — while the menu still shows the row that
    was right-clicked. An index would still be 2; it would name something else.

Timew.cmd.byStart closes the remaining window by making the resolve and the act one
process
: the walk finds the interval whose start matches $2 and execs timew on
that id, so the number never returns to Luau to go stale. A row already deleted from a
terminal resolves to nothing and says so — it cannot resolve to a different interval.

Two subprocesses, not one per id: the starts arrive from a single timew get over every
reference, and a token's position in that output is the id. 70 ms over 66 intervals,
against ~950 ms at ALL_WALK_MAX for the per-id spelling and Timew.exec's 3 s timeout.
The bound is ALL_WALK_MAX and the suite reads it out of both scripts, so a resolve
walk shorter than the list walk fails rather than leaving the oldest rows on screen
unactionable.

Ruling: no confirmation, and why that is not an omission

Backed by timew help delete, timew help continue, timew help undo on 1.9.1.

A two-step menu is structurally impossible. onActivate is enqueued with
makeScriptSnapshot() (plugin_panel.cpp:406-423), and that function leaves
pointerContext unset (:471-478) — the UI-callback path is the only one that fills it
in, at :173-182. So a second openContextMenu from an activation returns false.
There is no dialog primitive either. An "are you sure" step has nowhere to be drawn.

So the mis-click is made cheap and legible instead of impossible, on three counts:

  1. The header names the interval — date, time, tag, duration — so a right-click on
    the wrong row is visible before anything is chosen.
  2. Delete is behind a separator and is not the bottom item. The bottom edge is where
    an over-travelled pointer lands; undo sits there instead.
  3. It is reversible from the same menu. timew keeps a journal. Verified:
$ timew summary :ids     @2 alpha, deep work | @1 beta
$ timew delete @1        Deleted @1
$ timew summary :ids     @1 alpha, deep work
$ timew undo             Undo
$ timew summary :ids     @2 alpha, deep work | @1 beta      ← id and tags restored

Undo is gated on state.undoable, the one enabled flag this menu writes down. The
journal is global and older than the panel, so offering it unconditionally would invite a
user to revert something done in a terminal an hour ago with no way to see what it was.
Gated, the item means "undo what I just did here". It is cleared after undoing, because
the entry underneath belongs to whoever made it and timew undo cannot itself be undone.

Ruling: continue @id, not start <tag>

A row shows one string where an interval can carry several tags — timew's DOM prints
them as alpha "deep work" — so timew start <row.tag> would create a single tag
literally called alpha "deep work". timew continue @id copies the whole set:

$ byStart continue 2026-09-01T10:00:00
Tracking alpha "deep work"
$ timew get dom.tracked.1.tags
alpha "deep work"

It also stops whatever is running first (exit 0, no error), which is what switching tasks
means in a time tracker — so it is offered while tracking rather than greyed out.

Refresh after acting

Every action goes through the existing run, which already refetches and pokes the bar.
No second path. run grew one parameter for the undo flag and now reads Timew.exec's
ok, so a failed command arms nothing.

Trust boundary

A tag comes out of timew's database, which is to say out of whatever the user has ever
typed at a shell, and a label over 512 bytes makes the binding raise rather than
return (plugin_bindings.cpp:638-640) — taking the handler down with it. Tags are
clipped to 120 bytes for the menu; a 3000-byte tag is in the suite.

byStart carries three data elements, one of them a translated sentence — a locale is a
file on disk, so the hostile-bytes assert is pointed at the message slot as well as the
start.

Files

timew/parse.luau is in the diff beyond the task's file list, and unavoidably: the row
had nowhere to keep its identity. Parse.interval gains one nested capture and one field;
date, at, duration and tag are byte-identical.

Gates

$ nix shell nixpkgs#luau -c luau test.luau
viewModel: 12 states, 1739 field comparisons
ok

$ noctalia plugins lint timew
yuris/timew
  ok

0 errors, 0 warnings

plugin_api stays at 3 — the context menu needs no bump, it already works there. No
formatting settings, no gestures, no bar changes.

Mutation results — 13 applied, 13 killed

Mutation Killed by
context = row.start → context = i the context is row 2's own start, not its index: "2"
openRowMenu(i) → openRowMenu(1) at the call site the context is row 2's own start...: "2026-08-31T17:48:53"
drop the separator item header, continue, separator, delete, undo: got 4
enabled = state.undoable → enabled = true with the journal popped, undo is disabled rather than absent
run(Timew.cmd.undo(), false) → run(Timew.cmd.undo()) undoing pops the journal, so the entry underneath is no longer this panel's to offer
apply({ undoable = undoable ~= false }) → no-op a landed stop of this panel's is a change it may offer back
refresh() → fetchDays() in run the delete refetched through refresh: 1 commands, not 4
refresh() → nothing in run no queued command matching /bin/sh -c for i in $(seq 0 13)...
drop the id == "continue" or id == "delete" guard onActivate acted on the id "start", which this menu never published
#tag <= 120 → #tag <= 6000 items[1].label is 3023 bytes, not 1..512
-gt ALL_WALK_MAX → -gt DAY_WALK_MAX in byStart delete by start: argv[3] is ... -gt 60 ...
start = start → start = date in Parse.interval the row keeps its whole start: 2026-08-31

What a reviewer should challenge hardest

  1. undoable as panel state. It is the first field in state that no fetch answers
    and no viewModel field carries — the menu reads it directly, the way handlers read
    state.shown. If "the journal is global, so only offer this panel's own change" is
    the wrong call, the alternative is offering undo unconditionally and the field goes.
  2. No confirmation. The impossibility of a second menu is evidence-backed; "header +
    separator + position + timew undo" being enough is a judgement. The remaining hole
    is a delete followed by a write from a terminal before the user reaches for undo — the
    journal's top is then not the delete, and the item silently reverts the wrong thing.
    Nothing in the plugin can see that.
  3. continue while tracking stops the running clock, silently, with exit 0. Offered
    rather than disabled, on the argument that switching tasks is the common case. A user
    who reads the label as "also start this" loses the current interval's boundary — and
    timew undo does bring it back.
  4. byStart's alignment assumption: that a token's position in timew get <200 refs>
    is the dom id. It holds because ids 1..n are all valid and timew prints one value per
    reference in order, and the miss path is safe (no match → exit 1, translated toast),
    but a timew that ever skipped a reference would act on the wrong id.
  5. Byte-based tag clipping splits a UTF-8 codepoint at 120 bytes. Judged better than
    the alternative — one replacement glyph versus a luaL_error — but it is a visible
    artefact in a non-Latin locale.
Right-clicking a history row opens a menu of actions for that interval. Three things that needed a terminal before. ``` 31/08 12:00 · admin · 0:30 (header) Continue admin ──────────────────────────── (separator) Delete this interval Undo last change (disabled unless this panel changed something) ``` ## Where it can be bound is not a choice `pointerContext` has exactly one producer in the shell — `ui_tree_reconciler.cpp:1385`, inside `setOnRightClickWithPointer` — and `panel.openContextMenu` returns `false` without it (`plugin_bindings.cpp:760-765`). `onRightClick` is in turn absent from the `kFlex` and `kBox` whitelists, so no container can host a menu. That leaves the row's tag button, which is a `ui.button` only because `onClick` needed one; the dot beside it is a `ui.box` and cannot share this the way it shares the click. `onActivate` must be a **named global string**, not a closure (`plugin_bindings.cpp:650`, max 128 bytes). So this PR reintroduces exactly one named global — the thing #13 deleted 27 of — deliberately, and routes every row through it using the request's `context`. There is no per-row name and no numbering, which is what keeps the list uncapped. ## Ruling: stable identifier, not a row index **timew's ids move.** `@N` counts backwards from the newest interval, so every insert renumbers every id. Verified live rather than reasoned about: ``` $ timew summary :ids @2 = alpha "deep work", @1 = beta $ timew start gamma Tracking gamma $ timew continue @2 Tracking beta ← not the interval the user saw ``` An interval's **start** is the identifier that does not move: timew forbids overlap, so no two intervals share one, and nothing in this panel modifies a start. `Parse.interval` now keeps the whole timestamp beside the two clipped halves the list displays, and the menu carries it in `context`. **Positional is safe at open time and wrong at activate time**, which is why the index is spent in one place and the identifier crosses into the other: - **Open.** `openRowMenu(i)` indexes `state.shown`, exactly as `filterFromRow(i)` does. Safe by the same invariant: the shell resolves a closure out of the handler table the newest render installed (`luau_host.cpp:3084-3090`) and `commit` publishes `state.shown` off the very model it renders, so the closure and the list it indexes are always from one commit. - **Activate.** The menu is open across renders. The panel re-fetches on its own second tick and on all four state watches, so by the time an item is chosen `state.shown` may be shorter, reordered, or a different day's — while the menu still shows the row that was right-clicked. An index would still be `2`; it would name something else. `Timew.cmd.byStart` closes the remaining window by making the resolve and the act **one process**: the walk finds the interval whose start matches `$2` and `exec`s timew on that id, so the number never returns to Luau to go stale. A row already deleted from a terminal resolves to nothing and says so — it cannot resolve to a different interval. Two subprocesses, not one per id: the starts arrive from a single `timew get` over every reference, and a token's position in that output is the id. 70 ms over 66 intervals, against ~950 ms at `ALL_WALK_MAX` for the per-id spelling and `Timew.exec`'s 3 s timeout. The bound is `ALL_WALK_MAX` and the suite reads it out of *both* scripts, so a resolve walk shorter than the list walk fails rather than leaving the oldest rows on screen unactionable. ## Ruling: no confirmation, and why that is not an omission Backed by `timew help delete`, `timew help continue`, `timew help undo` on 1.9.1. **A two-step menu is structurally impossible.** `onActivate` is enqueued with `makeScriptSnapshot()` (`plugin_panel.cpp:406-423`), and that function leaves `pointerContext` unset (`:471-478`) — the UI-callback path is the only one that fills it in, at `:173-182`. So a second `openContextMenu` from an activation returns `false`. There is no dialog primitive either. An "are you sure" step has nowhere to be drawn. So the mis-click is made cheap and legible instead of impossible, on three counts: 1. **The header names the interval** — date, time, tag, duration — so a right-click on the wrong row is visible before anything is chosen. 2. **Delete is behind a separator and is not the bottom item.** The bottom edge is where an over-travelled pointer lands; undo sits there instead. 3. **It is reversible from the same menu.** timew keeps a journal. Verified: ``` $ timew summary :ids @2 alpha, deep work | @1 beta $ timew delete @1 Deleted @1 $ timew summary :ids @1 alpha, deep work $ timew undo Undo $ timew summary :ids @2 alpha, deep work | @1 beta ← id and tags restored ``` **Undo is gated on `state.undoable`**, the one `enabled` flag this menu writes down. The journal is global and older than the panel, so offering it unconditionally would invite a user to revert something done in a terminal an hour ago with no way to see what it was. Gated, the item means "undo what I just did here". It is cleared after undoing, because the entry underneath belongs to whoever made it and `timew undo` cannot itself be undone. ## Ruling: `continue @id`, not `start <tag>` A row shows one string where an interval can carry several tags — timew's DOM prints them as `alpha "deep work"` — so `timew start <row.tag>` would create a single tag literally called `alpha "deep work"`. `timew continue @id` copies the whole set: ``` $ byStart continue 2026-09-01T10:00:00 Tracking alpha "deep work" $ timew get dom.tracked.1.tags alpha "deep work" ``` It also stops whatever is running first (exit 0, no error), which is what switching tasks means in a time tracker — so it is offered while tracking rather than greyed out. ## Refresh after acting Every action goes through the existing `run`, which already refetches and pokes the bar. No second path. `run` grew one parameter for the undo flag and now reads `Timew.exec`'s `ok`, so a failed command arms nothing. ## Trust boundary A tag comes out of timew's database, which is to say out of whatever the user has ever typed at a shell, and a label over 512 bytes makes the binding **raise** rather than return (`plugin_bindings.cpp:638-640`) — taking the handler down with it. Tags are clipped to 120 bytes for the menu; a 3000-byte tag is in the suite. `byStart` carries three data elements, one of them a translated sentence — a locale is a file on disk, so the hostile-bytes assert is pointed at the message slot as well as the start. ## Files `timew/parse.luau` is in the diff beyond the task's file list, and unavoidably: the row had nowhere to keep its identity. `Parse.interval` gains one nested capture and one field; `date`, `at`, `duration` and `tag` are byte-identical. ## Gates ``` $ nix shell nixpkgs#luau -c luau test.luau viewModel: 12 states, 1739 field comparisons ok $ noctalia plugins lint timew yuris/timew ok 0 errors, 0 warnings ``` `plugin_api` stays at 3 — the context menu needs no bump, it already works there. No formatting settings, no gestures, no bar changes. ## Mutation results — 13 applied, 13 killed | Mutation | Killed by | |---|---| | `context = row.start` → `context = i` | `the context is row 2's own start, not its index: "2"` | | `openRowMenu(i)` → `openRowMenu(1)` at the call site | `the context is row 2's own start...: "2026-08-31T17:48:53"` | | drop the `separator` item | `header, continue, separator, delete, undo: got 4` | | `enabled = state.undoable` → `enabled = true` | `with the journal popped, undo is disabled rather than absent` | | `run(Timew.cmd.undo(), false)` → `run(Timew.cmd.undo())` | `undoing pops the journal, so the entry underneath is no longer this panel's to offer` | | `apply({ undoable = undoable ~= false })` → no-op | `a landed stop of this panel's is a change it may offer back` | | `refresh()` → `fetchDays()` in `run` | `the delete refetched through refresh: 1 commands, not 4` | | `refresh()` → nothing in `run` | `no queued command matching /bin/sh -c for i in $(seq 0 13)...` | | drop the `id == "continue" or id == "delete"` guard | `onActivate acted on the id "start", which this menu never published` | | `#tag <= 120` → `#tag <= 6000` | `items[1].label is 3023 bytes, not 1..512` | | `-gt ALL_WALK_MAX` → `-gt DAY_WALK_MAX` in `byStart` | `delete by start: argv[3] is ... -gt 60 ...` | | `start = start` → `start = date` in `Parse.interval` | `the row keeps its whole start: 2026-08-31` | ## What a reviewer should challenge hardest 1. **`undoable` as panel state.** It is the first field in `state` that no fetch answers and no `viewModel` field carries — the menu reads it directly, the way handlers read `state.shown`. If "the journal is global, so only offer this panel's own change" is the wrong call, the alternative is offering undo unconditionally and the field goes. 2. **No confirmation.** The impossibility of a second menu is evidence-backed; "header + separator + position + `timew undo`" being *enough* is a judgement. The remaining hole is a delete followed by a write from a terminal before the user reaches for undo — the journal's top is then not the delete, and the item silently reverts the wrong thing. Nothing in the plugin can see that. 3. **`continue` while tracking stops the running clock**, silently, with exit 0. Offered rather than disabled, on the argument that switching tasks is the common case. A user who reads the label as "also start this" loses the current interval's boundary — and `timew undo` does bring it back. 4. **`byStart`'s alignment assumption**: that a token's position in `timew get <200 refs>` is the dom id. It holds because ids 1..n are all valid and timew prints one value per reference in order, and the miss path is safe (no match → exit 1, translated toast), but a timew that ever skipped a reference would act on the wrong id. 5. **Byte-based tag clipping** splits a UTF-8 codepoint at 120 bytes. Judged better than the alternative — one replacement glyph versus a `luaL_error` — but it is a visible artefact in a non-Latin locale.
Right-clicking a history row opens Continue / Delete / Undo for that
interval, under a header naming it. Three things that needed a terminal
before.

The menu opens from `onRightClick` on the row's tag button, which is the
only place it can: `pointerContext` has one producer in the shell
(setOnRightClickWithPointer, ui_tree_reconciler.cpp:1385), the binding
returns false without it, and `onRightClick` is absent from the kFlex and
kBox whitelists. `onActivate` must be a named global string
(plugin_bindings.cpp:650), so this reintroduces exactly one named global
and routes every row through it.

Identity is the interval's own start timestamp, not a row index. timew's
`@N` counts backwards from the newest interval, so every insert
renumbers every id -- verified live: with `alpha` at @2, one
`timew start gamma` shifted it to @3 and `timew continue @2` resumed
`beta`. Parse.interval keeps the whole start, the menu carries it in
`context`, and Timew.cmd.byStart resolves it back to an id inside the
same shell process that acts on it, so the id never goes stale between
lookup and use. An index would be safe at open time (the closure and
`state.shown` come from one commit) and wrong at activate time, because
the panel re-fetches on its own tick while the menu is open.

Continue is `timew continue @id` rather than `timew start <tag>`: a row
shows one string where an interval can carry several tags, and continue
copies the whole set -- verified live, `alpha "deep work"` comes back
intact.

Delete is not confirmed, deliberately. A second menu is impossible --
onActivate runs under a snapshot with no pointer serial
(plugin_panel.cpp:406-423 against makeScriptSnapshot at :471-478) -- so
instead the header names the interval before anything can be chosen,
delete sits behind a separator and is not the bottom item, and timew's
journal makes it reversible from the same menu. Verified against 1.9.1:
delete, undo, the interval is back with its id and tags. Undo is offered
only for a change this panel made, since the journal is global and older
than the panel.

Every action goes through the existing `run`, so the refetch after a
state change is that one function's property.
yuris left a comment

Verdict: APPROVE

Reviewed by the integrator. The three rulings this PR rests on are each backed by live output,
and I re-derived all three independently rather than reading the argument.

Verified

Ids move, so start-based identity is necessary, not stylistic. Reproduced on this machine:
an interval sitting at @1 became @2 after a single unrelated timew start. A menu carrying
@N across a refresh would act on a different interval, exactly as the PR says.

Undo restores a delete, so the no-confirmation ruling holds. Reproduced end to end:
2 intervals -> timew delete -> 1 -> timew undo -> 2, tags and ids intact. That makes the
mis-click recoverable from the same menu, which is the substance behind the argument that a
confirm step is unavailable. The structural claim is also right: onActivate is enqueued with
makeScriptSnapshot(), which leaves pointerContext unset, so a second openContextMenu from
an activation returns false - a two-step menu genuinely cannot be built here.

Resolve-and-act in one process is the right shape. byStart never returns an id to Lua, so
there is no window where the number goes stale between resolving and acting. A row deleted from
a terminal resolves to nothing and says so, rather than resolving to its neighbour. This is the
part I would have expected to get wrong, and it is correct.

Gates: 12 states, 1739 field comparisons / ok; lint 0 errors, 0 warnings.

Mutations

Six against the new safety properties, each restored byte-identically (patterns verified to have
applied - an unapplied mutation reports as a survivor):

# Mutation Result
M1 undo armed even for non-journal commands killed
M2 separator becomes a header killed
M3 undo item always enabled killed
M5 Parse.interval loses its ISO start killed
M6 empty context accepted for a destructive action killed
M7 tag clipping disabled, re-exposing the 512-byte raise killed

M7 is the one worth naming: a tag over 512 bytes makes the binding raise rather than return,
taking the handler down with it, and tags come from whatever the user has ever typed at a shell.
Clipping to 120 bytes with a 3000-byte tag in the suite is the right defence, and it is tested.

Advisory

  1. state.undoable means "this panel changed something", and is cleared after undoing because
    the journal entry underneath belongs to someone else. That reasoning is sound but lives only
    in the PR body - it belongs in a comment beside the field, where the next reader will be.
  2. The menu offers Continue while tracking, relying on timew continue stopping the current
    interval first. True on 1.9.1 and verified in the PR, but it is an upstream behaviour the
    plugin now depends on silently; worth a line in the README when U8 syncs it.
**Verdict: APPROVE** Reviewed by the integrator. The three rulings this PR rests on are each backed by live output, and I re-derived all three independently rather than reading the argument. ## Verified **Ids move, so start-based identity is necessary, not stylistic.** Reproduced on this machine: an interval sitting at `@1` became `@2` after a single unrelated `timew start`. A menu carrying `@N` across a refresh would act on a different interval, exactly as the PR says. **Undo restores a delete, so the no-confirmation ruling holds.** Reproduced end to end: 2 intervals -> `timew delete` -> 1 -> `timew undo` -> 2, tags and ids intact. That makes the mis-click recoverable from the same menu, which is the substance behind the argument that a confirm step is unavailable. The structural claim is also right: `onActivate` is enqueued with `makeScriptSnapshot()`, which leaves `pointerContext` unset, so a second `openContextMenu` from an activation returns `false` - a two-step menu genuinely cannot be built here. **Resolve-and-act in one process is the right shape.** `byStart` never returns an id to Lua, so there is no window where the number goes stale between resolving and acting. A row deleted from a terminal resolves to nothing and says so, rather than resolving to its neighbour. This is the part I would have expected to get wrong, and it is correct. **Gates:** `12 states, 1739 field comparisons / ok`; lint `0 errors, 0 warnings`. ## Mutations Six against the new safety properties, each restored byte-identically (patterns verified to have applied - an unapplied mutation reports as a survivor): | # | Mutation | Result | |---|----------|--------| | M1 | undo armed even for non-journal commands | killed | | M2 | separator becomes a header | killed | | M3 | undo item always enabled | killed | | M5 | `Parse.interval` loses its ISO start | killed | | M6 | empty `context` accepted for a destructive action | killed | | M7 | tag clipping disabled, re-exposing the 512-byte raise | killed | M7 is the one worth naming: a tag over 512 bytes makes the binding *raise* rather than return, taking the handler down with it, and tags come from whatever the user has ever typed at a shell. Clipping to 120 bytes with a 3000-byte tag in the suite is the right defence, and it is tested. ## Advisory 1. `state.undoable` means "this panel changed something", and is cleared after undoing because the journal entry underneath belongs to someone else. That reasoning is sound but lives only in the PR body - it belongs in a comment beside the field, where the next reader will be. 2. The menu offers Continue while tracking, relying on `timew continue` stopping the current interval first. True on 1.9.1 and verified in the PR, but it is an upstream behaviour the plugin now depends on silently; worth a line in the README when U8 syncs it.
yuris merged commit 93a663c496 into main 2026-09-01 18:34:19 +00:00
yuris deleted branch upgrade/u5-ctxmenu 2026-09-01 18:34:19 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
yuris/noctalia-timew!15
No description provided.