fix(properties): fix regressions in status, pipestatus and terminal-width handling (#3399)

src/context.rs

@@ -43,9 +43,6 @@ pub struct Context<'a> {
     /// Properties to provide to modules.
     pub properties: Properties,
 
-    /// Pipestatus of processes in pipe
-    pub pipestatus: Option<Vec<String>>,
-
     /// Private field to store Git information for modules who need it
     repo: OnceCell<Repo>,
 
@@ -107,7 +104,7 @@ impl<'a> Context<'a> {
 
     /// Create a new instance of Context for the provided directory
     pub fn new_with_shell_and_path(
-        properties: Properties,
+        mut properties: Properties,
         shell: Shell,
         target: Target,
         path: PathBuf,
@@ -115,12 +112,24 @@ impl<'a> Context<'a> {
     ) -> Context<'a> {
         let config = StarshipConfig::initialize();
 
-        let pipestatus = properties
+        // If the vector is zero-length, we should pretend that we didn't get a
+        // pipestatus at all (since this is the input `--pipestatus=""`)
+        if properties
             .pipestatus
             .as_deref()
-            .map(Context::get_and_flatten_pipestatus)
+            .map_or(false, |p| p.len() == 1 && p[0].is_empty())
-            .flatten();
+        {
-        log::trace!("Received completed pipestatus of {:?}", pipestatus);
+            properties.pipestatus = None;
+        }
+        log::trace!(
+            "Received completed pipestatus of {:?}",
+            properties.pipestatus
+        );
+
+        // If status-code is empty, set it to None
+        if matches!(properties.status_code.as_deref(), Some("")) {
+            properties.status_code = None;
+        }
 
         // Canonicalize the current path to resolve symlinks, etc.
         // NOTE: On Windows this converts the path to extended-path syntax.
@@ -138,7 +147,6 @@ impl<'a> Context<'a> {
         Context {
             config,
             properties,
-            pipestatus,
             current_dir,
             logical_dir,
             dir_contents: OnceCell::new(),
@@ -201,27 +209,6 @@ impl<'a> Context<'a> {
         dir
     }
 
-    /// Reads and appropriately flattens multiple args for pipestatus
-    // TODO: Replace with value_delimiter = ' ' clap option?
-    pub fn get_and_flatten_pipestatus(args: &[String]) -> Option<Vec<String>> {
-        // Due to shell differences, we can potentially receive individual or space
-        // separated inputs, e.g. "0","1","2","0" is the same as "0 1 2 0" and
-        // "0 1", "2 0". We need to accept all these formats and return a Vec<String>
-        let parsed_vals = args
-            .iter()
-            .map(|x| x.split_ascii_whitespace())
-            .flatten()
-            .map(|x| x.to_string())
-            .collect::<Vec<String>>();
-        // If the vector is zero-length, we should pretend that we didn't get a
-        // pipestatus at all (since this is the input `--pipestatus=""`)
-        if parsed_vals.is_empty() {
-            None
-        } else {
-            Some(parsed_vals)
-        }
-    }
-
     /// Create a new module
     pub fn new_module(&self, name: &str) -> Module {
         let config = self.config.get_module_config(name);
@@ -571,10 +558,6 @@ pub enum Shell {
     Unknown,
 }
 
-fn default_width() -> usize {
-    terminal_size().map_or(80, |(w, _)| w.0 as usize)
-}
-
 /// Which kind of prompt target to print (main prompt, rprompt, ...)
 #[derive(Debug, Clone, Copy, PartialEq)]
 pub enum Target {
@@ -588,12 +571,12 @@ pub enum Target {
 pub struct Properties {
     /// The status code of the previously run command
     #[clap(short = 's', long = "status")]
-    pub status_code: Option<i32>,
+    pub status_code: Option<String>,
     /// Bash, Fish and Zsh support returning codes for each process in a pipeline.
-    #[clap(long)]
+    #[clap(long, value_delimiter = ' ')]
-    pipestatus: Option<Vec<String>>,
+    pub pipestatus: Option<Vec<String>>,
     /// The width of the current interactive terminal.
-    #[clap(short = 'w', long, default_value_t=default_width())]
+    #[clap(short = 'w', long, default_value_t=default_width(), parse(try_from_str=parse_width))]
     terminal_width: usize,
     /// The path that the prompt should render for.
     #[clap(short, long)]
@@ -632,6 +615,17 @@ fn parse_jobs(jobs: &str) -> Result<i64, ParseIntError> {
     jobs.trim().parse::<i64>()
 }
 
+fn default_width() -> usize {
+    terminal_size().map_or(80, |(w, _)| w.0 as usize)
+}
+
+fn parse_width(width: &str) -> Result<usize, ParseIntError> {
+    if width.is_empty() {
+        return Ok(default_width());
+    }
+    width.trim().parse::<usize>()
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

src/init/starship.fish

@@ -10,7 +10,7 @@ function fish_prompt
     # Account for changes in variable name between v2.7 and v3.0
     set STARSHIP_DURATION "$CMD_DURATION$cmd_duration"
     set STARSHIP_JOBS (count (jobs -p))
-    ::STARSHIP:: prompt --terminal-width="$COLUMNS" --status=$STARSHIP_CMD_STATUS --pipestatus=$STARSHIP_CMD_PIPESTATUS --keymap=$STARSHIP_KEYMAP --cmd-duration=$STARSHIP_DURATION --jobs=$STARSHIP_JOBS
+    ::STARSHIP:: prompt --terminal-width="$COLUMNS" --status=$STARSHIP_CMD_STATUS --pipestatus="$STARSHIP_CMD_PIPESTATUS" --keymap=$STARSHIP_KEYMAP --cmd-duration=$STARSHIP_DURATION --jobs=$STARSHIP_JOBS
 end
 
 function fish_right_prompt
@@ -25,7 +25,7 @@ function fish_right_prompt
     # Account for changes in variable name between v2.7 and v3.0
     set STARSHIP_DURATION "$CMD_DURATION$cmd_duration"
     set STARSHIP_JOBS (count (jobs -p))
-    ::STARSHIP:: prompt --right --terminal-width="$COLUMNS" --status=$STARSHIP_CMD_STATUS --pipestatus=$STARSHIP_CMD_PIPESTATUS --keymap=$STARSHIP_KEYMAP --cmd-duration=$STARSHIP_DURATION --jobs=$STARSHIP_JOBS
+    ::STARSHIP:: prompt --right --terminal-width="$COLUMNS" --status=$STARSHIP_CMD_STATUS --pipestatus="$STARSHIP_CMD_PIPESTATUS" --keymap=$STARSHIP_KEYMAP --cmd-duration=$STARSHIP_DURATION --jobs=$STARSHIP_JOBS
 end
 
 # Disable virtualenv prompt, it breaks starship

src/modules/character.rs

@@ -22,9 +22,9 @@ pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
     let config: CharacterConfig = CharacterConfig::try_load(module.config);
 
     let props = &context.properties;
-    let exit_code = props.status_code;
+    let exit_code = props.status_code.as_deref().unwrap_or("0");
     let keymap = props.keymap.as_str();
-    let exit_success = exit_code.unwrap_or_default() == 0;
+    let exit_success = exit_code == "0";
 
     // Match shell "keymap" names to normalized vi modes
     // NOTE: in vi mode, fish reports normal mode as "default".

src/modules/status.rs

@@ -28,9 +28,9 @@ pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
         return None;
     };
 
-    let exit_code = context.properties.status_code.unwrap_or_default();
+    let exit_code = context.properties.status_code.as_deref().unwrap_or("0");
 
-    let pipestatus_status = match &context.pipestatus {
+    let pipestatus_status = match &context.properties.pipestatus {
         None => PipeStatusStatus::Disabled,
         Some(ps) => match ps.len() > 1 {
             true => PipeStatusStatus::Pipe(ps),
@@ -44,7 +44,7 @@ pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
     };
 
     // Exit code is zero and pipestatus is all zero or disabled/missing
-    if exit_code == 0
+    if exit_code == "0"
         && (match pipestatus_status {
             PipeStatusStatus::Pipe(ps) => ps.iter().all(|s| s == "0"),
             _ => true,

src/test/mod.rs

@@ -123,7 +123,7 @@ impl<'a> ModuleRenderer<'a> {
     }
 
     pub fn status(mut self, status: i32) -> Self {
-        self.context.properties.status_code = Some(status);
+        self.context.properties.status_code = Some(status.to_string());
         self
     }
 
@@ -137,7 +137,7 @@ impl<'a> ModuleRenderer<'a> {
     }
 
     pub fn pipestatus(mut self, status: &[i32]) -> Self {
-        self.context.pipestatus = Some(
+        self.context.properties.pipestatus = Some(
             status
                 .iter()
                 .map(std::string::ToString::to_string)