From ffbbd0b0c393b218b208e009817430a2019288b4 Mon Sep 17 00:00:00 2001 From: foxnne Date: Sat, 17 Oct 2026 17:52:38 -0500 Subject: [PATCH] app: Undo names whose step it takes, and the bubble after it has a Redo When the step an undo would take was made by someone else, the Edit menu reads "Undo Claude's Draw" (and Redo likewise), and pressing it says "Undid Claude's Draw" on the document with a Redo that puts it back. The person's own steps read "Undo" as always. `Editor.stepToTake` finds the step and its author for the menu, the bubble and `fizzy.undo` alike. Co-Authored-By: Claude Opus 5.5 --- app/ActivityFeed.zig | 67 +++++++++++++++++++++++++++++++++------ src/editor/Editor.zig | 63 +++++++++++++++++++++++++++++++++--- src/editor/Keybinds.zig | 41 ++++++++++-------------- src/editor/menu_model.zig | 25 +++++++++++++-- tests/integration.zig | 26 +++++++++++++++ 5 files changed, 181 insertions(+), 41 deletions(-) diff --git a/app/ActivityFeed.zig b/app/ActivityFeed.zig index c70ae3d57..0448fce19 100644 --- a/app/ActivityFeed.zig +++ b/app/ActivityFeed.zig @@ -1,7 +1,9 @@ //! The commands just run on one document (`Host.activity`), shown small at the bottom right of //! its pane as they run: each fades in, stays a moment and fades out, the older ones rising as //! newer ones come. Drawn on a layer that takes no input, so it never covers the work, and on wall -//! time, so whatever runs the commands (a person, an agent, a tape) shows the same. +//! time, so whatever runs the commands (a person, an agent, a tape) shows the same. One that +//! offers an action ("Undid Claude's Draw" with its Redo) takes clicks on its button only, and +//! stays longer to be reached. const ActivityFeed = @This(); const std = @import("std"); @@ -16,6 +18,8 @@ seen: u64 = 0, const max = 5; const life_ns: i128 = 2400 * std.time.ns_per_ms; +/// An item with an action stays this long. +const action_life_ns: i128 = 6000 * std.time.ns_per_ms; const fade_ns: i128 = 250 * std.time.ns_per_ms; const pad: f32 = 8; const gap: f32 = 4; @@ -24,6 +28,19 @@ const Item = struct { title: [48]u8, len: u8, born: i128, + action: ?Action = null, + + fn life(self: Item) i128 { + return if (self.action != null) action_life_ns else life_ns; + } +}; + +/// A button on an item: its label, and the caller's own tag for it, which `draw` answers when +/// it is pressed. +const Action = struct { + label: [16]u8, + len: u8, + tag: u64, }; /// A feed showing only what runs from now on. @@ -57,14 +74,26 @@ pub fn push(self: *ActivityFeed, title: []const u8, now: i128) void { self.len += 1; } -/// Draw the feed in the bottom right of `area` (physical), newest lowest. -pub fn draw(self: *ActivityFeed, area: dvui.Rect.Physical, id_extra: usize, now: i128) void { - // The oldest go first. - while (self.len > 0 and now - self.items[0].born > life_ns) { - std.mem.copyForwards(Item, self.items[0 .. self.len - 1], self.items[1..self.len]); - self.len -= 1; +/// `push`, with a button labelled `label` on it: `draw` answers `tag` when it is pressed. +pub fn pushAction(self: *ActivityFeed, title: []const u8, label: []const u8, tag: u64, now: i128) void { + self.push(title, now); + var action: Action = .{ .label = undefined, .len = @intCast(@min(label.len, 16)), .tag = tag }; + @memcpy(action.label[0..action.len], label[0..action.len]); + self.items[self.len - 1].action = action; +} + +/// Draw the feed in the bottom right of `area` (physical), newest lowest. Answers the tag of the +/// action pressed this frame, if one was. +pub fn draw(self: *ActivityFeed, area: dvui.Rect.Physical, id_extra: usize, now: i128) ?u64 { + // Those whose time is up go. + var kept: usize = 0; + for (self.items[0..self.len]) |item| { + if (now - item.born > item.life()) continue; + self.items[kept] = item; + kept += 1; } - if (self.len == 0) return; + self.len = kept; + if (self.len == 0) return null; dvui.refresh(null, @src(), null); const r = area.toNatural(); @@ -89,9 +118,10 @@ pub fn draw(self: *ActivityFeed, area: dvui.Rect.Physical, id_extra: usize, now: const age = now - item.born; // In, then out, over `fade_ns` each; rising into place as it comes in. const in = @min(1, @as(f32, @floatFromInt(age)) / @as(f32, @floatFromInt(fade_ns))); - const out = @min(1, @as(f32, @floatFromInt(life_ns - age)) / @as(f32, @floatFromInt(fade_ns))); + const out = @min(1, @as(f32, @floatFromInt(item.life() - age)) / @as(f32, @floatFromInt(fade_ns))); const alpha = @max(0, @min(in, out)); - const w = font.textSize(item.title[0..item.len]).w + pad * 2; + const action_w = if (item.action) |a| font.textSize(a.label[0..a.len]).w + pad * 2 else 0; + const w = font.textSize(item.title[0..item.len]).w + pad * 2 + action_w; const rect: dvui.Rect = .{ .x = r.w - 12 - w, .y = bottom - h + (1 - in) * h * 0.5, .w = w, .h = h }; // Glass has no fade: it grows in from, and shrinks out to, a little smaller. const k = 0.6 + 0.4 * alpha; @@ -102,6 +132,7 @@ pub fn draw(self: *ActivityFeed, area: dvui.Rect.Physical, id_extra: usize, now: bottom -= (h + gap) * in; } core.dialogs.glassBubbles(layer.data().id, bubbles[0..self.len], rs.s, .{}); + var pressed: ?u64 = null; for (self.items[0..self.len], rects[0..self.len], alphas[0..self.len], 0..) |item, rect, alpha, n| { dvui.labelNoFmt(@src(), item.title[0..item.len], .{}, .{ .rect = rect, @@ -111,5 +142,21 @@ pub fn draw(self: *ActivityFeed, area: dvui.Rect.Physical, id_extra: usize, now: .margin = .{}, .color_text = .{ .color = ink.opacity(0.85 * alpha) }, }); + const a = item.action orelse continue; + // The button alone takes input, on a layer of its own over the feed's. + const bw = font.textSize(a.label[0..a.len]).w + pad * 2; + var hit: dvui.FloatingWidget = undefined; + hit.init(@src(), .{}, .{ .rect = .{ .x = r.x + rect.x + rect.w - bw, .y = r.y + rect.y, .w = bw, .h = rect.h }, .id_extra = id_extra *% 8 +% n }); + defer hit.deinit(); + if (dvui.button(@src(), a.label[0..a.len], .{}, .{ + .expand = .both, + .font = font, + .padding = .{ .x = pad, .y = pad / 2, .w = pad, .h = pad / 2 }, + .margin = .{}, + .background = false, + .border = .{}, + .color_text = .{ .color = dvui.themeGet().color(.highlight, .fill).opacity(alpha) }, + })) pressed = a.tag; } + return pressed; } diff --git a/src/editor/Editor.zig b/src/editor/Editor.zig index f5cadbd5a..b6be18bab 100644 --- a/src/editor/Editor.zig +++ b/src/editor/Editor.zig @@ -2960,7 +2960,7 @@ fn drawDocSurface(ctx: ?*anyopaque) anyerror!dvui.App.Result { if (claim_pill == null) if (ds.transport.place(area, now)) |bar| { area.h = @min(area.h, bar.y - area.y + 6 * dvui.windowRectScale().s); }; - ds.feed.draw(area, @truncate(ds.doc_id), now); + if (ds.feed.draw(area, @truncate(ds.doc_id), now)) |step| editor.redoFromFeed(doc, step); } // A menu inside the document only when its owner names one. The context is created *after* @@ -6147,19 +6147,74 @@ pub fn redo(editor: *Editor) !void { /// An undo or a redo of the active document, through its owner's command: its transport shows, /// and a key held down goes further each press (`HistoryTransport.press`), the command run that -/// many times. Nothing to undo, or a claimed document: the command as ever, and no bar. -fn historyStep(editor: *Editor, action: enum { undo, redo }) !void { +/// many times. Nothing to undo, or a claimed document: the command as ever, and no bar. An undo of +/// a step someone else made says so on the document, with a Redo (`ActivityFeed.pushAction`), in +/// place of the plain "Undo" its run would show. +fn historyStep(editor: *Editor, action: HistoryAction) !void { const doc = editor.activeDoc() orelse return; const name = @tagName(action); - const shows = editor.app.host.documentClaim(doc.id) == null and editor.activeDocCommandEnabled(name); + const host = &editor.app.host; + const shows = host.documentClaim(doc.id) == null and editor.activeDocCommandEnabled(name); const ds = if (shows) editor.doc_surfaces.get(doc.id) else null; const by = if (ds) |d| d.transport.press(dvui.frameTimeNS(), action == .redo) else 1; for (0..by) |_| { if (!editor.activeDocCommandEnabled(name)) break; + // Who made a step is kept only once someone besides the person has acted on it. + const others: ?StepToTake = if (action == .undo and host.step_authors.contains(doc.id)) blk: { + const t = editor.stepToTake(doc, .undo, host.arena()) catch break :blk null; + break :blk if (t.by.kind == .person) null else t; + } else null; + const t = others orelse { + try editor.runActiveDocCommand(name); + continue; + }; + // The run's own "Undo" goes unrecorded (as while a demo seeks), so the bubble below is + // the one it shows. The flag is put back before the feed draws. + const muted = host.activity_muted; + host.activity_muted = true; + defer host.activity_muted = muted; try editor.runActiveDocCommand(name); + var buf: [96]u8 = undefined; + const said = std.fmt.bufPrint(&buf, "Undid {s}'s {s}", .{ authorName(t.by), t.step.title }) catch "Undid a change"; + if (editor.doc_surfaces.get(doc.id)) |d| d.feed.pushAction(said, "Redo", t.step.id, dvui.frameTimeNS()); } } +/// Redo `doc`'s step `id` from its bubble's Redo: only while it is still the next one to redo. +pub fn redoFromFeed(editor: *Editor, doc: sdk.DocHandle, id: u64) void { + const t = editor.stepToTake(doc, .redo, editor.app.host.arena()) catch return; + if (t.step.id == id) _ = doc.owner.historyGoTo(doc, t.to); +} + +pub const HistoryAction = enum { undo, redo }; + +/// The step an undo or a redo of a document would take, the step it leaves on top (`to`, for +/// `historyGoTo`; 0 before the first), and who made it. +pub const StepToTake = struct { step: sdk.Plugin.HistoryLine.Entry, to: u64, by: sdk.Author }; + +/// What an undo or a redo of `doc` would take, from its owner's line (`historyLine`): +/// `error.NoLine` when the owner keeps none, `error.Nothing` when there is nothing to take. +pub fn stepToTake(editor: *Editor, doc: sdk.DocHandle, action: HistoryAction, arena: std.mem.Allocator) error{ NoLine, Nothing }!StepToTake { + const line = doc.owner.historyLine(doc, arena) orelse return error.NoLine; + const step: sdk.Plugin.HistoryLine.Entry, const to: u64 = switch (action) { + .undo => blk: { + if (line.at == 0) return error.Nothing; + break :blk .{ line.entries[line.at - 1], if (line.at >= 2) line.entries[line.at - 2].id else 0 }; + }, + .redo => blk: { + if (line.at >= line.entries.len) return error.Nothing; + break :blk .{ line.entries[line.at], line.entries[line.at].id }; + }, + }; + return .{ .step = step, .to = to, .by = editor.app.host.stepAuthor(doc.id, step.id) }; +} + +/// Who made a step, as a person reads it: "the person", else the author's label ("Claude"). +pub fn authorName(a: sdk.Author) []const u8 { + if (a.kind == .person) return "the person"; + return if (a.label.len > 0) a.label else @tagName(a.kind); +} + pub fn openInFileBrowser(editor: *Editor, path: []const u8) !void { // Darwin goes through `darwin_spawn` rather than `std.process.run`: the latter walks the // `environ` array captured at startup, which any `unsetenv` elsewhere in the process (SDL, diff --git a/src/editor/Keybinds.zig b/src/editor/Keybinds.zig index d274f687d..387438c21 100644 --- a/src/editor/Keybinds.zig +++ b/src/editor/Keybinds.zig @@ -442,40 +442,31 @@ fn cmdRedoWith(state: *anyopaque, args: HistoryStep.Args, call: *sdk.Command.Cal /// `historyGoTo`), so it reaches a document that is not active. With `expect`, only when that /// step is the one it would take, so a caller never takes back a step made since it looked /// (`plans/SHARED_HISTORY_PLAN.md`, step 5). Answers the step and whose it was. -fn historyStepWith(editor: *Editor, args: HistoryStep.Args, call: *sdk.Command.Call, action: enum { undo, redo }) !void { +fn historyStepWith(editor: *Editor, args: HistoryStep.Args, call: *sdk.Command.Call, action: Editor.HistoryAction) !void { const doc = if (args.path.len == 0) editor.activeDoc() orelse return call.fail("no document is active", .{}) else editor.docFromPath(args.path) orelse return call.fail("no open document at {s}", .{args.path}); - const line = doc.owner.historyLine(doc, call.arena) orelse { + const t = editor.stepToTake(doc, action, call.arena) catch |err| switch (err) { + error.Nothing => return call.fail("nothing to {s}", .{@tagName(action)}), // An owner with no line undoes the active document only, through its own command. - const active = editor.activeDoc(); - if (args.expect != 0 or active == null or active.?.id != doc.id) - return call.fail("{s} keeps no history line, so it can only undo the active document, with no expect", .{doc.owner.id}); - return switch (action) { - .undo => editor.undo(), - .redo => editor.redo(), - }; - }; - const host = &editor.app.host; - const step: sdk.Plugin.HistoryLine.Entry, const to: u64 = switch (action) { - .undo => blk: { - if (line.at == 0) return call.fail("nothing to undo", .{}); - break :blk .{ line.entries[line.at - 1], if (line.at >= 2) line.entries[line.at - 2].id else 0 }; - }, - .redo => blk: { - if (line.at >= line.entries.len) return call.fail("nothing to redo", .{}); - break :blk .{ line.entries[line.at], line.entries[line.at].id }; + error.NoLine => { + const active = editor.activeDoc(); + if (args.expect != 0 or active == null or active.?.id != doc.id) + return call.fail("{s} keeps no history line, so it can only {s} the active document, with no expect", .{ doc.owner.id, @tagName(action) }); + return switch (action) { + .undo => editor.undo(), + .redo => editor.redo(), + }; }, }; - const whose = host.stepAuthor(doc.id, step.id); - const by = if (whose.kind == .person) "the person" else if (whose.label.len > 0) whose.label else @tagName(whose.kind); - if (args.expect != 0 and args.expect != step.id) return call.fail( + const by = Editor.authorName(t.by); + if (args.expect != 0 and args.expect != t.step.id) return call.fail( "step {d} is not the one to {s}: that is step {d}, {s}, by {s}. The document changed since; fix it forward", - .{ args.expect, @tagName(action), step.id, step.title, by }, + .{ args.expect, @tagName(action), t.step.id, t.step.title, by }, ); - if (!doc.owner.historyGoTo(doc, to)) return call.fail("{s} could not {s} step {d}", .{ doc.owner.id, @tagName(action), step.id }); - try call.returns(.{ .step = step.id, .title = step.title, .by = by }); + if (!doc.owner.historyGoTo(doc, t.to)) return call.fail("{s} could not {s} step {d}", .{ doc.owner.id, @tagName(action), t.step.id }); + try call.returns(.{ .step = t.step.id, .title = t.step.title, .by = by }); } /// Set while `tick` is running a command because of a live key event. That event is never /// marked handled, so it goes on to reach the focused widget under its own steam — a clipboard diff --git a/src/editor/menu_model.zig b/src/editor/menu_model.zig index 080d17fa1..e9c7329fe 100644 --- a/src/editor/menu_model.zig +++ b/src/editor/menu_model.zig @@ -112,6 +112,27 @@ fn canSaveAll(editor: *Editor) bool { return false; } +/// "Undo Claude's Draw" when the step it would take is someone else's; "Undo" for the person's +/// own, so working alone reads as it always has. +fn undoTitle(editor: *Editor) [:0]const u8 { + return historyTitle(editor, .undo, "Undo", &undo_title); +} +fn redoTitle(editor: *Editor) [:0]const u8 { + return historyTitle(editor, .redo, "Redo", &redo_title); +} + +var undo_title: [96:0]u8 = undefined; +var redo_title: [96:0]u8 = undefined; + +fn historyTitle(editor: *Editor, action: Editor.HistoryAction, plain: [:0]const u8, buf: *[96:0]u8) [:0]const u8 { + const doc = editor.activeDoc() orelse return plain; + // Who made a step is kept only once someone besides the person has acted on the document. + if (!editor.app.host.step_authors.contains(doc.id)) return plain; + const t = editor.stepToTake(doc, action, editor.app.host.arena()) catch return plain; + if (t.by.kind == .person) return plain; + return std.fmt.bufPrintZ(buf, "{s} {s}'s {s}", .{ plain, Editor.authorName(t.by), t.step.title }) catch plain; +} + fn canUndo(editor: *Editor) bool { return editor.activeDocCommandEnabled("undo"); } @@ -193,8 +214,8 @@ const edit_items = [_]Item{ .native_always_enabled = true, } }, .separator, - .{ .command = .{ .id = "fizzy.undo", .title = .{ .static = "Undo" }, .sf_symbol = "arrow.uturn.backward", .enabled = canUndo } }, - .{ .command = .{ .id = "fizzy.redo", .title = .{ .static = "Redo" }, .sf_symbol = "arrow.uturn.forward", .enabled = canRedo } }, + .{ .command = .{ .id = "fizzy.undo", .title = .{ .dynamic = undoTitle }, .sf_symbol = "arrow.uturn.backward", .enabled = canUndo } }, + .{ .command = .{ .id = "fizzy.redo", .title = .{ .dynamic = redoTitle }, .sf_symbol = "arrow.uturn.forward", .enabled = canRedo } }, // Transform / Grid Layout are pixel-art concepts, not fizzy's own — pixi parents its own // section here rather than fizzy knowing about them. .{ .plugin_section = "fizzy.menu.edit" }, diff --git a/tests/integration.zig b/tests/integration.zig index 7e29cdd9a..a999f5dc6 100644 --- a/tests/integration.zig +++ b/tests/integration.zig @@ -7454,6 +7454,32 @@ test "headless: the whole editor comes up, opens a file, plays the tour and goes try std.testing.expectEqualStrings(before, (try read(host, notes_args)).text); } + // The person's Cmd-Z taking a step someone else made: the menu names it, and the pane says + // so with a Redo that puts it back. + { + const front = editor.activeDoc() orelse return error.TestUnexpectedResult; + const before = (try read(host, "")).text; + const front_write = try std.fmt.allocPrint(gpa, ".{{ .path = \"{s}\", .text = \"// by Claude\\n\" }}", .{main_zig}); + defer gpa.free(front_write); + const outer = host.act(.{ .kind = .agent, .plugin = "test", .session = 1, .label = "Claude" }); + try std.testing.expect(host.callCommand("text.write", front_write, host.arena()) == .ok); + host.endAct(outer); + const taken = try editor.stepToTake(front, .undo, host.arena()); + try std.testing.expectEqualStrings("Claude", fizzy.Editor.authorName(taken.by)); + + try editor.undo(); + try std.testing.expectEqualStrings(before, (try read(host, "")).text); + const ds = editor.doc_surfaces.get(front.id) orelse return error.TestUnexpectedResult; + const said = ds.feed.items[ds.feed.len - 1]; + try std.testing.expect(std.mem.startsWith(u8, said.title[0..said.len], "Undid Claude's ")); + const redo = said.action orelse return error.TestUnexpectedResult; + try std.testing.expectEqual(taken.step.id, redo.tag); + editor.redoFromFeed(front, redo.tag); + try std.testing.expectEqualStrings("// by Claude\n", (try read(host, "")).text); + try editor.undo(); + try std.testing.expectEqualStrings(before, (try read(host, "")).text); + } + // Save and Close, the unsaved-changes dialog's: the same write Save makes, then the tab // closes once it lands. try std.testing.expect(doc.owner.isDirty(doc));