Repository: osa1/tiny (language: rust)

Issue:
Add option for higher resolution timestamp
Configuration setting for enabling `HH:MM:SS` for each irc message.


Discussion / hints:
Should not be too hard to implement, just make sure to make it opt-in and add a configuration variable for it.

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 18d13404..b88fa4e2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -16,6 +16,10 @@
   location. The old location `$HOME/.tinyrc.yml` is still used when there isn't
   a config file in the new location, to avoid breakage. `$HOME/.config` is used
   for `$XDG_CONFIG_HOME` when the env variable is not available (#152).
+- Timestamps can now be configured using the new (optional) `timestamp_format`
+  field in the config. Value should be a valid `strftime()` format string (see
+  `man strftime` for details). Default value is the same as before (`%H:%M`).
+  (#161)
 
 # 2019/10/05: 0.5.0
 
diff --git a/README.md b/README.md
index ec098291..e20e6de0 100644
--- a/README.md
+++ b/README.md
@@ -38,6 +38,8 @@ tiny is an IRC client written in Rust.
 
 - Configurable colors
 
+- Configurable timestamps
+
 - SASL authentication
 
 - Configurable desktop notifications on new messages
@@ -169,7 +171,7 @@ Commands start with `/` character.
 - `/names`: List all nicks in the current channel. You can use `/names <nick>` to
   check if a specific nick is in the channel.
 
-- `/reload`: Reload configuration
+- `/reload`: Reload TUI colors from the config file
 
 - `/clear`: Clears tab contents
 
diff --git a/libtiny_tui/examples/bench.rs b/libtiny_tui/examples/bench.rs
index d600f093..5b7e65e8 100644
--- a/libtiny_tui/examples/bench.rs
+++ b/libtiny_tui/examples/bench.rs
@@ -9,7 +9,6 @@ use libtiny_tui::TUI;
 use libtiny_ui::*;
 use std::fs::File;
 use std::io::{BufRead, BufReader};
-use std::path::PathBuf;
 use tokio::runtime::current_thread::Runtime;
 
 fn main() {
@@ -20,13 +19,11 @@ fn main() {
     let lines = file_buffered.lines().map(Result::unwrap).collect();
 
     let mut executor = Runtime::new().unwrap();
-    let (tui, _) = TUI::run(PathBuf::from("../tiny/config.yml"), &mut executor);
+    let (tui, _) = TUI::run(None, &mut executor);
 
     tui.draw();
 
     executor.block_on(bench_task(tui, lines));
-
-    // executor.run();
 }
 
 async fn bench_task(tui: TUI, lines: Vec<String>) {
diff --git a/libtiny_tui/src/config.rs b/libtiny_tui/src/config.rs
index b1836837..7a915bbe 100644
--- a/libtiny_tui/src/config.rs
+++ b/libtiny_tui/src/config.rs
@@ -4,6 +4,7 @@
 use serde::de::{self, Deserializer, MapAccess, Visitor};
 use serde::Deserialize;
 use serde_yaml;
+use std::default::Default;
 use std::fs::File;
 use std::io::Read;
 use std::path::Path;
@@ -14,6 +15,23 @@ pub use termbox_simple::*;
 pub(crate) struct Config {
     #[serde(default)]
     pub(crate) colors: Colors,
+
+    /// A valid `strftime()` format string used when showing timestamps. Validated in `TUI::new`.
+    #[serde(default = "def_ts_fmt")]
+    pub(crate) timestamp_format: String,
+}
+
+pub(crate) fn def_ts_fmt() -> String {
+    "%H:%M".to_string()
+}
+
+impl Default for Config {
+    fn default() -> Config {
+        Config {
+            colors: Colors::default(),
+            timestamp_format: def_ts_fmt(),
+        }
+    }
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
diff --git a/libtiny_tui/src/lib.rs b/libtiny_tui/src/lib.rs
index 0e3472e2..b936a75a 100644
--- a/libtiny_tui/src/lib.rs
+++ b/libtiny_tui/src/lib.rs
@@ -41,7 +41,10 @@ pub struct TUI {
 }
 
 impl TUI {
-    pub fn run(config_path: PathBuf, runtime: &mut Runtime) -> (TUI, mpsc::Receiver<Event>) {
+    pub fn run(
+        config_path: Option<PathBuf>,
+        runtime: &mut Runtime,
+    ) -> (TUI, mpsc::Receiver<Event>) {
         let tui = Rc::new(RefCell::new(tui::TUI::new(config_path)));
         let inner = Rc::downgrade(&tui);
 
diff --git a/libtiny_tui/src/messaging.rs b/libtiny_tui/src/messaging.rs
index 70a0e7d5..59220c38 100644
--- a/libtiny_tui/src/messaging.rs
+++ b/libtiny_tui/src/messaging.rs
@@ -2,6 +2,7 @@ use term_input::Key;
 use termbox_simple::Termbox;
 
 use std::convert::From;
+use std::rc::Rc;
 
 use time::{self, Tm};
 
@@ -43,42 +44,22 @@ pub(crate) struct MessagingUI {
     show_current_nick: bool,
 
     last_activity_line: Option<ActivityLine>,
-    last_activity_ts: Option<Timestamp>,
-}
-
-/// Like `time::Tm`, but we only care about hour and minute parts.
-#[derive(PartialEq, Eq, Clone, Copy)]
-pub(crate) struct Timestamp {
-    hour: i32,
-    min: i32,
-}
+    last_activity_ts: Option<String>,
 
-impl Timestamp {
-    fn stamp(self, msg_area: &mut MsgArea) {
-        msg_area.set_style(SegStyle::SchemeStyle(SchemeStyle::Timestamp));
-        msg_area.add_text(&format!("{:02}:{:02} ", self.hour, self.min));
-    }
-}
-
-impl From<Tm> for Timestamp {
-    fn from(tm: Tm) -> Timestamp {
-        Timestamp {
-            hour: tm.tm_hour,
-            min: tm.tm_min,
-        }
-    }
+    /// A valid `time::strftime` format string for rendering timestamps.
+    ts_fmt: Rc<String>,
 }
 
 /// An activity line is just a line that we update on joins / leaves /
 /// disconnects. We group activities that happen in the same minute to avoid
 /// redundantly showing lines.
 struct ActivityLine {
-    ts: Timestamp,
+    ts: Tm,
     line_idx: usize,
 }
 
 impl MessagingUI {
-    pub(crate) fn new(width: i32, height: i32, status: bool) -> MessagingUI {
+    pub(crate) fn new(width: i32, height: i32, status: bool, ts_fmt: Rc<String>) -> MessagingUI {
         MessagingUI {
             msg_area: MsgArea::new(width, height - 1),
             input_field: TextField::new(width),
@@ -91,6 +72,7 @@ impl MessagingUI {
             show_current_nick: true,
             last_activity_line: None,
             last_activity_ts: None,
+            ts_fmt,
         }
     }
 
@@ -263,18 +245,23 @@ impl MessagingUI {
 // Adding new messages
 
 impl MessagingUI {
-    fn add_timestamp(&mut self, ts: Timestamp) {
-        if let Some(ts_) = self.last_activity_ts {
-            if ts_ != ts {
-                ts.stamp(&mut self.msg_area);
+    fn add_timestamp(&mut self, ts: Tm) {
+        let ts_str = time::strftime(&*self.ts_fmt, &ts).unwrap();
+        match self.last_activity_ts {
+            None => {
+                self.msg_area.add_timestamp(&ts_str);
+                self.last_activity_ts = Some(ts_str);
+            }
+            Some(ref last_activity_ts) => {
+                if last_activity_ts != &ts_str {
+                    self.msg_area.add_timestamp(&ts_str);
+                    self.last_activity_ts = Some(ts_str);
+                }
             }
-        } else {
-            ts.stamp(&mut self.msg_area);
         }
-        self.last_activity_ts = Some(ts);
     }
 
-    pub(crate) fn show_topic(&mut self, topic: &str, ts: Timestamp) {
+    pub(crate) fn show_topic(&mut self, topic: &str, ts: Tm) {
         self.add_timestamp(ts);
 
         self.msg_area
@@ -317,7 +304,7 @@ impl MessagingUI {
         &mut self,
         sender: &str,
         msg: &str,
-        ts: Timestamp,
+        ts: Tm,
         highlight: bool,
         is_action: bool,
     ) {
@@ -354,7 +341,7 @@ impl MessagingUI {
         self.msg_area.flush_line();
     }
 
-    pub(crate) fn add_msg(&mut self, msg: &str, ts: Timestamp) {
+    pub(crate) fn add_msg(&mut self, msg: &str, ts: Tm) {
         self.reset_activity_line();
 
         self.add_timestamp(ts);
@@ -364,7 +351,7 @@ impl MessagingUI {
         self.msg_area.flush_line();
     }
 
-    pub(crate) fn add_err_msg(&mut self, msg: &str, ts: Timestamp) {
+    pub(crate) fn add_err_msg(&mut self, msg: &str, ts: Tm) {
         self.reset_activity_line();
 
         self.add_timestamp(ts);
@@ -396,7 +383,7 @@ impl MessagingUI {
         self.nicks.clear();
     }
 
-    pub(crate) fn join(&mut self, nick: &str, ts: Option<Timestamp>) {
+    pub(crate) fn join(&mut self, nick: &str, ts: Option<Tm>) {
         if self.show_status && !self.nicks.contains(nick) {
             if let Some(ts) = ts {
                 let line_idx = self.get_activity_line_idx(ts);
@@ -413,7 +400,7 @@ impl MessagingUI {
         self.nicks.insert(nick);
     }
 
-    pub(crate) fn part(&mut self, nick: &str, ts: Option<Timestamp>) {
+    pub(crate) fn part(&mut self, nick: &str, ts: Option<Tm>) {
         self.nicks.remove(nick);
 
         if self.show_status {
@@ -445,7 +432,7 @@ impl MessagingUI {
         self.show_status
     }
 
-    pub(crate) fn nick(&mut self, old_nick: &str, new_nick: &str, ts: Timestamp) {
+    pub(crate) fn nick(&mut self, old_nick: &str, new_nick: &str, ts: Tm) {
         self.nicks.remove(old_nick);
         self.nicks.insert(new_nick);
 
@@ -465,7 +452,7 @@ impl MessagingUI {
         self.last_activity_line = None;
     }
 
-    fn get_activity_line_idx(&mut self, ts: Timestamp) -> usize {
+    fn get_activity_line_idx(&mut self, ts: Tm) -> usize {
         // borrow checker strikes again
         if let Some(ref mut l) = self.last_activity_line {
             if l.ts == ts {
diff --git a/libtiny_tui/src/msg_area/line.rs b/libtiny_tui/src/msg_area/line.rs
index 982185a7..6dca2533 100644
--- a/libtiny_tui/src/msg_area/line.rs
+++ b/libtiny_tui/src/msg_area/line.rs
@@ -60,7 +60,7 @@ pub(crate) enum SchemeStyle {
 }
 
 impl Seg {
-    pub(crate) fn style(&self, colors: &Colors) -> config::Style {
+    fn style(&self, colors: &Colors) -> config::Style {
         match self.style {
             SegStyle::Fixed(style) => style,
             SegStyle::Index(idx) => config::Style {
@@ -106,17 +106,17 @@ impl Line {
         if self.current_seg.text.is_empty() {
             self.current_seg.style = style;
         } else if self.current_seg.style != style {
-            let seg = mem::replace(
-                &mut self.current_seg,
-                Seg {
-                    text: String::new(),
-                    style,
-                },
-            );
-            self.segs.push(seg);
+            self.flush_current_seg(style);
         }
     }
 
+    pub(crate) fn add_timestamp(&mut self, ts_str: &str) {
+        debug_assert!(self.segs.is_empty());
+        self.set_style(SegStyle::SchemeStyle(SchemeStyle::Timestamp));
+        self.add_text(ts_str);
+        self.add_char(' ');
+    }
+
     pub(crate) fn add_text(&mut self, str: &str) {
         fn push_color(ret: &mut String, irc_fg: u8, irc_bg: Option<u8>) {
             ret.push(TERMBOX_COLOR_PREFIX);
@@ -263,6 +263,17 @@ impl Line {
             }
         }
     }
+
+    fn flush_current_seg(&mut self, style: SegStyle) {
+        let seg = mem::replace(
+            &mut self.current_seg,
+            Seg {
+                text: String::new(),
+                style,
+            },
+        );
+        self.segs.push(seg);
+    }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
diff --git a/libtiny_tui/src/msg_area/mod.rs b/libtiny_tui/src/msg_area/mod.rs
index 52c0d7b8..fd038723 100644
--- a/libtiny_tui/src/msg_area/mod.rs
+++ b/libtiny_tui/src/msg_area/mod.rs
@@ -143,6 +143,10 @@ impl MsgArea {
         self.line_buf.set_style(style);
     }
 
+    pub(crate) fn add_timestamp(&mut self, ts_str: &str) {
+        self.line_buf.add_timestamp(ts_str);
+    }
+
     pub(crate) fn add_text(&mut self, str: &str) {
         self.line_buf.add_text(str);
     }
diff --git a/libtiny_tui/src/tui.rs b/libtiny_tui/src/tui.rs
index 5c1f34c4..aadf5598 100644
--- a/libtiny_tui/src/tui.rs
+++ b/libtiny_tui/src/tui.rs
@@ -3,12 +3,13 @@
 #![allow(clippy::too_many_arguments)]
 
 use std::path::PathBuf;
+use std::rc::Rc;
 use std::str;
 use std::str::SplitWhitespace;
 use time::Tm;
 
-use crate::config::{parse_config, Colors, Config, Style};
-use crate::messaging::{MessagingUI, Timestamp};
+use crate::config::{def_ts_fmt, parse_config, Colors, Config, Style};
+use crate::messaging::MessagingUI;
 use crate::notifier::Notifier;
 use crate::statusline::{draw_statusline, statusline_visible};
 use crate::tab::{Tab, TabStyle};
@@ -60,23 +61,54 @@ pub(crate) struct TUI {
     show_statusline: bool,
     /// Is there room for statusline?
     statusline_visible: bool,
-    /// Config file path
-    config_path: PathBuf,
+
+    /// A valid `time::strftime` format string for rendering timestamps.
+    ts_fmt: Rc<String>,
+
+    /// Config file path, used in `/reload` command to reload TUI colors.
+    config_path: Option<PathBuf>,
 }
 
 impl TUI {
-    pub(crate) fn new(config_path: PathBuf) -> TUI {
-        let tb = Termbox::init().unwrap(); // TODO: check errors
-
-        // This is now done in reload_config() below
-        // tb.set_clear_attributes(colors.clear.fg as u8, colors.clear.bg as u8);
+    pub(crate) fn new(config_path: Option<PathBuf>) -> TUI {
+        let mut tb = Termbox::init().unwrap(); // TODO: check errors
 
         let width = tb.width() as i32;
         let height = tb.height() as i32;
 
+        let config_or_error = match config_path {
+            None => Ok(Config::default()),
+            Some(ref config_path) => parse_config(config_path),
+        };
+
+        let (colors, ts_fmt, opt_err) = match config_or_error {
+            Err(err) => (
+                Colors::default(),
+                def_ts_fmt(),
+                Some(format!("Can't parse TUI config: {:?}", err)),
+            ),
+            Ok(Config {
+                colors,
+                timestamp_format,
+            }) => {
+                // Validate timestamp format
+                let (timestamp_format, err) = match time::strftime(&timestamp_format, &time::now())
+                {
+                    Err(err) => (
+                        def_ts_fmt(),
+                        Some(format!("Invalid timestamp_format field: {}", err)),
+                    ),
+                    Ok(_) => (timestamp_format, None),
+                };
+                (colors, timestamp_format, err)
+            }
+        };
+
+        tb.set_clear_attributes(colors.clear.fg as u8, colors.clear.bg as u8);
+
         let mut tui = TUI {
             tb,
-            colors: Colors::default(),
+            colors,
             tabs: Vec::new(),
             active_idx: 0,
             width,
@@ -84,6 +116,7 @@ impl TUI {
             h_scroll: 0,
             show_statusline: false,
             statusline_visible: statusline_visible(width, height),
+            ts_fmt: Rc::new(ts_fmt),
             config_path,
         };
 
@@ -95,7 +128,10 @@ impl TUI {
             &MsgTarget::Server { serv: "mentions" },
         );
 
-        tui.reload_config();
+        if let Some(err) = opt_err {
+            tui.add_client_err_msg(&err, &MsgTarget::CurrentTab);
+        }
+
         tui
     }
 
@@ -197,23 +233,28 @@ impl TUI {
                 true
             }
             Some("reload") => {
-                self.reload_config();
+                self.reload_colors();
                 true
             }
             _ => false,
         }
     }
 
-    pub(crate) fn reload_config(&mut self) {
-        match parse_config(&self.config_path) {
-            Err(err) => {
-                self.add_client_err_msg(
-                    &format!("Can't parse TUI config: {:?}", err),
-                    &MsgTarget::CurrentTab,
-                );
-            }
-            Ok(Config { colors }) => {
-                self.set_colors(colors);
+    fn reload_colors(&mut self) {
+        if let Some(ref config_path) = self.config_path {
+            match parse_config(config_path) {
+                Err(err) => {
+                    self.add_client_err_msg(
+                        &format!("Can't parse TUI config: {:?}", err),
+                        &MsgTarget::CurrentTab,
+                    );
+                }
+                Ok(Config {
+                    colors,
+                    timestamp_format: _,
+                }) => {
+                    self.set_colors(colors);
+                }
             }
         }
     }
@@ -265,7 +306,12 @@ impl TUI {
         self.tabs.insert(
             idx,
             Tab {
-                widget: MessagingUI::new(self.width, self.height - 1 - statusline_height, status),
+                widget: MessagingUI::new(
+                    self.width,
+                    self.height - 1 - statusline_height,
+                    status,
+                    self.ts_fmt.clone(),
+                ),
                 src,
                 style: TabStyle::Normal,
                 switch,
@@ -1049,7 +1095,7 @@ impl TUI {
     ) {
         self.apply_to_target(target, &|tab: &mut Tab, _| {
             tab.widget
-                .add_privmsg(sender, msg, Timestamp::from(ts), highlight, is_action);
+                .add_privmsg(sender, msg, ts, highlight, is_action);
             let nick = tab.widget.get_nick();
             if let Some(nick_) = nick {
                 tab.notifier
@@ -1062,7 +1108,7 @@ impl TUI {
     /// and debug log tabs. Timestamped and logged.
     pub(crate) fn add_msg(&mut self, msg: &str, ts: Tm, target: &MsgTarget) {
         self.apply_to_target(target, &|tab: &mut Tab, _| {
-            tab.widget.add_msg(msg, Timestamp::from(ts));
+            tab.widget.add_msg(msg, ts);
         });
     }
 
@@ -1070,14 +1116,14 @@ impl TUI {
     /// nickname is in use etc. Timestamped and logged.
     pub(crate) fn add_err_msg(&mut self, msg: &str, ts: Tm, target: &MsgTarget) {
         self.apply_to_target(target, &|tab: &mut Tab, _| {
-            tab.widget.add_err_msg(msg, Timestamp::from(ts));
+            tab.widget.add_err_msg(msg, ts);
         });
     }
 
     pub(crate) fn set_topic(&mut self, title: &str, ts: Tm, serv: &str, chan: &str) {
         let target = MsgTarget::Chan { serv, chan };
         self.apply_to_target(&target, &|tab: &mut Tab, _| {
-            tab.widget.show_topic(title, Timestamp::from(ts));
+            tab.widget.show_topic(title, ts);
         });
     }
 
@@ -1090,13 +1136,13 @@ impl TUI {
 
     pub(crate) fn add_nick(&mut self, nick: &str, ts: Option<Tm>, target: &MsgTarget) {
         self.apply_to_target(target, &|tab: &mut Tab, _| {
-            tab.widget.join(nick, ts.map(Timestamp::from));
+            tab.widget.join(nick, ts);
         });
     }
 
     pub(crate) fn remove_nick(&mut self, nick: &str, ts: Option<Tm>, target: &MsgTarget) {
         self.apply_to_target(target, &|tab: &mut Tab, _| {
-            tab.widget.part(nick, ts.map(Timestamp::from));
+            tab.widget.part(nick, ts);
         });
     }
 
@@ -1108,7 +1154,7 @@ impl TUI {
         target: &MsgTarget,
     ) {
         self.apply_to_target(target, &|tab: &mut Tab, _| {
-            tab.widget.nick(old_nick, new_nick, Timestamp::from(ts));
+            tab.widget.nick(old_nick, new_nick, ts);
             // TODO: Does this actually rename the tab?
             tab.update_source(&|src: &mut MsgSource| {
                 if let MsgSource::User { ref mut nick, .. } = *src {
diff --git a/tiny/config.yml b/tiny/config.yml
index 16ef876f..debc6a2d 100644
--- a/tiny/config.yml
+++ b/tiny/config.yml
@@ -37,6 +37,10 @@ defaults:
 # Where to put log files
 log_dir: '{}'
 
+# How to show timestamps. Should be a valid strftime() format string. Run `man
+# strftime` in your shell for strftime format specification.
+timestamp_format: '%H:%M'
+
 # Color theme based on 256 colors. Colors can be defined as color indices
 # (0-255) or with their names.
 #
diff --git a/tiny/src/main.rs b/tiny/src/main.rs
index 6aac1b57..814aa50c 100644
--- a/tiny/src/main.rs
+++ b/tiny/src/main.rs
@@ -78,7 +78,7 @@ fn run(
     let mut executor = tokio::runtime::current_thread::Runtime::new().unwrap();
 
     // Create TUI task
-    let (tui, rcv_tui_ev) = TUI::run(config_path.clone(), &mut executor);
+    let (tui, rcv_tui_ev) = TUI::run(Some(config_path.clone()), &mut executor);
     tui.draw();
 
     // Create logger


Test patch: