From 4f001149c71c1104c01355bae2d4fb8c3e9ad6b4 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Tue, 18 Aug 2026 16:48:33 +0200 Subject: [PATCH 01/10] fix(header): stop hover controls from shoving the diff chip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The compact project header pins the base-compare chip (branch +N -M) to the right edge of the git status area, with the hide/focus buttons in the cluster to its right. Revealing those buttons on hover widened the cluster, so the chip slid left every time the pointer entered the corner. Hand the buttons to the git status row instead. They render just left of the chip, where the flex spacer absorbs their width and the chip stays put. The comfortable two-row layout keeps them in the header row — there the chip already sits on a row of its own. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NsgnZNRhcTnZdEZj8Tm2hp --- .../src/views/panels/project_column.rs | 201 ++++++++++-------- .../src/git_header/status_pill.rs | 6 + 2 files changed, 113 insertions(+), 94 deletions(-) diff --git a/crates/okena-app/src/views/panels/project_column.rs b/crates/okena-app/src/views/panels/project_column.rs index 8f7fd0e5d..038360f5a 100644 --- a/crates/okena-app/src/views/panels/project_column.rs +++ b/crates/okena-app/src/views/panels/project_column.rs @@ -677,98 +677,119 @@ impl ProjectColumn { // crashes GPUI (prepaint and paint can see different hover states). let show_reveal = is_focused_view || self.header_hovered; + let has_git = git_status + .as_ref() + .and_then(|g| g.branch.as_ref()) + .is_some(); + + let reveal_controls: Option = show_reveal.then(|| { + h_flex() + .gap(px(2.0)) + .child( + div() + .id("hide-project-btn") + .cursor_pointer() + // Uniform horizontal padding (not a fixed width) + // so every header button sits 5px from its + // neighbours regardless of glyph size — a small + // dot no longer floats in a wide box. + .px(px(5.0)) + .h(px(24.0)) + .flex() + .items_center() + .justify_center() + .rounded(px(4.0)) + .hover(|s| s.bg(rgb(t.bg_hover))) + .on_mouse_down(MouseButton::Left, |_, _, cx| { + cx.stop_propagation(); + }) + .on_click(move |_, _window, cx| { + cx.stop_propagation(); + focus_manager_for_hide.update(cx, |fm, cx| { + workspace_for_hide.update(cx, |ws, cx| { + ws.toggle_project_overview_visibility( + fm, + window_id_for_hide, + &project_id_for_hide, + cx, + ); + }); + }); + }) + .child( + svg() + .path(vis_icon) + .size(px(14.0)) + .text_color(rgb(t.text_secondary)), + ) + .tooltip(move |_window, cx| Tooltip::new(vis_tooltip).build(_window, cx)), + ) + .child( + div() + .id("fullscreen-project-btn") + .cursor_pointer() + .px(px(5.0)) + .h(px(24.0)) + .flex() + .items_center() + .justify_center() + .rounded(px(4.0)) + .hover(|s| s.bg(rgb(t.bg_hover))) + .on_mouse_down(MouseButton::Left, |_, _, cx| { + cx.stop_propagation(); + }) + .on_click(move |_, _window, cx| { + cx.stop_propagation(); + let pid = project_id.clone(); + focus_manager.update(cx, |fm, cx| { + workspace.update(cx, |ws, cx| { + // Toggle: when already focused, clear + // focus to return to the overview. + let target = if is_focused_view { None } else { Some(pid) }; + ws.set_focused_project(fm, target, cx); + }); + cx.notify(); + }); + }) + .child( + svg() + .path(focus_icon) + .size(px(14.0)) + .text_color(rgb(t.text_secondary)), + ) + .tooltip(move |_window, cx| Tooltip::new(focus_tooltip).build(_window, cx)), + ) + .into_any_element() + }); + + // In the compact row the base-compare chip is pinned to the right edge + // of the git status area, so growing the button cluster on its right + // shoved the chip sideways on every hover. Hand the buttons to the git + // status row instead: they land left of the chip and the flex spacer + // absorbs their width. The comfortable layout keeps them in the header + // row — there the chip lives on a row of its own. + let (inline_reveal, header_reveal) = if !is_comfortable && has_git { + (reveal_controls, None) + } else { + (None, reveal_controls) + }; + + let git_status_el = self.git_header.update(cx, |gh, cx| { + gh.render_git_status(git_status.clone(), inline_reveal, &t, cx) + }); + let right_controls = h_flex() .gap(px(8.0)) .child(self.render_hidden_taskbar(project, t, cx)) - // All four action buttons share one cluster with a single, uniform - // gap. Absent buttons (the hover-revealed hide/fullscreen while the - // header isn't hovered, or an empty hook/service indicator) leave - // the flex layout entirely, so a gap only ever appears between - // buttons that are actually visible. + // The header buttons share one cluster with a single, uniform gap. + // Absent buttons (an empty hook/service indicator, or the + // hover-revealed pair once it has been handed to the git status + // row) leave the flex layout entirely, so a gap only ever appears + // between buttons that are actually visible. .child( h_flex() .gap(px(2.0)) - .when(show_reveal, |d| { - d.child( - div() - .id("hide-project-btn") - .cursor_pointer() - // Uniform horizontal padding (not a fixed width) - // so every header button sits 5px from its - // neighbours regardless of glyph size — a small - // dot no longer floats in a wide box. - .px(px(5.0)) - .h(px(24.0)) - .flex() - .items_center() - .justify_center() - .rounded(px(4.0)) - .hover(|s| s.bg(rgb(t.bg_hover))) - .on_mouse_down(MouseButton::Left, |_, _, cx| { - cx.stop_propagation(); - }) - .on_click(move |_, _window, cx| { - cx.stop_propagation(); - focus_manager_for_hide.update(cx, |fm, cx| { - workspace_for_hide.update(cx, |ws, cx| { - ws.toggle_project_overview_visibility( - fm, - window_id_for_hide, - &project_id_for_hide, - cx, - ); - }); - }); - }) - .child( - svg() - .path(vis_icon) - .size(px(14.0)) - .text_color(rgb(t.text_secondary)), - ) - .tooltip(move |_window, cx| { - Tooltip::new(vis_tooltip).build(_window, cx) - }), - ) - .child( - div() - .id("fullscreen-project-btn") - .cursor_pointer() - .px(px(5.0)) - .h(px(24.0)) - .flex() - .items_center() - .justify_center() - .rounded(px(4.0)) - .hover(|s| s.bg(rgb(t.bg_hover))) - .on_mouse_down(MouseButton::Left, |_, _, cx| { - cx.stop_propagation(); - }) - .on_click(move |_, _window, cx| { - cx.stop_propagation(); - let pid = project_id.clone(); - focus_manager.update(cx, |fm, cx| { - workspace.update(cx, |ws, cx| { - // Toggle: when already focused, clear - // focus to return to the overview. - let target = - if is_focused_view { None } else { Some(pid) }; - ws.set_focused_project(fm, target, cx); - }); - cx.notify(); - }); - }) - .child( - svg() - .path(focus_icon) - .size(px(14.0)) - .text_color(rgb(t.text_secondary)), - ) - .tooltip(move |_window, cx| { - Tooltip::new(focus_tooltip).build(_window, cx) - }), - ) - }) + .when_some(header_reveal, |d, controls| d.child(controls)) .child({ self.hook_panel .update(cx, |hp, cx| hp.render_hook_indicator(&t, cx)) @@ -779,14 +800,6 @@ impl ProjectColumn { }), ); - let git_status_el = self.git_header.update(cx, |gh, cx| { - gh.render_git_status(git_status.clone(), &t, cx) - }); - let has_git = git_status - .as_ref() - .and_then(|g| g.branch.as_ref()) - .is_some(); - let context_menu_handler = { let request_broker = self.request_broker.clone(); let project_id = self.project_id.clone(); diff --git a/crates/okena-views-git/src/git_header/status_pill.rs b/crates/okena-views-git/src/git_header/status_pill.rs index 037a0585f..9acf8fcb3 100644 --- a/crates/okena-views-git/src/git_header/status_pill.rs +++ b/crates/okena-views-git/src/git_header/status_pill.rs @@ -22,9 +22,14 @@ impl GitHeader { /// /// `current_branch` is the branch name from the git status watcher /// (passed in because the watcher lives in the main app). + /// + /// `inline_controls` is dropped in just left of the right-aligned + /// base-compare chip. The header's hover-revealed buttons go there so + /// revealing them eats the flex spacer instead of shoving the chip left. pub fn render_git_status( &self, status: Option, + inline_controls: Option, t: &ThemeColors, cx: &mut Context, ) -> AnyElement { @@ -233,6 +238,7 @@ impl GitHeader { // Flexible spacer: pushes the base-compare chip to the right // edge of the header row. .child(div().flex_1().min_w(px(8.0))) + .when_some(inline_controls, |d, controls| d.child(controls)) // Branch-vs-base comparison, shown only when a base exists, // as a labeled `⎇ main +N −M` chip. Doubles as the "review // changes" affordance: clicking opens a three-dot diff of From 1690d127aecc7efb08e96dac26f7194bda0667cf Mon Sep 17 00:00:00 2001 From: David Matejka Date: Tue, 18 Aug 2026 17:17:16 +0200 Subject: [PATCH 02/10] feat(terminal): let the user mark a pane unread MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bell indicator so far only came from the shell ringing BEL. Add a manual toggle so a pane can be flagged to come back to: "Mark as Unread" in the terminal context menu (flips to "Mark as Read" when a bell is already lit), and a ToggleUnread action on cmd-u / ctrl-shift-u. The render path clears the bell on every frame the pane is focused, which would undo the mark instantly. A `manual_unread` flag holds it: set by hand, skipped by the clear, released when focus leaves — so the mark survives while you are looking at the pane, and the next visit clears it like any other bell. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NsgnZNRhcTnZdEZj8Tm2hp --- crates/okena-app/src/keybindings/config.rs | 7 ++ .../okena-app/src/keybindings/descriptions.rs | 13 ++- crates/okena-app/src/keybindings/mod.rs | 3 +- crates/okena-app/src/views/overlay_manager.rs | 93 +++++++++++++++---- crates/okena-app/src/views/window/handlers.rs | 13 +++ crates/okena-terminal/src/terminal/meta.rs | 36 +++++++ crates/okena-terminal/src/terminal/mod.rs | 7 ++ .../okena-terminal/src/terminal/tests/mod.rs | 1 + .../src/terminal/tests/unread.rs | 73 +++++++++++++++ crates/okena-views-terminal/src/actions.rs | 1 + .../src/layout/terminal_pane/actions.rs | 9 ++ .../src/layout/terminal_pane/render.rs | 13 ++- .../src/overlays/terminal_context_menu.rs | 31 +++++++ 13 files changed, 275 insertions(+), 25 deletions(-) create mode 100644 crates/okena-terminal/src/terminal/tests/unread.rs diff --git a/crates/okena-app/src/keybindings/config.rs b/crates/okena-app/src/keybindings/config.rs index 9f393f854..fdc8da65d 100644 --- a/crates/okena-app/src/keybindings/config.rs +++ b/crates/okena-app/src/keybindings/config.rs @@ -198,6 +198,13 @@ impl KeybindingConfig { KeybindingEntry::new("ctrl-shift-m", Some("TerminalPane")), ], ); + bindings.insert( + "ToggleUnread".to_string(), + vec![ + KeybindingEntry::new("cmd-u", Some("TerminalPane")), + KeybindingEntry::new("ctrl-shift-u", Some("TerminalPane")), + ], + ); bindings.insert( "Copy".to_string(), vec![ diff --git a/crates/okena-app/src/keybindings/descriptions.rs b/crates/okena-app/src/keybindings/descriptions.rs index 23cae9f5f..61cbc964f 100644 --- a/crates/okena-app/src/keybindings/descriptions.rs +++ b/crates/okena-app/src/keybindings/descriptions.rs @@ -12,8 +12,8 @@ use super::{ ShowFileSearch, ShowHookLog, ShowKeybindings, ShowLogConsole, ShowProfileManager, ShowProjectSwitcher, ShowSessionManager, ShowSettings, ShowThemeSelector, SplitHorizontal, SplitVertical, StartAllServices, StopAllServices, ToggleFullscreen, TogglePaneSwitcher, - ToggleProjectLayout, ToggleProjectVisibility, ToggleSidebar, ToggleSidebarAutoHide, ZoomIn, - ZoomOut, + ToggleProjectLayout, ToggleProjectVisibility, ToggleSidebar, ToggleSidebarAutoHide, + ToggleUnread, ZoomIn, ZoomOut, }; /// Get human-readable descriptions for all actions @@ -160,6 +160,15 @@ pub fn get_action_descriptions() -> HashMap<&'static str, ActionDescription> { factory: || Box::new(MinimizeTerminal), }, ); + map.insert( + "ToggleUnread", + ActionDescription { + name: "Mark as Unread", + description: "Raise (or clear) the terminal's unread bell mark", + category: "Terminal", + factory: || Box::new(ToggleUnread), + }, + ); map.insert( "Copy", ActionDescription { diff --git a/crates/okena-app/src/keybindings/mod.rs b/crates/okena-app/src/keybindings/mod.rs index 1b6614ada..b5829ceb6 100644 --- a/crates/okena-app/src/keybindings/mod.rs +++ b/crates/okena-app/src/keybindings/mod.rs @@ -64,7 +64,7 @@ pub use okena_views_terminal::actions::{ FocusPrevTerminal, FocusRight, FocusUp, FullscreenNextTerminal, FullscreenPrevTerminal, JumpToNextFailedCommand, JumpToNextPrompt, JumpToPreviousFailedCommand, JumpToPreviousPrompt, MinimizeTerminal, Paste, ResetZoom, Search, SearchNext, SearchPrev, SendBacktab, SendEscape, - SendTab, SplitHorizontal, SplitVertical, ToggleFullscreen, ZoomIn, ZoomOut, + SendTab, SplitHorizontal, SplitVertical, ToggleFullscreen, ToggleUnread, ZoomIn, ZoomOut, }; // Sidebar-specific actions (defined in okena-views-sidebar crate) @@ -388,6 +388,7 @@ fn create_keybinding(action: &str, keystroke: &str, context: Option<&str>) -> Op "AddTab" => Some(KeyBinding::new(keystroke, AddTab, context)), "CloseTerminal" => Some(KeyBinding::new(keystroke, CloseTerminal, context)), "MinimizeTerminal" => Some(KeyBinding::new(keystroke, MinimizeTerminal, context)), + "ToggleUnread" => Some(KeyBinding::new(keystroke, ToggleUnread, context)), "FocusNextTerminal" => Some(KeyBinding::new(keystroke, FocusNextTerminal, context)), "FocusPrevTerminal" => Some(KeyBinding::new(keystroke, FocusPrevTerminal, context)), "FocusLeft" => Some(KeyBinding::new(keystroke, FocusLeft, context)), diff --git a/crates/okena-app/src/views/overlay_manager.rs b/crates/okena-app/src/views/overlay_manager.rs index edc348e38..a2b6af1d6 100644 --- a/crates/okena-app/src/views/overlay_manager.rs +++ b/crates/okena-app/src/views/overlay_manager.rs @@ -131,10 +131,14 @@ pub enum OverlayManagerEvent { }, /// Context menu: Add terminal to project - AddTerminal { project_id: String }, + AddTerminal { + project_id: String, + }, /// Context menu: Create worktree from project - CreateWorktree { project_id: String }, + CreateWorktree { + project_id: String, + }, /// Context menu: Rename project RenameProject { @@ -157,7 +161,9 @@ pub enum OverlayManagerEvent { }, /// Context menu: Close worktree project (opens the confirm dialog) - CloseWorktree { project_id: String }, + CloseWorktree { + project_id: String, + }, /// Context menu: Open the daemon-backed worktree list. ManageWorktrees { @@ -174,19 +180,29 @@ pub enum OverlayManagerEvent { }, /// Context menu: Delete project - DeleteProject { project_id: String }, + DeleteProject { + project_id: String, + }, /// Context menu: Toggle a project's pinned flag - ToggleProjectPinned { project_id: String }, + ToggleProjectPinned { + project_id: String, + }, /// Folder context menu: Delete folder - DeleteFolder { folder_id: String }, + DeleteFolder { + folder_id: String, + }, /// Context menu: Configure hooks for a project - ConfigureHooks { project_id: String }, + ConfigureHooks { + project_id: String, + }, /// Context menu: Quick create worktree (one-click) - QuickCreateWorktree { project_id: String }, + QuickCreateWorktree { + project_id: String, + }, /// Color picker: project color was changed (for remote sync) ProjectColorChanged { @@ -195,7 +211,9 @@ pub enum OverlayManagerEvent { }, /// Color picker: a worktree project's color override was reset to its parent - WorktreeColorReset { project_id: String }, + WorktreeColorReset { + project_id: String, + }, /// Color picker: folder color was changed FolderColorChanged { @@ -204,10 +222,14 @@ pub enum OverlayManagerEvent { }, /// Context menu: Reload services (okena.yaml) for a project - ReloadServices { project_id: String }, + ReloadServices { + project_id: String, + }, /// Context menu: Focus parent project of a worktree - FocusParent { project_id: String }, + FocusParent { + project_id: String, + }, /// Project switcher: Focus a specific project FocusProject(String), @@ -220,10 +242,14 @@ pub enum OverlayManagerEvent { ToggleProjectVisibility(String), /// Remote connect dialog: connection paired and ready - RemoteConnected { config: RemoteConnectionConfig }, + RemoteConnected { + config: RemoteConnectionConfig, + }, /// Remote context menu: reconnect to a connection - RemoteReconnect { connection_id: String }, + RemoteReconnect { + connection_id: String, + }, /// Remote context menu: open pair dialog RemotePair { @@ -238,13 +264,20 @@ pub enum OverlayManagerEvent { }, /// Remote pair dialog: user submitted a code - RemotePaired { connection_id: String, code: String }, + RemotePaired { + connection_id: String, + code: String, + }, /// Remote context menu: remove a connection - RemoteRemoveConnection { connection_id: String }, + RemoteRemoveConnection { + connection_id: String, + }, /// Terminal context menu: copy - TerminalCopy { terminal_id: String }, + TerminalCopy { + terminal_id: String, + }, /// Terminal context menu: annotate the selection and send it back. /// The host owns the terminals, so only it can snapshot the selected text. TerminalAnnotate { @@ -252,11 +285,20 @@ pub enum OverlayManagerEvent { position: gpui::Point, }, /// Terminal context menu: paste - TerminalPaste { terminal_id: String }, + TerminalPaste { + terminal_id: String, + }, /// Terminal context menu: clear - TerminalClear { terminal_id: String }, + TerminalClear { + terminal_id: String, + }, + TerminalToggleUnread { + terminal_id: String, + }, /// Terminal context menu: select all - TerminalSelectAll { terminal_id: String }, + TerminalSelectAll { + terminal_id: String, + }, /// Terminal context menu: split TerminalSplit { project_id: String, @@ -289,7 +331,10 @@ pub enum OverlayManagerEvent { }, /// File viewer blame click: open the named commit in the diff viewer. - OpenCommitFromBlame { project_id: String, hash: String }, + OpenCommitFromBlame { + project_id: String, + hash: String, + }, OpenFileExternally { path: String, @@ -1258,6 +1303,7 @@ impl OverlayManager { layout_path: Vec, position: gpui::Point, has_selection: bool, + has_bell: bool, link_url: Option, cx: &mut Context, ) { @@ -1271,6 +1317,7 @@ impl OverlayManager { layout_path, position, has_selection, + has_bell, link_url, cx, ) @@ -1316,6 +1363,12 @@ impl OverlayManager { terminal_id: terminal_id.clone(), }); } + TerminalContextMenuEvent::ToggleUnread { terminal_id } => { + this.hide_terminal_context_menu(cx); + cx.emit(OverlayManagerEvent::TerminalToggleUnread { + terminal_id: terminal_id.clone(), + }); + } TerminalContextMenuEvent::Split { project_id, layout_path, diff --git a/crates/okena-app/src/views/window/handlers.rs b/crates/okena-app/src/views/window/handlers.rs index 776af43b9..983857e5a 100644 --- a/crates/okena-app/src/views/window/handlers.rs +++ b/crates/okena-app/src/views/window/handlers.rs @@ -804,6 +804,13 @@ impl WindowView { terminal.clear(); } } + OverlayManagerEvent::TerminalToggleUnread { terminal_id } => { + let terminals = self.terminals.lock(); + if let Some(terminal) = terminals.get(terminal_id) { + terminal.toggle_unread(); + } + cx.notify(); + } OverlayManagerEvent::TerminalSelectAll { terminal_id } => { let terminals = self.terminals.lock(); if let Some(terminal) = terminals.get(terminal_id) { @@ -1200,6 +1207,11 @@ impl WindowView { has_selection, link_url, } => { + let has_bell = self + .terminals + .lock() + .get(&terminal_id) + .is_some_and(|t| t.has_bell()); self.overlay_manager.update(cx, |om, cx| { om.show_terminal_context_menu( terminal_id, @@ -1207,6 +1219,7 @@ impl WindowView { layout_path, position, has_selection, + has_bell, link_url, cx, ); diff --git a/crates/okena-terminal/src/terminal/meta.rs b/crates/okena-terminal/src/terminal/meta.rs index 0a9383d0a..135c66a28 100644 --- a/crates/okena-terminal/src/terminal/meta.rs +++ b/crates/okena-terminal/src/terminal/meta.rs @@ -88,6 +88,42 @@ impl Terminal { /// Clear the bell notification flag (call when terminal receives focus) pub fn clear_bell(&self) { *self.has_bell.lock() = false; + self.manual_unread + .store(false, std::sync::atomic::Ordering::Relaxed); + } + + /// Raise the bell by hand — "mark as unread". Unlike a BEL from the shell + /// this survives the render path's clear-on-focus (see `manual_unread`), + /// so it sticks on the pane the user is currently looking at. + pub fn mark_unread(&self) { + *self.has_bell.lock() = true; + self.manual_unread + .store(true, std::sync::atomic::Ordering::Relaxed); + } + + /// Toggle the manual unread mark. Returns the new state. Any bell counts as + /// read here, so this also dismisses one the shell rang. + pub fn toggle_unread(&self) -> bool { + if self.has_bell() { + self.clear_bell(); + false + } else { + self.mark_unread(); + true + } + } + + /// Whether the bell is currently held by a manual unread mark. + pub fn is_manually_unread(&self) -> bool { + self.manual_unread + .load(std::sync::atomic::Ordering::Relaxed) + } + + /// Drop the manual hold while keeping the bell lit (call when the pane + /// loses focus). The mark has done its job — the next focus clears it. + pub fn release_manual_unread(&self) { + self.manual_unread + .store(false, std::sync::atomic::Ordering::Relaxed); } /// Consume the one-shot "bell rang since last drain" edge. Returns true if diff --git a/crates/okena-terminal/src/terminal/mod.rs b/crates/okena-terminal/src/terminal/mod.rs index e0e588ddc..f12a45e02 100644 --- a/crates/okena-terminal/src/terminal/mod.rs +++ b/crates/okena-terminal/src/terminal/mod.rs @@ -160,6 +160,12 @@ pub struct Terminal { /// exactly once instead of on every batch while `has_bell` stays set. pub(super) bell_pending: Arc, + /// "The user marked this pane unread by hand" flag. Holds `has_bell` + /// against the render path's clear-on-focus, so marking the pane you are + /// looking at actually sticks; released when focus leaves, so the next + /// visit clears the bell like any other. GPUI thread only. + pub(super) manual_unread: AtomicBool, + /// Sticky "this pane raised a desktop notification" flag, mirroring /// `has_bell` but for OSC 9/777 alerts. Set by the app when it actually /// fires a notification (so it already honors the user's settings and the @@ -419,6 +425,7 @@ impl Terminal { title, has_bell, bell_pending, + manual_unread: AtomicBool::new(false), has_notification: AtomicBool::new(false), pending_clipboard, pending_clipboard_reads, diff --git a/crates/okena-terminal/src/terminal/tests/mod.rs b/crates/okena-terminal/src/terminal/tests/mod.rs index 746729634..ab6d753c3 100644 --- a/crates/okena-terminal/src/terminal/tests/mod.rs +++ b/crates/okena-terminal/src/terminal/tests/mod.rs @@ -6,6 +6,7 @@ mod osc; mod prompt_jump; mod resize_authority; mod snapshot_watermark; +mod unread; mod url_detect; mod xterm_color; diff --git a/crates/okena-terminal/src/terminal/tests/unread.rs b/crates/okena-terminal/src/terminal/tests/unread.rs new file mode 100644 index 000000000..73c908be1 --- /dev/null +++ b/crates/okena-terminal/src/terminal/tests/unread.rs @@ -0,0 +1,73 @@ +//! Manual "mark as unread" — the bell raised by hand rather than by BEL. + +use super::super::Terminal; +use super::super::types::TerminalSize; +use super::NullTransport; +use std::sync::Arc; + +fn terminal() -> Terminal { + Terminal::new( + "t".to_string(), + TerminalSize::default(), + Arc::new(NullTransport), + "/tmp".to_string(), + ) +} + +#[test] +fn marking_unread_lights_the_bell_and_holds_it() { + let terminal = terminal(); + + assert!(!terminal.has_bell()); + terminal.mark_unread(); + + assert!(terminal.has_bell(), "the mark lights the bell indicator"); + assert!( + terminal.is_manually_unread(), + "the hold is what survives the render path's clear-on-focus" + ); +} + +#[test] +fn a_bell_from_the_shell_is_not_held() { + let terminal = terminal(); + + terminal.process_output(b"\x07"); + + assert!(terminal.has_bell()); + assert!( + !terminal.is_manually_unread(), + "a BEL still clears as soon as the pane is focused" + ); +} + +#[test] +fn releasing_the_hold_keeps_the_bell_lit() { + let terminal = terminal(); + terminal.mark_unread(); + + terminal.release_manual_unread(); + + assert!(terminal.has_bell(), "focus leaving does not read the pane"); + assert!( + !terminal.is_manually_unread(), + "the next visit clears the bell like any other" + ); +} + +#[test] +fn toggle_flips_both_ways_and_clears_a_shell_bell() { + let terminal = terminal(); + + assert!(terminal.toggle_unread(), "off -> unread"); + assert!(terminal.has_bell()); + + assert!(!terminal.toggle_unread(), "unread -> read"); + assert!(!terminal.has_bell()); + assert!(!terminal.is_manually_unread()); + + // A bell the shell rang reads as unread too, so the toggle dismisses it. + terminal.process_output(b"\x07"); + assert!(!terminal.toggle_unread(), "shell bell -> read"); + assert!(!terminal.has_bell()); +} diff --git a/crates/okena-views-terminal/src/actions.rs b/crates/okena-views-terminal/src/actions.rs index 321f6d4f9..b6b9375a5 100644 --- a/crates/okena-views-terminal/src/actions.rs +++ b/crates/okena-views-terminal/src/actions.rs @@ -38,5 +38,6 @@ gpui::actions!( JumpToPreviousFailedCommand, JumpToNextFailedCommand, AnnotateSelection, + ToggleUnread, ] ); diff --git a/crates/okena-views-terminal/src/layout/terminal_pane/actions.rs b/crates/okena-views-terminal/src/layout/terminal_pane/actions.rs index 364988e72..a2bcda6b4 100644 --- a/crates/okena-views-terminal/src/layout/terminal_pane/actions.rs +++ b/crates/okena-views-terminal/src/layout/terminal_pane/actions.rs @@ -46,6 +46,15 @@ impl TerminalPane { } } + /// Toggle the pane's unread mark — the bell indicator the shell raises on + /// BEL, set by hand so a pane can be flagged to come back to. + pub(super) fn handle_toggle_unread(&mut self, cx: &mut Context) { + if let Some(ref terminal) = self.terminal { + terminal.toggle_unread(); + cx.notify(); + } + } + pub(super) fn handle_fullscreen(&mut self, cx: &mut Context) { if let Some(ref id) = self.terminal_id { let action = ActionRequest::SetFullscreen { diff --git a/crates/okena-views-terminal/src/layout/terminal_pane/render.rs b/crates/okena-views-terminal/src/layout/terminal_pane/render.rs index ea5f2d834..ec89614e9 100644 --- a/crates/okena-views-terminal/src/layout/terminal_pane/render.rs +++ b/crates/okena-views-terminal/src/layout/terminal_pane/render.rs @@ -6,8 +6,8 @@ use crate::actions::{ FocusNextTerminal, FocusPrevTerminal, FocusRight, FocusUp, FullscreenNextTerminal, FullscreenPrevTerminal, JumpToNextFailedCommand, JumpToNextPrompt, JumpToPreviousFailedCommand, JumpToPreviousPrompt, MinimizeTerminal, Paste, ResetZoom, Search, SearchNext, SearchPrev, - SendBacktab, SendEscape, SendTab, SplitHorizontal, SplitVertical, ToggleFullscreen, ZoomIn, - ZoomOut, + SendBacktab, SendEscape, SendTab, SplitHorizontal, SplitVertical, ToggleFullscreen, + ToggleUnread, ZoomIn, ZoomOut, }; use crate::layout::navigation::NavigationDirection; use crate::terminal_view_settings; @@ -66,9 +66,12 @@ impl Render for TerminalPane { let is_focused = window.is_window_active() && focus_handle.is_focused(window); let has_bell = self.terminal.as_ref().is_some_and(|t| t.has_bell()); + // A hand-set "unread" mark holds the bell against this clear, or + // marking the focused pane would be undone on the very next frame. if is_focused && has_bell && let Some(ref terminal) = self.terminal + && !terminal.is_manually_unread() { terminal.clear_bell(); } @@ -93,6 +96,9 @@ impl Render for TerminalPane { && let Some(ref terminal) = self.terminal { terminal.mark_as_viewed(); + // Focus has left, so the mark no longer needs holding: the bell + // stays lit and the next visit clears it like any other. + terminal.release_manual_unread(); } self.was_focused = is_focused; @@ -154,6 +160,9 @@ impl Render for TerminalPane { .on_action(cx.listener(|this, _: &MinimizeTerminal, _window, cx| { this.handle_minimize(cx); })) + .on_action(cx.listener(|this, _: &ToggleUnread, _window, cx| { + this.handle_toggle_unread(cx); + })) .on_action(cx.listener(|this, _: &Copy, _window, cx| { this.handle_copy(cx); })) diff --git a/crates/okena-views-terminal/src/overlays/terminal_context_menu.rs b/crates/okena-views-terminal/src/overlays/terminal_context_menu.rs index 62dc0373d..d51d3721e 100644 --- a/crates/okena-views-terminal/src/overlays/terminal_context_menu.rs +++ b/crates/okena-views-terminal/src/overlays/terminal_context_menu.rs @@ -29,6 +29,10 @@ pub enum TerminalContextMenuEvent { SelectAll { terminal_id: String, }, + /// Flip the pane's unread (bell) mark. + ToggleUnread { + terminal_id: String, + }, Split { project_id: String, layout_path: Vec, @@ -53,18 +57,24 @@ pub struct TerminalContextMenu { layout_path: Vec, position: Point, has_selection: bool, + /// Whether the pane currently carries a bell/unread mark — flips the + /// menu's mark-unread entry into a mark-read one. + has_bell: bool, /// URL at the right-click position (if any). link_url: Option, focus_handle: FocusHandle, } impl TerminalContextMenu { + // Context-menu setup: params are position/state inputs, not a group. + #[allow(clippy::too_many_arguments)] pub fn new( terminal_id: String, project_id: String, layout_path: Vec, position: Point, has_selection: bool, + has_bell: bool, link_url: Option, cx: &mut Context, ) -> Self { @@ -75,6 +85,7 @@ impl TerminalContextMenu { layout_path, position, has_selection, + has_bell, link_url, focus_handle, } @@ -217,6 +228,26 @@ impl Render for TerminalContextMenu { }); })), ) + // Mark as Unread / Read — the bell indicator, by hand + .child( + menu_item( + "ctx-toggle-unread", + "icons/bell.svg", + if self.has_bell { + "Mark as Read" + } else { + "Mark as Unread" + }, + &t, + ) + .on_click(cx.listener( + |this, _, _window, cx| { + cx.emit(TerminalContextMenuEvent::ToggleUnread { + terminal_id: this.terminal_id.clone(), + }); + }, + )), + ) .child(menu_separator(&t)) // Split Horizontal .child( From bb687885d9053fb760c8ab10fe5f29a9b06755d1 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Tue, 18 Aug 2026 17:29:52 +0200 Subject: [PATCH 03/10] fix(terminal): drop the slack from the wrapped-URL width guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 2 extends a URL onto the next row when a TUI wraps the line itself. 6c750fc6 added the "a continuation can never be wider than the row it continues" guard, but gave it the same +3 slack the surrounding guards had at the time. 149e4c3d removed that slack from the others — the layout edge is exact, so slack only lets prose in — and missed this one. Claude Code's PR line lands exactly on the boundary: the URL row ends at column 58, the next row is 61 wide, and 61 > 58 + 3 is false. ● Hotovo — https://github.com/contember/webmaster/pull/567 (feat/browser-and-edge-worker-sentry) → main, samostatně od Compare against the edge itself. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01RusYtFivQgiGheLoKVcTaK --- crates/okena-terminal/src/terminal/links.rs | 6 +++-- .../src/terminal/tests/url_detect.rs | 24 +++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/crates/okena-terminal/src/terminal/links.rs b/crates/okena-terminal/src/terminal/links.rs index bd8fd8fe5..512becfa1 100644 --- a/crates/okena-terminal/src/terminal/links.rs +++ b/crates/okena-terminal/src/terminal/links.rs @@ -328,8 +328,10 @@ impl Terminal { // A mid-token wrap fills the row to the layout edge, so a // continuation can never be wider than the row it // continues. A wider next row means the break was a word - // break — the URL ended on its own line. - if next_rtrimmed.chars().count() > url_end_col + 3 { + // break — the URL ended on its own line. No slack: the + // edge is exact, and slack is what let a 3-column-longer + // continuation through. + if next_rtrimmed.chars().count() > url_end_col { break; } diff --git a/crates/okena-terminal/src/terminal/tests/url_detect.rs b/crates/okena-terminal/src/terminal/tests/url_detect.rs index f647d4969..c28e387d8 100644 --- a/crates/okena-terminal/src/terminal/tests/url_detect.rs +++ b/crates/okena-terminal/src/terminal/tests/url_detect.rs @@ -439,3 +439,27 @@ fn detect_url_not_extended_across_a_trailing_dash() { "https://github.com/contember/webmaster/pull/564" ); } + +#[test] +fn detect_url_not_extended_by_a_barely_longer_next_line() { + // The URL ends its row at column 58 and the next row is 61 wide, so the + // URL never reached the layout edge — no wrap. The old +3 slack put this + // exactly on the boundary and absorbed the branch name. Reproduces + // Claude Code's PR line: "● Hotovo — https://…/pull/567" / + // "(feat/browser-and-edge-worker-sentry) → main, samostatně od". + let links = detect_urls_in( + "\u{25cf} Hotovo \u{2014} https://github.com/contember/webmaster/pull/567\r\n (feat/browser-and-edge-worker-sentry) \u{2192} main, samostatn\u{11b} od\r\n", + 80, + ); + let url_links: Vec<&DetectedLink> = links.iter().filter(|l| l.is_url).collect(); + assert_eq!( + url_links.len(), + 1, + "Branch name on the next line must not be absorbed: {:?}", + links + ); + assert_eq!( + url_links[0].text, + "https://github.com/contember/webmaster/pull/567" + ); +} From 48741daa487bbab756229bae7219632013a7f126 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Tue, 18 Aug 2026 17:31:34 +0200 Subject: [PATCH 04/10] feat(terminal): show the unread bell where the pane is hidden MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bell only ever reached the pane's attention border and the sidebar, so a terminal in an inactive tab or minimized to the header taskbar carried its mark invisibly — the one case where being told to come back matters most, now that the mark can be set by hand. Tabs and the minimized/detached chips report it with the sidebar's glyph and color: bell.svg in `border_bell`, outranking the hook, waiting and active states. Both read `has_bell || has_notification`, the same pair the pane border shows, since the chip is standing in for that border. Toggling now also refreshes the windows. The mark shows in four places and the sidebar sits behind a `.cached()` wrapper that a notify from the pane never reaches; one keypress is far too rare for the cost to matter. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NsgnZNRhcTnZdEZj8Tm2hp --- .../src/views/panels/project_column.rs | 53 +++++++++++++------ crates/okena-app/src/views/window/handlers.rs | 11 ++-- .../src/layout/tabs/mod.rs | 19 ++++--- .../src/layout/terminal_pane/actions.rs | 4 ++ 4 files changed, 62 insertions(+), 25 deletions(-) diff --git a/crates/okena-app/src/views/panels/project_column.rs b/crates/okena-app/src/views/panels/project_column.rs index 038360f5a..5f56dcab8 100644 --- a/crates/okena-app/src/views/panels/project_column.rs +++ b/crates/okena-app/src/views/panels/project_column.rs @@ -450,13 +450,15 @@ impl ProjectColumn { let workspace = self.workspace.clone(); let project_id = self.project_id.clone(); - let terminal_name = { - let osc_title = self - .terminals - .lock() - .get(&terminal_id) - .and_then(|t| t.title()); - project.terminal_display_name(&terminal_id, osc_title) + // A minimized terminal has no pane to carry the attention + // border, so its chip reports the same two signals. + let (terminal_name, has_bell) = { + let terminals = self.terminals.lock(); + let terminal = terminals.get(&terminal_id); + let osc_title = terminal.and_then(|t| t.title()); + let bell = + terminal.is_some_and(|t| t.has_bell() || t.has_notification()); + (project.terminal_display_name(&terminal_id, osc_title), bell) }; div() @@ -473,9 +475,17 @@ impl ProjectColumn { .text_size(ui_text_sm(cx)) .child( svg() - .path("icons/terminal-minimized.svg") + .path(if has_bell { + "icons/bell.svg" + } else { + "icons/terminal-minimized.svg" + }) .size(px(10.0)) - .text_color(rgb(t.text_muted)), + .text_color(if has_bell { + rgb(t.border_bell) + } else { + rgb(t.text_muted) + }), ) .child(div().text_color(rgb(t.text_primary)).child(terminal_name)) .on_click(move |_, _window, cx| { @@ -493,13 +503,13 @@ impl ProjectColumn { let workspace = self.workspace.clone(); let terminal_id_for_click = terminal_id.clone(); - let terminal_name = { - let osc_title = self - .terminals - .lock() - .get(&terminal_id) - .and_then(|t| t.title()); - project.terminal_display_name(&terminal_id, osc_title) + let (terminal_name, has_bell) = { + let terminals = self.terminals.lock(); + let terminal = terminals.get(&terminal_id); + let osc_title = terminal.and_then(|t| t.title()); + let bell = + terminal.is_some_and(|t| t.has_bell() || t.has_notification()); + (project.terminal_display_name(&terminal_id, osc_title), bell) }; div() @@ -511,8 +521,19 @@ impl ProjectColumn { .border_color(rgb(t.border)) .bg(rgb(t.bg_hover)) .hover(|s| s.bg(rgb(t.bg_selection))) + .flex() + .items_center() + .gap(px(4.0)) .text_size(ui_text_sm(cx)) .text_color(rgb(t.text_primary)) + .when(has_bell, |d| { + d.child( + svg() + .path("icons/bell.svg") + .size(px(10.0)) + .text_color(rgb(t.border_bell)), + ) + }) .child(format!("\u{2197} {}", terminal_name)) .on_click(move |_, _window, cx| { workspace.update(cx, |ws, cx| { diff --git a/crates/okena-app/src/views/window/handlers.rs b/crates/okena-app/src/views/window/handlers.rs index 983857e5a..67edf7677 100644 --- a/crates/okena-app/src/views/window/handlers.rs +++ b/crates/okena-app/src/views/window/handlers.rs @@ -805,11 +805,16 @@ impl WindowView { } } OverlayManagerEvent::TerminalToggleUnread { terminal_id } => { - let terminals = self.terminals.lock(); - if let Some(terminal) = terminals.get(terminal_id) { - terminal.toggle_unread(); + { + let terminals = self.terminals.lock(); + if let Some(terminal) = terminals.get(terminal_id) { + terminal.toggle_unread(); + } } cx.notify(); + // Same reason as the keybinding path: the sidebar row sits + // behind a `.cached()` wrapper a plain notify won't reach. + cx.refresh_windows(); } OverlayManagerEvent::TerminalSelectAll { terminal_id } => { let terminals = self.terminals.lock(); diff --git a/crates/okena-views-terminal/src/layout/tabs/mod.rs b/crates/okena-views-terminal/src/layout/tabs/mod.rs index 2fadd273b..c26c61fd9 100644 --- a/crates/okena-views-terminal/src/layout/tabs/mod.rs +++ b/crates/okena-views-terminal/src/layout/tabs/mod.rs @@ -392,14 +392,17 @@ impl LayoutContainer { _ => None, }; - let (is_waiting, idle_label, progress) = terminal_id.as_ref().map_or((false, None, None), |tid| { + let (is_waiting, idle_label, progress, has_bell) = terminal_id.as_ref().map_or((false, None, None, false), |tid| { let guard = terminals.lock(); - guard.get(tid).map_or((false, None, None), |t| { + guard.get(tid).map_or((false, None, None, false), |t| { let progress = t.progress(); + // An inactive tab hides its pane, so the tab stands in for the + // pane's attention border and reports the same two signals. + let bell = t.has_bell() || t.has_notification(); if t.is_waiting_for_input() { - (true, Some(t.idle_duration_display()), progress) + (true, Some(t.idle_duration_display()), progress, bell) } else { - (false, None, progress) + (false, None, progress, bell) } }) }); @@ -496,12 +499,16 @@ impl LayoutContainer { })) .into_any_element() } else { - let icon_color = if is_hook { rgb(t.term_yellow) } else if is_waiting { rgb(t.border_idle) } else if is_active { rgb(t.success) } else { rgb(t.text_muted) }; + // Bell outranks the rest: it is the one state the user is + // being asked to come back to. Same glyph and color as the + // sidebar's terminal rows. + let icon_color = if has_bell { rgb(t.border_bell) } else if is_hook { rgb(t.term_yellow) } else if is_waiting { rgb(t.border_idle) } else if is_active { rgb(t.success) } else { rgb(t.text_muted) }; + let icon_path = if has_bell { "icons/bell.svg" } else { "icons/terminal.svg" }; h_flex() .gap(px(6.0)) .overflow_hidden() .text_ellipsis() - .child(svg().path("icons/terminal.svg").size(px(12.0)).flex_shrink_0().text_color(icon_color)) + .child(svg().path(icon_path).size(px(12.0)).flex_shrink_0().text_color(icon_color)) .child(tab_label.clone()) .children(idle_label.as_ref().map(|d| { div().text_size(ui_text_sm(cx)).text_color(rgb(t.border_idle)).child(d.clone()) diff --git a/crates/okena-views-terminal/src/layout/terminal_pane/actions.rs b/crates/okena-views-terminal/src/layout/terminal_pane/actions.rs index a2bcda6b4..051ae8e32 100644 --- a/crates/okena-views-terminal/src/layout/terminal_pane/actions.rs +++ b/crates/okena-views-terminal/src/layout/terminal_pane/actions.rs @@ -52,6 +52,10 @@ impl TerminalPane { if let Some(ref terminal) = self.terminal { terminal.toggle_unread(); cx.notify(); + // The mark shows in four places, and the sidebar is a `.cached()` + // sibling this pane's notify never reaches. One keypress is far + // too rare for the cost of bypassing the caches to matter. + cx.refresh_windows(); } } From 403fb237fb170c0d56d6d2f8937e18f76ed1e84d Mon Sep 17 00:00:00 2001 From: David Matejka Date: Tue, 18 Aug 2026 18:12:51 +0200 Subject: [PATCH 05/10] fix(terminal): require a wrapped URL to continue its last token MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The width guards cannot decide the narrow case: word-wrapping at width W produces rows no wider than W, and Claude Code's PR line lands the URL row exactly on W, making it the widest row in the block. ● Hotovo — https://github.com/contember/webmaster/pull/567 (feat/browser-and-edge-worker-sentry) → main, samostatně od #566, nepřekrývají se). A hard wrap breaks a token mid-way — it never starts a new one, so read the break lexically instead: - an opening paren starts a bracketed token, so it is a parenthetical after the URL, not the URL's tail; - a wholly numeric last segment (`/pull/567`, `/issues/42`) can only continue with more digits, or with a delimiter opening the next segment. The numeric rule covers GitHub PR and issue links without knowing about GitHub, and still joins a wrap that falls inside the number (`…/pull/56` + `7`), which an explicit list of complete URL shapes would have had to special-case anyway. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01RusYtFivQgiGheLoKVcTaK --- crates/okena-terminal/src/terminal/links.rs | 24 +++++++ .../src/terminal/tests/url_detect.rs | 63 +++++++++++++++++++ 2 files changed, 87 insertions(+) diff --git a/crates/okena-terminal/src/terminal/links.rs b/crates/okena-terminal/src/terminal/links.rs index 512becfa1..351330a8e 100644 --- a/crates/okena-terminal/src/terminal/links.rs +++ b/crates/okena-terminal/src/terminal/links.rs @@ -354,6 +354,30 @@ impl Terminal { break; } + // ── The continuation must read as a continuation of the + // URL's last token. A hard wrap breaks a token mid-way; + // it never starts a new one. ── + let first = content.chars().next(); + + // An opening paren starts a bracketed token, so this is a + // parenthetical following the URL, not the URL's tail. + if first == Some('(') { + break; + } + + // A wholly numeric last segment (`/pull/567`, `/issues/42`) + // can only continue with more digits, or with a delimiter + // that starts the next segment. + let last_segment = extended_url.rsplit('/').next().unwrap_or(""); + let continues_a_number = + first.is_some_and(|c| c.is_ascii_digit() || matches!(c, '/' | '?' | '#')); + if !last_segment.is_empty() + && last_segment.bytes().all(|b| b.is_ascii_digit()) + && !continues_a_number + { + break; + } + // Take URL-compatible chars as extension. let ext_char_len = content.chars().take_while(|c| url_char(*c)).count(); if ext_char_len == 0 { diff --git a/crates/okena-terminal/src/terminal/tests/url_detect.rs b/crates/okena-terminal/src/terminal/tests/url_detect.rs index c28e387d8..fd655b11d 100644 --- a/crates/okena-terminal/src/terminal/tests/url_detect.rs +++ b/crates/okena-terminal/src/terminal/tests/url_detect.rs @@ -463,3 +463,66 @@ fn detect_url_not_extended_by_a_barely_longer_next_line() { "https://github.com/contember/webmaster/pull/567" ); } + +#[test] +fn detect_url_not_extended_into_a_parenthetical() { + // Same PR line, narrower terminal: the URL row (58) is now the widest row + // in the block, so no width guard can tell it from a mid-token wrap. The + // continuation opens a paren, which starts a new token — and the URL ends + // in a wholly numeric segment, which only digits can continue. + let links = detect_urls_in( + " Pushed to feat/browser-and-edge-worker-sentry, created\r\n PR #567, ran 3 shell commands\r\n\u{25cf} Hotovo \u{2014} https://github.com/contember/webmaster/pull/567\r\n(feat/browser-and-edge-worker-sentry) \u{2192} main, samostatn\u{11b}\r\nod #566, nep\u{159}ekr\u{fd}vaj\u{ed} se).\r\n", + 60, + ); + let url_links: Vec<&DetectedLink> = links.iter().filter(|l| l.is_url).collect(); + assert_eq!( + url_links.len(), + 1, + "Branch name on the next line must not be absorbed: {:?}", + links + ); + assert_eq!( + url_links[0].text, + "https://github.com/contember/webmaster/pull/567" + ); +} + +#[test] +fn detect_url_extended_across_a_split_number() { + // The counterpart of the rule above: a genuine wrap can fall inside the + // PR number, and a digit does continue a numeric segment. + let links = detect_urls_in( + " https://github.com/contember/webmaster/pull/56\r\n 7\r\n", + 48, + ); + let url_links: Vec<&DetectedLink> = links + .iter() + .filter(|l| l.text == "https://github.com/contember/webmaster/pull/567") + .collect(); + assert_eq!( + url_links.len(), + 2, + "A digit must still continue a wrapped PR number: {:?}", + links + ); +} + +#[test] +fn detect_url_extended_across_a_split_path_segment() { + // A non-numeric last segment stays extendable — the numeric rule must not + // leak into ordinary path wraps. + let links = detect_urls_in( + " https://github.com/contember/webmaster/tree/feat/brow\r\n ser-sentry\r\n", + 56, + ); + let url_links: Vec<&DetectedLink> = links + .iter() + .filter(|l| l.text == "https://github.com/contember/webmaster/tree/feat/browser-sentry") + .collect(); + assert_eq!( + url_links.len(), + 2, + "Ordinary path wrap must still join: {:?}", + links + ); +} From 7629a6ae1b27eec3a960b072e6de0484247ff5cd Mon Sep 17 00:00:00 2001 From: David Matejka Date: Wed, 19 Aug 2026 11:19:43 +0200 Subject: [PATCH 06/10] feat(git): put the current branch on top of the branch picker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The picker listed locals in git's ref order, so the branch you are on could sit anywhere in the list. Lead the LOCAL section with it — it is the row users scan for, and it becomes the default keyboard selection. Extracts the filter/order pass out of `recompute_branch_filtered` into a pure `branch_nav_items` so the ordering is unit-testable. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NsgnZNRhcTnZdEZj8Tm2hp --- .../src/git_header/branch_picker.rs | 116 ++++++++++++++---- 1 file changed, 92 insertions(+), 24 deletions(-) diff --git a/crates/okena-views-git/src/git_header/branch_picker.rs b/crates/okena-views-git/src/git_header/branch_picker.rs index 32160f414..a459a4b7c 100644 --- a/crates/okena-views-git/src/git_header/branch_picker.rs +++ b/crates/okena-views-git/src/git_header/branch_picker.rs @@ -83,29 +83,7 @@ impl GitHeader { /// and on every filter-input change. pub(super) fn recompute_branch_filtered(&mut self, cx: &mut Context) { let filter = self.branch_picker_filter.read(cx).value().to_lowercase(); - let current = self.branch_picker_list.current.clone(); - let matches = |b: &String| filter.is_empty() || b.to_lowercase().contains(&filter); - - let mut items = Vec::new(); - for b in &self.branch_picker_list.local { - if matches(b) { - items.push(BranchNavItem { - name: b.clone(), - kind: BranchKind::Local, - is_current: current.as_deref() == Some(b.as_str()), - }); - } - } - for b in &self.branch_picker_list.remote { - if matches(b) { - items.push(BranchNavItem { - name: b.clone(), - kind: BranchKind::Remote, - is_current: false, - }); - } - } - self.branch_picker_filtered = items; + self.branch_picker_filtered = branch_nav_items(&self.branch_picker_list, &filter); self.branch_picker_selected = 0; self.branch_picker_scroll.scroll_to_item(0); } @@ -643,6 +621,40 @@ impl GitHeader { } } +/// Build the flat, display-ordered nav list from a loaded branch list and a +/// lowercased filter string: locals first, then remotes. +/// +/// The current branch leads the LOCAL section — it is what users scan for, and +/// putting it on top also makes it the default keyboard selection. The rest +/// keep git's ordering. +fn branch_nav_items(list: &BranchList, filter: &str) -> Vec { + let is_current = |b: &str| list.current.as_deref() == Some(b); + let matches = |b: &str| filter.is_empty() || b.to_lowercase().contains(filter); + + let mut local: Vec<&String> = list.local.iter().collect(); + local.sort_by_key(|b| !is_current(b)); + + local + .into_iter() + .filter(|b| matches(b)) + .map(|b| BranchNavItem { + name: b.clone(), + kind: BranchKind::Local, + is_current: is_current(b), + }) + .chain( + list.remote + .iter() + .filter(|b| matches(b)) + .map(|b| BranchNavItem { + name: b.clone(), + kind: BranchKind::Remote, + is_current: false, + }), + ) + .collect() +} + /// Map a flat selection index (local-first) to its child position within the /// scroll container, so `ScrollHandle::scroll_to_item` lands on the right row. /// @@ -664,7 +676,63 @@ fn branch_row_child_index(local_count: usize, selected: usize) -> usize { #[cfg(test)] mod tests { - use super::branch_row_child_index; + use super::{BranchKind, BranchList, branch_nav_items, branch_row_child_index}; + + fn list(current: Option<&str>, local: &[&str], remote: &[&str]) -> BranchList { + BranchList { + local: local.iter().map(|s| s.to_string()).collect(), + remote: remote.iter().map(|s| s.to_string()).collect(), + current: current.map(|s| s.to_string()), + } + } + + fn rows(items: &[super::BranchNavItem]) -> Vec<(&str, BranchKind, bool)> { + items + .iter() + .map(|b| (b.name.as_str(), b.kind, b.is_current)) + .collect() + } + + #[test] + fn current_branch_leads_the_local_section() { + let items = branch_nav_items(&list(Some("feature"), &["main", "feature", "wip"], &[]), ""); + assert_eq!( + rows(&items), + vec![ + ("feature", BranchKind::Local, true), + ("main", BranchKind::Local, false), + ("wip", BranchKind::Local, false), + ] + ); + } + + #[test] + fn remotes_follow_locals_and_detached_head_keeps_order() { + let items = branch_nav_items(&list(None, &["main", "wip"], &["origin/release"]), ""); + assert_eq!( + rows(&items), + vec![ + ("main", BranchKind::Local, false), + ("wip", BranchKind::Local, false), + ("origin/release", BranchKind::Remote, false), + ] + ); + } + + #[test] + fn filter_matches_case_insensitively_and_drops_the_current_branch() { + let items = branch_nav_items( + &list(Some("feature"), &["main", "feature"], &["origin/Main-2"]), + "main", + ); + assert_eq!( + rows(&items), + vec![ + ("main", BranchKind::Local, false), + ("origin/Main-2", BranchKind::Remote, false), + ] + ); + } #[test] fn child_index_within_local_section() { From f8cd0bd9a65b997eac92f430e7463d37d00567c1 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Wed, 19 Aug 2026 11:32:55 +0200 Subject: [PATCH 07/10] feat(git): show branch state in the branch picker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The picker listed bare names in git's ref order, so choosing a branch meant remembering which one is current work, whether it is pushed, and whether a worktree already holds it — the last one only surfacing as a failed checkout. Collect per-branch metadata in one `git for-each-ref` pass: tip commit time, upstream tracking counts, and the worktree holding each branch. One subprocess beats a per-branch gix rev-walk by a wide margin — git answers ahead/behind for every ref off its own commit-graph (~10ms for ~70 refs here) — and it rides the existing `BranchList` wire type as an additive field, so a remote host without it degrades to plain names. Each row now carries, right-aligned: the holding worktree, `local` / `gone` / `↑N ↓M` against the upstream, and the tip age. Sections order by recency, with the current branch still leading LOCAL. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NsgnZNRhcTnZdEZj8Tm2hp --- crates/okena-git/src/lib.rs | 26 +- crates/okena-git/src/repository/branch.rs | 264 +++++++++++++++++ crates/okena-git/src/repository/mod.rs | 6 +- .../src/diff_viewer/provider.rs | 3 + crates/okena-views-git/src/git_header.rs | 6 +- .../src/git_header/branch_picker.rs | 271 ++++++++++++++++-- 6 files changed, 540 insertions(+), 36 deletions(-) diff --git a/crates/okena-git/src/lib.rs b/crates/okena-git/src/lib.rs index 1d77e7aeb..bcee5423f 100644 --- a/crates/okena-git/src/lib.rs +++ b/crates/okena-git/src/lib.rs @@ -16,19 +16,19 @@ pub use diff::{ }; pub use error::{GitError, GitResult}; pub use repository::{ - BranchList, CloneProgress, HeadSnapshot, OrphanedWorktree, VerifiedWorktree, - checkout_local_branch, checkout_remote_branch, clone_dir_name, clone_repository, - compute_target_paths, count_ahead_behind, count_unpushed_commits, create_and_checkout_branch, - create_worktree, create_worktree_with_start_point, delete_local_branch, delete_remote_branch, - discard_file_changes, fetch_all, fetch_and_fast_forward, finish_clone_repository, - get_available_branches_for_worktree, get_current_branch, get_default_branch, get_head_snapshot, - get_repo_common_dir, get_repo_root, has_uncommitted_changes, is_complete_checkout, - list_branches, list_branches_classified, list_linked_worktree_paths, list_pull_requests, - merge_branch, move_worktree, parse_clone_progress, project_path_in_worktree, push_branch, - rebase_onto, remove_orphaned_worktree, remove_worktree, remove_worktree_fast, - resolve_git_root_and_subdir, resolve_review_base, stage_file, start_clone_repository, - stash_changes, stash_pop, unstage_file, validate_clone_url, verify_linked_worktree_fresh, - verify_orphaned_worktree, + BranchDetail, BranchList, CloneProgress, HeadSnapshot, OrphanedWorktree, UpstreamState, + VerifiedWorktree, checkout_local_branch, checkout_remote_branch, clone_dir_name, + clone_repository, compute_target_paths, count_ahead_behind, count_unpushed_commits, + create_and_checkout_branch, create_worktree, create_worktree_with_start_point, + delete_local_branch, delete_remote_branch, discard_file_changes, fetch_all, + fetch_and_fast_forward, finish_clone_repository, get_available_branches_for_worktree, + get_current_branch, get_default_branch, get_head_snapshot, get_repo_common_dir, get_repo_root, + has_uncommitted_changes, is_complete_checkout, list_branches, list_branches_classified, + list_linked_worktree_paths, list_pull_requests, merge_branch, move_worktree, + parse_clone_progress, project_path_in_worktree, push_branch, rebase_onto, + remove_orphaned_worktree, remove_worktree, remove_worktree_fast, resolve_git_root_and_subdir, + resolve_review_base, stage_file, start_clone_repository, stash_changes, stash_pop, + unstage_file, validate_clone_url, verify_linked_worktree_fresh, verify_orphaned_worktree, }; /// Validate that a git ref (branch name, commit hash, revision) doesn't look diff --git a/crates/okena-git/src/repository/branch.rs b/crates/okena-git/src/repository/branch.rs index c3d619232..c46862a02 100644 --- a/crates/okena-git/src/repository/branch.rs +++ b/crates/okena-git/src/repository/branch.rs @@ -2,6 +2,7 @@ //! plus default-branch resolution, rebase, merge, stash, and per-file //! stage/unstage/discard. +use std::collections::HashMap; use std::path::Path; use serde::{Deserialize, Serialize}; @@ -249,6 +250,164 @@ pub struct BranchList { pub remote: Vec, /// Current HEAD branch name (`None` if detached). pub current: Option, + /// Per-branch metadata, keyed by the same names used in `local`/`remote`. + /// Empty when the metadata pass failed, or when it comes from a remote host + /// that predates this field — consumers must treat a missing entry as + /// "unknown" and still show the branch. + #[serde(default)] + pub details: HashMap, +} + +/// What a branch picker can show beside the name: how recently the branch +/// moved, how it sits against its upstream, and whether another worktree holds +/// it. Collected for every branch in one [`collect_branch_details`] pass. +#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct BranchDetail { + /// Committer time of the branch tip, as a Unix timestamp. + #[serde(default)] + pub committed_at: Option, + /// How the branch sits against its configured upstream. + #[serde(default)] + pub upstream: UpstreamState, + /// Worktree holding this branch, when it is not the one we are asking + /// from. Checking such a branch out fails, so the UI can say why up front. + #[serde(default)] + pub worktree: Option, +} + +/// A branch's relation to its configured upstream ref. +#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum UpstreamState { + /// No upstream configured — the branch exists only locally. + #[default] + Untracked, + /// The upstream ref is configured but no longer exists on the remote. + Gone, + /// Tracking `name`, `ahead`/`behind` commits apart from it. Both zero + /// means in sync. + Tracked { + name: String, + ahead: usize, + behind: usize, + }, +} + +/// One line per branch: ` `, +/// tab-separated. Tabs cannot appear in a ref name, and git emits none inside +/// these fields. +const DETAIL_FORMAT: &str = concat!( + "%(refname:short)\t", + "%(committerdate:unix)\t", + "%(upstream:short)\t", + "%(upstream:track)\t", + "%(worktreepath)\t", + "%(HEAD)" +); + +/// Collect [`BranchDetail`] for every branch in a single `git for-each-ref`. +/// +/// One subprocess beats a per-branch `gix` rev-walk by a wide margin here: git +/// answers ahead/behind for every ref off its own commit-graph in a single +/// pass (~10ms for ~70 refs). Soft-fails to an empty map — the branch list is +/// still usable without metadata, e.g. against a git too old for +/// `%(worktreepath)` (< 2.23). +fn collect_branch_details(path: &Path) -> HashMap { + let Ok(p) = path_str(path) else { + return HashMap::new(); + }; + // `LC_ALL=C` keeps `%(upstream:track)` in English; git translates it, and + // the counts are parsed back out of that text below. + let output = safe_output(command("git").env("LC_ALL", "C").args([ + "-C", + p, + "for-each-ref", + "--format", + DETAIL_FORMAT, + "refs/heads", + "refs/remotes", + ])); + match output { + Ok(output) if output.status.success() => { + parse_branch_details(&String::from_utf8_lossy(&output.stdout)) + } + Ok(output) => { + log::warn!( + "git for-each-ref failed for {}: {}", + path.display(), + String::from_utf8_lossy(&output.stderr).trim() + ); + HashMap::new() + } + Err(error) => { + log::warn!("git for-each-ref failed for {}: {error}", path.display()); + HashMap::new() + } + } +} + +/// Parse the [`DETAIL_FORMAT`] output. Short lines are skipped rather than +/// failing the whole map — a branch without metadata still lists fine. +fn parse_branch_details(stdout: &str) -> HashMap { + let mut map = HashMap::new(); + for line in stdout.lines() { + let mut fields = line.split('\t'); + let (Some(name), Some(time), Some(upstream), Some(track), Some(worktree), Some(head)) = ( + fields.next(), + fields.next(), + fields.next(), + fields.next(), + fields.next(), + fields.next(), + ) else { + continue; + }; + if name.is_empty() { + continue; + } + map.insert( + name.to_string(), + BranchDetail { + committed_at: time.parse().ok(), + upstream: parse_upstream_track(upstream, track), + // The worktree we are asking from is where a checkout lands + // anyway; only another one blocks it. + worktree: (head.trim() != "*" && !worktree.is_empty()) + .then(|| worktree.to_string()), + }, + ); + } + map +} + +/// Turn `%(upstream:short)` + `%(upstream:track)` into an [`UpstreamState`]. +/// +/// `track` is empty both for a branch without an upstream and for one in sync +/// with it, which is why the upstream name is read alongside it. Otherwise it +/// reads `[gone]`, `[ahead N]`, `[behind N]` or `[ahead N, behind M]`. +fn parse_upstream_track(upstream: &str, track: &str) -> UpstreamState { + if upstream.is_empty() { + return UpstreamState::Untracked; + } + let inner = track.trim().trim_start_matches('[').trim_end_matches(']'); + if inner == "gone" { + return UpstreamState::Gone; + } + let mut ahead = 0; + let mut behind = 0; + for part in inner.split(',') { + let mut words = part.split_whitespace(); + match (words.next(), words.next().and_then(|n| n.parse().ok())) { + (Some("ahead"), Some(n)) => ahead = n, + (Some("behind"), Some(n)) => behind = n, + _ => {} + } + } + UpstreamState::Tracked { + name: upstream.to_string(), + ahead, + behind, + } } /// List branches classified into local vs. remote. @@ -299,6 +458,7 @@ pub fn list_branches_classified(path: &Path) -> BranchList { current: head_branch_short(&repo), local, remote, + details: collect_branch_details(path), } } @@ -364,6 +524,110 @@ pub fn create_and_checkout_branch( mod tests { use super::*; use crate::repository::status::get_current_branch; + + /// Sample `for-each-ref` output in [`DETAIL_FORMAT`]: current branch, + /// a branch held by another worktree, one whose upstream is gone, and a + /// remote-only ref. + const SAMPLE: &str = concat!( + "main\t1700000000\torigin/main\t[behind 3]\t/repo\t*\n", + "feature\t1699000000\torigin/feature\t[ahead 2, behind 1]\t/repo/../wt-feature\t \n", + "stale\t1698000000\torigin/stale\t[gone]\t\t \n", + "local-only\t1697000000\t\t\t\t \n", + "origin/release\t1696000000\t\t\t\t \n", + ); + + #[test] + fn parses_tracking_counts_and_recency() { + let details = parse_branch_details(SAMPLE); + + assert_eq!( + details["main"], + BranchDetail { + committed_at: Some(1700000000), + upstream: UpstreamState::Tracked { + name: "origin/main".to_string(), + ahead: 0, + behind: 3, + }, + // HEAD marker set: this is our own worktree, not a blocker. + worktree: None, + } + ); + assert_eq!( + details["feature"].upstream, + UpstreamState::Tracked { + name: "origin/feature".to_string(), + ahead: 2, + behind: 1, + } + ); + assert_eq!( + details["feature"].worktree.as_deref(), + Some("/repo/../wt-feature") + ); + } + + #[test] + fn parses_missing_and_gone_upstreams() { + let details = parse_branch_details(SAMPLE); + + assert_eq!(details["stale"].upstream, UpstreamState::Gone); + assert_eq!(details["local-only"].upstream, UpstreamState::Untracked); + assert_eq!(details["origin/release"].upstream, UpstreamState::Untracked); + assert_eq!(details["origin/release"].committed_at, Some(1696000000)); + } + + #[test] + fn in_sync_branch_is_tracked_not_untracked() { + // Empty `%(upstream:track)` means either "no upstream" or "in sync"; + // the upstream name is what tells them apart. + let details = parse_branch_details("main\t1700000000\torigin/main\t\t\t*\n"); + assert_eq!( + details["main"].upstream, + UpstreamState::Tracked { + name: "origin/main".to_string(), + ahead: 0, + behind: 0, + } + ); + } + + #[test] + fn short_and_empty_lines_are_skipped() { + let details = parse_branch_details("broken-line\t1700000000\n\nmain\t1\t\t\t\t*\n"); + assert!(!details.contains_key("broken-line")); + assert!(details.contains_key("main")); + } + + #[test] + fn classified_list_carries_details_from_real_git() { + let (_tmp, repo) = init_temp_repo(); + let wt_tmp = tempfile::tempdir().expect("create worktree tempdir"); + let wt_path = wt_tmp.path().join("wt-feat"); + git_in( + &repo, + &[ + "worktree", + "add", + wt_path.to_str().expect("utf-8 path"), + "-b", + "feat", + ], + ); + + let list = list_branches_classified(&repo); + + let main = &list.details["main"]; + assert!(main.committed_at.is_some_and(|t| t > 0)); + // No remote in a temp repo, so nothing tracks anything. + assert_eq!(main.upstream, UpstreamState::Untracked); + assert_eq!(main.worktree, None, "our own worktree must not be flagged"); + assert!( + list.details["feat"].worktree.is_some(), + "a branch held by another worktree must report its path" + ); + } + use crate::repository::test_support::{git_in, init_temp_repo}; use std::path::PathBuf; diff --git a/crates/okena-git/src/repository/mod.rs b/crates/okena-git/src/repository/mod.rs index 775be5175..ae312d1b6 100644 --- a/crates/okena-git/src/repository/mod.rs +++ b/crates/okena-git/src/repository/mod.rs @@ -24,9 +24,9 @@ pub mod status; pub mod worktree; pub use branch::{ - BranchList, checkout_local_branch, checkout_remote_branch, create_and_checkout_branch, - delete_local_branch, delete_remote_branch, discard_file_changes, fetch_all, - get_available_branches_for_worktree, get_default_branch, list_branches, + BranchDetail, BranchList, UpstreamState, checkout_local_branch, checkout_remote_branch, + create_and_checkout_branch, delete_local_branch, delete_remote_branch, discard_file_changes, + fetch_all, get_available_branches_for_worktree, get_default_branch, list_branches, list_branches_classified, merge_branch, push_branch, rebase_onto, resolve_base_ref, resolve_review_base, stage_file, stash_changes, stash_pop, unstage_file, }; diff --git a/crates/okena-views-git/src/diff_viewer/provider.rs b/crates/okena-views-git/src/diff_viewer/provider.rs index 133aef8a5..cb285b8b0 100644 --- a/crates/okena-views-git/src/diff_viewer/provider.rs +++ b/crates/okena-views-git/src/diff_viewer/provider.rs @@ -36,6 +36,9 @@ pub trait GitProvider: Send + Sync + 'static { local, remote, current: None, + // The name-only fallback carries no per-branch metadata; the + // picker degrades to plain names. + details: Default::default(), }) } diff --git a/crates/okena-views-git/src/git_header.rs b/crates/okena-views-git/src/git_header.rs index ce46e05d4..089c309aa 100644 --- a/crates/okena-views-git/src/git_header.rs +++ b/crates/okena-views-git/src/git_header.rs @@ -4,7 +4,7 @@ //! Extracted from `ProjectColumn` to keep that view thin. Implementation //! is split across the `git_header/` submodules — one per concern. -use okena_git::{BranchList, CommitLogEntry, FileDiffSummary}; +use okena_git::{BranchDetail, BranchList, CommitLogEntry, FileDiffSummary}; use okena_ui::simple_input::{InputChangedEvent, SimpleInputState}; use okena_workspace::request_broker::RequestBroker; use okena_workspace::state::Workspace; @@ -61,6 +61,10 @@ struct BranchNavItem { name: String, kind: BranchKind, is_current: bool, + /// Tip time, upstream state and holding worktree, as reported by + /// `list_branches_classified`. Defaults (all unknown) when the host did + /// not report metadata for this branch. + detail: BranchDetail, } /// State for the right-click context menu on a commit row in the graph. diff --git a/crates/okena-views-git/src/git_header/branch_picker.rs b/crates/okena-views-git/src/git_header/branch_picker.rs index a459a4b7c..cb6a79676 100644 --- a/crates/okena-views-git/src/git_header/branch_picker.rs +++ b/crates/okena-views-git/src/git_header/branch_picker.rs @@ -3,14 +3,17 @@ use super::{BranchKind, BranchNavItem, BranchPickerStatus, GitHeader}; +use std::cmp::Reverse; + use okena_core::theme::ThemeColors; -use okena_git::BranchList; +use okena_git::{BranchDetail, BranchList, UpstreamState}; use okena_ui::simple_input::SimpleInput; use okena_ui::theme::with_alpha; use okena_ui::tokens::{ui_text_md, ui_text_ms, ui_text_sm}; use gpui::prelude::*; use gpui::*; +use gpui_component::tooltip::Tooltip; use gpui_component::{h_flex, v_flex}; impl GitHeader { @@ -317,17 +320,21 @@ impl GitHeader { _ => None, }; - let row = |name: String, - is_current: bool, + let row = |item: &BranchNavItem, is_selected: bool, - kind: BranchKind, key: String, cx: &mut Context| -> AnyElement { + let BranchNavItem { + name, + kind, + is_current, + detail, + } = item.clone(); let name_for_click = name.clone(); let is_remote = kind == BranchKind::Remote; h_flex() - .id(ElementId::Name(key.into())) + .id(ElementId::Name(key.clone().into())) .px(px(10.0)) .py(px(4.0)) .gap(px(6.0)) @@ -358,16 +365,21 @@ impl GitHeader { .min_w_0() .text_ellipsis() .overflow_hidden() + // Without this a name with a `/` wraps to a second line + // instead of truncating, and the meta column jumps. + .whitespace_nowrap() .child(name), ) .when(is_current, |d| { d.child( div() + .flex_shrink_0() .text_size(ui_text_sm(cx)) .text_color(rgb(t.term_cyan)) .child("HEAD"), ) }) + .children(branch_meta(&detail, kind, &key, t, cx)) .on_mouse_down(MouseButton::Left, |_, _, cx| { cx.stop_propagation(); }) @@ -391,7 +403,7 @@ impl GitHeader { v_flex() .id("branch-picker-popover") .occlude() - .w(px(320.0)) + .w(px(420.0)) .max_h(px(420.0)) .bg(rgb(t.bg_primary)) .border_1() @@ -496,10 +508,8 @@ impl GitHeader { .iter() .map(|(flat, b)| { row( - b.name.clone(), - b.is_current, + b, *flat == selected, - BranchKind::Local, format!("branch-picker-row-{}", flat), cx, ) @@ -509,10 +519,8 @@ impl GitHeader { .iter() .map(|(flat, b)| { row( - b.name.clone(), - false, + b, *flat == selected, - BranchKind::Remote, format!("branch-picker-row-{}", flat), cx, ) @@ -624,15 +632,27 @@ impl GitHeader { /// Build the flat, display-ordered nav list from a loaded branch list and a /// lowercased filter string: locals first, then remotes. /// -/// The current branch leads the LOCAL section — it is what users scan for, and -/// putting it on top also makes it the default keyboard selection. The rest -/// keep git's ordering. +/// Within each section the most recently committed branch comes first, which +/// is the order people actually work in; branches with no reported tip time +/// (older hosts, metadata unavailable) sink to the bottom of their section. +/// The current branch overrides that and leads the LOCAL section — it is what +/// users scan for, and putting it on top also makes it the default keyboard +/// selection. fn branch_nav_items(list: &BranchList, filter: &str) -> Vec { let is_current = |b: &str| list.current.as_deref() == Some(b); let matches = |b: &str| filter.is_empty() || b.to_lowercase().contains(filter); + let detail = |b: &str| list.details.get(b).cloned().unwrap_or_default(); + let tip_time = |b: &str| { + list.details + .get(b) + .and_then(|d| d.committed_at) + .unwrap_or(i64::MIN) + }; let mut local: Vec<&String> = list.local.iter().collect(); - local.sort_by_key(|b| !is_current(b)); + local.sort_by_key(|b| (!is_current(b), Reverse(tip_time(b)))); + let mut remote: Vec<&String> = list.remote.iter().collect(); + remote.sort_by_key(|b| Reverse(tip_time(b))); local .into_iter() @@ -641,20 +661,162 @@ fn branch_nav_items(list: &BranchList, filter: &str) -> Vec { name: b.clone(), kind: BranchKind::Local, is_current: is_current(b), + detail: detail(b), }) .chain( - list.remote - .iter() + remote + .into_iter() .filter(|b| matches(b)) .map(|b| BranchNavItem { name: b.clone(), kind: BranchKind::Remote, is_current: false, + detail: detail(b), }), ) .collect() } +/// Right-hand metadata cluster for a branch row: the worktree holding the +/// branch, how it sits against its upstream, and how recently it moved. +/// +/// Each part is omitted when it has nothing to say, so an ordinary in-sync +/// branch shows only its age. Upstream state is local-only — a remote ref does +/// not track anything. +fn branch_meta( + detail: &BranchDetail, + kind: BranchKind, + key: &str, + t: &ThemeColors, + cx: &App, +) -> Vec { + let mut parts: Vec = Vec::new(); + + // A branch checked out elsewhere cannot be checked out here — git refuses + // it — so name the worktree before the user tries. + if let Some(path) = detail.worktree.as_deref() { + let tooltip = format!("Checked out in {path}"); + let label = path + .rsplit(['/', '\\']) + .find(|part| !part.is_empty()) + .unwrap_or(path) + .to_string(); + parts.push( + h_flex() + .id(ElementId::Name(format!("{key}-worktree").into())) + .flex_shrink_0() + .gap(px(3.0)) + .items_center() + .text_size(ui_text_sm(cx)) + .text_color(rgb(t.term_yellow)) + .child( + svg() + .path("icons/folder.svg") + .size(px(9.0)) + .text_color(rgb(t.term_yellow)), + ) + .child( + div() + .max_w(px(70.0)) + .overflow_hidden() + .whitespace_nowrap() + .text_ellipsis() + .child(label), + ) + .tooltip(move |window, cx| Tooltip::new(tooltip.clone()).build(window, cx)) + .into_any_element(), + ); + } + + if kind == BranchKind::Local { + match &detail.upstream { + UpstreamState::Untracked => parts.push( + div() + .id(ElementId::Name(format!("{key}-untracked").into())) + .flex_shrink_0() + .text_size(ui_text_sm(cx)) + .text_color(rgb(t.text_muted)) + .child("local") + .tooltip(|window, cx| { + Tooltip::new("No upstream — never pushed").build(window, cx) + }) + .into_any_element(), + ), + UpstreamState::Gone => parts.push( + div() + .id(ElementId::Name(format!("{key}-gone").into())) + .flex_shrink_0() + .text_size(ui_text_sm(cx)) + .text_color(rgb(t.term_red)) + .child("gone") + .tooltip(|window, cx| { + Tooltip::new("Upstream branch no longer exists on the remote") + .build(window, cx) + }) + .into_any_element(), + ), + UpstreamState::Tracked { + name, + ahead, + behind, + } => { + let (ahead, behind) = (*ahead, *behind); + if ahead > 0 || behind > 0 { + let tooltip = { + let mut lines = Vec::new(); + if ahead > 0 { + lines.push(format!("{ahead} ahead of {name}")); + } + if behind > 0 { + lines.push(format!("{behind} behind {name}")); + } + lines.join("\n") + }; + parts.push( + h_flex() + .id(ElementId::Name(format!("{key}-track").into())) + .flex_shrink_0() + .gap(px(4.0)) + .items_center() + .text_size(ui_text_sm(cx)) + .when(ahead > 0, |d| { + d.child( + div() + .text_color(rgb(t.term_green)) + .child(format!("\u{2191}{ahead}")), + ) + }) + .when(behind > 0, |d| { + d.child( + div() + .text_color(rgb(t.term_yellow)) + .child(format!("\u{2193}{behind}")), + ) + }) + .tooltip(move |window, cx| { + Tooltip::new(tooltip.clone()).build(window, cx) + }) + .into_any_element(), + ); + } + } + } + } + + if let Some(timestamp) = detail.committed_at { + parts.push( + div() + .flex_shrink_0() + .text_size(ui_text_sm(cx)) + .text_color(rgb(t.text_muted)) + .child(okena_git::format_relative_time(timestamp)) + .into_any_element(), + ); + } + + parts +} + /// Map a flat selection index (local-first) to its child position within the /// scroll container, so `ScrollHandle::scroll_to_item` lands on the right row. /// @@ -676,16 +838,46 @@ fn branch_row_child_index(local_count: usize, selected: usize) -> usize { #[cfg(test)] mod tests { - use super::{BranchKind, BranchList, branch_nav_items, branch_row_child_index}; + use super::{BranchDetail, BranchKind, BranchList, branch_nav_items, branch_row_child_index}; + /// Build a list whose branches carry no metadata — the pre-metadata host + /// case, where display order falls back to git's ref order. fn list(current: Option<&str>, local: &[&str], remote: &[&str]) -> BranchList { BranchList { local: local.iter().map(|s| s.to_string()).collect(), remote: remote.iter().map(|s| s.to_string()).collect(), current: current.map(|s| s.to_string()), + details: Default::default(), } } + /// Same, with a tip timestamp per branch name. + fn list_with_times( + current: Option<&str>, + local: &[(&str, i64)], + remote: &[(&str, i64)], + ) -> BranchList { + let mut list = list( + current, + &local.iter().map(|(n, _)| *n).collect::>(), + &remote.iter().map(|(n, _)| *n).collect::>(), + ); + list.details = local + .iter() + .chain(remote) + .map(|(name, at)| { + ( + name.to_string(), + BranchDetail { + committed_at: Some(*at), + ..Default::default() + }, + ) + }) + .collect(); + list + } + fn rows(items: &[super::BranchNavItem]) -> Vec<(&str, BranchKind, bool)> { items .iter() @@ -734,6 +926,47 @@ mod tests { ); } + #[test] + fn sections_are_ordered_by_recency_behind_the_current_branch() { + let items = branch_nav_items( + &list_with_times( + Some("feature"), + &[("main", 300), ("feature", 100), ("wip", 200)], + &[("origin/old", 50), ("origin/new", 400)], + ), + "", + ); + assert_eq!( + rows(&items), + vec![ + // Current branch wins over its older tip time. + ("feature", BranchKind::Local, true), + ("main", BranchKind::Local, false), + ("wip", BranchKind::Local, false), + ("origin/new", BranchKind::Remote, false), + ("origin/old", BranchKind::Remote, false), + ] + ); + } + + #[test] + fn branches_without_a_tip_time_sink_below_dated_ones() { + let mut list = list_with_times(None, &[("dated", 100)], &[]); + list.local.push("undated".to_string()); + // Git's ref order puts `undated` last anyway, so check the reverse too. + list.local.reverse(); + + let items = branch_nav_items(&list, ""); + + assert_eq!( + rows(&items), + vec![ + ("dated", BranchKind::Local, false), + ("undated", BranchKind::Local, false), + ] + ); + } + #[test] fn child_index_within_local_section() { // 3 local rows: LOCAL header at child 0, rows at children 1, 2, 3. From e386102de2cb9a2e6673e400ae7d083a81d4cfb0 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Wed, 19 Aug 2026 11:51:42 +0200 Subject: [PATCH 08/10] feat(git): add row actions to the branch picker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Picking a branch was all-or-nothing: click it and you are on it. Looking at what a branch contains first meant checking it out, or leaving for the commit log popover and finding the branch again there. Add a row context menu with the three things that need no checkout: Show History, Compare with Current, Copy Branch Name. The first two route into what already exists — the commit log popover already renders an arbitrary branch's graph, and `DiffMode::BranchCompare` already backs a three-dot diff — so this is wiring, not a second implementation. History seeds the popover's own branch dropdown from the picker's loaded list, so it costs one commit-graph fetch and no more. Reachable by right-click and, since the picker is keyboard-driven, by the menu key / shift-F10 on the selected row, with arrows and Enter inside the menu. Compare is disabled on the current branch — it would diff nothing — and navigation steps past it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NsgnZNRhcTnZdEZj8Tm2hp --- crates/okena-views-git/src/git_header.rs | 19 ++ .../src/git_header/branch_picker.rs | 296 +++++++++++++++++- .../src/git_header/commit_log.rs | 24 ++ 3 files changed, 334 insertions(+), 5 deletions(-) diff --git a/crates/okena-views-git/src/git_header.rs b/crates/okena-views-git/src/git_header.rs index 089c309aa..892911d41 100644 --- a/crates/okena-views-git/src/git_header.rs +++ b/crates/okena-views-git/src/git_header.rs @@ -67,6 +67,18 @@ struct BranchNavItem { detail: BranchDetail, } +/// State for the context menu on a branch row in the picker. Captured when +/// the menu opens so it survives the list being refiltered underneath it. +pub(super) struct BranchRowContextMenu { + pub(super) position: Point, + pub(super) name: String, + /// Comparing the current branch with itself has nothing to show, so the + /// compare item is disabled for it. + pub(super) is_current: bool, + /// Keyboard-highlighted item, as an index into `BRANCH_ROW_ACTIONS`. + pub(super) selected: usize, +} + /// State for the right-click context menu on a commit row in the graph. /// Captured once at open time so the menu doesn't need a live reference /// back to the underlying `CommitLogEntry`. @@ -131,6 +143,11 @@ pub struct GitHeader { /// selected row in view. branch_picker_scroll: ScrollHandle, branch_picker_create_mode: bool, + /// Open context menu for one branch row, if any. + pub(super) branch_row_menu: Option, + /// On-screen bounds of the keyboard-selected row, so the menu key can + /// anchor the menu under it the same way a right-click does. + branch_row_bounds: Bounds, branch_picker_create_name: Entity, branch_picker_status: BranchPickerStatus, @@ -207,6 +224,8 @@ impl GitHeader { branch_picker_selected: 0, branch_picker_scroll: ScrollHandle::new(), branch_picker_create_mode: false, + branch_row_menu: None, + branch_row_bounds: Bounds::default(), branch_picker_create_name, branch_picker_status: BranchPickerStatus::Idle, ci_checks_visible: false, diff --git a/crates/okena-views-git/src/git_header/branch_picker.rs b/crates/okena-views-git/src/git_header/branch_picker.rs index cb6a79676..bacf6285b 100644 --- a/crates/okena-views-git/src/git_header/branch_picker.rs +++ b/crates/okena-views-git/src/git_header/branch_picker.rs @@ -1,15 +1,17 @@ //! Branch switcher popover — filter/select a local or remote branch, or //! create a new one from the current HEAD. -use super::{BranchKind, BranchNavItem, BranchPickerStatus, GitHeader}; +use super::{BranchKind, BranchNavItem, BranchPickerStatus, BranchRowContextMenu, GitHeader}; use std::cmp::Reverse; use okena_core::theme::ThemeColors; +use okena_core::types::DiffMode; use okena_git::{BranchDetail, BranchList, UpstreamState}; use okena_ui::simple_input::SimpleInput; use okena_ui::theme::with_alpha; use okena_ui::tokens::{ui_text_md, ui_text_ms, ui_text_sm}; +use okena_workspace::requests::{OverlayRequest, ProjectOverlay, ProjectOverlayKind}; use gpui::prelude::*; use gpui::*; @@ -140,6 +142,7 @@ impl GitHeader { self.branch_picker_visible = false; self.branch_picker_create_mode = false; self.branch_picker_status = BranchPickerStatus::Idle; + self.branch_row_menu = None; // Restore the previously-focused terminal so typing resumes there. let workspace = self.workspace.clone(); self.focus_manager.update(cx, |fm, cx| { @@ -257,6 +260,148 @@ impl GitHeader { .detach(); } + /// Open the row context menu for `item`, anchored at `position`. + fn open_branch_row_menu( + &mut self, + item: &BranchNavItem, + position: Point, + cx: &mut Context, + ) { + self.branch_row_menu = Some(BranchRowContextMenu { + position, + name: item.name.clone(), + is_current: item.is_current, + selected: 0, + }); + cx.notify(); + } + + /// Open the row menu for the keyboard-selected branch, anchored under it + /// so it lands where a right-click on that row would have put it. + fn open_selected_branch_menu(&mut self, cx: &mut Context) { + let Some(item) = self + .branch_picker_filtered + .get(self.branch_picker_selected) + .cloned() + else { + return; + }; + let bounds = self.branch_row_bounds; + let position = point( + bounds.origin.x + px(24.0), + bounds.origin.y + bounds.size.height, + ); + self.open_branch_row_menu(&item, position, cx); + } + + /// Open a three-dot diff of `branch` against the current one, without + /// checking anything out. Base is the current branch, so the diff reads as + /// "what `branch` adds" — the same orientation as the commit log's compare. + fn compare_branch_with_current(&mut self, branch: String, cx: &mut Context) { + let base = self + .current_branch + .clone() + .unwrap_or_else(|| "HEAD".to_string()); + let project_id = self.project_id.clone(); + self.hide_branch_picker(cx); + self.request_broker.update(cx, |broker, cx| { + broker.push_overlay_request( + OverlayRequest::Project(ProjectOverlay { + project_id, + kind: ProjectOverlayKind::DiffViewer { + file: None, + mode: Some(DiffMode::BranchCompare { base, head: branch }), + commit_message: None, + commits: None, + commit_index: None, + }, + }), + cx, + ); + }); + } + + /// Move the menu selection by `delta`, stepping past items this row + /// disables so Enter always lands on something that runs. + fn move_branch_menu_selection(&mut self, delta: isize, cx: &mut Context) { + let Some(menu) = self.branch_row_menu.as_mut() else { + return; + }; + menu.selected = next_menu_index(menu.selected, delta, menu.is_current); + cx.notify(); + } + + /// Run one row action against the branch the menu was opened on, and close + /// the menu. A no-op for an action this row disables. + fn run_branch_row_action(&mut self, action: BranchRowAction, cx: &mut Context) { + let Some(menu) = self.branch_row_menu.as_ref() else { + return; + }; + if !action.is_enabled(menu.is_current) { + return; + } + let branch = menu.name.clone(); + self.branch_row_menu = None; + match action { + BranchRowAction::History => self.show_branch_history(branch, cx), + BranchRowAction::Compare => self.compare_branch_with_current(branch, cx), + BranchRowAction::CopyName => { + cx.write_to_clipboard(ClipboardItem::new_string(branch)); + cx.notify(); + } + } + } + + /// Render the context menu for a branch row. Returns `None` when no menu + /// is open. + fn render_branch_row_menu( + &self, + t: &ThemeColors, + cx: &mut Context, + ) -> Option { + use okena_ui::menu::{context_menu_panel, menu_item_conditional}; + + let menu = self.branch_row_menu.as_ref()?; + let position = menu.position; + let is_current = menu.is_current; + let selected = menu.selected; + + let mut panel = context_menu_panel("branch-row-context-menu", t).on_mouse_down_out( + cx.listener(|this, _, _, cx| { + this.branch_row_menu = None; + cx.notify(); + }), + ); + for (index, action) in BRANCH_ROW_ACTIONS.iter().enumerate() { + let action = *action; + let enabled = action.is_enabled(is_current); + panel = panel.child( + menu_item_conditional( + ElementId::Name(format!("branch-row-ctx-{}", action.id()).into()), + action.icon(), + action.label(), + enabled, + t, + ) + // Same highlight as the picker's own selected row, so keyboard + // focus reads the same in both. + .when(index == selected, |d| { + d.bg(with_alpha(t.border_active, 0.15)) + }) + .when(enabled, |d| { + d.on_click( + cx.listener(move |this, _, _, cx| this.run_branch_row_action(action, cx)), + ) + }), + ); + } + + Some( + deferred(anchored().position(position).snap_to_window().child(panel)) + .into_any_element(), + ) + } + /// Render the branch switcher popover anchored under the branch chip. /// Returns a zero-size element when the popover is hidden. pub fn render_branch_picker( @@ -332,6 +477,7 @@ impl GitHeader { detail, } = item.clone(); let name_for_click = name.clone(); + let item_for_menu = item.clone(); let is_remote = kind == BranchKind::Remote; h_flex() .id(ElementId::Name(key.clone().into())) @@ -380,9 +526,32 @@ impl GitHeader { ) }) .children(branch_meta(&detail, kind, &key, t, cx)) + .when(is_selected, |d| { + // Feed the keyboard-selected row's bounds back so the menu + // key can anchor under it. Assigned without notifying — + // this runs every layout pass. + let entity = cx.entity().clone(); + d.relative().child( + canvas( + move |bounds, _window, app| { + entity.update(app, |this, _| this.branch_row_bounds = bounds); + }, + |_, _, _, _| {}, + ) + .absolute() + .size_full(), + ) + }) .on_mouse_down(MouseButton::Left, |_, _, cx| { cx.stop_propagation(); }) + .on_mouse_down( + MouseButton::Right, + cx.listener(move |this, event: &MouseDownEvent, _window, cx| { + this.open_branch_row_menu(&item_for_menu, event.position, cx); + cx.stop_propagation(); + }), + ) .on_click(cx.listener(move |this, _, _window, cx| { this.checkout_branch(name_for_click.clone(), kind, cx); })) @@ -398,7 +567,7 @@ impl GitHeader { .child(label) }; - deferred( + let popover = deferred( anchored().position(position).snap_to_window().child( v_flex() .id("branch-picker-popover") @@ -439,7 +608,38 @@ impl GitHeader { } return; } + // An open row menu takes the keys until it closes. + if this.branch_row_menu.is_some() { + match key { + "escape" => { + this.branch_row_menu = None; + cx.notify(); + } + "up" => this.move_branch_menu_selection(-1, cx), + "down" => this.move_branch_menu_selection(1, cx), + "enter" => { + if let Some(action) = this + .branch_row_menu + .as_ref() + .map(|menu| BRANCH_ROW_ACTIONS[menu.selected]) + { + this.run_branch_row_action(action, cx); + } + } + _ => {} + } + cx.stop_propagation(); + return; + } match key { + "menu" => { + this.open_selected_branch_menu(cx); + cx.stop_propagation(); + } + "f10" if event.keystroke.modifiers.shift => { + this.open_selected_branch_menu(cx); + cx.stop_propagation(); + } "up" => { this.select_prev_branch(cx); cx.stop_propagation(); @@ -624,8 +824,75 @@ impl GitHeader { }), ), ), - ) - .into_any_element() + ); + + // Mount the row menu as a sibling so it overlays the popover — + // `Deferred` takes a single child, so the pair needs a parent div. + div() + .child(popover) + .when_some(self.render_branch_row_menu(t, cx), |d, menu| d.child(menu)) + .into_any_element() + } +} + +/// What the row context menu offers, in menu order. All three work without +/// checking the branch out. +#[derive(Clone, Copy, PartialEq, Eq)] +enum BranchRowAction { + History, + Compare, + CopyName, +} + +const BRANCH_ROW_ACTIONS: [BranchRowAction; 3] = [ + BranchRowAction::History, + BranchRowAction::Compare, + BranchRowAction::CopyName, +]; + +/// Step the menu selection by `delta`, skipping items disabled for this row +/// and stopping at the ends. Returns `current` when nothing selectable lies +/// that way. +fn next_menu_index(current: usize, delta: isize, is_current: bool) -> usize { + let count = BRANCH_ROW_ACTIONS.len() as isize; + let mut index = current as isize; + for _ in 0..count { + index = (index + delta).clamp(0, count - 1); + if BRANCH_ROW_ACTIONS[index as usize].is_enabled(is_current) { + return index as usize; + } + } + current +} + +impl BranchRowAction { + fn id(self) -> &'static str { + match self { + Self::History => "history", + Self::Compare => "compare", + Self::CopyName => "copy", + } + } + + fn icon(self) -> &'static str { + match self { + Self::History => "icons/git-commit.svg", + Self::Compare => "icons/git-pull-request.svg", + Self::CopyName => "icons/copy.svg", + } + } + + fn label(self) -> &'static str { + match self { + Self::History => "Show History", + Self::Compare => "Compare with Current", + Self::CopyName => "Copy Branch Name", + } + } + + /// Comparing the current branch against itself would diff nothing. + fn is_enabled(self, is_current: bool) -> bool { + !(self == Self::Compare && is_current) } } @@ -838,7 +1105,10 @@ fn branch_row_child_index(local_count: usize, selected: usize) -> usize { #[cfg(test)] mod tests { - use super::{BranchDetail, BranchKind, BranchList, branch_nav_items, branch_row_child_index}; + use super::{ + BranchDetail, BranchKind, BranchList, BranchRowAction, branch_nav_items, + branch_row_child_index, next_menu_index, + }; /// Build a list whose branches carry no metadata — the pre-metadata host /// case, where display order falls back to git's ref order. @@ -967,6 +1237,22 @@ mod tests { ); } + #[test] + fn menu_navigation_stops_at_the_ends() { + // [History, Compare, Copy] — all selectable on a non-current branch. + assert_eq!(next_menu_index(0, 1, false), 1); + assert_eq!(next_menu_index(2, 1, false), 2); + assert_eq!(next_menu_index(0, -1, false), 0); + } + + #[test] + fn menu_navigation_skips_compare_on_the_current_branch() { + assert!(!BranchRowAction::Compare.is_enabled(true)); + // Down from History lands on Copy, not the disabled Compare. + assert_eq!(next_menu_index(0, 1, true), 2); + assert_eq!(next_menu_index(2, -1, true), 0); + } + #[test] fn child_index_within_local_section() { // 3 local rows: LOCAL header at child 0, rows at children 1, 2, 3. diff --git a/crates/okena-views-git/src/git_header/commit_log.rs b/crates/okena-views-git/src/git_header/commit_log.rs index 618866b75..f6c6b7e2c 100644 --- a/crates/okena-views-git/src/git_header/commit_log.rs +++ b/crates/okena-views-git/src/git_header/commit_log.rs @@ -75,6 +75,30 @@ impl GitHeader { .detach(); } + /// Open the commit log popover pointed at `branch`, without checking it + /// out — the "look before you switch" path out of the branch picker. + /// + /// Seeds the popover's own branch dropdown from the picker's list, which + /// is already loaded, so this costs one commit-graph fetch and no more. + pub(super) fn show_branch_history(&mut self, branch: String, cx: &mut Context) { + let branches = self + .branch_picker_list + .local + .iter() + .chain(&self.branch_picker_list.remote) + .cloned() + .collect(); + self.hide_branch_picker(cx); + self.commit_log_branches = branches; + self.commit_log_compare_mode = false; + self.commit_log_compare_base = None; + self.commit_log_compare_head = None; + self.commit_log_picker_target = BranchPickerTarget::Graph; + self.commit_log_visible = true; + self.diff_popover_visible = false; + self.switch_commit_log_branch(Some(branch), cx); + } + fn switch_commit_log_branch(&mut self, branch: Option, cx: &mut Context) { self.commit_log_branch = branch.clone(); self.commit_log_branch_picker = false; From 5087a0682ba1acf4977a28147586c5e30047bb52 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Wed, 19 Aug 2026 13:50:16 +0200 Subject: [PATCH 09/10] fix(markdown): wrap long frontmatter values The value column of a frontmatter entry is a flex child, so `min-width: auto` pinned it to its unwrapped single-line width: a long `description:` painted straight past the edge of the metadata card. `min-width: 0` lets it shrink to the card and wrap inside it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DcL4r46s9HMDmrzQkLzUMo --- crates/okena-markdown/src/render.rs | 75 ++++++++++++++++++++--------- 1 file changed, 51 insertions(+), 24 deletions(-) diff --git a/crates/okena-markdown/src/render.rs b/crates/okena-markdown/src/render.rs index 6c5f7666b..47ab2cfca 100644 --- a/crates/okena-markdown/src/render.rs +++ b/crates/okena-markdown/src/render.rs @@ -568,22 +568,35 @@ impl MarkdownDocument { }; match value { + // `min-width: 0` lets the value column shrink below its unwrapped + // width so a long scalar wraps inside the card instead of painting + // past its edge. FmValue::Scalar(s) => h_flex() + .w_full() .gap(px(8.0)) .items_baseline() .child(key_label().min_w(px(120.0)).flex_shrink_0()) - .child(div().flex_1().text_color(rgb(c.body)).child(s.clone())), + .child( + div() + .flex_1() + .min_w_0() + .text_color(rgb(c.body)) + .child(s.clone()), + ), FmValue::Empty => h_flex() + .w_full() .gap(px(8.0)) .items_baseline() .child(key_label().min_w(px(120.0)).flex_shrink_0()) .child(div().italic().text_color(rgb(c.muted)).child("\u{2014}")), FmValue::List(items) => v_flex() + .w_full() .gap(px(2.0)) .child(key_label()) .child(Self::render_fm_list(items, t, cx)), - FmValue::Map(sub) => v_flex().gap(px(2.0)).child(key_label()).child( + FmValue::Map(sub) => v_flex().w_full().gap(px(2.0)).child(key_label()).child( v_flex() + .w_full() .gap(px(4.0)) .pl(px(16.0)) .children(sub.iter().map(|(k, v)| Self::render_fm_entry(k, v, t, cx))), @@ -594,35 +607,49 @@ impl MarkdownDocument { /// Render a frontmatter sequence as a bulleted, indented list. fn render_fm_list(items: &[FmValue], t: &ThemeColors, cx: &App) -> Div { let c = MdColors::new(t); - let mut list = v_flex().gap(px(2.0)).pl(px(16.0)); + let mut list = v_flex().w_full().gap(px(2.0)).pl(px(16.0)); for item in items { - list = list.child(match item { - FmValue::Scalar(s) => h_flex() - .gap(px(8.0)) - .items_baseline() - .child(div().text_color(rgb(c.muted)).child("\u{2022}")) - .child(div().text_color(rgb(c.body)).child(s.clone())), - FmValue::Empty => h_flex() - .gap(px(8.0)) - .child(div().text_color(rgb(c.muted)).child("\u{2022}")), - FmValue::List(inner) => v_flex() - .child(div().text_color(rgb(c.muted)).child("\u{2022}")) - .child(Self::render_fm_list(inner, t, cx)), - FmValue::Map(sub) => v_flex() - .gap(px(4.0)) - .child(div().text_color(rgb(c.muted)).child("\u{2022}")) - .child( + list = + list.child(match item { + FmValue::Scalar(s) => h_flex() + .w_full() + .gap(px(8.0)) + .items_baseline() + .child( + div() + .flex_shrink_0() + .text_color(rgb(c.muted)) + .child("\u{2022}"), + ) + .child( + div() + .flex_1() + .min_w_0() + .text_color(rgb(c.body)) + .child(s.clone()), + ), + FmValue::Empty => h_flex() + .gap(px(8.0)) + .child(div().text_color(rgb(c.muted)).child("\u{2022}")), + FmValue::List(inner) => v_flex() + .w_full() + .child(div().text_color(rgb(c.muted)).child("\u{2022}")) + .child(Self::render_fm_list(inner, t, cx)), + FmValue::Map(sub) => { v_flex() + .w_full() .gap(px(4.0)) - .pl(px(16.0)) - .children(sub.iter().map(|(k, v)| Self::render_fm_entry(k, v, t, cx))), - ), - }); + .child(div().text_color(rgb(c.muted)).child("\u{2022}")) + .child(v_flex().w_full().gap(px(4.0)).pl(px(16.0)).children( + sub.iter().map(|(k, v)| Self::render_fm_entry(k, v, t, cx)), + )) + } + }); } list } - /// Render inline elements with selection highlighting. + /// Render inline elements as one wrapping row of word tokens. pub(crate) fn render_inlines_with_selection( inlines: &[Inline], t: &ThemeColors, From 55be6bcb507ec9c53e7ba3f617ae50d5053b61ae Mon Sep 17 00:00:00 2001 From: David Matejka Date: Wed, 19 Aug 2026 13:50:22 +0200 Subject: [PATCH 10/10] fix(markdown): keep inlines on one line instead of breaking after each run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A paragraph was a wrapping flex row with one item per inline run, so a run long enough to wrap occupied a full-width box and whatever followed it — a code chip, the text after a bold lead-in — started on the next line. Inline elements are now flattened into that row as word tokens: text splits on word boundaries (each token keeping its trailing whitespace), and bold/italic/link hand their emphasis down to the tokens instead of wrapping them in a container. Code stays a single chip. Measured at 600px: text with code chips went from 276px to the 161px plain-text baseline, a bold lead-in from 184px to 161px. Both shapes are covered by tests. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DcL4r46s9HMDmrzQkLzUMo --- crates/okena-markdown/src/render.rs | 289 +++++++++++-------- crates/okena-markdown/tests/inline_layout.rs | 45 +++ 2 files changed, 214 insertions(+), 120 deletions(-) diff --git a/crates/okena-markdown/src/render.rs b/crates/okena-markdown/src/render.rs index 47ab2cfca..6b72a0fd5 100644 --- a/crates/okena-markdown/src/render.rs +++ b/crates/okena-markdown/src/render.rs @@ -47,6 +47,65 @@ fn highlighted_code_line( build_styled_text_with_backgrounds(spans, &bg_ranges) } +/// Emphasis a word token inherits from the inline elements enclosing it. +/// +/// Inline elements are flattened into one wrapping row rather than nested into +/// containers: a container holding a run that wraps measures as a full-width +/// box, so whatever follows it starts on a new line instead of continuing the +/// current one. Emphasis rides down to the tokens instead. +#[derive(Clone, Copy, Default)] +struct InlineStyle { + bold: bool, + italic: bool, + link: bool, +} + +impl InlineStyle { + fn apply(self, el: Div, c: &MdColors) -> Div { + el.when(self.bold, |el| el.font_weight(FontWeight::BOLD)) + .when(self.italic, |el| el.italic()) + .when(self.link, |el| el.text_color(rgb(c.link)).underline()) + } +} + +/// Split a text run into word tokens, each keeping its trailing whitespace. +/// +/// One element per word is what lets the row wrap between words. Leading +/// whitespace becomes its own token, so a break there leaves the space behind +/// on the previous line. +fn word_tokens(text: &str) -> Vec<&str> { + let mut tokens = Vec::new(); + let mut start = 0usize; + let mut prev_ws = false; + for (i, ch) in text.char_indices() { + let ws = ch.is_whitespace(); + if prev_ws && !ws && i > start { + tokens.push(&text[start..i]); + start = i; + } + prev_ws = ws; + } + if start < text.len() { + tokens.push(&text[start..]); + } + tokens +} + +/// Narrow a character selection range to the `len` characters at `offset`. +fn sub_selection( + selection: Option<(usize, usize)>, + offset: usize, + len: usize, +) -> Option<(usize, usize)> { + selection.and_then(|(s, e)| { + if e <= offset || s >= offset + len { + None + } else { + Some((s.saturating_sub(offset), (e - offset).min(len))) + } + }) +} + impl MarkdownDocument { /// Number of top-level blocks in the document. Each maps to one list item. pub fn node_count(&self) -> usize { @@ -657,31 +716,14 @@ impl MarkdownDocument { selection: Option<(usize, usize)>, ) -> Div { let mut elements: Vec
= Vec::new(); - let mut offset = 0usize; - - for inline in inlines { - let inline_len = match inline { - Inline::Text(text) => char_len(text), - Inline::Code(code) => char_len(code), - Inline::Bold(children) | Inline::Italic(children) => { - Self::inlines_text_length(children) - } - Inline::Link { children, .. } => Self::inlines_text_length(children), - }; - - let inline_sel = selection.and_then(|(s, e)| { - if e <= offset || s >= offset + inline_len { - None - } else { - Some((s.saturating_sub(offset), (e - offset).min(inline_len))) - } - }); - - elements.push(Self::render_inline_with_selection( - inline, t, cx, inline_sel, - )); - offset += inline_len; - } + Self::push_inlines( + inlines, + InlineStyle::default(), + t, + cx, + selection, + &mut elements, + ); div() .flex() @@ -699,31 +741,72 @@ impl MarkdownDocument { .children(elements) } - /// Render a single inline element with selection. - fn render_inline_with_selection( + /// Text length of one inline element, in characters. + fn inline_text_length(inline: &Inline) -> usize { + match inline { + Inline::Text(text) => char_len(text), + Inline::Code(code) => char_len(code), + Inline::Bold(children) | Inline::Italic(children) => { + Self::inlines_text_length(children) + } + Inline::Link { children, .. } => Self::inlines_text_length(children), + } + } + + /// Flatten inline elements into `out`, handing each one the slice of the + /// selection range that falls inside it. + fn push_inlines( + inlines: &[Inline], + style: InlineStyle, + t: &ThemeColors, + cx: &App, + selection: Option<(usize, usize)>, + out: &mut Vec
, + ) { + let mut offset = 0usize; + for inline in inlines { + let len = Self::inline_text_length(inline); + let inline_sel = sub_selection(selection, offset, len); + Self::push_inline(inline, style, t, cx, inline_sel, out); + offset += len; + } + } + + /// Append one inline element to `out`: text as word tokens, code as a chip, + /// emphasis as style handed down to the tokens inside it. + fn push_inline( inline: &Inline, + style: InlineStyle, t: &ThemeColors, cx: &App, selection: Option<(usize, usize)>, - ) -> Div { + out: &mut Vec
, + ) { let selection_bg = rgba(0x3390ff40); let c = MdColors::new(t); match inline { Inline::Text(text) => { - if let Some((start, end)) = selection { - let (before, selected, after) = slice_by_chars(text, start, end); - div() - .flex() - .min_w_0() - .child(div().min_w_0().child(before)) - .child(div().min_w_0().bg(selection_bg).child(selected)) - .child(div().min_w_0().child(after)) - } else { - // `min-width: 0` lets a long run shrink to the column width - // and wrap inside it. On `min-width: auto` the run keeps its - // unwrapped width and paints past the edge of the column. - div().min_w_0().child(text.clone()) + let mut offset = 0usize; + for token in word_tokens(text) { + let token_len = char_len(token); + let el = match sub_selection(selection, offset, token_len) { + Some((start, end)) => { + let (before, selected, after) = slice_by_chars(token, start, end); + div() + .flex() + .min_w_0() + .child(div().min_w_0().child(before)) + .child(div().min_w_0().bg(selection_bg).child(selected)) + .child(div().min_w_0().child(after)) + } + // `min-width: 0` lets one very long word (a bare URL, + // say) shrink to the column width and wrap inside it + // instead of painting past the edge. + None => div().min_w_0().child(token.to_string()), + }; + out.push(style.apply(el, &c)); + offset += token_len; } } Inline::Code(code) => { @@ -737,86 +820,52 @@ impl MarkdownDocument { .bg(rgb(c.inline_code_bg)) .text_color(rgb(c.body)) }; - if let Some((start, end)) = selection { - let (before, selected, after) = slice_by_chars(code, start, end); - chip(div()) - .flex() - .child(div().child(before)) - .child(div().bg(selection_bg).child(selected)) - .child(div().child(after)) - } else { - chip(div()).child(code.clone()) - } - } - Inline::Bold(children) => { - let mut container = div().font_weight(FontWeight::BOLD).flex().flex_wrap(); - let mut offset = 0usize; - for child in children { - let child_len = match child { - Inline::Text(t) => char_len(t), - Inline::Code(c) => char_len(c), - Inline::Bold(ch) | Inline::Italic(ch) => Self::inlines_text_length(ch), - Inline::Link { children: ch, .. } => Self::inlines_text_length(ch), - }; - let child_sel = selection.and_then(|(s, e)| { - if e <= offset || s >= offset + child_len { - None - } else { - Some((s.saturating_sub(offset), (e - offset).min(child_len))) - } - }); - container = container - .child(Self::render_inline_with_selection(child, t, cx, child_sel)); - offset += child_len; - } - container - } - Inline::Italic(children) => { - let mut container = div().italic().flex().flex_wrap(); - let mut offset = 0usize; - for child in children { - let child_len = match child { - Inline::Text(t) => char_len(t), - Inline::Code(c) => char_len(c), - Inline::Bold(ch) | Inline::Italic(ch) => Self::inlines_text_length(ch), - Inline::Link { children: ch, .. } => Self::inlines_text_length(ch), - }; - let child_sel = selection.and_then(|(s, e)| { - if e <= offset || s >= offset + child_len { - None - } else { - Some((s.saturating_sub(offset), (e - offset).min(child_len))) - } - }); - container = container - .child(Self::render_inline_with_selection(child, t, cx, child_sel)); - offset += child_len; - } - container - } - Inline::Link { children, .. } => { - let mut container = div().text_color(rgb(c.link)).underline().flex().flex_wrap(); - let mut offset = 0usize; - for child in children { - let child_len = match child { - Inline::Text(t) => char_len(t), - Inline::Code(c) => char_len(c), - Inline::Bold(ch) | Inline::Italic(ch) => Self::inlines_text_length(ch), - Inline::Link { children: ch, .. } => Self::inlines_text_length(ch), - }; - let child_sel = selection.and_then(|(s, e)| { - if e <= offset || s >= offset + child_len { - None - } else { - Some((s.saturating_sub(offset), (e - offset).min(child_len))) - } - }); - container = container - .child(Self::render_inline_with_selection(child, t, cx, child_sel)); - offset += child_len; - } - container + let el = match selection { + Some((start, end)) => { + let (before, selected, after) = slice_by_chars(code, start, end); + chip(div()) + .flex() + .child(div().child(before)) + .child(div().bg(selection_bg).child(selected)) + .child(div().child(after)) + } + None => chip(div()).child(code.clone()), + }; + out.push(style.apply(el, &c)); } + Inline::Bold(children) => Self::push_inlines( + children, + InlineStyle { + bold: true, + ..style + }, + t, + cx, + selection, + out, + ), + Inline::Italic(children) => Self::push_inlines( + children, + InlineStyle { + italic: true, + ..style + }, + t, + cx, + selection, + out, + ), + Inline::Link { children, .. } => Self::push_inlines( + children, + InlineStyle { + link: true, + ..style + }, + t, + cx, + selection, + out, + ), } } diff --git a/crates/okena-markdown/tests/inline_layout.rs b/crates/okena-markdown/tests/inline_layout.rs index 9d8c54382..7d80917f8 100644 --- a/crates/okena-markdown/tests/inline_layout.rs +++ b/crates/okena-markdown/tests/inline_layout.rs @@ -73,3 +73,48 @@ fn paragraph_with_long_trailing_text_is_not_inflated(cx: &mut TestAppContext) { "paragraph block height {height}px is inflated (expected a handful of text lines)" ); } + +/// Long text runs separated by inline code — the shape where a run that wraps +/// used to occupy a full-width box and push the chip after it onto a new line. +const MIXED: &str = "If it is a one-way door with a wide blast radius, data migration, \ + public API or security model, say so and offer `comprehensive-plan` instead. \ + Anything that is hard to unship deserves the slower path, so reach for \ + `comprehensive-plan` again. When the change is cheap to revert and the blast \ + radius is small, `weigh-options` is enough on its own. Deliberation that costs \ + more than the mistake is waste, and `weigh-smallest` cuts it down."; + +/// The same prose without the code spans, as the baseline line count. +const MIXED_PLAIN: &str = "If it is a one-way door with a wide blast radius, data migration, \ + public API or security model, say so and offer comprehensive-plan instead. \ + Anything that is hard to unship deserves the slower path, so reach for \ + comprehensive-plan again. When the change is cheap to revert and the blast \ + radius is small, weigh-options is enough on its own. Deliberation that costs \ + more than the mistake is waste, and weigh-smallest cuts it down."; + +#[gpui::test] +fn inline_code_does_not_force_a_line_break(cx: &mut TestAppContext) { + // Pre-fix this was 276px against a 161px baseline — five lines lost to the + // break each chip forced after the run before it. + let mixed = measure_block_height(cx, MIXED, 600.0); + let plain = measure_block_height(cx, MIXED_PLAIN, 600.0); + assert!( + mixed <= plain, + "inline code added lines: {mixed}px against a {plain}px plain-text baseline" + ); +} + +#[gpui::test] +fn bold_does_not_force_a_line_break(cx: &mut TestAppContext) { + // Bold leads, plain text follows: the text after it used to start on its own + // line (184px against the same 161px baseline). + let bold = measure_block_height( + cx, + &format!("**The right one is X** — {MIXED_PLAIN}"), + 600.0, + ); + let plain = measure_block_height(cx, &format!("The right one is X — {MIXED_PLAIN}"), 600.0); + assert!( + bold <= plain, + "bold added lines: {bold}px against a {plain}px plain-text baseline" + ); +}