From 9c02a1b070b90668c97968b848421ad2de9d459b Mon Sep 17 00:00:00 2001 From: Lionel Flandrin Date: Sat, 26 Jun 2021 18:50:44 +0100 Subject: [PATCH] Make command implementation return a Result<()> The error message is displayed with cx.editor.set_error. --- helix-term/src/commands.rs | 415 +++++++++++++++++++++++-------------- helix-view/src/editor.rs | 5 +- 2 files changed, 263 insertions(+), 157 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index e35b36f9..8a9ffb91 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -23,7 +23,7 @@ use helix_view::{ Document, DocumentId, Editor, ViewId, }; -use anyhow::anyhow; +use anyhow::{anyhow, bail}; use helix_lsp::{ lsp, util::{lsp_pos_to_pos, lsp_range_to_range, pos_to_lsp_pos, range_to_lsp_range}, @@ -1161,34 +1161,52 @@ mod cmd { pub alias: Option<&'static str>, pub doc: &'static str, // params, flags, helper, completer - pub fun: fn(&mut compositor::Context, &[&str], PromptEvent), + pub fun: fn(&mut compositor::Context, &[&str], PromptEvent) -> anyhow::Result<()>, pub completer: Option, } - fn quit(cx: &mut compositor::Context, _args: &[&str], _event: PromptEvent) { + fn quit( + cx: &mut compositor::Context, + _args: &[&str], + _event: PromptEvent, + ) -> anyhow::Result<()> { // last view and we have unsaved changes - if cx.editor.tree.views().count() == 1 && buffers_remaining_impl(cx.editor) { - return; + if cx.editor.tree.views().count() == 1 { + buffers_remaining_impl(cx.editor)? } + cx.editor .close(view!(cx.editor).id, /* close_buffer */ false); + + Ok(()) } - fn force_quit(cx: &mut compositor::Context, _args: &[&str], _event: PromptEvent) { + fn force_quit( + cx: &mut compositor::Context, + _args: &[&str], + _event: PromptEvent, + ) -> anyhow::Result<()> { cx.editor .close(view!(cx.editor).id, /* close_buffer */ false); + + Ok(()) } - fn open(cx: &mut compositor::Context, args: &[&str], _event: PromptEvent) { + fn open( + cx: &mut compositor::Context, + args: &[&str], + _event: PromptEvent, + ) -> anyhow::Result<()> { match args.get(0) { Some(path) => { - // TODO: handle error - let _ = cx.editor.open(path.into(), Action::Replace); + let _ = cx.editor.open(path.into(), Action::Replace)?; + + Ok(()) } None => { - cx.editor.set_error("wrong argument count".to_string()); + bail!("wrong argument count"); } - }; + } } fn write_impl>( @@ -1200,11 +1218,11 @@ mod cmd { if let Some(path) = path { if let Err(err) = doc.set_path(path.as_ref()) { - return Err(anyhow!("invalid filepath: {}", err)); + bail!("invalid filepath: {}", err); }; } if doc.path().is_none() { - return Err(anyhow!("cannot write a buffer without a filename")); + bail!("cannot write a buffer without a filename"); } let fmt = doc.auto_format().map(|fmt| { let shared = fmt.shared(); @@ -1220,21 +1238,33 @@ mod cmd { Ok(tokio::spawn(doc.format_and_save(fmt))) } - fn write(cx: &mut compositor::Context, args: &[&str], _event: PromptEvent) { - match write_impl(cx, args.first()) { - Err(e) => cx.editor.set_error(e.to_string()), - Ok(handle) => { - cx.jobs - .add(Job::new(handle.unwrap_or_else(|e| Err(e.into()))).wait_before_exiting()); - } - }; + fn write( + cx: &mut compositor::Context, + args: &[&str], + _event: PromptEvent, + ) -> anyhow::Result<()> { + let handle = write_impl(cx, args.first())?; + cx.jobs + .add(Job::new(handle.unwrap_or_else(|e| Err(e.into()))).wait_before_exiting()); + + Ok(()) } - fn new_file(cx: &mut compositor::Context, _args: &[&str], _event: PromptEvent) { + fn new_file( + cx: &mut compositor::Context, + _args: &[&str], + _event: PromptEvent, + ) -> anyhow::Result<()> { cx.editor.new_file(Action::Replace); + + Ok(()) } - fn format(cx: &mut compositor::Context, _args: &[&str], _event: PromptEvent) { + fn format( + cx: &mut compositor::Context, + _args: &[&str], + _event: PromptEvent, + ) -> anyhow::Result<()> { let (_, doc) = current!(cx.editor); if let Some(format) = doc.format() { @@ -1242,9 +1272,14 @@ mod cmd { make_format_callback(doc.id(), doc.version(), Modified::LeaveModified, format); cx.jobs.callback(callback); } - } - fn set_indent_style(cx: &mut compositor::Context, args: &[&str], _event: PromptEvent) { + Ok(()) + } + fn set_indent_style( + cx: &mut compositor::Context, + args: &[&str], + _event: PromptEvent, + ) -> anyhow::Result<()> { use IndentStyle::*; // If no argument, report current indent style. @@ -1256,7 +1291,7 @@ mod cmd { Spaces(n) if (2..=8).contains(&n) => format!("{} spaces", n), _ => "error".into(), // Shouldn't happen. }); - return; + return Ok(()); } // Attempt to parse argument as an indent style. @@ -1274,15 +1309,19 @@ mod cmd { if let Some(s) = style { let doc = doc_mut!(cx.editor); doc.indent_style = s; + + Ok(()) } else { - // Invalid argument. - cx.editor - .set_error(format!("invalid indent style '{}'", args[0],)); + bail!("invalid indent style '{}'", args[0]); } } /// Sets or reports the current document's line ending setting. - fn set_line_ending(cx: &mut compositor::Context, args: &[&str], _event: PromptEvent) { + fn set_line_ending( + cx: &mut compositor::Context, + args: &[&str], + _event: PromptEvent, + ) -> anyhow::Result<()> { use LineEnding::*; // If no argument, report current line ending setting. @@ -1298,7 +1337,8 @@ mod cmd { // These should never be a document's default line ending. VT | LS | PS => "error".into(), }); - return; + + return Ok(()); } // Attempt to parse argument as a line ending. @@ -1314,70 +1354,66 @@ mod cmd { if let Some(le) = line_ending { doc_mut!(cx.editor).line_ending = le; + Ok(()) } else { - // Invalid argument. - cx.editor - .set_error(format!("invalid line ending '{}'", args[0],)); + bail!("invalid line ending '{}'", args[0]); } } - fn earlier(cx: &mut compositor::Context, args: &[&str], _event: PromptEvent) { - let uk = match args.join(" ").parse::() { - Ok(uk) => uk, - Err(msg) => { - cx.editor.set_error(msg); - return; - } - }; + fn earlier( + cx: &mut compositor::Context, + args: &[&str], + _event: PromptEvent, + ) -> anyhow::Result<()> { + let uk = args + .join(" ") + .parse::() + .map_err(|s| anyhow!(s))?; + let (view, doc) = current!(cx.editor); - doc.earlier(view.id, uk) + doc.earlier(view.id, uk); + + Ok(()) } - fn later(cx: &mut compositor::Context, args: &[&str], _event: PromptEvent) { - let uk = match args.join(" ").parse::() { - Ok(uk) => uk, - Err(msg) => { - cx.editor.set_error(msg); - return; - } - }; + fn later( + cx: &mut compositor::Context, + args: &[&str], + _event: PromptEvent, + ) -> anyhow::Result<()> { + let uk = args + .join(" ") + .parse::() + .map_err(|s| anyhow!(s))?; let (view, doc) = current!(cx.editor); - doc.later(view.id, uk) + doc.later(view.id, uk); + + Ok(()) } - fn write_quit(cx: &mut compositor::Context, args: &[&str], event: PromptEvent) { - match write_impl(cx, args.first()) { - Ok(handle) => { - if let Err(e) = helix_lsp::block_on(handle) { - cx.editor.set_error(e.to_string()); - } else { - quit(cx, &[], event); - } - } - Err(e) => { - cx.editor.set_error(e.to_string()); - } - } + fn write_quit( + cx: &mut compositor::Context, + args: &[&str], + event: PromptEvent, + ) -> anyhow::Result<()> { + let handle = write_impl(cx, args.first())?; + helix_lsp::block_on(handle)?; + quit(cx, &[], event) } - fn force_write_quit(cx: &mut compositor::Context, args: &[&str], event: PromptEvent) { - match write_impl(cx, args.first()) { - Ok(handle) => { - if let Err(e) = helix_lsp::block_on(handle) { - cx.editor.set_error(e.to_string()); - } else { - force_quit(cx, &[], event); - } - } - Err(e) => { - cx.editor.set_error(e.to_string()); - } - } + fn force_write_quit( + cx: &mut compositor::Context, + args: &[&str], + event: PromptEvent, + ) -> anyhow::Result<()> { + let handle = write_impl(cx, args.first())?; + helix_lsp::block_on(handle)?; + force_quit(cx, &[], event) } - /// Returns `true` if there are modified buffers remaining and sets editor error, - /// otherwise returns `false` - fn buffers_remaining_impl(editor: &mut Editor) -> bool { + /// Results an error if there are modified buffers remaining and sets editor error, + /// otherwise returns `Ok(())` + fn buffers_remaining_impl(editor: &mut Editor) -> anyhow::Result<()> { let modified: Vec<_> = editor .documents() .filter(|doc| doc.is_modified()) @@ -1388,16 +1424,13 @@ mod cmd { }) .collect(); if !modified.is_empty() { - let err = format!( + bail!( "{} unsaved buffer(s) remaining: {:?}", modified.len(), modified ); - editor.set_error(err); - true - } else { - false } + Ok(()) } fn write_all_impl( @@ -1406,7 +1439,7 @@ mod cmd { _event: PromptEvent, quit: bool, force: bool, - ) { + ) -> anyhow::Result<()> { let mut errors = String::new(); // save all documents @@ -1419,11 +1452,10 @@ mod cmd { // TODO: handle error. let _ = helix_lsp::block_on(tokio::spawn(doc.save())); } - editor.set_error(errors); if quit { - if !force && buffers_remaining_impl(editor) { - return; + if !force { + buffers_remaining_impl(editor)?; } // close all views @@ -1432,23 +1464,42 @@ mod cmd { editor.close(view_id, false); } } + + bail!(errors) } - fn write_all(cx: &mut compositor::Context, args: &[&str], event: PromptEvent) { + fn write_all( + cx: &mut compositor::Context, + args: &[&str], + event: PromptEvent, + ) -> anyhow::Result<()> { write_all_impl(&mut cx.editor, args, event, false, false) } - fn write_all_quit(cx: &mut compositor::Context, args: &[&str], event: PromptEvent) { + fn write_all_quit( + cx: &mut compositor::Context, + args: &[&str], + event: PromptEvent, + ) -> anyhow::Result<()> { write_all_impl(&mut cx.editor, args, event, true, false) } - fn force_write_all_quit(cx: &mut compositor::Context, args: &[&str], event: PromptEvent) { + fn force_write_all_quit( + cx: &mut compositor::Context, + args: &[&str], + event: PromptEvent, + ) -> anyhow::Result<()> { write_all_impl(&mut cx.editor, args, event, true, true) } - fn quit_all_impl(editor: &mut Editor, _args: &[&str], _event: PromptEvent, force: bool) { - if !force && buffers_remaining_impl(editor) { - return; + fn quit_all_impl( + editor: &mut Editor, + _args: &[&str], + _event: PromptEvent, + force: bool, + ) -> anyhow::Result<()> { + if !force { + buffers_remaining_impl(editor)?; } // close all views @@ -1456,57 +1507,82 @@ mod cmd { for view_id in views { editor.close(view_id, false); } + + Ok(()) } - fn quit_all(cx: &mut compositor::Context, args: &[&str], event: PromptEvent) { + fn quit_all( + cx: &mut compositor::Context, + args: &[&str], + event: PromptEvent, + ) -> anyhow::Result<()> { quit_all_impl(&mut cx.editor, args, event, false) } - fn force_quit_all(cx: &mut compositor::Context, args: &[&str], event: PromptEvent) { + fn force_quit_all( + cx: &mut compositor::Context, + args: &[&str], + event: PromptEvent, + ) -> anyhow::Result<()> { quit_all_impl(&mut cx.editor, args, event, true) } - fn theme(cx: &mut compositor::Context, args: &[&str], _event: PromptEvent) { + fn theme( + cx: &mut compositor::Context, + args: &[&str], + _event: PromptEvent, + ) -> anyhow::Result<()> { let theme = if let Some(theme) = args.first() { theme } else { - cx.editor.set_error("theme name not provided".into()); - return; + bail!("theme name not provided"); }; - cx.editor.set_theme_from_name(theme); + cx.editor.set_theme_from_name(theme) } fn yank_main_selection_to_clipboard( cx: &mut compositor::Context, _args: &[&str], _event: PromptEvent, - ) { - yank_main_selection_to_clipboard_impl(&mut cx.editor); + ) -> anyhow::Result<()> { + yank_main_selection_to_clipboard_impl(&mut cx.editor) } - fn yank_joined_to_clipboard(cx: &mut compositor::Context, args: &[&str], _event: PromptEvent) { + fn yank_joined_to_clipboard( + cx: &mut compositor::Context, + args: &[&str], + _event: PromptEvent, + ) -> anyhow::Result<()> { let (_, doc) = current!(cx.editor); let separator = args .first() .copied() .unwrap_or_else(|| doc.line_ending.as_str()); - yank_joined_to_clipboard_impl(&mut cx.editor, separator); + yank_joined_to_clipboard_impl(&mut cx.editor, separator) } - fn paste_clipboard_after(cx: &mut compositor::Context, _args: &[&str], _event: PromptEvent) { - paste_clipboard_impl(&mut cx.editor, Paste::After); + fn paste_clipboard_after( + cx: &mut compositor::Context, + _args: &[&str], + _event: PromptEvent, + ) -> anyhow::Result<()> { + paste_clipboard_impl(&mut cx.editor, Paste::After) } - fn paste_clipboard_before(cx: &mut compositor::Context, _args: &[&str], _event: PromptEvent) { - paste_clipboard_impl(&mut cx.editor, Paste::After); + fn paste_clipboard_before( + cx: &mut compositor::Context, + _args: &[&str], + _event: PromptEvent, + ) -> anyhow::Result<()> { + paste_clipboard_impl(&mut cx.editor, Paste::After) } fn replace_selections_with_clipboard( cx: &mut compositor::Context, _args: &[&str], _event: PromptEvent, - ) { + ) -> anyhow::Result<()> { let (view, doc) = current!(cx.editor); match cx.editor.clipboard_provider.get_contents() { @@ -1520,71 +1596,89 @@ mod cmd { doc.apply(&transaction, view.id); doc.append_changes_to_history(view.id); + Ok(()) + } + Err(e) => { + log::error!("Couldn't get system clipboard contents: {:?}", e); + bail!("Couldn't get system clipboard contents: {:?}", e); } - Err(e) => log::error!("Couldn't get system clipboard contents: {:?}", e), } } - fn show_clipboard_provider(cx: &mut compositor::Context, _args: &[&str], _event: PromptEvent) { + fn show_clipboard_provider( + cx: &mut compositor::Context, + _args: &[&str], + _event: PromptEvent, + ) -> anyhow::Result<()> { cx.editor .set_status(cx.editor.clipboard_provider.name().into()); + Ok(()) } - fn change_current_directory(cx: &mut compositor::Context, args: &[&str], _event: PromptEvent) { - let dir = match args.first() { - Some(dir) => dir, - None => { - cx.editor.set_error("target directory not provided".into()); - return; - } - }; + fn change_current_directory( + cx: &mut compositor::Context, + args: &[&str], + _event: PromptEvent, + ) -> anyhow::Result<()> { + let dir = args + .first() + .ok_or_else(|| anyhow!("target directory not provided"))?; if let Err(e) = std::env::set_current_dir(dir) { - cx.editor.set_error(format!( - "Couldn't change the current working directory: {:?}", - e - )); - return; + bail!("Couldn't change the current working directory: {:?}", e); } match std::env::current_dir() { - Ok(cwd) => cx.editor.set_status(format!( - "Current working directory is now {}", - cwd.display() - )), - Err(e) => cx - .editor - .set_error(format!("Couldn't get the new working directory: {}", e)), + Ok(cwd) => { + cx.editor.set_status(format!( + "Current working directory is now {}", + cwd.display() + )); + Ok(()) + } + Err(e) => bail!("Couldn't get the new working directory: {}", e), } } - fn show_current_directory(cx: &mut compositor::Context, _args: &[&str], _event: PromptEvent) { + fn show_current_directory( + cx: &mut compositor::Context, + _args: &[&str], + _event: PromptEvent, + ) -> anyhow::Result<()> { match std::env::current_dir() { - Ok(cwd) => cx - .editor - .set_status(format!("Current working directory is {}", cwd.display())), - Err(e) => cx - .editor - .set_error(format!("Couldn't get the current working directory: {}", e)), + Ok(cwd) => { + cx.editor + .set_status(format!("Current working directory is {}", cwd.display())); + Ok(()) + } + Err(e) => bail!("Couldn't get the current working directory: {}", e), } } /// Sets the [`Document`]'s encoding.. - fn set_encoding(cx: &mut compositor::Context, args: &[&str], _: PromptEvent) { + fn set_encoding( + cx: &mut compositor::Context, + args: &[&str], + _event: PromptEvent, + ) -> anyhow::Result<()> { let (_, doc) = current!(cx.editor); if let Some(label) = args.first() { doc.set_encoding(label) - .unwrap_or_else(|e| cx.editor.set_error(e.to_string())); } else { let encoding = doc.encoding().name().to_string(); - cx.editor.set_status(encoding) + cx.editor.set_status(encoding); + Ok(()) } } /// Reload the [`Document`] from its source file. - fn reload(cx: &mut compositor::Context, _args: &[&str], _: PromptEvent) { + fn reload( + cx: &mut compositor::Context, + _args: &[&str], + _event: PromptEvent, + ) -> anyhow::Result<()> { let (view, doc) = current!(cx.editor); - doc.reload(view.id).unwrap(); + doc.reload(view.id) } pub const TYPABLE_COMMAND_LIST: &[TypableCommand] = &[ @@ -1849,7 +1943,9 @@ fn command_mode(cx: &mut Context) { } if let Some(cmd) = cmd::COMMANDS.get(parts[0]) { - (cmd.fun)(cx, &parts[1..], event); + if let Err(e) = (cmd.fun)(cx, &parts[1..], event) { + cx.editor.set_error(format!("{}", e)); + } } else { cx.editor .set_error(format!("no such command: '{}'", parts[0])); @@ -2758,7 +2854,7 @@ fn yank(cx: &mut Context) { cx.editor.set_status(msg) } -fn yank_joined_to_clipboard_impl(editor: &mut Editor, separator: &str) { +fn yank_joined_to_clipboard_impl(editor: &mut Editor, separator: &str) -> anyhow::Result<()> { let (view, doc) = current!(editor); let values: Vec = doc @@ -2776,17 +2872,20 @@ fn yank_joined_to_clipboard_impl(editor: &mut Editor, separator: &str) { if let Err(e) = editor.clipboard_provider.set_contents(joined) { log::error!("Couldn't set system clipboard content: {:?}", e); + bail!("Couldn't set system clipboard content: {:?}", e); } editor.set_status(msg); + + Ok(()) } fn yank_joined_to_clipboard(cx: &mut Context) { let line_ending = current!(cx.editor).1.line_ending; - yank_joined_to_clipboard_impl(&mut cx.editor, line_ending.as_str()); + let _ = yank_joined_to_clipboard_impl(&mut cx.editor, line_ending.as_str()); } -fn yank_main_selection_to_clipboard_impl(editor: &mut Editor) { +fn yank_main_selection_to_clipboard_impl(editor: &mut Editor) -> anyhow::Result<()> { let (view, doc) = current!(editor); let value = doc @@ -2796,13 +2895,15 @@ fn yank_main_selection_to_clipboard_impl(editor: &mut Editor) { if let Err(e) = editor.clipboard_provider.set_contents(value.into_owned()) { log::error!("Couldn't set system clipboard content: {:?}", e); + bail!("Couldn't set system clipboard content: {:?}", e); } editor.set_status("yanked main selection to system clipboard".to_owned()); + Ok(()) } fn yank_main_selection_to_clipboard(cx: &mut Context) { - yank_main_selection_to_clipboard_impl(&mut cx.editor); + let _ = yank_main_selection_to_clipboard_impl(&mut cx.editor); } #[derive(Copy, Clone)] @@ -2850,7 +2951,7 @@ fn paste_impl( Some(transaction) } -fn paste_clipboard_impl(editor: &mut Editor, action: Paste) { +fn paste_clipboard_impl(editor: &mut Editor, action: Paste) -> anyhow::Result<()> { let (view, doc) = current!(editor); match editor @@ -2861,18 +2962,22 @@ fn paste_clipboard_impl(editor: &mut Editor, action: Paste) { Ok(Some(transaction)) => { doc.apply(&transaction, view.id); doc.append_changes_to_history(view.id); + Ok(()) + } + Ok(None) => Ok(()), + Err(e) => { + log::error!("Couldn't get system clipboard contents: {:?}", e); + bail!("Couldn't get system clipboard contents: {:?}", e); } - Ok(None) => {} - Err(e) => log::error!("Couldn't get system clipboard contents: {:?}", e), } } fn paste_clipboard_after(cx: &mut Context) { - paste_clipboard_impl(&mut cx.editor, Paste::After); + let _ = paste_clipboard_impl(&mut cx.editor, Paste::After); } fn paste_clipboard_before(cx: &mut Context) { - paste_clipboard_impl(&mut cx.editor, Paste::Before); + let _ = paste_clipboard_impl(&mut cx.editor, Paste::Before); } fn replace_with_yanked(cx: &mut Context) { diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 4f01cce4..c80535ed 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -97,16 +97,17 @@ impl Editor { self._refresh(); } - pub fn set_theme_from_name(&mut self, theme: &str) { + pub fn set_theme_from_name(&mut self, theme: &str) -> anyhow::Result<()> { let theme = match self.theme_loader.load(theme.as_ref()) { Ok(theme) => theme, Err(e) => { log::warn!("failed setting theme `{}` - {}", theme, e); - return; + anyhow::bail!("failed setting theme `{}` - {}", theme, e); } }; self.set_theme(theme); + Ok(()) } fn _refresh(&mut self) {