Refactor and fix IRC formatting handling (#360)

This PR implements IRC formatting parsing, as described in
https://modern.ircdocs.horse/formatting.html

Previously we were only parsing the color codes (starting with `\x03`), rest of
the formatting characters were ignored.

We now parse all formatting characters, but formatting characters other than
two-digit color codes, "reverse" (reverses current background and foreground
colors), and "reset" (resets formatting to default) are currently ignored.
Handling those is left for another PR.

Logger now filters out all control characters before writing to a file.

I also refactored mapping IRC colors to termbox colors. Previously, for some
colors, we used ANSI terminal colors in range 0-15, but those colors change
depending on the terminal color scheme (not sure if this is part of the ANSI
standard, or just something terminals do to implement color schemes). So we now
map IRC color codes to ANSI colors in range 16-255.

ARCHITECTURE.md

@@ -103,6 +103,19 @@ for user input events and a TUI handle to update the TUI. The types are:
 | term_input     | Input handling (reading events from `stdin`) |
 | termbox_simple | Terminal manipulation (drawing) |
 | libtiny_common | The "channel name" type |
+| libtiny_wire   | Parsing IRC message formatting characters (colors etc.) |
+
+### libtiny_logger
+
+Implements logging IRC events (incoming messages, user left/joined etc.) to
+user-specified log directory.
+
+#### Dependencies of `libtiny_logger`:
+
+| Dependency     | Used for      |
+| -------------- | ------------- |
+| libtiny_common | The "channel name" type |
+| libtiny_wire   | Filtering out IRC message formatting characters (colors etc.) |
 
 ### libtiny_wire
 

CHANGELOG.md

@@ -11,6 +11,12 @@
 - `/join` (without arguments) now rejoins the current channel. (#334)
 - Key bindings can be configured in the config file. See the [wiki
   page][key-bindings-wiki] for details. (#328, #336)
+- Handling of IRC formatting characters (colors etc.) in TUI and logger
+  improved: 
+  - TUI now handles "reset" control character, to reset the text style to the
+    default.
+  - Logger now filters out all control characters before writing to the file.
+  (#360)
 
 [key-bindings-wiki]: https://github.com/osa1/tiny/wiki/Configuring-key-bindings
 

assets/README.md

@@ -0,0 +1,6 @@
+Update `crate_deps.png` with:
+
+```
+dot -Tpng crate_deps.dot > crate_deps.png
+optipng crate_deps.png
+```

assets/crate_deps.dot

@@ -24,8 +24,10 @@ digraph mygraph {
     "libtiny_client" -> "libtiny_wire"
 
     "libtiny_logger" -> "libtiny_common"
+    "libtiny_logger" -> "libtiny_wire"
 
     "libtiny_tui" -> "libtiny_common"
+    "libtiny_tui" -> "libtiny_wire"
     "libtiny_tui" -> "term_input"
     "libtiny_tui" -> "termbox_simple"
 

crates/libtiny_logger/Cargo.toml

@@ -7,5 +7,6 @@ edition = "2018"
 
 [dependencies]
 libtiny_common = { path = "../libtiny_common" }
+libtiny_wire = { path = "../libtiny_wire" }
 log = "0.4"
 time = "0.1"

crates/libtiny_logger/src/lib.rs

@@ -9,6 +9,7 @@ use std::rc::Rc;
 use time::Tm;
 
 use libtiny_common::{ChanName, ChanNameRef, MsgTarget};
+use libtiny_wire::formatting::remove_irc_control_chars;
 
 #[macro_use]
 extern crate log;
@@ -306,6 +307,7 @@ impl LoggerInner {
         _highlight: bool,
         is_action: bool,
     ) {
+        let msg = remove_irc_control_chars(msg);
         self.apply_to_target(target, |fd: &mut File, report_err: &dyn Fn(String)| {
             let io_ret = if is_action {
                 writeln!(fd, "[{}] {} {}", strf(&ts), sender, msg)

crates/libtiny_tui/Cargo.toml

@@ -14,6 +14,7 @@ desktop-notifications = ["notify-rust"]
 
 [dependencies]
 libtiny_common = { path = "../libtiny_common" }
+libtiny_wire = { path = "../libtiny_wire" }
 log = "0.4"
 notify-rust = { version = "3", optional = true }
 serde = { version = "1.0", features = ["derive"] }

crates/libtiny_tui/src/config.rs

@@ -34,10 +34,10 @@ fn default_max_nick_length() -> usize {
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub struct Style {
-    /// Termbox fg.
+    /// Termbox fg
     pub fg: u16,
 
-    /// Termbox bg.
+    /// Termbox bg
     pub bg: u16,
 }
 

crates/libtiny_tui/src/msg_area/line.rs

@@ -1,10 +1,7 @@
-use crate::line_split::LineType;
-use crate::{
-    config::{Colors, Style},
-    line_split::LineDataCache,
-    utils::translate_irc_control_chars,
-};
-use std::mem;
+use crate::config::{Colors, Style};
+use crate::line_split::{LineDataCache, LineType};
+
+use libtiny_wire::formatting::{parse_irc_formatting, Color, IrcFormatEvent};
 use termbox_simple::{self, Termbox};
 
 /// A single line added to the widget. May be rendered as multiple lines on the
@@ -13,6 +10,7 @@ use termbox_simple::{self, Termbox};
 pub(crate) struct Line {
     /// Line segments.
     segments: Vec<StyledString>,
+
     /// The segment we're currently extending.
     current_seg: StyledString,
 
@@ -35,7 +33,7 @@ pub(crate) enum SegStyle {
     /// of the color list, so make sure to use mod.
     NickColor(usize),
 
-    /// A style from the current color scheme.
+    // Rest of the styles are from the color scheme
     UserMsg,
     ErrMsg,
     Topic,
@@ -78,9 +76,6 @@ impl Default for StyledString {
     }
 }
 
-// TODO get rid of this
-const TERMBOX_COLOR_PREFIX: char = '\x00';
-
 impl Line {
     pub(crate) fn new() -> Line {
         Line {
@@ -103,7 +98,7 @@ impl Line {
         if self.current_seg.string.is_empty() {
             self.current_seg.style = style;
         } else if self.current_seg.style != style {
-            let seg = mem::replace(
+            let seg = std::mem::replace(
                 &mut self.current_seg,
                 StyledString {
                     string: String::new(),
@@ -115,31 +110,36 @@ impl Line {
     }
 
     fn add_text_inner(&mut self, str: &str) {
-        fn push_color(ret: &mut String, irc_fg: u8, irc_bg: Option<u8>) {
-            ret.push(TERMBOX_COLOR_PREFIX);
-            ret.push(0 as char); // style
-            ret.push(irc_color_to_termbox(irc_fg) as char);
-            ret.push(
-                irc_bg
-                    .map(irc_color_to_termbox)
-                    .unwrap_or(termbox_simple::TB_DEFAULT as u8) as char,
-            );
-        }
-        let str = translate_irc_control_chars(str, push_color);
-        self.current_seg.string.reserve(str.len());
-
-        let mut iter = str.chars();
-        while let Some(char) = iter.next() {
-            if char == TERMBOX_COLOR_PREFIX {
-                let st = iter.next().unwrap() as u8;
-                let fg = iter.next().unwrap() as u8;
-                let bg = iter.next().unwrap() as u8;
-                let fg = (u16::from(st) << 8) | u16::from(fg);
-                let bg = u16::from(bg);
-                let style = Style { fg, bg };
-                self.set_message_style(SegStyle::Fixed(style));
-            } else if char > '\x08' {
-                self.current_seg.string.push(char);
+        for format_event in parse_irc_formatting(str) {
+            match format_event {
+                IrcFormatEvent::Bold
+                | IrcFormatEvent::Italic
+                | IrcFormatEvent::Underline
+                | IrcFormatEvent::Strikethrough
+                | IrcFormatEvent::Monospace => {
+                    // TODO
+                }
+                IrcFormatEvent::Text(text) => {
+                    self.current_seg.string.push_str(text);
+                }
+                IrcFormatEvent::Color { fg, bg } => {
+                    let style = SegStyle::Fixed(Style {
+                        fg: u16::from(irc_color_to_termbox(fg)),
+                        bg: bg
+                            .map(|bg| u16::from(irc_color_to_termbox(bg)))
+                            .unwrap_or(termbox_simple::TB_DEFAULT),
+                    });
+
+                    self.set_message_style(style);
+                }
+                IrcFormatEvent::ReverseColor => {
+                    if let SegStyle::Fixed(Style { fg, bg }) = self.current_seg.style {
+                        self.set_message_style(SegStyle::Fixed(Style { fg: bg, bg: fg }));
+                    }
+                }
+                IrcFormatEvent::Reset => {
+                    self.set_message_style(SegStyle::UserMsg);
+                }
             }
         }
     }
@@ -150,7 +150,6 @@ impl Line {
     }
 
     pub(crate) fn add_char(&mut self, char: char, style: SegStyle) {
-        assert_ne!(char, TERMBOX_COLOR_PREFIX);
         self.set_message_style(style);
         self.current_seg.string.push(char);
     }
@@ -230,28 +229,28 @@ impl Line {
 
 ////////////////////////////////////////////////////////////////////////////////
 
-// IRC colors: http://en.wikichip.org/wiki/irc/colors
 // Termbox colors: http://www.calmar.ws/vim/256-xterm-24bit-rgb-color-chart.html
 //                 (alternatively just run `cargo run --example colors`)
-fn irc_color_to_termbox(irc_color: u8) -> u8 {
+fn irc_color_to_termbox(irc_color: Color) -> u8 {
     match irc_color {
-        0 => 15,  // white
-        1 => 0,   // black
-        2 => 17,  // navy
-        3 => 2,   // green
-        4 => 9,   // red
-        5 => 88,  // maroon
-        6 => 5,   // purple
-        7 => 130, // olive
-        8 => 11,  // yellow
-        9 => 10,  // light green
-        10 => 6,  // teal
-        11 => 14, // cyan
-        12 => 12, // awful blue
-        13 => 13, // magenta
-        14 => 8,  // gray
-        15 => 7,  // light gray
-        _ => termbox_simple::TB_DEFAULT as u8,
+        Color::White => 255,
+        Color::Black => 16,
+        Color::Blue => 21,
+        Color::Green => 46,
+        Color::Red => 196,
+        Color::Brown => 88,
+        Color::Magenta => 93,
+        Color::Orange => 210,
+        Color::Yellow => 228,
+        Color::LightGreen => 154,
+        Color::Cyan => 75,
+        Color::LightCyan => 39,
+        Color::LightBlue => 38,
+        Color::Pink => 129,
+        Color::Grey => 243,
+        Color::LightGrey => 249,
+        Color::Default => termbox_simple::TB_DEFAULT as u8,
+        Color::Ansi(ansi_color) => ansi_color,
     }
 }
 

crates/libtiny_tui/src/notifier.rs

@@ -1,4 +1,6 @@
-use crate::{utils::remove_irc_control_chars, MsgTarget};
+use crate::MsgTarget;
+
+use libtiny_wire::formatting::remove_irc_control_chars;
 
 #[cfg(feature = "desktop-notifications")]
 use notify_rust::Notification;

crates/libtiny_tui/src/utils.rs

@@ -52,106 +52,3 @@ pub(crate) fn is_nick_char(c: char) -> bool {
         || c == '-' // not valid according to RFC 2812 but servers accept it and I've seen nicks with
                     // this char in the wild
 }
-
-////////////////////////////////////////////////////////////////////////////////
-
-use std::{iter::Peekable, str::Chars};
-
-/// Parse at least one, at most two digits. Does not consume the iterator when
-/// result is `None`.
-fn parse_color_code(chars: &mut Peekable<Chars>) -> Option<u8> {
-    fn to_dec(ch: char) -> Option<u8> {
-        ch.to_digit(10).map(|c| c as u8)
-    }
-
-    let c1_char = *chars.peek()?;
-    let c1_digit = match to_dec(c1_char) {
-        None => {
-            return None;
-        }
-        Some(c1_digit) => {
-            chars.next();
-            c1_digit
-        }
-    };
-
-    match chars.peek().cloned() {
-        None => Some(c1_digit),
-        Some(c2) => match to_dec(c2) {
-            None => Some(c1_digit),
-            Some(c2_digit) => {
-                chars.next();
-                Some(c1_digit * 10 + c2_digit)
-            }
-        },
-    }
-}
-
-////////////////////////////////////////////////////////////////////////////////
-
-/// Translate IRC color codes using the callback, replace tabs with 8 spaces, and remove other
-/// ASCII control characters from the input.
-pub(crate) fn translate_irc_control_chars(
-    str: &str,
-    push_color: fn(ret: &mut String, fg: u8, bg: Option<u8>),
-) -> String {
-    let mut ret = String::with_capacity(str.len());
-    let mut iter = str.chars().peekable();
-
-    while let Some(char) = iter.next() {
-        if char == '\x03' {
-            match parse_color_code(&mut iter) {
-                None => {
-                    // just skip the control char
-                }
-                Some(fg) => {
-                    if let Some(char) = iter.peek().cloned() {
-                        if char == ',' {
-                            iter.next(); // consume ','
-                            match parse_color_code(&mut iter) {
-                                None => {
-                                    // comma was not part of the color code,
-                                    // add it to the new string
-                                    push_color(&mut ret, fg, None);
-                                    ret.push(char);
-                                }
-                                Some(bg) => {
-                                    push_color(&mut ret, fg, Some(bg));
-                                }
-                            }
-                        } else {
-                            push_color(&mut ret, fg, None);
-                        }
-                    } else {
-                        push_color(&mut ret, fg, None);
-                    }
-                }
-            }
-        } else if char == '\t' {
-            ret.push_str("        ");
-        } else if !char.is_ascii_control() {
-            ret.push(char);
-        }
-    }
-
-    ret
-}
-
-/// Like `translate_irc_control_chars`, but skips color codes.
-pub(crate) fn remove_irc_control_chars(str: &str) -> String {
-    fn push_color(_ret: &mut String, _fg: u8, _bg: Option<u8>) {}
-    translate_irc_control_chars(str, push_color)
-}
-
-#[test]
-fn test_translate_irc_control_chars() {
-    assert_eq!(
-        remove_irc_control_chars("  Le Voyageur imprudent  "),
-        "  Le Voyageur imprudent  "
-    );
-    assert_eq!(remove_irc_control_chars("\x0301,02foo"), "foo");
-    assert_eq!(remove_irc_control_chars("\x0301,2foo"), "foo");
-    assert_eq!(remove_irc_control_chars("\x031,2foo"), "foo");
-    assert_eq!(remove_irc_control_chars("\x031,foo"), ",foo");
-    assert_eq!(remove_irc_control_chars("\x03,foo"), ",foo");
-}