Skip to content

Commit

Permalink
address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@4e554c4c
4e554c4c committed Oct 26, 2024
1 parent da976f9 commit bd31605
Show file tree
Hide file tree
Showing 8 changed files with 55 additions and 70 deletions.
2 changes: 1 addition & 1 deletion data/src/buffer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ impl Upstream {
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,
Expand Down
4 changes: 2 additions & 2 deletions data/src/history/manager.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -597,11 +597,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::<String>())
.format(prefixes.iter().collect::<String>())
.chars()
.count()
+ 1
Expand Down
4 changes: 2 additions & 2 deletions data/src/input.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,11 @@ impl Input {

pub fn messages(&self, user: User, channel_users: &[User], chantypes: &[char], statusmsg: &[char]) -> Option<Vec<Message>> {
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 {
Expand Down
55 changes: 21 additions & 34 deletions data/src/isupport.rs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use irc::proto;
use std::str::FromStr;

// Utilized ISUPPORT parameters should have an associated Kind enum variant
Expand Down Expand Up @@ -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::<u16>() {
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),
});
}
}
}
});
Expand Down Expand Up @@ -143,10 +140,9 @@ impl FromStr for Operation {
let chars = value.chars().collect::<Vec<_>>();
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(
Expand Down Expand Up @@ -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 {
Expand All @@ -351,13 +343,8 @@ impl FromStr for Operation {
))),
"STATUSMSG" => {
let chars = value.chars().collect::<Vec<_>>();
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![];
Expand Down
26 changes: 13 additions & 13 deletions data/src/message.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ pub enum Target {
Channel {
channel: Channel,
source: Source,
prefix: Vec<char>,
prefixes: Vec<char>,
},
Query {
nick: Nick,
Expand All @@ -126,10 +126,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,
Target::Highlights { .. } => None,
Expand Down Expand Up @@ -625,28 +625,28 @@ 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,
source: source::Source::Server(Some(source::Server::new(
source::server::Kind::Part,
Some(user?.nickname().to_owned()),
))),
prefix: Default::default(),
prefixes: Default::default(),
}),
Command::JOIN(channel, _) => Some(Target::Channel {
channel,
source: source::Source::Server(Some(source::Server::new(
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();
Expand All @@ -656,15 +656,15 @@ fn target(
source::server::Kind::ReplyTopic,
None,
))),
prefix: Default::default(),
prefixes: Default::default(),
})
}
Command::Numeric(RPL_CHANNELMODEIS, params) => {
let channel = params.get(1)?.clone();
Some(Target::Channel {
channel,
source: source::Source::Server(None),
prefix: Default::default(),
prefixes: Default::default(),
})
}
Command::Numeric(RPL_AWAY, params) => {
Expand All @@ -687,12 +687,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)) => {
Expand Down Expand Up @@ -721,12 +721,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)) => {
Expand Down
2 changes: 1 addition & 1 deletion data/src/message/broadcast.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ fn expand(
Target::Channel {
channel,
source: source.clone(),
prefix: Default::default(),
prefixes: Default::default(),
},
content.clone(),
)
Expand Down
18 changes: 8 additions & 10 deletions irc/proto/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,6 @@ pub fn command(command: &str, parameters: Vec<String>) -> 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
Expand All @@ -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
Expand All @@ -84,10 +76,12 @@ pub fn parse_channel_from_target(
// We parse the target by finding the first character in chantypes, and returing (even if that
// 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)?;
// a valid channel name with those prefixes.
let chan_index = target.find(chantypes)?;

// will not panic, since `find` always returns a valid codepoint index
// This will not panic, since `find` always returns a valid codepoint index.
// We call `find` -> `split_at` because it is an _inclusive_ split, which includes the match.
// We need to return this since the channel target includes its chantype.
let (prefix, chan) = target.split_at(chan_index);
if prefix.chars().all(|ref c| statusmsg_prefixes.contains(c)) {
Some((prefix.chars().collect(), chan.to_owned()))
Expand All @@ -110,6 +104,10 @@ macro_rules! command {
mod tests {
use super::*;


// Reference: https://defs.ircdocs.horse/defs/chanmembers
const CHANNEL_MEMBERSHIP_PREFIXES: &[char] = &['~', '&', '!', '@', '%', '+'];

#[test]
fn is_channel_correct() {
let chantypes = DEFAULT_CHANNEL_PREFIXES;
Expand Down
14 changes: 7 additions & 7 deletions src/buffer/channel.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,18 +61,18 @@ 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("")
.width(width)
.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);

Expand Down Expand Up @@ -145,7 +145,7 @@ pub fn view<'a>(

let timestamp_nickname_row = row![]
.push_maybe(timestamp)
.push_maybe(prefix)
.push_maybe(prefixes)
.push(nick)
.push(space);

Expand Down Expand Up @@ -193,7 +193,7 @@ pub fn view<'a>(
container(
row![]
.push_maybe(timestamp)
.push_maybe(prefix)
.push_maybe(prefixes)
.push(marker)
.push(space)
.push(message),
Expand All @@ -216,7 +216,7 @@ pub fn view<'a>(
container(
row![]
.push_maybe(timestamp)
.push_maybe(prefix)
.push_maybe(prefixes)
.push(marker)
.push(space)
.push(message),
Expand All @@ -243,7 +243,7 @@ pub fn view<'a>(
container(
row![]
.push_maybe(timestamp)
.push_maybe(prefix)
.push_maybe(prefixes)
.push(marker)
.push(space)
.push(message),
Expand Down

0 comments on commit bd31605

Please sign in to comment.