diff --git a/README.md b/README.md index 963a852..32d5169 100644 --- a/README.md +++ b/README.md @@ -293,6 +293,8 @@ Native LSP currently provides synchronization, diagnostics, go-to-definition, an By default, vamp loads `$XDG_CONFIG_HOME/vamp/config.rb`, or `~/.config/vamp/config.rb` when that environment variable is unset. `--config path` selects a file explicitly; `--no-config` bypasses Ruby entirely. Set `VAMP_RUBY` to select a different Ruby executable. +Rejected configuration opens a centered floating **Configuration error** window, sized like the floating file explorer. It shows the selected config path, full failure reason, and explains that default settings and bindings are in use. The diagnostic remains visible even if background messages arrive; editing keys and paste are ignored until **Enter** or **Escape** dismisses it. Use arrows (or Ctrl+h/j/k/l) to scroll long paths/reasons horizontally or vertically, and **Home** to reset the view. Correct the config and restart vamp to apply it. Invalid individual preferences/bindings that do not reject the whole configuration still report ordinary notices. + ```ruby Vamp.configure do |config| config.leader = "space" diff --git a/src/editor.rs b/src/editor.rs index 45026fc..3056b66 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -62,6 +62,13 @@ pub struct Palette { pub selected: usize, } +pub struct StartupDiagnostic { + pub lines: Vec, + pub top: usize, + pub left: usize, + max_column: usize, +} + pub struct Hover { text: String, line_starts: Vec, @@ -878,6 +885,7 @@ pub struct Editor { leader_display: String, palette: Option, hover: Option, + startup_diagnostic: Option, project_root: PathBuf, file_picker: Option, file_explorer_side: DockSide, @@ -936,6 +944,7 @@ impl Editor { leader_display: String::new(), palette: None, hover: None, + startup_diagnostic: None, project_root: std::env::current_dir()?, file_picker: None, file_explorer_side: DockSide::Right, @@ -1217,6 +1226,25 @@ impl Editor { self.palette.as_ref() } + pub fn startup_diagnostic(&self) -> Option<&StartupDiagnostic> { + self.startup_diagnostic.as_ref() + } + + pub(crate) fn show_startup_diagnostic(&mut self, text: String) { + let lines = split_lines(&text); + let max_column = lines + .iter() + .map(|line| line.graphemes(true).count()) + .max() + .unwrap_or(0); + self.startup_diagnostic = Some(StartupDiagnostic { + lines, + top: 0, + left: 0, + max_column, + }); + } + /// Borrowed popup state, visible only while its source remains current. pub fn hover(&self) -> Option<&Hover> { self.hover.as_ref().filter(|hover| { @@ -1981,6 +2009,30 @@ impl Editor { Input::Ctrl(ch) => control_key(ch), input => input, }; + if let Some(diagnostic) = &mut self.startup_diagnostic { + match input { + Input::Enter | Input::Escape => self.startup_diagnostic = None, + Input::Up | Input::Ctrl('k') => diagnostic.top = diagnostic.top.saturating_sub(1), + Input::Down | Input::Ctrl('j') => { + diagnostic.top = diagnostic + .top + .saturating_add(1) + .min(diagnostic.lines.len() - 1) + } + Input::Left | Input::Ctrl('h') => { + diagnostic.left = diagnostic.left.saturating_sub(1) + } + Input::Right | Input::Ctrl('l') => { + diagnostic.left = diagnostic.left.saturating_add(1).min(diagnostic.max_column) + } + Input::Home => { + diagnostic.top = 0; + diagnostic.left = 0; + } + _ => {} + } + return None; + } if self.handle_hover(&input) { return None; } diff --git a/src/lsp_config.rs b/src/lsp_config.rs index 60f850d..4ddfb71 100644 --- a/src/lsp_config.rs +++ b/src/lsp_config.rs @@ -1,5 +1,4 @@ use crate::lsp::ServerConfig; -use serde::Deserialize; use serde_json::Value; pub fn resolve(value: Value) -> Result, String> { @@ -81,12 +80,6 @@ pub fn resolve(value: Value) -> Result, String> { Ok(servers) } -pub(crate) fn deserialize<'de, D: serde::Deserializer<'de>>( - deserializer: D, -) -> Result, D::Error> { - resolve(Value::deserialize(deserializer)?).map_err(serde::de::Error::custom) -} - fn command(value: Value, language: &str) -> Result, String> { let error = || { format!( diff --git a/src/ruby.rs b/src/ruby.rs index 12cc31a..8f31465 100644 --- a/src/ruby.rs +++ b/src/ruby.rs @@ -95,7 +95,8 @@ pub fn load_config(path: &Path) -> Result { }); } config.bindings = bindings; - config.lsp = lsp; + config.lsp = + crate::lsp_config::resolve(lsp).map_err(|error| format!("Ruby config: {error}"))?; config.leader = leader; config.file_explorer_side = file_explorer_side; config.file_picker_view = file_picker_view; @@ -379,8 +380,9 @@ enum Response { Config { version: u8, plugins: Vec, - #[serde(deserialize_with = "crate::lsp_config::deserialize")] - lsp: Vec, + // Keep this wire field required; resolve its configuration after decoding. + #[serde(deserialize_with = "serde_json::Value::deserialize")] + lsp: serde_json::Value, bindings: Vec, leader: String, file_explorer_side: String, @@ -1085,33 +1087,40 @@ end"#, } let path = fixture.file( "invalid.rb", - r#"Vamp.configure { |config| config.lsp["unlisted"] = true }"#, + r#"Vamp.configure { |config| config.lsp["ruby"] = true }"#, ); let error = load_config(&path).unwrap_err(); + assert!(!error.contains("Malformed Ruby protocol frame"), "{error}"); assert!( - error.contains("unlisted") && error.contains("preset"), + error.contains("\"ruby\"") && error.contains("command") && error.contains("extensions"), "{error}" ); } #[test] - fn lsp_wire_configuration_is_validated_before_being_consumed() { + fn lsp_wire_configuration_is_resolved_after_decoding() { let mut frame = serde_json::json!({ "version": 1, "type": "config", "plugins": [], "lsp": {}, "bindings": [], "leader": "space", "file_explorer_side": "right", "file_picker_view": "tree", "file_explorer_view": "tree", "clipboard_mode": "system" }); - assert!(matches!( - serde_json::from_value::(frame.clone()).unwrap(), - Response::Config { lsp, .. } if lsp.is_empty() - )); + let Response::Config { lsp, .. } = + serde_json::from_value::(frame.clone()).unwrap() + else { + panic!("expected config frame"); + }; + assert!(crate::lsp_config::resolve(lsp).unwrap().is_empty()); frame["lsp"] = serde_json::json!({"rust": true}); - assert!(matches!( - serde_json::from_value::(frame.clone()).unwrap(), - Response::Config { lsp, .. } - if lsp.len() == 1 && lsp[0].command == ["rust-analyzer"] - )); + let Response::Config { lsp, .. } = + serde_json::from_value::(frame.clone()).unwrap() + else { + panic!("expected config frame"); + }; + assert_eq!( + crate::lsp_config::resolve(lsp).unwrap()[0].command, + ["rust-analyzer"] + ); for value in [ serde_json::json!(null), serde_json::json!([]), @@ -1127,10 +1136,12 @@ end"#, serde_json::json!({"rust": {"unexpected": true}}), ] { frame["lsp"] = value; - assert!( - serde_json::from_value::(frame.clone()).is_err(), - "{frame}" - ); + let Response::Config { lsp, .. } = + serde_json::from_value::(frame.clone()).unwrap() + else { + panic!("expected config frame"); + }; + assert!(crate::lsp_config::resolve(lsp).is_err(), "{frame}"); } let languages: serde_json::Map = (0..17) .map(|index| { @@ -1141,11 +1152,50 @@ end"#, }) .collect(); frame["lsp"] = serde_json::Value::Object(languages); - assert!(serde_json::from_value::(frame.clone()).is_err()); + let Response::Config { lsp, .. } = + serde_json::from_value::(frame.clone()).unwrap() + else { + panic!("expected config frame"); + }; + assert!(crate::lsp_config::resolve(lsp).is_err()); frame.as_object_mut().unwrap().remove("lsp"); assert!(serde_json::from_value::(frame).is_err()); } + #[test] + fn malformed_config_wire_remains_a_protocol_error() { + let fixture = Fixture::new(); + for mutation in [ + "return '['", + "frame.delete('lsp')", + "frame['leader'] = 7", + "frame['unexpected'] = true", + ] { + let source = format!( + r#" +Vamp.configure {{ |config| config.lsp["ruby"] = true }} +module JSON + class << self + alias_method :original_generate, :generate + def generate(frame) + if frame['type'] == 'config' + {mutation} + end + original_generate(frame) + end + end +end +"# + ); + let path = fixture.file("config.rb", &source); + let error = load_config(&path).unwrap_err(); + assert!( + error.starts_with("Malformed Ruby protocol frame: "), + "{mutation}: {error}" + ); + } + } + #[test] fn crash_hang_callback_failure_and_fresh_host_recovery() { let fixture = Fixture::new(); diff --git a/src/session.rs b/src/session.rs index 4e7d045..a4e6d59 100644 --- a/src/session.rs +++ b/src/session.rs @@ -46,8 +46,8 @@ pub struct EditorSession { } impl EditorSession { - /// File errors fail opening; config and plugin errors leave core editing - /// available and are reported in the editor's message. + /// File errors fail opening; rejected configuration leaves core editing + /// available and presents a persistent startup diagnostic. pub fn open( file: Option, capabilities: Vec, @@ -57,7 +57,10 @@ impl EditorSession { let mut notices = Vec::new(); let config = match config { Some(path) => load_config(path).unwrap_or_else(|error| { - notices.push(format!("config disabled: {error}")); + editor.show_startup_diagnostic(format!( + "{}\n\nConfiguration was rejected; using default settings and bindings.\n\n{error}", + path.display() + )); Config::default() }), None => Config::default(), @@ -2317,7 +2320,17 @@ end let mut session = fixture.open(&format!( "Vamp.configure do |config|\n config.leader = \",\"\n config.bind \"i\", nil\n {expression}\nend" )); - assert!(session.editor().message().contains(error), "{expression}"); + assert!( + session + .editor() + .startup_diagnostic() + .unwrap() + .lines + .iter() + .any(|line| line.contains(error)), + "{expression}" + ); + session.handle(Input::Enter); session.handle(Input::Char('i')); session.handle(Input::Char('界')); session.handle(Input::Escape); @@ -2338,24 +2351,56 @@ end } #[test] - fn broken_config_falls_back_to_core_without_failing_the_session() { - let fixture = Fixture::new(); - let mut session = fixture.open("raise 'broken config'"); - assert!( - session + fn rejected_config_diagnostic_persists_until_dismissed_and_default_editing_survives() { + for (config, reason, dismiss) in [ + ("raise 'broken config'", "broken config", Input::Enter), + ( + "Vamp.configure { |config| config.lsp['ruby'] = true }", + "ruby", + Input::Escape, + ), + ] { + let fixture = Fixture::new(); + let mut session = fixture.open(config); + let diagnostic = session .editor() - .message() - .contains("config disabled: Ruby config: RuntimeError: broken config") - ); - session.handle(Input::Char('i')); - session.handle(Input::Char('x')); - session.handle(Input::Escape); - session.execute("wq"); - assert!(session.editor().should_quit()); - assert_eq!( - fs::read_to_string(fixture.0.join("buffer.txt")).unwrap(), - "x" - ); + .startup_diagnostic() + .expect("config failure must be visible"); + let text = diagnostic.lines.join("\n"); + assert!(text.contains(&fixture.0.join("config.rb").display().to_string())); + assert!(text.contains(reason)); + session.editor.set_message("unrelated background message"); + session.poll(); + for input in [ + Input::Char('i'), + Input::Paste("accidental edit".into()), + Input::Ctrl('p'), + ] { + session.handle(input); + } + assert_eq!( + session + .editor() + .startup_diagnostic() + .unwrap() + .lines + .join("\n"), + text + ); + assert_eq!(session.editor().mode(), Mode::Normal); + assert_eq!(session.editor().lines(), &[""]); + session.handle(dismiss); + assert!(session.editor().startup_diagnostic().is_none()); + session.handle(Input::Char('i')); + session.handle(Input::Char('x')); + session.handle(Input::Escape); + session.execute("wq"); + assert!(session.editor().should_quit()); + assert_eq!( + fs::read_to_string(fixture.0.join("buffer.txt")).unwrap(), + "x" + ); + } } fn drain_until(session: &mut EditorSession, ready: impl Fn(&EditorSession) -> bool) { diff --git a/src/tui.rs b/src/tui.rs index f57170d..2e1dc24 100644 --- a/src/tui.rs +++ b/src/tui.rs @@ -29,7 +29,7 @@ use unicode_segmentation::UnicodeSegmentation; use unicode_width::UnicodeWidthStr; use crate::{ - editor::{Editor, Hover, Input, Mode, Palette}, + editor::{Editor, Hover, Input, Mode, Palette, StartupDiagnostic}, files::{DockSide, FilePicker, FilePresentation, FileView}, plugin::{Diagnostic, DiagnosticSeverity}, session::EditorSession, @@ -605,18 +605,7 @@ fn draw( } if let Some(picker) = editor.file_picker() { if picker.presentation == FilePresentation::Floating { - let [_, column, _] = Layout::horizontal([ - Constraint::Fill(1), - Constraint::Length(area.width.min(100)), - Constraint::Fill(1), - ]) - .areas(area); - let [_, panel, _] = Layout::vertical([ - Constraint::Fill(1), - Constraint::Length(area.height.min(24)), - Constraint::Fill(1), - ]) - .areas(column); + let panel = floating_panel(area); frame.render_widget(Clear, panel); draw_file_list(frame, picker, panel, editor.file_navigation_hint()); } else if let Some(panel) = dock { @@ -634,6 +623,71 @@ fn draw( if let Some(hover) = editor.hover() { draw_hover(frame, hover, viewport); } + if let Some(diagnostic) = editor.startup_diagnostic() { + draw_startup_diagnostic(frame, diagnostic, area); + } +} + +fn floating_panel(area: Rect) -> Rect { + let [_, column, _] = Layout::horizontal([ + Constraint::Fill(1), + Constraint::Length(area.width.min(100)), + Constraint::Fill(1), + ]) + .areas(area); + let [_, panel, _] = Layout::vertical([ + Constraint::Fill(1), + Constraint::Length(area.height.min(24)), + Constraint::Fill(1), + ]) + .areas(column); + panel +} + +fn draw_startup_diagnostic(frame: &mut Frame, diagnostic: &StartupDiagnostic, area: Rect) { + let panel = floating_panel(area); + frame.render_widget(Clear, panel); + let bordered = panel.width >= 4 && panel.height >= 4; + let block = Block::default() + .borders(if bordered { + Borders::ALL + } else { + Borders::NONE + }) + .border_style(Style::default().fg(Color::Red)) + .title(" Configuration error "); + let content = if bordered { block.inner(panel) } else { panel }; + if bordered { + frame.render_widget(block, panel); + } + let [text_area, help_area] = Layout::vertical([ + Constraint::Fill(1), + Constraint::Length(u16::from(content.height >= 3)), + ]) + .areas(content); + let lines: Vec = diagnostic + .lines + .iter() + .skip(diagnostic.top) + .take(usize::from(text_area.height)) + .map(|line| { + let line = display_text(line); + Line::from(visible_line( + &line, + display_column(&line, diagnostic.left), + usize::from(text_area.width), + )) + }) + .collect(); + frame.render_widget(Paragraph::new(lines), text_area); + frame.render_widget( + Paragraph::new("Arrows scroll · Home reset · Enter/Esc continue") + .style(Style::default().fg(Color::DarkGray)), + help_area, + ); + if text_area.width > 0 && text_area.height > 0 { + frame.set_cursor_position((text_area.x, text_area.y)); + } } fn draw_hover(frame: &mut Frame, hover: &Hover, area: Rect) { @@ -1746,74 +1800,6 @@ end assert_eq!(viewports.syntax.len(), 1); } - #[test] - fn highlighted_editor_text_stays_out_of_docked_and_floating_picker_surfaces() { - let directory = Directory::new(); - let path = directory.file("panels.rs", &vec!["// note"; 30].join("\n")); - let area = Rect::new(3, 2, 100, 24); - for (name, side) in [ - ("left", Some(DockSide::Left)), - ("right", Some(DockSide::Right)), - ("top", Some(DockSide::Top)), - ("bottom", Some(DockSide::Bottom)), - ("floating", None), - ] { - let mut editor = Editor::open(Some(path.clone()), vec![]).unwrap(); - if side.is_some() { - editor.set_file_explorer_side(name).unwrap(); - } - editor.handle(Input::Char(' ')); - editor.handle(Input::Char(if side.is_some() { 'e' } else { 'o' })); - let mut terminal = Terminal::with_options( - ratatui::backend::TestBackend::new(110, 30), - ratatui::TerminalOptions { - viewport: ratatui::Viewport::Fixed(area), - }, - ) - .unwrap(); - let mut viewports = BufferViewports::default(); - terminal - .draw(|frame| viewports.draw(frame, &editor)) - .unwrap(); - let workspace = Rect::new(area.x, area.y, area.width, area.height - 2); - let (text, panel) = if let Some(side) = side { - let (text, panel) = dock_layout(workspace, side, true); - (text, panel.unwrap()) - } else { - (workspace, Rect::new(area.x + 8, area.y + 3, 84, 18)) - }; - let mut expected = Terminal::with_options( - ratatui::backend::TestBackend::new(110, 30), - ratatui::TerminalOptions { - viewport: ratatui::Viewport::Fixed(panel), - }, - ) - .unwrap(); - expected - .draw(|frame| { - draw_file_list( - frame, - editor.file_picker().unwrap(), - panel, - editor.file_navigation_hint(), - ) - }) - .unwrap(); - let buffer = terminal.backend().buffer(); - let expected = expected.backend().buffer(); - for y in panel.y..panel.bottom() { - for x in panel.x..panel.right() { - assert_eq!(buffer[(x, y)], expected[(x, y)], "{name}: {x},{y}"); - } - } - assert_eq!(buffer[(text.x + 3, text.y)].symbol(), "/"); - assert_eq!(buffer[(text.x + 3, text.y)].fg, Color::DarkGray); - assert_eq!(buffer[(text.x, text.y)].fg, Color::Cyan); - assert_eq!(buffer[(area.x, area.bottom() - 2)].bg, Color::Cyan); - assert_eq!(buffer[(0, 0)].fg, Color::Reset); - } - } - #[test] fn translate_distinguishes_control_j_from_enter_and_normalizes_control_letters() { for (code, modifiers, expected) in [ @@ -2784,6 +2770,50 @@ end assert_eq!(buffer[(0, 0)].symbol(), " "); } + #[test] + fn startup_diagnostic_scrolls_long_unicode_reasons_and_dismisses_without_editing() { + let directory = Directory::new(); + let source = directory.file("buffer.txt", "keep"); + let config = directory.file("config.rb", "raise \"#{'x' * 160}界END\\nlast reason\""); + let mut session = + EditorSession::open(Some(source), vec!["text".into()], Some(&config)).unwrap(); + let mut terminal = Terminal::new(ratatui::backend::TestBackend::new(110, 26)).unwrap(); + let mut viewports = BufferViewports::default(); + terminal + .draw(|frame| viewports.draw(frame, session.editor())) + .unwrap(); + let text = screen_text(&terminal); + assert!(text.contains("Configuration error")); + assert!(text.contains("config.rb")); + assert!(text.contains("default settings and bindings")); + assert!(text.contains("last reason")); + assert!(!text.contains("界END")); + for _ in 0..100 { + session.handle(Input::Right); + } + terminal + .draw(|frame| viewports.draw(frame, session.editor())) + .unwrap(); + let scrolled = screen_text(&terminal); + assert!( + scrolled.contains("界") && scrolled.contains("END"), + "{scrolled}" + ); + for (width, height) in [(4, 4), (1, 1)] { + let mut tiny = + Terminal::new(ratatui::backend::TestBackend::new(width, height)).unwrap(); + tiny.draw(|frame| viewports.draw(frame, session.editor())) + .unwrap(); + } + session.handle(Input::Enter); + terminal + .draw(|frame| viewports.draw(frame, session.editor())) + .unwrap(); + assert!(!screen_text(&terminal).contains("Configuration error")); + assert_eq!(session.editor().lines(), &["keep"]); + assert_eq!(session.editor().cursor(), crate::editor::Cursor::default()); + } + #[test] fn picker_conversion_focus_palette_and_closure_preserve_buffer_viewport_cache() { let directory = Directory::new();