From 2b67b1ace571df0a26457aba8461ebecf90552ea Mon Sep 17 00:00:00 2001 From: Calvin Lee Date: Thu, 24 Oct 2024 13:40:58 +0000 Subject: [PATCH] address review comments --- data/src/buffer.rs | 2 +- data/src/history/manager.rs | 4 +-- data/src/input.rs | 4 +-- data/src/isupport.rs | 55 +++++++++++++---------------------- data/src/message.rs | 26 ++++++++--------- data/src/message/broadcast.rs | 2 +- irc/proto/src/lib.rs | 22 ++++++-------- src/buffer/channel.rs | 14 ++++----- 8 files changed, 56 insertions(+), 73 deletions(-) diff --git a/data/src/buffer.rs b/data/src/buffer.rs index 0761ab06..e521c97d 100644 --- a/data/src/buffer.rs +++ b/data/src/buffer.rs @@ -45,7 +45,7 @@ impl Buffer { Self::Channel(_, channel) => message::Target::Channel { channel, source: message::Source::Server(source), - prefix: Default::default(), + prefixes: Default::default(), }, Self::Query(_, nick) => message::Target::Query { nick, diff --git a/data/src/history/manager.rs b/data/src/history/manager.rs index de264d4e..acb7e7b7 100644 --- a/data/src/history/manager.rs +++ b/data/src/history/manager.rs @@ -681,11 +681,11 @@ impl Data { filtered .iter() .filter_map(|message| { - message.target.prefix().map(|prefix| { + message.target.prefixes().map(|prefixes| { buffer_config .status_message_prefix .brackets - .format(prefix.iter().collect::()) + .format(prefixes.iter().collect::()) .chars() .count() + 1 diff --git a/data/src/input.rs b/data/src/input.rs index 4fad3d11..e8af403c 100644 --- a/data/src/input.rs +++ b/data/src/input.rs @@ -67,11 +67,11 @@ impl Input { pub fn messages(&self, user: User, channel_users: &[User], chantypes: &[char], statusmsg: &[char]) -> Option> { let to_target = |target: &str, source| { - if let Some((prefix, channel)) = proto::parse_channel_from_target(target, chantypes, statusmsg) { + if let Some((prefixes, channel)) = proto::parse_channel_from_target(target, chantypes, statusmsg) { Some(message::Target::Channel { channel, source, - prefix, + prefixes, }) } else if let Ok(user) = User::try_from(target) { Some(message::Target::Query { diff --git a/data/src/isupport.rs b/data/src/isupport.rs index 53377471..946f1eec 100644 --- a/data/src/isupport.rs +++ b/data/src/isupport.rs @@ -1,4 +1,3 @@ -use irc::proto; use std::str::FromStr; // Utilized ISUPPORT parameters should have an associated Kind enum variant @@ -89,23 +88,21 @@ impl FromStr for Operation { value.split(',').for_each(|channel_limit| { if let Some((prefix, limit)) = channel_limit.split_once(':') { if limit.is_empty() { - prefix.chars().for_each(|c| { - if proto::CHANNEL_PREFIXES.contains(&c) { - channel_limits.push(ChannelLimit { - prefix: c, - limit: None, - }); - } - }); + for c in prefix.chars() { + // TODO validate after STATUSMSG received + channel_limits.push(ChannelLimit { + prefix: c, + limit: None, + }); + } } else if let Ok(limit) = limit.parse::() { - prefix.chars().for_each(|c| { - if proto::CHANNEL_PREFIXES.contains(&c) { - channel_limits.push(ChannelLimit { - prefix: c, - limit: Some(limit), - }); - } - }); + for c in prefix.chars() { + // TODO validate after STATUSMSG received + channel_limits.push(ChannelLimit { + prefix: c, + limit: Some(limit), + }); + } } } }); @@ -143,10 +140,9 @@ impl FromStr for Operation { let chars = value.chars().collect::>(); if chars.is_empty() { Ok(Operation::Add(Parameter::CHANTYPES(None))) - } else if chars.iter().all(|c| proto::CHANNEL_PREFIXES.contains(c)) { - Ok(Operation::Add(Parameter::CHANTYPES(Some(chars)))) } else { - Err("value must only contain channel types if specified") + // TODO validate after STATUSMSG is received + Ok(Operation::Add(Parameter::CHANTYPES(Some(chars)))) } } "CHATHISTORY" => Ok(Operation::Add(Parameter::CHATHISTORY( @@ -331,13 +327,9 @@ impl FromStr for Operation { let mut prefix_maps = vec![]; if let Some((modes, prefixes)) = value.split_once(')') { - modes.chars().skip(1).zip(prefixes.chars()).for_each( - |(mode, prefix)| { - if proto::CHANNEL_MEMBERSHIP_PREFIXES.contains(&prefix) { - prefix_maps.push(PrefixMap { mode, prefix }) - } - }, - ); + for (mode, prefix) in modes.chars().skip(1).zip(prefixes.chars()) { + prefix_maps.push(PrefixMap { mode, prefix }) + } Ok(Operation::Add(Parameter::PREFIX(prefix_maps))) } else { @@ -351,13 +343,8 @@ impl FromStr for Operation { ))), "STATUSMSG" => { let chars = value.chars().collect::>(); - if chars.iter() - .all(|c| proto::CHANNEL_MEMBERSHIP_PREFIXES.contains(c)) - { - Ok(Operation::Add(Parameter::STATUSMSG(chars))) - } else { - Err("unknown channel membership prefix(es)") - } + // TODO validate that STATUSMSG ⊂ PREFIX after ISUPPORT ends + Ok(Operation::Add(Parameter::STATUSMSG(chars))) } "TARGMAX" => { let mut command_target_limits = vec![]; diff --git a/data/src/message.rs b/data/src/message.rs index e1cb7580..97cfd23a 100644 --- a/data/src/message.rs +++ b/data/src/message.rs @@ -110,7 +110,7 @@ pub enum Target { Channel { channel: Channel, source: Source, - prefix: Vec, + prefixes: Vec, }, Query { nick: Nick, @@ -120,10 +120,10 @@ pub enum Target { } impl Target { - pub fn prefix(&self) -> Option<&[char]> { + pub fn prefixes(&self) -> Option<&[char]> { match self { Target::Server { .. } => None, - Target::Channel { prefix, .. } => Some(prefix), + Target::Channel { prefixes, .. } => Some(prefixes), Target::Query { .. } => None, Target::Logs => None, } @@ -549,12 +549,12 @@ fn target( Command::MODE(target, ..) if proto::is_channel(&target, chantypes) => Some(Target::Channel { channel: target, source: source::Source::Server(None), - prefix: Default::default(), + prefixes: Default::default(), }), Command::TOPIC(channel, _) | Command::KICK(channel, _, _) => Some(Target::Channel { channel, source: source::Source::Server(None), - prefix: Default::default(), + prefixes: Default::default(), }), Command::PART(channel, _) => Some(Target::Channel { channel, @@ -562,7 +562,7 @@ fn target( source::server::Kind::Part, Some(user?.nickname().to_owned()), ))), - prefix: Default::default(), + prefixes: Default::default(), }), Command::JOIN(channel, _) => Some(Target::Channel { channel, @@ -570,7 +570,7 @@ fn target( source::server::Kind::Join, Some(user?.nickname().to_owned()), ))), - prefix: Default::default(), + prefixes: Default::default(), }), Command::Numeric(RPL_TOPIC | RPL_TOPICWHOTIME, params) => { let channel = params.get(1)?.clone(); @@ -580,7 +580,7 @@ fn target( source::server::Kind::ReplyTopic, None, ))), - prefix: Default::default(), + prefixes: Default::default(), }) } Command::Numeric(RPL_CHANNELMODEIS, params) => { @@ -588,7 +588,7 @@ fn target( Some(Target::Channel { channel, source: source::Source::Server(None), - prefix: Default::default(), + prefixes: Default::default(), }) } Command::Numeric(RPL_AWAY, params) => { @@ -611,12 +611,12 @@ fn target( }; match (proto::parse_channel_from_target(&target, chantypes, statusmsg), user) { - (Some((prefix, channel)), Some(user)) => { + (Some((prefixes, channel)), Some(user)) => { let source = source(resolve_attributes(&user, &channel).unwrap_or(user)); Some(Target::Channel { channel, source, - prefix, + prefixes, }) } (None, Some(user)) => { @@ -645,12 +645,12 @@ fn target( }; match (proto::parse_channel_from_target(&target, chantypes, statusmsg), user) { - (Some((prefix, channel)), Some(user)) => { + (Some((prefixes, channel)), Some(user)) => { let source = source(resolve_attributes(&user, &channel).unwrap_or(user)); Some(Target::Channel { channel, source, - prefix, + prefixes, }) } (None, Some(user)) => { diff --git a/data/src/message/broadcast.rs b/data/src/message/broadcast.rs index 3c6503ba..17293211 100644 --- a/data/src/message/broadcast.rs +++ b/data/src/message/broadcast.rs @@ -43,7 +43,7 @@ fn expand( Target::Channel { channel, source: source.clone(), - prefix: Default::default(), + prefixes: Default::default(), }, content.clone(), ) diff --git a/irc/proto/src/lib.rs b/irc/proto/src/lib.rs index ae9602c5..bb7dcdc3 100644 --- a/irc/proto/src/lib.rs +++ b/irc/proto/src/lib.rs @@ -47,10 +47,6 @@ pub fn command(command: &str, parameters: Vec) -> Message { command: Command::new(command, parameters), } } - -/// Reference: https://defs.ircdocs.horse/defs/chantypes -pub const CHANNEL_PREFIXES: &[char] = &['#', '&', '+', '!']; - /// Reference: https://defs.ircdocs.horse/defs/chantypes /// /// Channel types which should be used if the CHANTYPES ISUPPORT is not returned @@ -67,10 +63,6 @@ pub const CHANNEL_BLACKLIST_CHARS: &[char] = &[',', '\u{07}', ',']; pub fn is_channel(target: &str, chantypes: &[char]) -> bool { target.starts_with(chantypes) && !target.contains(CHANNEL_BLACKLIST_CHARS) } - -// Reference: https://defs.ircdocs.horse/defs/chanmembers -pub const CHANNEL_MEMBERSHIP_PREFIXES: &[char] = &['~', '&', '!', '@', '%', '+']; - /// https://modern.ircdocs.horse/#channels /// /// Given a target, split it into a channel name (beginning with a character in `chantypes`) and a @@ -85,10 +77,7 @@ pub fn parse_channel_from_target( // character is in statusmsg_prefixes) // If the characters before the first chantypes character are all valid prefixes, then we have // a valid channel name with those prefixes. let chan_index = target.find(chantypes)?; - let chan_index = target.find(chantypes)?; - - // will not panic, since `find` always returns a valid codepoint index - let (prefix, chan) = target.split_at(chan_index); + let (prefix, chan) = target.split_once(chantypes)?; if prefix.chars().all(|ref c| statusmsg_prefixes.contains(c)) { Some((prefix.chars().collect(), chan.to_owned())) } else { @@ -110,6 +99,13 @@ macro_rules! command { mod tests { use super::*; + + /// Reference: https://defs.ircdocs.horse/defs/chantypes + const CHANNEL_PREFIXES: &[char] = &['#', '&', '+', '!']; + + // Reference: https://defs.ircdocs.horse/defs/chanmembers + const CHANNEL_MEMBERSHIP_PREFIXES: &[char] = &['~', '&', '!', '@', '%', '+']; + #[test] fn is_channel_correct() { let chantypes = DEFAULT_CHANNEL_PREFIXES; @@ -126,7 +122,7 @@ mod tests { #[test] fn parse_channel() { - let chantypes = DEFAULT_CHANNEL_PREFIXES; + let chantypes = CHANNEL_PREFIXES; let prefixes = CHANNEL_MEMBERSHIP_PREFIXES; assert_eq!( parse_channel_from_target("#foo", chantypes, prefixes), diff --git a/src/buffer/channel.rs b/src/buffer/channel.rs index 64680bf4..70c20cf3 100644 --- a/src/buffer/channel.rs +++ b/src/buffer/channel.rs @@ -59,7 +59,7 @@ pub fn view<'a>( selectable_text(timestamp).style(theme::selectable_text::timestamp) }); - let prefix = message.target.prefix().map_or( + let prefixes = message.target.prefixes().map_or( max_nick_width.and_then(|_| { max_prefix_width.map(|width| { selectable_text("") @@ -67,10 +67,10 @@ pub fn view<'a>( .horizontal_alignment(alignment::Horizontal::Right) }) }), - |prefix| { + |prefixes| { let text = selectable_text(format!( "{} ", - config.buffer.status_message_prefix.brackets.format(String::from_iter(prefix)) + config.buffer.status_message_prefix.brackets.format(String::from_iter(prefixes)) )) .style(theme::selectable_text::tertiary); @@ -138,7 +138,7 @@ pub fn view<'a>( let timestamp_nickname_row = row![] .push_maybe(timestamp) - .push_maybe(prefix) + .push_maybe(prefixes) .push(nick) .push(space); @@ -186,7 +186,7 @@ pub fn view<'a>( container( row![] .push_maybe(timestamp) - .push_maybe(prefix) + .push_maybe(prefixes) .push(marker) .push(space) .push(message), @@ -209,7 +209,7 @@ pub fn view<'a>( container( row![] .push_maybe(timestamp) - .push_maybe(prefix) + .push_maybe(prefixes) .push(marker) .push(space) .push(message), @@ -236,7 +236,7 @@ pub fn view<'a>( container( row![] .push_maybe(timestamp) - .push_maybe(prefix) + .push_maybe(prefixes) .push(marker) .push(space) .push(message),