From 318bdd895590c97dd53f8d3661d76fa1c0cd67a0 Mon Sep 17 00:00:00 2001 From: cyqsimon <28627918+cyqsimon@users.noreply.github.com> Date: Tue, 6 Feb 2024 23:34:03 +0800 Subject: 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 --- atuin-client/src/settings.rs | 98 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 89 insertions(+), 9 deletions(-) (limited to 'atuin-client/src/settings.rs') diff --git a/atuin-client/src/settings.rs b/atuin-client/src/settings.rs index 833311c8..7abacd0d 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 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: <+|->[:[:]] +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 { + // 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, } 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(()) + } +} -- cgit v1.3.1