diff --git a/src/client/mod.rs b/src/client/mod.rs index 95a723cc..24f4f995 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -1287,6 +1287,9 @@ fn run_client_with_mode( let socket_path = client_socket_path(); let shell_config = client_rendered_shell.then(|| { shell::ClientShellConfig::from_config(&loaded_config.config) + .with_startup_config_diagnostic(crate::config::config_diagnostic_summary( + &loaded_config.diagnostics, + )) .with_local_endpoint(&socket_path) }); let mouse_capture = loaded_config.config.ui.mouse_capture; diff --git a/src/client/shell.rs b/src/client/shell.rs index 1e82bfdb..7223c437 100644 --- a/src/client/shell.rs +++ b/src/client/shell.rs @@ -277,6 +277,7 @@ mod tests { ClientShellSnapshot { boot_id: "boot-1".into(), revision: 1, + config_diagnostic: None, focused_workspace_id: Some("ws_1".into()), focused_tab_id: Some("tab_1".into()), focused_pane_id: Some("pane_1".into()), @@ -489,6 +490,124 @@ mod tests { ); } + #[test] + fn startup_config_diagnostics_are_client_rendered_and_persist_until_replaced() { + let config = ClientShellConfig::from_config(&Config::default()) + .with_startup_config_diagnostic(Some("local config warning".into())); + let mut state = ClientShellState::new(config); + let mut shared_snapshot = snapshot(); + shared_snapshot.config_diagnostic = Some("local config warning".into()); + state.set_snapshot(Box::new(shared_snapshot)); + assert_eq!( + state.config_diagnostic.as_deref(), + Some("client + endpoint: local config warning") + ); + + let mut endpoint_snapshot = snapshot(); + endpoint_snapshot.config_diagnostic = Some("endpoint config warning".into()); + state.set_snapshot(Box::new(endpoint_snapshot)); + state.set_pane_surface(surface()); + + let frame = state.compose(106, 20).expect("diagnostic frame"); + let text = frame + .cells + .chunks(frame.width as usize) + .map(|row| { + row.iter() + .map(|cell| cell.symbol.as_str()) + .collect::() + }) + .collect::>() + .join("\n"); + assert!(text.contains("client: local config warning")); + assert!(text.contains("endpoint: endpoint config warning")); + + state.handle_input_bytes(b"x"); + assert!(state.config_diagnostic.is_some()); + + state.set_snapshot(Box::new(snapshot())); + assert_eq!( + state.config_diagnostic.as_deref(), + Some("local config warning") + ); + } + + #[test] + fn config_diagnostic_offsets_only_the_pane_rows_it_overlaps() { + let mut config = ClientShellConfig::from_config(&Config::default()); + config.toast_delay_seconds = 0; + let mut state = ClientShellState::new(config); + let mut endpoint_snapshot = snapshot(); + endpoint_snapshot.config_diagnostic = Some("one-line warning".into()); + state.set_snapshot(Box::new(endpoint_snapshot)); + state.set_pane_surface(surface()); + state.visible_notification = Some(ClientVisibleNotification { + event: SemanticNotification { + kind: SemanticNotificationKind::Custom, + title: "notification".into(), + body: None, + sound: None, + agent: None, + workspace_id: None, + tab_id: None, + pane_id: None, + position: Some(crate::config::ToastHerdrPosition::TopRight), + }, + deadline: std::time::Instant::now(), + }); + + state.compose(106, 20).expect("one-line frame"); + let pane_area = state.layout(106, 20).pane_surface; + assert_eq!(state.hits.notification_toast.y, pane_area.y); + + let mut endpoint_snapshot = snapshot(); + endpoint_snapshot.config_diagnostic = Some("first warning\nsecond warning".into()); + state.set_snapshot(Box::new(endpoint_snapshot)); + state.compose(106, 20).expect("two-line frame"); + assert_eq!(state.hits.notification_toast.y, pane_area.y + 1); + } + + #[test] + fn endpoint_reload_result_does_not_override_snapshot_diagnostic_authority() { + let mut state = ClientShellState::new(ClientShellConfig::from_config(&Config::default())); + let mut endpoint_snapshot = snapshot(); + endpoint_snapshot.config_diagnostic = Some("endpoint warning".into()); + state.set_snapshot(Box::new(endpoint_snapshot)); + state.pending_requests.insert( + "reload-1".into(), + PendingEndpointRequest { + boot_id: "boot-1".into(), + confirmation_workspace_id: None, + kind: PendingEndpointKind::ReloadConfig, + }, + ); + + state.handle_endpoint_result( + "boot-1", + "reload-1", + Ok(crate::api::schema::ResponseResult::ConfigReload { + status: crate::config::ConfigReloadStatus::Partial, + diagnostics: vec!["keybinding warning".into()], + }), + ); + assert_eq!(state.config_diagnostic.as_deref(), Some("endpoint warning")); + + state.set_snapshot(Box::new(snapshot())); + assert!(state.config_diagnostic.is_none()); + } + + #[test] + fn live_client_config_keeps_sound_diagnostics() { + let mut shell_config = ClientShellConfig::from_config(&Config::default()); + let mut config = Config::default(); + config.ui.sound.path = Some(std::path::PathBuf::from("invalid.wav")); + + let diagnostics = shell_config.apply_live_config(&config, &[], &[]); + assert!(diagnostics + .iter() + .any(|diagnostic| diagnostic.contains("expected an mp3 file"))); + } + #[test] fn client_composes_popup_terminal_content_inside_client_owned_chrome() { let mut state = ClientShellState::new(ClientShellConfig::from_config(&Config::default())); diff --git a/src/client/shell/actions.rs b/src/client/shell/actions.rs index 320807ea..e504c134 100644 --- a/src/client/shell/actions.rs +++ b/src/client/shell/actions.rs @@ -548,13 +548,6 @@ impl ClientShellState { } PendingEndpointKind::ReloadConfig => { let repaint = match result { - Ok(crate::api::schema::ResponseResult::ConfigReload { - diagnostics, .. - }) if !diagnostics.is_empty() => { - self.endpoint_error = - crate::config::config_diagnostic_summary(&diagnostics); - true - } Ok(crate::api::schema::ResponseResult::ConfigReload { .. }) => false, Ok(_) => { self.endpoint_error = diff --git a/src/client/shell/composition.rs b/src/client/shell/composition.rs index 23f4c66c..49c859a1 100644 --- a/src/client/shell/composition.rs +++ b/src/client/shell/composition.rs @@ -1,5 +1,23 @@ use super::*; +fn diagnostic_overlap_rows(diagnostic_area: Rect, target: Rect, rendered_rows: u16) -> u16 { + let diagnostic_bottom = diagnostic_area + .y + .saturating_add(rendered_rows.min(diagnostic_area.height)); + diagnostic_bottom + .min(target.bottom()) + .saturating_sub(diagnostic_area.y.max(target.y)) +} + +fn clipboard_feedback_starts_at_top(position: crate::config::ToastClipboardPosition) -> bool { + matches!( + position, + crate::config::ToastClipboardPosition::TopLeft + | crate::config::ToastClipboardPosition::TopCenter + | crate::config::ToastClipboardPosition::TopRight + ) +} + impl ClientShellState { pub(crate) fn compose(&mut self, cols: u16, rows: u16) -> Option { let snapshot = self.snapshot.as_deref()?; @@ -246,26 +264,58 @@ impl ClientShellState { } } self.hits.notification_toast = Rect::default(); - if let Some(notification) = self.visible_notification.as_ref() { + let mut diagnostic_pane_overlap = 0; + if self.config_diagnostic.is_some() || self.visible_notification.is_some() { let cursor = frame.cursor.clone(); let mut composed = frame.to_ratatui_buffer()?; - self.hits.notification_toast = notifications::render_visible_notification( - &mut composed, - layout.pane_surface, - notification, - self.config.toast_position, - &self.config.palette, - ); + if let Some(diagnostic) = self.config_diagnostic.as_deref() { + let diagnostic_area = if layout.mobile_header.is_empty() { + Rect::new(0, 0, cols, rows) + } else { + layout.pane_surface + }; + let rendered_rows = crate::ui::render_config_diagnostic_buffer( + &mut composed, + diagnostic_area, + diagnostic, + &self.config.palette, + ); + diagnostic_pane_overlap = + diagnostic_overlap_rows(diagnostic_area, layout.pane_surface, rendered_rows); + } + if let Some(notification) = self.visible_notification.as_ref() { + self.hits.notification_toast = notifications::render_visible_notification( + &mut composed, + layout.pane_surface, + notification, + self.config.toast_position, + diagnostic_pane_overlap, + &self.config.palette, + ); + } frame.replace_from_ratatui_buffer_preserving_effects(&composed, cursor); } if let Some(feedback) = self.copy_feedback.as_ref() { let cursor = frame.cursor.clone(); let mut composed = frame.to_ratatui_buffer()?; + let base_offset = + if clipboard_feedback_starts_at_top(self.config.clipboard_toast_position) { + diagnostic_pane_overlap + } else { + 0 + }; + let offset = crate::ui::copy_feedback_offset_for_toast( + layout.pane_surface, + feedback, + base_offset, + self.config.clipboard_toast_position, + self.hits.notification_toast, + ); crate::ui::render_copy_feedback_buffer( &mut composed, layout.pane_surface, feedback, - 0, + offset, self.config.clipboard_toast_position, &self.config.palette, ); diff --git a/src/client/shell/config.rs b/src/client/shell/config.rs index 555295dd..49c9348f 100644 --- a/src/client/shell/config.rs +++ b/src/client/shell/config.rs @@ -1,6 +1,31 @@ use super::*; +pub(super) fn merged_config_diagnostic( + local: Option<&str>, + endpoint: Option<&str>, +) -> Option { + match (local, endpoint) { + (Some(local), Some(endpoint)) if local == endpoint => { + Some(format!("client + endpoint: {local}")) + } + (Some(local), Some(endpoint)) => Some(format!("client: {local}\nendpoint: {endpoint}")), + (Some(local), None) => Some(local.to_owned()), + (None, Some(endpoint)) => Some(endpoint.to_owned()), + (None, None) => None, + } +} + impl ClientShellState { + fn set_local_config_diagnostic(&mut self, diagnostic: Option) { + self.local_config_diagnostic = diagnostic; + self.config_diagnostic = merged_config_diagnostic( + self.local_config_diagnostic.as_deref(), + self.snapshot + .as_deref() + .and_then(|snapshot| snapshot.config_diagnostic.as_deref()), + ); + } + pub(super) fn persist_chrome_preferences(&mut self, outcome: &mut ClientShellInput) { let Some(path) = self.config.preferences_path.as_deref() else { return; @@ -38,10 +63,14 @@ impl ClientShellState { if self.agent_panel_sort_manual { self.config.agent_panel_sort = agent_panel_sort; } - self.endpoint_error = crate::config::config_diagnostic_summary(&diagnostics); + self.set_local_config_diagnostic(crate::config::config_diagnostic_summary( + &diagnostics, + )); } Err(diagnostics) => { - self.endpoint_error = crate::config::config_diagnostic_summary(&diagnostics); + self.set_local_config_diagnostic(crate::config::config_diagnostic_summary( + &diagnostics, + )); } } } @@ -91,9 +120,15 @@ impl ClientShellConfig { ), preferences_path: None, preferences: preferences::ClientChromePreferences::default(), + startup_config_diagnostic: None, } } + pub(crate) fn with_startup_config_diagnostic(mut self, diagnostic: Option) -> Self { + self.startup_config_diagnostic = diagnostic; + self + } + pub(crate) fn with_local_endpoint(self, socket_path: &std::path::Path) -> Self { self.with_preferences_path(preferences::path_for_local_endpoint(socket_path)) } @@ -133,6 +168,7 @@ impl ClientShellConfig { diagnostics.push(format!("{diagnostic}; keeping previous [ui] settings")); } else { let ui = &config.ui; + diagnostics.extend(ui.sound.diagnostics()); self.sidebar_width = ui.sidebar_width; self.sidebar_min_width = ui.sidebar_min_width; self.sidebar_max_width = ui.sidebar_max_width; diff --git a/src/client/shell/notifications.rs b/src/client/shell/notifications.rs index b189c0dc..1a349567 100644 --- a/src/client/shell/notifications.rs +++ b/src/client/shell/notifications.rs @@ -9,6 +9,7 @@ pub(super) fn render_visible_notification( area: Rect, notification: &ClientVisibleNotification, default_position: crate::config::ToastHerdrPosition, + top_offset: u16, palette: &Palette, ) -> Rect { if area.is_empty() { @@ -32,7 +33,9 @@ pub(super) fn render_visible_notification( }; let y = match position { crate::config::ToastHerdrPosition::TopLeft - | crate::config::ToastHerdrPosition::TopRight => area.y, + | crate::config::ToastHerdrPosition::TopRight => area + .y + .saturating_add(top_offset.min(area.height.saturating_sub(height))), crate::config::ToastHerdrPosition::BottomLeft | crate::config::ToastHerdrPosition::BottomRight => area.bottom().saturating_sub(height), }; diff --git a/src/client/shell/state.rs b/src/client/shell/state.rs index a2aba444..d1e1e765 100644 --- a/src/client/shell/state.rs +++ b/src/client/shell/state.rs @@ -37,6 +37,7 @@ pub(crate) struct ClientShellConfig { pub(super) worktree_directory: std::path::PathBuf, pub(super) preferences_path: Option, pub(super) preferences: preferences::ClientChromePreferences, + pub(super) startup_config_diagnostic: Option, } #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -766,6 +767,8 @@ pub(crate) struct ClientShellState { pub(super) pending_notifications: Vec, pub(super) visible_notification: Option, pub(super) outer_focused: Option, + pub(super) local_config_diagnostic: Option, + pub(super) config_diagnostic: Option, pub(super) endpoint_error: Option, } @@ -779,6 +782,7 @@ pub(super) struct WorkspaceEntry { impl ClientShellState { pub(crate) fn new(mut config: ClientShellConfig) -> Self { let preferences = config.preferences; + let local_config_diagnostic = config.startup_config_diagnostic.take(); let sidebar_collapsed = preferences .sidebar_collapsed .unwrap_or(config.sidebar_start_collapsed); @@ -855,6 +859,8 @@ impl ClientShellState { pending_notifications: Vec::new(), visible_notification: None, outer_focused: None, + config_diagnostic: local_config_diagnostic.clone(), + local_config_diagnostic, endpoint_error: None, } } @@ -891,6 +897,10 @@ impl ClientShellState { } pub(crate) fn set_snapshot(&mut self, snapshot: Box) { + self.config_diagnostic = super::config::merged_config_diagnostic( + self.local_config_diagnostic.as_deref(), + snapshot.config_diagnostic.as_deref(), + ); if self .pane_surface .as_ref() diff --git a/src/protocol/wire.rs b/src/protocol/wire.rs index 3d12cdb4..14850eb5 100644 --- a/src/protocol/wire.rs +++ b/src/protocol/wire.rs @@ -796,6 +796,8 @@ pub struct ClientShellSnapshot { pub boot_id: String, /// Monotonic replacement revision within one endpoint boot. pub revision: u64, + /// Endpoint startup/reload config warning, filtered for client-owned keybindings. + pub config_diagnostic: Option, pub focused_workspace_id: Option, pub focused_tab_id: Option, pub focused_pane_id: Option, @@ -1916,6 +1918,7 @@ mod tests { let msg = ServerMessage::ClientShellSnapshot(Box::new(ClientShellSnapshot { boot_id: "boot-1".into(), revision: 1, + config_diagnostic: Some("endpoint config warning".into()), focused_workspace_id: Some("w1".into()), focused_tab_id: Some("w1:t1".into()), focused_pane_id: Some("w1:p1".into()), diff --git a/src/server/client_shell.rs b/src/server/client_shell.rs index 80f1d67c..dc64e518 100644 --- a/src/server/client_shell.rs +++ b/src/server/client_shell.rs @@ -7,6 +7,7 @@ pub(super) fn snapshot( app: &app::App, boot_id: &str, revision: u64, + config_diagnostic: Option<&str>, ) -> protocol::ClientShellSnapshot { let snapshot = app.session_snapshot(); let workspaces = snapshot @@ -148,6 +149,7 @@ pub(super) fn snapshot( protocol::ClientShellSnapshot { boot_id: boot_id.to_owned(), revision, + config_diagnostic: config_diagnostic.map(str::to_owned), focused_workspace_id: snapshot.focused_workspace_id, focused_tab_id: snapshot.focused_tab_id, focused_pane_id: snapshot.focused_pane_id, diff --git a/src/server/headless.rs b/src/server/headless.rs index 0de220e2..42a21f59 100644 --- a/src/server/headless.rs +++ b/src/server/headless.rs @@ -3293,6 +3293,7 @@ impl HeadlessServer { &self.app, &self.client_shell_boot_id, connection.shell_projection_revision, + self.server_config_diagnostic_without_keybindings.as_deref(), ); connection.shell_snapshot = Some(snapshot.clone()); self.clients.insert(client_id, connection); @@ -4862,7 +4863,14 @@ impl HeadlessServer { let shell_snapshot_template = render_targets .iter() .any(|(_, _, _, _, mode)| matches!(mode, ClientConnectionMode::ClientShell)) - .then(|| client_shell_snapshot(&self.app, &self.client_shell_boot_id, 0)); + .then(|| { + client_shell_snapshot( + &self.app, + &self.client_shell_boot_id, + 0, + self.server_config_diagnostic_without_keybindings.as_deref(), + ) + }); let mut broken_clients: Vec = Vec::new(); let mut deferred_frame = false; for (client_id, (cols, rows), cell_size, is_foreground, mode) in render_targets { @@ -5529,6 +5537,10 @@ fn server_config_diagnostic_summaries(diagnostics: &[String]) -> (Option } fn is_keybinding_config_diagnostic(diagnostic: &str) -> bool { + if diagnostic.starts_with("config parse error:") || diagnostic.starts_with("config read error:") + { + return false; + } diagnostic.contains("keybinding") || diagnostic.contains("keys.") } @@ -6510,6 +6522,8 @@ mod tests { server.app.state.active = Some(0); server.app.state.selected = 0; server.app.state.mode = crate::app::Mode::Terminal; + server.server_config_diagnostic_without_keybindings = + Some("endpoint config warning".into()); let (writer, control_rx, render_rx) = test_client_writer(); assert!( @@ -6527,6 +6541,10 @@ mod tests { ServerMessage::ClientShellSnapshot(snapshot) => { assert_eq!(snapshot.workspaces.len(), 1); assert_eq!(snapshot.workspaces[0].label, "shell-only-label"); + assert_eq!( + snapshot.config_diagnostic.as_deref(), + Some("endpoint config warning") + ); } other => panic!("expected client shell snapshot, got {other:?}"), } @@ -7085,6 +7103,19 @@ new_tab = "prefix+t" .any(|binding| binding.label == "prefix+c")); } + #[test] + fn server_keybinding_filter_keeps_whole_config_failures() { + assert!(!is_keybinding_config_diagnostic( + "config parse error: invalid value at `keys.new_tab = @`; using defaults" + )); + assert!(!is_keybinding_config_diagnostic( + "config read error: permission denied at keys.toml; using defaults" + )); + assert!(is_keybinding_config_diagnostic( + "unsafe direct keybinding: keys.close_pane would intercept typing" + )); + } + #[test] fn local_keybinding_client_hides_server_keybinding_warnings() { let mut server = test_headless_server(); diff --git a/src/ui.rs b/src/ui.rs index a169ab90..30703335 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -56,11 +56,11 @@ pub(crate) use self::sidebar::agent_panel_entries_from; #[cfg(test)] pub(crate) use self::sidebar::workspace_drop_indicator_row; use self::sidebar::{render_sidebar, render_sidebar_collapsed}; -pub(crate) use self::status::render_copy_feedback_buffer; use self::status::{ copy_feedback_rect, render_config_diagnostic, render_copy_feedback, render_toast_notification, toast_notification_rect, }; +pub(crate) use self::status::{render_config_diagnostic_buffer, render_copy_feedback_buffer}; pub(crate) use self::tab_surface::{ compute_tab_surface, render_tab_surface, resize_tab_surface, TabSurfaceLayout, }; @@ -550,7 +550,7 @@ fn render_notifications(app: &AppState, frame: &mut Frame, terminal_area: Rect) } } -fn copy_feedback_offset_for_toast( +pub(crate) fn copy_feedback_offset_for_toast( area: Rect, feedback: &crate::app::state::CopyFeedback, base_offset: u16, diff --git a/src/ui/status.rs b/src/ui/status.rs index 83a74176..50d283da 100644 --- a/src/ui/status.rs +++ b/src/ui/status.rs @@ -3,7 +3,7 @@ use ratatui::{ layout::{Constraint, Layout, Rect}, style::{Color, Modifier, Style}, text::{Line, Span}, - widgets::{Block, Borders, Clear, Paragraph}, + widgets::{Block, Borders, Clear, Paragraph, Widget}, Frame, }; @@ -180,10 +180,20 @@ pub(crate) fn render_copy_feedback_buffer( } pub(super) fn render_config_diagnostic(frame: &mut Frame, area: Rect, message: &str, p: &Palette) { + render_config_diagnostic_buffer(frame.buffer_mut(), area, message, p); +} + +pub(crate) fn render_config_diagnostic_buffer( + buffer: &mut Buffer, + area: Rect, + message: &str, + p: &Palette, +) -> u16 { let style = Style::default() .fg(panel_contrast_fg(p)) .bg(p.yellow) .add_modifier(Modifier::BOLD); + let mut rendered_rows = 0u16; for (row, line) in message .lines() @@ -193,16 +203,19 @@ pub(super) fn render_config_diagnostic(frame: &mut Frame, area: Rect, message: & { let text = format!(" {line} "); let width = (text.len() as u16).min(area.width); - let notif_area = Rect::new( + let diagnostic_area = Rect::new( area.x + area.width.saturating_sub(width), area.y + row as u16, width, 1, ); - frame.render_widget(Clear, notif_area); - frame.render_widget(Paragraph::new(Span::styled(text, style)), notif_area); + Clear.render(diagnostic_area, buffer); + Paragraph::new(Span::styled(text, style)).render(diagnostic_area, buffer); + rendered_rows = rendered_rows.saturating_add(1); } + + rendered_rows } pub(super) fn state_icon_symbol(