Convert Align and Placeholder into enums

Also move number and style logic out of `format_and_paint_line_numbers()`
and into a separate `linenumbers_and_styles()` function.

4 files changed, 144 insertions, 78 deletions

diff --git a/src/features/line_numbers.rs b/src/features/line_numbers.rs
index eee57b03..8aeb9918 100644
--- a/src/features/line_numbers.rs
+++ b/src/features/line_numbers.rs
@@ -8,7 +8,7 @@ use crate::delta::State;
 use crate::features::hyperlinks;
 use crate::features::side_by_side::{Left, PanelSide, Right};
 use crate::features::OptionValueFunction;
-use crate::format;
+use crate::format::{self, Align, Placeholder};
 use crate::minusplus::*;
 use crate::style::Style;
 
@@ -61,14 +61,11 @@ pub fn make_feature() -> Vec<(String, OptionValueFunction)> {
     ])
 }
 
-/// Return a vec of `ansi_term::ANSIGenericString`s representing the left and right fields of the
-/// two-column line number display.
-pub fn format_and_paint_line_numbers<'a>(
+pub fn linenumbers_and_styles<'a>(
     line_numbers_data: &'a mut LineNumbersData,
     state: &State,
-    side_by_side_panel: Option<PanelSide>,
     config: &'a config::Config,
-) -> Vec<ansi_term::ANSIGenericString<'a, str>> {
+) -> Option<(MinusPlus<Option<usize>>, MinusPlus<Style>)> {
     let nr_left = line_numbers_data.line_number[Left];
     let nr_right = line_numbers_data.line_number[Right];
     let (minus_style, zero_style, plus_style) = (
@@ -93,9 +90,23 @@ pub fn format_and_paint_line_numbers<'a>(
             ((None, Some(nr_right)), (minus_style, plus_style))
         }
         State::HunkPlusWrapped => ((None, None), (minus_style, plus_style)),
-        _ => return Vec::new(),
+        _ => return None,
     };
+    Some((
+        MinusPlus::new(minus_number, plus_number),
+        MinusPlus::new(minus_style, plus_style),
+    ))
+}
 
+/// Return a vec of `ansi_term::ANSIGenericString`s representing the left and right fields of the
+/// two-column line number display.
+pub fn format_and_paint_line_numbers<'a>(
+    line_numbers_data: &'a LineNumbersData,
+    side_by_side_panel: Option<PanelSide>,
+    styles: MinusPlus<Style>,
+    line_numbers: MinusPlus<Option<usize>>,
+    config: &'a config::Config,
+) -> Vec<ansi_term::ANSIGenericString<'a, str>> {
     let mut formatted_numbers = Vec::new();
 
     let (emit_left, emit_right) = match (config.side_by_side, side_by_side_panel) {
@@ -107,28 +118,22 @@ pub fn format_and_paint_line_numbers<'a>(
 
     if emit_left {
         formatted_numbers.extend(format_and_paint_line_number_field(
-            &line_numbers_data.format_data[Left],
+            line_numbers_data,
+            Minus,
             &config.line_numbers_left_style,
-            minus_number,
-            plus_number,
-            line_numbers_data.hunk_max_line_number_width,
-            &minus_style,
-            &plus_style,
-            &line_numbers_data.plus_file,
+            &styles,
+            &line_numbers,
             config,
         ));
     }
 
     if emit_right {
         formatted_numbers.extend(format_and_paint_line_number_field(
-            &line_numbers_data.format_data[Right],
+            line_numbers_data,
+            Plus,
             &config.line_numbers_right_style,
-            minus_number,
-            plus_number,
-            line_numbers_data.hunk_max_line_number_width,
-            &minus_style,
-            &plus_style,
-            &line_numbers_data.plus_file,
+            &styles,
+            &line_numbers,
             config,
         ));
     }
@@ -205,45 +210,48 @@ impl<'a> LineNumbersData<'a> {
 
 #[allow(clippy::too_many_arguments)]
 fn format_and_paint_line_number_field<'a>(
-    format_data: &'a [format::FormatStringPlaceholderData<'a>],
+    line_numbers_data: &'a LineNumbersData,
+    side: MinusPlusIndex,
     style: &Style,
-    minus_number: Option<usize>,
-    plus_number: Option<usize>,
-    min_field_width: usize,
-    minus_number_style: &Style,
-    plus_number_style: &Style,
-    plus_file: &str,
+    styles: &MinusPlus<Style>,
+    line_numbers: &MinusPlus<Option<usize>>,
     config: &config::Config,
 ) -> Vec<ansi_term::ANSIGenericString<'a, str>> {
+    let min_field_width = line_numbers_data.hunk_max_line_number_width;
+
+    let format_data = &line_numbers_data.format_data[side];
+    let plus_file = &line_numbers_data.plus_file;
+
     let mut ansi_strings = Vec::new();
     let mut suffix = "";
     for placeholder in format_data {
         ansi_strings.push(style.paint(placeholder.prefix.as_str()));
 
-        let alignment_spec = placeholder.alignment_spec.unwrap_or("^");
         let width = if let Some(placeholder_width) = placeholder.width {
             max(placeholder_width, min_field_width)
         } else {
             min_field_width
         };
 
+        let alignment_spec = placeholder
+            .alignment_spec
+            .as_ref()
+            .unwrap_or(&Align::Center);
         match placeholder.placeholder {
-            Some("nm") => ansi_strings.push(minus_number_style.paint(format_line_number(
-                minus_number,
-                alignment_spec,
-                width,
-                None,
-                config,
-            ))),
-            Some("np") => ansi_strings.push(plus_number_style.paint(format_line_number(
-                plus_number,
-                alignment_spec,
-                width,
-                Some(plus_file),
-                config,
-            ))),
+            Some(Placeholder::NumberMinus) => ansi_strings.push(styles[Minus].paint(
+                format_line_number(line_numbers[Minus], alignment_spec, width, None, config),
+            )),
+            Some(Placeholder::NumberPlus) => {
+                ansi_strings.push(styles[Plus].paint(format_line_number(
+                    line_numbers[Plus],
+                    alignment_spec,
+                    width,
+                    Some(plus_file),
+                    config,
+                )))
+            }
             None => {}
-            Some(_) => unreachable!(),
+            _ => unreachable!("Invalid placeholder"),
         }
         suffix = placeholder.suffix.as_str();
     }
@@ -254,7 +262,7 @@ fn format_and_paint_line_number_field<'a>(
 /// Return line number formatted according to `alignment` and `width`.
 fn format_line_number(
     line_number: Option<usize>,
-    alignment: &str,
+    alignment: &Align,
     width: usize,
     plus_file: Option<&str>,
     config: &config::Config,
@@ -285,7 +293,7 @@ pub mod tests {
             format::parse_line_number_format("{nm}", &LINE_NUMBERS_PLACEHOLDER_REGEX),
             vec![format::FormatStringPlaceholderData {
                 prefix: "".into(),
-                placeholder: Some("nm"),
+                placeholder: Some(Placeholder::NumberMinus),
                 alignment_spec: None,
                 width: None,
                 suffix: "".into(),
@@ -301,7 +309,7 @@ pub mod tests {
             format::parse_line_number_format("{np:4}", &LINE_NUMBERS_PLACEHOLDER_REGEX),
             vec![format::FormatStringPlaceholderData {
                 prefix: "".into(),
-                placeholder: Some("np"),
+                placeholder: Some(Placeholder::NumberPlus),
                 alignment_spec: None,
                 width: Some(4),
                 suffix: "".into(),
@@ -317,8 +325,8 @@ pub mod tests {
             format::parse_line_number_format("{np:>4}", &LINE_NUMBERS_PLACEHOLDER_REGEX),
             vec![format::FormatStringPlaceholderData {
                 prefix: "".into(),
-                placeholder: Some("np"),
-                alignment_spec: Some(">"),
+                placeholder: Some(Placeholder::NumberPlus),
+                alignment_spec: Some(Align::Right),
                 width: Some(4),
                 suffix: "".into(),
                 prefix_len: 0,
@@ -333,8 +341,8 @@ pub mod tests {
             format::parse_line_number_format("{np:_>4}", &LINE_NUMBERS_PLACEHOLDER_REGEX),
             vec![format::FormatStringPlaceholderData {
                 prefix: "".into(),
-                placeholder: Some("np"),
-                alignment_spec: Some(">"),
+                placeholder: Some(Placeholder::NumberPlus),
+                alignment_spec: Some(Align::Right),
                 width: Some(4),
                 suffix: "".into(),
                 prefix_len: 0,
@@ -349,8 +357,8 @@ pub mod tests {
             format::parse_line_number_format("__{np:_>4}@@", &LINE_NUMBERS_PLACEHOLDER_REGEX),
             vec![format::FormatStringPlaceholderData {
                 prefix: "__".into(),
-                placeholder: Some("np"),
-                alignment_spec: Some(">"),
+                placeholder: Some(Placeholder::NumberPlus),
+                alignment_spec: Some(Align::Right),
                 width: Some(4),
                 suffix: "@@".into(),
                 prefix_len: 2,
@@ -369,8 +377,8 @@ pub mod tests {
             vec![
                 format::FormatStringPlaceholderData {
                     prefix: "__".into(),
-                    placeholder: Some("nm"),
-                    alignment_spec: Some("<"),
+                    placeholder: Some(Placeholder::NumberMinus),
+                    alignment_spec: Some(Align::Left),
                     width: Some(3),
                     suffix: "@@---{np:_>4}**".into(),
                     prefix_len: 2,
@@ -378,8 +386,8 @@ pub mod tests {
                 },
                 format::FormatStringPlaceholderData {
                     prefix: "@@---".into(),
-                    placeholder: Some("np"),
-                    alignment_spec: Some(">"),
+                    placeholder: Some(Placeholder::NumberPlus),
+                    alignment_spec: Some(Align::Right),
                     width: Some(4),
                     suffix: "**".into(),
                     prefix_len: 5,
@@ -417,7 +425,7 @@ pub mod tests {
             vec![format::FormatStringPlaceholderData {
                 prefix: long.into(),
                 prefix_len: long.len(),
-                placeholder: Some("nm"),
+                placeholder: Some(Placeholder::NumberMinus),
                 alignment_spec: None,
                 width: None,
                 suffix: long.into(),
diff --git a/src/format.rs b/src/format.rs
index 0b77e699..57a39790 100644
--- a/src/format.rs
+++ b/src/format.rs
@@ -1,13 +1,57 @@
+use std::convert::{TryFrom, TryInto};
+
 use regex::Regex;
 use smol_str::SmolStr;
 use unicode_segmentation::UnicodeSegmentation;
 
+#[derive(Debug, PartialEq)]
+pub enum Placeholder<'a> {
+    NumberMinus,
+    NumberPlus,
+    Str(&'a str),
+}
+
+impl<'a> TryFrom<Option<&'a str>> for Placeholder<'a> {
+    type Error = ();
+    fn try_from(from: Option<&'a str>) -> Result<Self, Self::Error> {
+        match from {
+            Some(placeholder) if placeholder == "nm" => Ok(Placeholder::NumberMinus),
+            Some(placeholder) if placeholder == "np" => Ok(Placeholder::NumberPlus),
+            Some(placeholder) => Ok(Placeholder::Str(placeholder)),
+            _ => Err(()),
+        }
+    }
+}
+
+#[derive(Debug, PartialEq)]
+pub enum Align {
+    Left,
+    Center,
+    Right,
+}
+
+impl TryFrom<Option<&str>> for Align {
+    type Error = ();
+    fn try_from(from: Option<&str>) -> Result<Self, Self::Error> {
+        match from {
+            Some(alignment) if alignment == "<" => Ok(Align::Left),
+            Some(alignment) if alignment == ">" => Ok(Align::Right),
+            Some(alignment) if alignment == "^" => Ok(Align::Center),
+            Some(alignment) => {
+                debug_assert!(false, "Unknown Alignment: {}", alignment);
+                Err(())
+            }
+            None => Err(()),
+        }
+    }
+}
+
 #[derive(Debug, Default, PartialEq)]
 pub struct FormatStringPlaceholderData<'a> {
     pub prefix: SmolStr,
     pub prefix_len: usize,
-    pub placeholder: Option<&'a str>,
-    pub alignment_spec: Option<&'a str>,
+    pub placeholder: Option<Placeholder<'a>>,
+    pub alignment_spec: Option<Align>,
     pub width: Option<usize>,
     pub suffix: SmolStr,
     pub suffix_len: usize,
@@ -20,7 +64,9 @@ impl<'a> FormatStringPlaceholderData<'a> {
         (
             self.prefix_len
                 + std::cmp::max(
-                    self.placeholder.map_or(0, |_| hunk_max_line_number_width),
+                    self.placeholder
+                        .as_ref()
+                        .map_or(0, |_| hunk_max_line_number_width),
                     self.width.unwrap_or(0),
                 ),
             self.suffix_len,
@@ -66,8 +112,8 @@ pub fn parse_line_number_format<'a>(
         format_data.push(FormatStringPlaceholderData {
             prefix,
             prefix_len,
-            placeholder: captures.get(1).map(|m| m.as_str()),
-            alignment_spec: captures.get(3).map(|m| m.as_str()),
+            placeholder: captures.get(1).map(|m| m.as_str()).try_into().ok(),
+            alignment_spec: captures.get(3).map(|m| m.as_str()).try_into().ok(),
             width: captures.get(4).map(|m| {
                 m.as_str()
                     .parse()
@@ -90,14 +136,14 @@ pub fn parse_line_number_format<'a>(
             suffix_len: format_string.graphemes(true).count(),
         })
     }
+
     format_data
 }
 
-pub fn pad(s: &str, width: usize, alignment: &str) -> String {
+pub fn pad(s: &str, width: usize, alignment: &Align) -> String {
     match alignment {
-        "<" => format!("{0:<1$}", s, width),
-        "^" => format!("{0:^1$}", s, width),
-        ">" => format!("{0:>1$}", s, width),
-        _ => unreachable!(),
+        Align::Left => format!("{0:<1$}", s, width),
+        Align::Center => format!("{0:^1$}", s, width),
+        Align::Right => format!("{0:>1$}", s, width),
     }
 }
diff --git a/src/handlers/blame.rs b/src/handlers/blame.rs
index a82b8995..2c1b2222 100644
--- a/src/handlers/blame.rs
+++ b/src/handlers/blame.rs
@@ -5,7 +5,7 @@ use regex::Regex;
 use crate::color;
 use crate::config;
 use crate::delta::{self, State, StateMachine};
-use crate::format;
+use crate::format::{self, Placeholder};
 use crate::paint::BgShouldFill;
 use crate::style::Style;
 
@@ -150,18 +150,23 @@ pub fn format_blame_metadata(
     for placeholder in format_data {
         s.push_str(placeholder.prefix.as_str());
 
-        let alignment_spec = placeholder.alignment_spec.unwrap_or("<");
+        let alignment_spec = placeholder
+            .alignment_spec
+            .as_ref()
+            .unwrap_or(&format::Align::Left);
         let width = placeholder.width.unwrap_or(15);
 
         let pad = |s| format::pad(s, width, alignment_spec);
         match placeholder.placeholder {
-            Some("timestamp") => s.push_str(&pad(
-                &chrono_humanize::HumanTime::from(blame.time).to_string()
+            Some(Placeholder::Str("timestamp")) => s.push_str(&pad(
+                &chrono_humanize::HumanTime::from(blame.time).to_string(),
             )),
-            Some("author") => s.push_str(&pad(blame.author)),
-            Some("commit") => s.push_str(&pad(&delta::format_raw_line(blame.commit, config))),
+            Some(Placeholder::Str("author")) => s.push_str(&pad(blame.author)),
+            Some(Placeholder::Str("commit")) => {
+                s.push_str(&pad(&delta::format_raw_line(blame.commit, config)))
+            }
             None => {}
-            Some(_) => unreachable!(),
+            _ => unreachable!("Unexpected `git blame` input"),
         }
         suffix = placeholder.suffix.as_str();
     }
diff --git a/src/paint.rs b/src/paint.rs
index 9b7e7fbe..3ad88628 100644
--- a/src/paint.rs
+++ b/src/paint.rs
@@ -524,12 +524,19 @@ impl<'a> Painter<'a> {
 
         let output_line_numbers = config.line_numbers && line_numbers_data.is_some();
         if output_line_numbers {
-            ansi_strings.extend(line_numbers::format_and_paint_line_numbers(
+            if let Some((line_numbers, styles)) = line_numbers::linenumbers_and_styles(
                 line_numbers_data.as_mut().unwrap(),
                 state,
-                side_by_side_panel,
                 config,
-            ))
+            ) {
+                ansi_strings.extend(line_numbers::format_and_paint_line_numbers(
+                    line_numbers_data.as_ref().unwrap(),
+                    side_by_side_panel,
+                    styles,
+                    line_numbers,
+                    config,
+                ))
+            }
         }
         match state {
             State::HunkMinus(Some(raw_line)) | State::HunkPlus(Some(raw_line)) => {