chore: remove unused name from Segment and remove some of the misleading underscores (#1584)

* chore: Remove unused name from Segment and remove some of the misleading underscores

* chore: Access members of `Segment` directly

5 files changed, 57 insertions, 113 deletions

diff --git a/src/formatter/parser.rs b/src/formatter/parser.rs
index 929a037f6..f393cb8be 100644
--- a/src/formatter/parser.rs
+++ b/src/formatter/parser.rs
@@ -7,50 +7,50 @@ use super::model::*;
 #[grammar = "formatter/spec.pest"]
 struct IdentParser;
 
-fn _parse_value(value: Pair<Rule>) -> FormatElement {
+fn parse_value(value: Pair<Rule>) -> FormatElement {
     match value.as_rule() {
-        Rule::text => FormatElement::Text(_parse_text(value).into()),
-        Rule::variable => FormatElement::Variable(_parse_variable(value).into()),
-        Rule::textgroup => FormatElement::TextGroup(_parse_textgroup(value)),
+        Rule::text => FormatElement::Text(parse_text(value).into()),
+        Rule::variable => FormatElement::Variable(parse_variable(value).into()),
+        Rule::textgroup => FormatElement::TextGroup(parse_textgroup(value)),
         Rule::conditional => {
-            FormatElement::Conditional(_parse_format(value.into_inner().next().unwrap()))
+            FormatElement::Conditional(parse_format(value.into_inner().next().unwrap()))
         }
         _ => unreachable!(),
     }
 }
 
-fn _parse_textgroup(textgroup: Pair<Rule>) -> TextGroup {
+fn parse_textgroup(textgroup: Pair<Rule>) -> TextGroup {
     let mut inner_rules = textgroup.into_inner();
     let format = inner_rules.next().unwrap();
     let style = inner_rules.next().unwrap();
 
     TextGroup {
-        format: _parse_format(format),
-        style: _parse_style(style),
+        format: parse_format(format),
+        style: parse_style(style),
     }
 }
 
-fn _parse_variable(variable: Pair<Rule>) -> &str {
+fn parse_variable(variable: Pair<Rule>) -> &str {
     variable.into_inner().next().unwrap().as_str()
 }
 
-fn _parse_text(text: Pair<Rule>) -> String {
+fn parse_text(text: Pair<Rule>) -> String {
     text.into_inner()
         .map(|pair| pair.as_str().chars())
         .flatten()
         .collect()
 }
 
-fn _parse_format(format: Pair<Rule>) -> Vec<FormatElement> {
-    format.into_inner().map(_parse_value).collect()
+fn parse_format(format: Pair<Rule>) -> Vec<FormatElement> {
+    format.into_inner().map(parse_value).collect()
 }
 
-fn _parse_style(style: Pair<Rule>) -> Vec<StyleElement> {
+fn parse_style(style: Pair<Rule>) -> Vec<StyleElement> {
     style
         .into_inner()
         .map(|pair| match pair.as_rule() {
             Rule::string => StyleElement::Text(pair.as_str().into()),
-            Rule::variable => StyleElement::Variable(_parse_variable(pair).into()),
+            Rule::variable => StyleElement::Variable(parse_variable(pair).into()),
             _ => unreachable!(),
         })
         .collect()
@@ -60,7 +60,7 @@ pub fn parse(format: &str) -> Result<Vec<FormatElement>, Error<Rule>> {
     IdentParser::parse(Rule::expression, format).map(|pairs| {
         pairs
             .take_while(|pair| pair.as_rule() != Rule::EOI)
-            .map(_parse_value)
+            .map(parse_value)
             .collect()
     })
 }
diff --git a/src/formatter/string_formatter.rs b/src/formatter/string_formatter.rs
index 2cc9fdb3f..7db1c8532 100644
--- a/src/formatter/string_formatter.rs
+++ b/src/formatter/string_formatter.rs
@@ -204,13 +204,13 @@ impl<'a> StringFormatter<'a> {
     /// - Format string in meta variables fails to parse
     /// - Variable mapper returns an error.
     pub fn parse(self, default_style: Option<Style>) -> Result<Vec<Segment>, StringFormatterError> {
-        fn _parse_textgroup<'a>(
+        fn parse_textgroup<'a>(
             textgroup: TextGroup<'a>,
             variables: &'a VariableMapType<'a>,
             style_variables: &'a StyleVariableMapType<'a>,
         ) -> Result<Vec<Segment>, StringFormatterError> {
-            let style = _parse_style(textgroup.style, style_variables);
-            _parse_format(
+            let style = parse_style(textgroup.style, style_variables);
+            parse_format(
                 textgroup.format,
                 style.transpose()?,
                 &variables,
@@ -218,7 +218,7 @@ impl<'a> StringFormatter<'a> {
             )
         }
 
-        fn _parse_style<'a>(
+        fn parse_style<'a>(
             style: Vec<StyleElement>,
             variables: &'a StyleVariableMapType<'a>,
         ) -> Option<Result<Style, StringFormatterError>> {
@@ -244,7 +244,7 @@ impl<'a> StringFormatter<'a> {
                 .transpose()
         }
 
-        fn _parse_format<'a>(
+        fn parse_format<'a>(
             format: Vec<FormatElement<'a>>,
             style: Option<Style>,
             variables: &'a VariableMapType<'a>,
@@ -254,13 +254,13 @@ impl<'a> StringFormatter<'a> {
                 .into_iter()
                 .map(|el| {
                     match el {
-                        FormatElement::Text(text) => Ok(vec![_new_segment("_text", text, style)]),
+                        FormatElement::Text(text) => Ok(vec![Segment::new(style, text)]),
                         FormatElement::TextGroup(textgroup) => {
                             let textgroup = TextGroup {
                                 format: textgroup.format,
                                 style: textgroup.style,
                             };
-                            _parse_textgroup(textgroup, &variables, &style_variables)
+                            parse_textgroup(textgroup, &variables, &style_variables)
                         }
                         FormatElement::Variable(name) => variables
                             .get(name.as_ref())
@@ -271,21 +271,17 @@ impl<'a> StringFormatter<'a> {
                                     .into_iter()
                                     .map(|mut segment| {
                                         // Derive upper style if the style of segments are none.
-                                        if !segment.has_style() {
-                                            if let Some(style) = style {
-                                                segment.set_style(style);
-                                            }
-                                        }
+                                        if segment.style.is_none() {
+                                            segment.style = style;
+                                        };
                                         segment
                                     })
                                     .collect()),
-                                VariableValue::Plain(text) => {
-                                    Ok(vec![_new_segment(name, text, style)])
-                                }
+                                VariableValue::Plain(text) => Ok(vec![Segment::new(style, text)]),
                                 VariableValue::Meta(format) => {
                                     let formatter = StringFormatter {
                                         format,
-                                        variables: _clone_without_meta(variables),
+                                        variables: clone_without_meta(variables),
                                         style_variables: style_variables.clone(),
                                     };
                                     formatter.parse(style)
@@ -295,7 +291,7 @@ impl<'a> StringFormatter<'a> {
                         FormatElement::Conditional(format) => {
                             // Show the conditional format string if all the variables inside are not
                             // none.
-                            fn _should_show_elements<'a>(
+                            fn should_show_elements<'a>(
                                 format_elements: &[FormatElement],
                                 variables: &'a VariableMapType<'a>,
                             ) -> bool {
@@ -311,8 +307,8 @@ impl<'a> StringFormatter<'a> {
                                                     // check the format string inside it.
                                                     VariableValue::Meta(meta_elements) => {
                                                         let meta_variables =
-                                                            _clone_without_meta(variables);
-                                                        _should_show_elements(
+                                                            clone_without_meta(variables);
+                                                        should_show_elements(
                                                             &meta_elements,
                                                             &meta_variables,
                                                         )
@@ -328,10 +324,10 @@ impl<'a> StringFormatter<'a> {
                                 })
                             }
 
-                            let should_show: bool = _should_show_elements(&format, variables);
+                            let should_show: bool = should_show_elements(&format, variables);
 
                             if should_show {
-                                _parse_format(format, style, variables, style_variables)
+                                parse_format(format, style, variables, style_variables)
                             } else {
                                 Ok(Vec::new())
                             }
@@ -342,7 +338,7 @@ impl<'a> StringFormatter<'a> {
             Ok(results?.into_iter().flatten().collect())
         }
 
-        _parse_format(
+        parse_format(
             self.format,
             default_style,
             &self.variables,
@@ -363,20 +359,7 @@ impl<'a> StyleVariableHolder<String> for StringFormatter<'a> {
     }
 }
 
-/// Helper function to create a new segment
-fn _new_segment(
-    name: impl Into<String>,
-    value: impl Into<String>,
-    style: Option<Style>,
-) -> Segment {
-    Segment {
-        _name: name.into(),
-        value: value.into(),
-        style,
-    }
-}
-
-fn _clone_without_meta<'a>(variables: &VariableMapType<'a>) -> VariableMapType<'a> {
+fn clone_without_meta<'a>(variables: &VariableMapType<'a>) -> VariableMapType<'a> {
     VariableMapType::from_iter(variables.iter().map(|(key, value)| {
         let value = match value {
             Some(Ok(value)) => match value {
@@ -523,13 +506,9 @@ mod tests {
             .unwrap()
             .map_variables_to_segments(|variable| match variable {
                 "var" => Some(Ok(vec![
-                    _new_segment("_1".to_owned(), "styless".to_owned(), None),
-                    _new_segment("_2".to_owned(), "styled".to_owned(), styled_style),
-                    _new_segment(
-                        "_3".to_owned(),
-                        "styled_no_modifier".to_owned(),
-                        styled_no_modifier_style,
-                    ),
+                    Segment::new(None, "styless"),
+                    Segment::new(styled_style, "styled"),
+                    Segment::new(styled_no_modifier_style, "styled_no_modifier"),
                 ])),
                 _ => None,
             });
diff --git a/src/module.rs b/src/module.rs
index e8d40f66e..3aa1cfea2 100644
--- a/src/module.rs
+++ b/src/module.rs
@@ -66,7 +66,7 @@ pub struct Module<'a> {
     pub config: Option<&'a toml::Value>,
 
     /// The module's name, to be used in configuration and logging.
-    _name: String,
+    name: String,
 
     /// The module's description
     description: String,
@@ -80,7 +80,7 @@ impl<'a> Module<'a> {
     pub fn new(name: &str, desc: &str, config: Option<&'a toml::Value>) -> Module<'a> {
         Module {
             config,
-            _name: name.to_string(),
+            name: name.to_string(),
             description: desc.to_string(),
             segments: Vec::new(),
         }
@@ -93,7 +93,7 @@ impl<'a> Module<'a> {
 
     /// Get module's name
     pub fn get_name(&self) -> &String {
-        &self._name
+        &self.name
     }
 
     /// Get module's description
@@ -103,12 +103,17 @@ impl<'a> Module<'a> {
 
     /// Whether a module has non-empty segments
     pub fn is_empty(&self) -> bool {
-        self.segments.iter().all(|segment| segment.is_empty())
+        self.segments
+            .iter()
+            .all(|segment| segment.value.trim().is_empty())
     }
 
     /// Get values of the module's segments
     pub fn get_segments(&self) -> Vec<&str> {
-        self.segments.iter().map(Segment::get_value).collect()
+        self.segments
+            .iter()
+            .map(|segment| segment.value.as_str())
+            .collect()
     }
 
     /// Returns a vector of colored ANSIString elements to be later used with
@@ -159,7 +164,7 @@ mod tests {
         let desc = "This is a unit test";
         let module = Module {
             config: None,
-            _name: name.to_string(),
+            name: name.to_string(),
             description: desc.to_string(),
             segments: Vec::new(),
         };
@@ -173,9 +178,9 @@ mod tests {
         let desc = "This is a unit test";
         let module = Module {
             config: None,
-            _name: name.to_string(),
+            name: name.to_string(),
             description: desc.to_string(),
-            segments: vec![Segment::new("test_segment")],
+            segments: vec![Segment::new(None, "")],
         };
 
         assert!(module.is_empty());
diff --git a/src/modules/line_break.rs b/src/modules/line_break.rs
index 04d966d64..e485458d4 100644
--- a/src/modules/line_break.rs
+++ b/src/modules/line_break.rs
@@ -7,11 +7,7 @@ pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
 
     let mut module = context.new_module("line_break");
 
-    module.set_segments(vec![Segment {
-        _name: "line_break".to_string(),
-        style: None,
-        value: LINE_ENDING.to_string(),
-    }]);
+    module.set_segments(vec![Segment::new(None, LINE_ENDING)]);
 
     Some(module)
 }
diff --git a/src/segment.rs b/src/segment.rs
index 13f527083..200a93b9e 100644
--- a/src/segment.rs
+++ b/src/segment.rs
@@ -6,9 +6,6 @@ use std::fmt;
 /// (e.g. The version that software is running).
 #[derive(Clone)]
 pub struct Segment {
-    /// The segment's name, to be used in configuration and logging.
-    pub _name: String,
-
     /// The segment's style. If None, will inherit the style of the module containing it.
     pub style: Option<Style>,
 
@@ -17,43 +14,15 @@ pub struct Segment {
 }
 
 impl Segment {
-    /// Creates a new segment with default fields.
-    pub fn new(name: &str) -> Self {
-        Self {
-            _name: name.to_string(),
-            style: None,
-            value: "".to_string(),
-        }
-    }
-
-    /// Sets the style of the segment.
-    ///
-    /// Accepts either `Color` or `Style`.
-    pub fn set_style<T>(&mut self, style: T) -> &mut Self
-    where
-        T: Into<Style>,
-    {
-        self.style = Some(style.into());
-        self
-    }
-
-    /// Check if the segment has a style
-    pub fn has_style(&self) -> bool {
-        self.style.is_some()
-    }
-
-    /// Sets the value of the segment.
-    pub fn set_value<T>(&mut self, value: T) -> &mut Self
+    /// Creates a new segment.
+    pub fn new<T>(style: Option<Style>, value: T) -> Self
     where
         T: Into<String>,
     {
-        self.value = value.into();
-        self
-    }
-
-    /// Gets the value of the segment.
-    pub fn get_value(&self) -> &str {
-        &self.value
+        Self {
+            style,
+            value: value.into(),
+        }
     }
 
     // Returns the ANSIString of the segment value, not including its prefix and suffix
@@ -63,11 +32,6 @@ impl Segment {
             None => ANSIString::from(&self.value),
         }
     }
-
-    /// Determines if the segment contains a value.
-    pub fn is_empty(&self) -> bool {
-        self.value.trim().is_empty()
-    }
 }
 
 impl fmt::Display for Segment {