From 8ca8773705a78561a7215a923d2a2be553ecbd4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= Date: Tue, 7 Nov 2023 14:15:28 +0100 Subject: [PATCH] Refactor command handling: - Move all handling code to the `cmd` module, make everything in the module private. - Remove custom `Option` type `CmdParseResult` and commented-out code. - Document why an `unwrap` when getting the client of a tab cannot fail. --- crates/tiny/src/cmd.rs | 150 +++++++++++++++++++---------------------- crates/tiny/src/ui.rs | 56 +++------------ 2 files changed, 79 insertions(+), 127 deletions(-) diff --git a/crates/tiny/src/cmd.rs b/crates/tiny/src/cmd.rs index b0011278..27fc02c2 100644 --- a/crates/tiny/src/cmd.rs +++ b/crates/tiny/src/cmd.rs @@ -8,75 +8,77 @@ use libtiny_tui::config::Chan; use std::borrow::Borrow; use std::str::FromStr; -pub(crate) struct CmdArgs<'a> { - pub args: &'a str, - pub defaults: &'a Defaults, - pub ui: &'a UI, - pub clients: &'a mut Vec, - pub src: MsgSource, -} +pub(crate) fn run_cmd( + cmd: &str, + src: MsgSource, + defaults: &Defaults, + ui: &UI, + clients: &mut Vec, +) { + match parse_cmd(cmd) { + Some(ParsedCmd { cmd, args }) => { + let cmd_args = CmdArgs { + args, + defaults, + ui, + clients, + src, + }; + (cmd.cmd_fn)(cmd_args); + } -pub(crate) struct Cmd { - /// Command name. E.g. if this is `"cmd"`, `/cmd ...` will call this command. - pub(crate) name: &'static str, - /// Command function. - pub(crate) cmd_fn: fn(CmdArgs), - /// Command description - description: &'static str, - /// Command usage - usage: &'static str, + None => { + ui.add_client_err_msg( + &format!("Unsupported command: \"/{}\"", cmd), + &MsgTarget::CurrentTab, + ); + } + } } -//////////////////////////////////////////////////////////////////////////////////////////////////// - -pub(crate) enum ParseCmdResult<'a> { - /// Command name parsing successful - Ok { - cmd: &'static Cmd, - - /// Rest of the command after extracting command name - rest: &'a str, - }, +struct ParsedCmd<'a> { + cmd: &'static Cmd, - // Command name is ambiguous, here are possible values - // Ambiguous(Vec<&'static str>), - /// Unknown command - Unknown, + /// Rest of the command after extracting command name. + args: &'a str, } -pub(crate) fn parse_cmd(cmd: &str) -> ParseCmdResult { - match cmd.split_whitespace().next() { - None => ParseCmdResult::Unknown, - Some(cmd_name) => { - let mut ws_idxs = utils::split_whitespace_indices(cmd); - ws_idxs.next(); // cmd_name - let rest = { - match ws_idxs.next() { - None => "", - Some(rest_idx) => &cmd[rest_idx..], - } - }; - // let mut possibilities: Vec<&'static Cmd> = vec![]; - for cmd in &CMDS { - if cmd_name == cmd.name { - // exact match, return - return ParseCmdResult::Ok { cmd, rest }; - } - } - ParseCmdResult::Unknown - // match possibilities.len() { - // 0 => - // ParseCmdResult::Unknown, - // 1 => - // ParseCmdResult::Ok { - // cmd: possibilities[0], - // rest, - // }, - // _ => - // ParseCmdResult::Ambiguous(possibilities.into_iter().map(|cmd| cmd.name).collect()), - // } +fn parse_cmd(cmd: &str) -> Option { + let cmd_name = cmd.split_whitespace().next()?; + let mut ws_idxs = utils::split_whitespace_indices(cmd); + ws_idxs.next(); // cmd_name + let rest = match ws_idxs.next() { + None => "", + Some(rest_idx) => &cmd[rest_idx..], + }; + for cmd in &CMDS { + if cmd_name == cmd.name { + return Some(ParsedCmd { cmd, args: rest }); } } + None +} + +struct CmdArgs<'a> { + args: &'a str, + defaults: &'a Defaults, + ui: &'a UI, + clients: &'a mut Vec, + src: MsgSource, +} + +struct Cmd { + /// Command name. If this is `"cmd"`, `/cmd ...` will call this command. + name: &'static str, + + /// Command function. + cmd_fn: fn(CmdArgs), + + /// Command description. Shown in `/help` and error messages. + description: &'static str, + + /// Command usage. Shown in `/help` and error messages. + usage: &'static str, } fn find_client_idx(clients: &[Client], serv_name: &str) -> Option { @@ -573,27 +575,13 @@ fn help(args: CmdArgs) { #[test] fn test_parse_cmd() { - let ret = parse_cmd("msg NickServ identify notMyPassword"); - match ret { - ParseCmdResult::Ok { cmd, rest } => { - assert_eq!(cmd.name, "msg"); - assert_eq!(rest, "NickServ identify notMyPassword"); - } - _ => { - panic!("Can't parse cmd"); - } - } + let ParsedCmd { cmd, args } = parse_cmd("msg NickServ identify notMyPassword").unwrap(); + assert_eq!(cmd.name, "msg"); + assert_eq!(args, "NickServ identify notMyPassword"); - let ret = parse_cmd("join #foo"); - match ret { - ParseCmdResult::Ok { cmd, rest } => { - assert_eq!(cmd.name, "join"); - assert_eq!(rest, "#foo"); - } - _ => { - panic!("Can't parse cmd"); - } - } + let ParsedCmd { cmd, args } = parse_cmd("join #foo").unwrap(); + assert_eq!(cmd.name, "join"); + assert_eq!(args, "#foo"); } #[test] diff --git a/crates/tiny/src/ui.rs b/crates/tiny/src/ui.rs index a3efd902..b60ac2e0 100644 --- a/crates/tiny/src/ui.rs +++ b/crates/tiny/src/ui.rs @@ -1,6 +1,6 @@ //! UI event handling -use crate::cmd::{parse_cmd, CmdArgs, ParseCmdResult}; +use crate::cmd::run_cmd; use crate::config; use libtiny_client::Client; use libtiny_common::{ChanNameRef, MsgSource, MsgTarget, TabStyle}; @@ -124,54 +124,23 @@ fn handle_input_ev( client.quit(msg.clone()); } } + Msg { msg, source } => { send_msg(ui, clients, &source, msg, false); } + Lines { lines, source } => { for line in lines.into_iter() { send_msg(ui, clients, &source, line, false) } } - Cmd { cmd, source } => handle_cmd(defaults, ui, clients, source, &cmd), - } -} -fn handle_cmd( - defaults: &config::Defaults, - ui: &UI, - clients: &mut Vec, - src: MsgSource, - cmd: &str, -) { - match parse_cmd(cmd) { - ParseCmdResult::Ok { cmd, rest } => { - let cmd_args = CmdArgs { - args: rest, - defaults, - ui, - clients, - src, - }; - (cmd.cmd_fn)(cmd_args); + Cmd { cmd, source } => { + run_cmd(&cmd, source, defaults, ui, clients); } - // ParseCmdResult::Ambiguous(vec) => { - // self.ui.add_client_err_msg( - // &format!("Unsupported command: \"/{}\"", msg), - // &MsgTarget::CurrentTab, - // ); - // self.ui.add_client_err_msg( - // &format!("Did you mean one of {:?} ?", vec), - // &MsgTarget::CurrentTab, - // ); - // }, - ParseCmdResult::Unknown => ui.add_client_err_msg( - &format!("Unsupported command: \"/{}\"", cmd), - &MsgTarget::CurrentTab, - ), } } -// TODO: move this somewhere else pub(crate) fn send_msg( ui: &UI, clients: &mut Vec, @@ -194,24 +163,19 @@ pub(crate) fn send_msg( return; } + // We only remove a client when its server tab is closed (which also closes its channel tabs), + // so if a tab exists its client must also be available. let client = clients .iter_mut() .find(|client| client.get_serv_name() == src.serv_name()) .unwrap(); - // TODO: For errors: - // - // ui.add_client_err_msg( - // &format!("Can't find server: {}", serv), - // &MsgTarget::CurrentTab, - // ); - - // `ui_target`: Where to show the message on ui - // `msg_target`: Actual PRIVMSG target to send to the server + // `ui_target`: Where to show the message on ui. + // `msg_target`: Actual PRIVMSG target to send to the server. let (ui_target, msg_target): (MsgTarget, &str) = { match src { MsgSource::Serv { .. } => { - // we don't split raw messages to 512-bytes long chunks + // We don't split raw messages to 512-bytes long chunks. client.raw_msg(&msg); return; }