Repository: LiveSplit/livesplit-core
Pull request #447 (2021-09-30 19:24:19)

Remove the `Short` Time Formatter
It turns out we don't actually really need it anywhere. It's the same as the `SegmentTime` time formatter, except on `None` it pretends there to be a value, which doesn't really make all that much sense. The run editor even wraps it in an `Empty` wrapper that replaces its `None` value with an empty string, which is the same as just wrapping the `SegmentTime` one.


Patch:
diff --git a/src/run/editor/cleaning.rs b/src/run/editor/cleaning.rs
index 17d6484..f82d9fe 100644
--- a/src/run/editor/cleaning.rs
+++ b/src/run/editor/cleaning.rs
@@ -6,13 +6,13 @@
 //! occurrences of this and allows you to delete them individually if any of
 //! them seem wrong.
 
-use crate::analysis::sum_of_segments::{best, track_branch, Prediction};
-use crate::platform::prelude::*;
-use crate::platform::Local;
-use crate::timing::formatter::{Short, TimeFormatter};
-use crate::{Attempt, Run, Segment, TimeSpan, TimingMethod};
-use core::fmt;
-use core::mem::replace;
+use crate::{
+    analysis::sum_of_segments::{best, track_branch, Prediction},
+    platform::{prelude::*, Local},
+    timing::formatter::{SegmentTime, TimeFormatter},
+    Attempt, Run, Segment, TimeSpan, TimingMethod,
+};
+use core::{fmt, mem::replace};
 
 /// A Sum of Best Cleaner allows you to interactively remove potential issues in
 /// the Segment History that lead to an inaccurate Sum of Best. If you skip a
@@ -67,7 +67,7 @@ pub struct CleanUp {
 
 impl fmt::Display for PotentialCleanUp<'_> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let short = Short::new();
+        let short = SegmentTime::new();
 
         let method = match self.method {
             TimingMethod::RealTime => "Real Time",
diff --git a/src/run/editor/state.rs b/src/run/editor/state.rs
index 2500a98..00b5254 100644
--- a/src/run/editor/state.rs
+++ b/src/run/editor/state.rs
@@ -4,7 +4,7 @@ use crate::{
     platform::prelude::*,
     run::RunMetadata,
     settings::{CachedImageId, ImageData},
-    timing::formatter::{none_wrapper::EmptyWrapper, Accuracy, Short, TimeFormatter},
+    timing::formatter::{none_wrapper::EmptyWrapper, Accuracy, SegmentTime, TimeFormatter},
 };
 use serde::{Deserialize, Serialize};
 
@@ -114,7 +114,7 @@ impl State {
 impl Editor {
     /// Calculates the Run Editor's state in order to visualize it.
     pub fn state(&mut self) -> State {
-        let formatter = EmptyWrapper::new(Short::with_accuracy(Accuracy::Hundredths));
+        let formatter = EmptyWrapper::new(SegmentTime::with_accuracy(Accuracy::Hundredths));
 
         let icon_change = self
             .game_icon_id
diff --git a/src/run/parser/face_split.rs b/src/run/parser/face_split.rs
index 4bf27b3..e1674fd 100644
--- a/src/run/parser/face_split.rs
+++ b/src/run/parser/face_split.rs
@@ -2,8 +2,7 @@
 
 use crate::{settings::Image, timing, RealTime, Run, Segment, Time, TimeSpan};
 use alloc::borrow::Cow;
-use core::num::ParseIntError;
-use core::result::Result as StdResult;
+use core::{num::ParseIntError, result::Result as StdResult};
 use snafu::{OptionExt, ResultExt};
 use std::io::{self, BufRead};
 
@@ -74,7 +73,7 @@ fn parse_time(time: &str) -> StdResult<Time, timing::ParseError> {
     }
 }
 
-fn replace<'a>(text: &'a str, a: &'a str, b: &str) -> Cow<'a, str> {
+fn replace<'text>(text: &'text str, a: &str, b: &str) -> Cow<'text, str> {
     if text.contains(a) {
         text.replace(a, b).into()
     } else {
diff --git a/src/timing/formatter/mod.rs b/src/timing/formatter/mod.rs
index f9153e8..6bb8e0b 100644
--- a/src/timing/formatter/mod.rs
+++ b/src/timing/formatter/mod.rs
@@ -3,16 +3,16 @@
 //!
 //! # Examples
 //!
-//! Using a Short Time Formatter to format a Time Span.
+//! Using a [`SegmentTime`] [`TimeFormatter`] to format a [`TimeSpan`].
 //!
 //! ```
-//! use livesplit_core::timing::formatter::{Short, TimeFormatter};
+//! use livesplit_core::timing::formatter::{SegmentTime, TimeFormatter};
 //! use livesplit_core::TimeSpan;
 //!
-//! // Create the Short Time Formatter.
-//! let formatter = Short::new();
+//! // Create the SegmentTime TimeFormatter.
+//! let formatter = SegmentTime::new();
 //!
-//! // Create a Time Span.
+//! // Create a TimeSpan.
 //! let time = TimeSpan::from_seconds(-(4.0 * 60.0 + 23.5));
 //!
 //! // Format it with the formatter.
@@ -28,22 +28,18 @@ mod digits_format;
 pub mod none_wrapper;
 mod regular;
 mod segment_time;
-mod short;
 pub mod timer;
 
-pub use self::accuracy::Accuracy;
-pub use self::complete::Complete;
-pub use self::days::Days;
-pub use self::delta::Delta;
-pub use self::digits_format::DigitsFormat;
-pub use self::regular::Regular;
-pub use self::segment_time::SegmentTime;
-pub use self::short::Short;
+pub use self::{
+    accuracy::Accuracy, complete::Complete, days::Days, delta::Delta, digits_format::DigitsFormat,
+    regular::Regular, segment_time::SegmentTime,
+};
 
-use crate::platform::math::f64::{abs, floor};
-use crate::TimeSpan;
-use core::cmp::min;
-use core::fmt::Display;
+use crate::{
+    platform::math::f64::{abs, floor},
+    TimeSpan,
+};
+use core::{cmp::min, fmt::Display};
 
 /// Time Formatters can be used to format optional Time Spans in various ways.
 pub trait TimeFormatter<'a> {
diff --git a/src/timing/formatter/segment_time.rs b/src/timing/formatter/segment_time.rs
index 7e0b6e2..13f518d 100644
--- a/src/timing/formatter/segment_time.rs
+++ b/src/timing/formatter/segment_time.rs
@@ -11,8 +11,7 @@ pub struct Inner {
 /// Segment Times. This specifically means that the fractional part of the time
 /// is always shown and the minutes and hours are only shown when necessary. The
 /// default accuracy is to show 2 digits of the fractional part, but this can be
-/// configured. Unlike the Short Time Formatter, the Segment Time Formatter
-/// shows a dash when there's an empty time.
+/// configured.
 ///
 /// # Example Formatting
 ///
@@ -96,3 +95,15 @@ impl Display for Inner {
         }
     }
 }
+
+#[test]
+fn test() {
+    let time = "4:20.69".parse::<TimeSpan>().unwrap();
+    let formatted = SegmentTime::new().format(time).to_string();
+    assert!(
+        // Modern processors
+        formatted == "4:20.69" ||
+        // Old x86 processors are apparently less precise
+        formatted == "4:20.68"
+    );
+}
diff --git a/src/timing/formatter/short.rs b/src/timing/formatter/short.rs
deleted file mode 100644
index 2677154..0000000
--- a/src/timing/formatter/short.rs
+++ /dev/null
@@ -1,114 +0,0 @@
-use super::{Accuracy, TimeFormatter, MINUS};
-use crate::TimeSpan;
-use core::fmt::{Display, Formatter, Result};
-
-pub struct Inner {
-    time: Option<TimeSpan>,
-    accuracy: Accuracy,
-}
-
-/// The Short Time Formatter is suitable for situations where you want a short
-/// visualization of a Time Span, but also a somewhat accurate one. This
-/// specifically means that the fractional part of the time is always shown and
-/// the minutes and hours are only shown when necessary. The default accuracy is
-/// to show 2 digits of the fractional part, but this can be configured.
-///
-/// # Example Formatting
-///
-/// * Empty Time `0.00`
-/// * Seconds `23.12`
-/// * Minutes `12:34.98`
-/// * Hours `12:34:56.12`
-/// * Negative Times `−23.12`
-pub struct Short {
-    accuracy: Accuracy,
-}
-
-impl Short {
-    /// Creates a new Short Time Formatter that uses hundredths for showing the
-    /// fractional part.
-    pub const fn new() -> Self {
-        Short {
-            accuracy: Accuracy::Hundredths,
-        }
-    }
-
-    /// Creates a new Short Time Formatter that uses the accuracy provided for
-    /// showing the fractional part.
-    pub const fn with_accuracy(accuracy: Accuracy) -> Self {
-        Short { accuracy }
-    }
-}
-
-impl Default for Short {
-    fn default() -> Self {
-        Self::new()
-    }
-}
-
-impl TimeFormatter<'_> for Short {
-    type Inner = Inner;
-
-    fn format<T>(&self, time: T) -> Self::Inner
-    where
-        T: Into<Option<TimeSpan>>,
-    {
-        Inner {
-            time: time.into(),
-            accuracy: self.accuracy,
-        }
-    }
-}
-
-impl Display for Inner {
-    fn fmt(&self, f: &mut Formatter<'_>) -> Result {
-        if let Some(time) = self.time {
-            let mut total_seconds = time.total_seconds();
-            if total_seconds < 0.0 {
-                total_seconds *= -1.0;
-                write!(f, "{}", MINUS)?;
-            }
-            let seconds = total_seconds % 60.0;
-            let total_minutes = (total_seconds / 60.0) as u64;
-            let minutes = total_minutes % 60;
-            let hours = total_minutes / 60;
-            if hours > 0 {
-                write!(
-                    f,
-                    "{}:{:02}:{}",
-                    hours,
-                    minutes,
-                    self.accuracy.format_seconds(seconds, true)
-                )
-            } else if total_minutes > 0 {
-                write!(
-                    f,
-                    "{}:{}",
-                    minutes,
-                    self.accuracy.format_seconds(seconds, true)
-                )
-            } else {
-                write!(f, "{}", self.accuracy.format_seconds(seconds, false))
-            }
-        } else {
-            match self.accuracy {
-                Accuracy::Seconds => write!(f, "0"),
-                Accuracy::Tenths => write!(f, "0.0"),
-                Accuracy::Hundredths => write!(f, "0.00"),
-                Accuracy::Milliseconds => write!(f, "0.000"),
-            }
-        }
-    }
-}
-
-#[test]
-fn test() {
-    let time = "4:20.69".parse::<TimeSpan>().unwrap();
-    let formatted = Short::new().format(time).to_string();
-    assert!(
-        // Modern processors
-        formatted == "4:20.69" ||
-        // Old x86 processors are apparently less precise
-        formatted == "4:20.68"
-    );
-}


Test patch:
diff --git a/src/comparison/tests/balanced_pb.rs b/src/comparison/tests/balanced_pb.rs
index 48859e1..640b76a 100644
--- a/src/comparison/tests/balanced_pb.rs
+++ b/src/comparison/tests/balanced_pb.rs
@@ -1,7 +1,9 @@
-use crate::comparison::balanced_pb::{BalancedPB, NAME};
-use crate::tests_helper::{run_with_splits, run_with_splits_opt};
-use crate::timing::formatter::{Short, TimeFormatter};
-use crate::{Run, Segment, TimeSpan, Timer};
+use crate::{
+    comparison::balanced_pb::{BalancedPB, NAME},
+    tests_helper::{run_with_splits, run_with_splits_opt},
+    timing::formatter::{SegmentTime, TimeFormatter},
+    Run, Segment, TimeSpan, Timer,
+};
 
 #[test]
 fn test() {
@@ -41,13 +43,13 @@ fn test() {
     {
         let run = timer.run();
         assert_eq!(
-            Short::new()
+            SegmentTime::new()
                 .format(run.segment(0).comparison(NAME).game_time)
                 .to_string(),
             "0.46"
         );
         assert_eq!(
-            Short::new()
+            SegmentTime::new()
                 .format(run.segment(1).comparison(NAME).game_time)
                 .to_string(),
             "2.53"