Add timezone configuration option & CLI overrides (#1517)

* Allow specifying a timezone in history search/list

* Fix clippy complaints

* Add a bit more comment on supporting named timezones

* Add rudimentary tests

* Ditch local timezone test

* Timezone configuration support

* Set default timezone to `local`

* `--tz` -> `--timezone`

`--tz` is kept as a visible alias

1 files changed, 89 insertions, 9 deletions

diff --git a/atuin-client/src/settings.rs b/atuin-client/src/settings.rs
index 833311c8f..7abacd0de 100644
--- a/atuin-client/src/settings.rs
+++ b/atuin-client/src/settings.rs
@@ -1,6 +1,7 @@
 use std::{
     collections::HashMap,
     convert::TryFrom,
+    fmt,
     io::prelude::*,
     path::{Path, PathBuf},
     str::FromStr,
@@ -11,13 +12,18 @@ use clap::ValueEnum;
 use config::{
     builder::DefaultState, Config, ConfigBuilder, Environment, File as ConfigFile, FileFormat,
 };
-use eyre::{eyre, Context, Result};
+use eyre::{bail, eyre, Context, Error, Result};
 use fs_err::{create_dir_all, File};
 use parse_duration::parse;
 use regex::RegexSet;
 use semver::Version;
 use serde::Deserialize;
-use time::{format_description::well_known::Rfc3339, OffsetDateTime};
+use serde_with::DeserializeFromStr;
+use time::{
+    format_description::{well_known::Rfc3339, FormatItem},
+    macros::format_description,
+    OffsetDateTime, UtcOffset,
+};
 use uuid::Uuid;
 
 pub const HISTORY_PAGE_SIZE: i64 = 100;
@@ -123,6 +129,46 @@ impl From<Dialect> for interim::Dialect {
     }
 }
 
+/// Type wrapper around `time::UtcOffset` to support a wider variety of timezone formats.
+///
+/// Note that the parsing of this struct needs to be done before starting any
+/// multithreaded runtime, otherwise it will fail on most Unix systems.
+///
+/// See: https://github.com/atuinsh/atuin/pull/1517#discussion_r1447516426
+#[derive(Clone, Copy, Debug, Eq, PartialEq, DeserializeFromStr)]
+pub struct Timezone(pub UtcOffset);
+impl fmt::Display for Timezone {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        self.0.fmt(f)
+    }
+}
+/// format: <+|-><hour>[:<minute>[:<second>]]
+static OFFSET_FMT: &[FormatItem<'_>] =
+    format_description!("[offset_hour sign:mandatory padding:none][optional [:[offset_minute padding:none][optional [:[offset_second padding:none]]]]]");
+impl FromStr for Timezone {
+    type Err = Error;
+
+    fn from_str(s: &str) -> Result<Self> {
+        // local timezone
+        if matches!(s.to_lowercase().as_str(), "l" | "local") {
+            let offset = UtcOffset::current_local_offset()?;
+            return Ok(Self(offset));
+        }
+
+        // offset from UTC
+        if let Ok(offset) = UtcOffset::parse(s, OFFSET_FMT) {
+            return Ok(Self(offset));
+        }
+
+        // IDEA: Currently named timezones are not supported, because the well-known crate
+        // for this is `chrono_tz`, which is not really interoperable with the datetime crate
+        // that we currently use - `time`. If ever we migrate to using `chrono`, this would
+        // be a good feature to add.
+
+        bail!(r#""{s}" is not a valid timezone spec"#)
+    }
+}
+
 #[derive(Clone, Debug, Deserialize, Copy)]
 pub enum Style {
     #[serde(rename = "auto")]
@@ -251,6 +297,7 @@ pub struct Sync {
 #[derive(Clone, Debug, Deserialize)]
 pub struct Settings {
     pub dialect: Dialect,
+    pub timezone: Timezone,
     pub style: Style,
     pub auto_sync: bool,
     pub update_check: bool,
@@ -305,11 +352,6 @@ pub struct Settings {
     // config! Keep secrets and settings apart.
     #[serde(skip)]
     pub session_token: String,
-
-    // This is determined at startup and cached.
-    // This is due to non-threadsafe get-env limitations.
-    #[serde(skip)]
-    pub local_tz: Option<time::UtcOffset>,
 }
 
 impl Settings {
@@ -488,6 +530,7 @@ impl Settings {
             .set_default("key_path", key_path.to_str())?
             .set_default("session_path", session_path.to_str())?
             .set_default("dialect", "us")?
+            .set_default("timezone", "local")?
             .set_default("auto_sync", true)?
             .set_default("update_check", cfg!(feature = "check-update"))?
             .set_default("sync_address", "https://api.atuin.sh")?
@@ -599,8 +642,6 @@ impl Settings {
             settings.session_token = String::from("not logged in");
         }
 
-        settings.local_tz = time::UtcOffset::current_local_offset().ok();
-
         Ok(settings)
     }
 
@@ -621,3 +662,42 @@ impl Default for Settings {
             .expect("Could not deserialize config")
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use std::str::FromStr;
+
+    use eyre::Result;
+
+    use super::Timezone;
+
+    #[test]
+    fn can_parse_offset_timezone_spec() -> Result<()> {
+        assert_eq!(Timezone::from_str("+02")?.0.as_hms(), (2, 0, 0));
+        assert_eq!(Timezone::from_str("-04")?.0.as_hms(), (-4, 0, 0));
+        assert_eq!(Timezone::from_str("+05:30")?.0.as_hms(), (5, 30, 0));
+        assert_eq!(Timezone::from_str("-09:30")?.0.as_hms(), (-9, -30, 0));
+
+        // single digit hours are allowed
+        assert_eq!(Timezone::from_str("+2")?.0.as_hms(), (2, 0, 0));
+        assert_eq!(Timezone::from_str("-4")?.0.as_hms(), (-4, 0, 0));
+        assert_eq!(Timezone::from_str("+5:30")?.0.as_hms(), (5, 30, 0));
+        assert_eq!(Timezone::from_str("-9:30")?.0.as_hms(), (-9, -30, 0));
+
+        // fully qualified form
+        assert_eq!(Timezone::from_str("+09:30:00")?.0.as_hms(), (9, 30, 0));
+        assert_eq!(Timezone::from_str("-09:30:00")?.0.as_hms(), (-9, -30, 0));
+
+        // these offsets don't really exist but are supported anyway
+        assert_eq!(Timezone::from_str("+0:5")?.0.as_hms(), (0, 5, 0));
+        assert_eq!(Timezone::from_str("-0:5")?.0.as_hms(), (0, -5, 0));
+        assert_eq!(Timezone::from_str("+01:23:45")?.0.as_hms(), (1, 23, 45));
+        assert_eq!(Timezone::from_str("-01:23:45")?.0.as_hms(), (-1, -23, -45));
+
+        // require a leading sign for clarity
+        assert!(Timezone::from_str("5").is_err());
+        assert!(Timezone::from_str("10:30").is_err());
+
+        Ok(())
+    }
+}