Refactor and fix IRC formatting handling

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,5 @@
+Update `crate_deps.png` with:
+
+```
+dot -Tpng crate_deps.dot > 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");
-}