ronantakizawa/codereview-bench huggingface dataset:
Repository: starship/starship
File: src/configs/hg_branch.rs (Rust)

Code before:
use serde::{Deserialize, Serialize};

#[derive(Clone, Deserialize, Serialize)]
#[cfg_attr(
    feature = "config-schema",
    derive(schemars::JsonSchema),
    schemars(deny_unknown_fields)
)]
#[serde(default)]
pub struct HgBranchConfig<'a> {
    pub symbol: &'a str,
    pub style: &'a str,
    pub format: &'a str,
    pub truncation_length: i64,
    pub truncation_symbol: &'a str,
    pub disabled: bool,
}

impl<'a> Default for HgBranchConfig<'a> {
    fn default() -> Self {
        HgBranchConfig {
            symbol: " ",
            style: "bold purple",
            format: "on [$symbol$branch:$topic]($style) ",
            truncation_length: std::i64::MAX,
            truncation_symbol: "…",
            disabled: true,
        }
    }
}


Review comment (suggestion):
```suggestion
            format: "on [$symbol$branch(:$topic)]($style) ",
```

Code after:
use serde::{Deserialize, Serialize};

#[derive(Clone, Deserialize, Serialize)]
#[cfg_attr(
    feature = "config-schema",
    derive(schemars::JsonSchema),
    schemars(deny_unknown_fields)
)]
#[serde(default)]
pub struct HgBranchConfig<'a> {
    pub symbol: &'a str,
    pub style: &'a str,
    pub format: &'a str,
    pub truncation_length: i64,
    pub truncation_symbol: &'a str,
    pub disabled: bool,
}

impl<'a> Default for HgBranchConfig<'a> {
    fn default() -> Self {
        HgBranchConfig {
            symbol: " ",
            style: "bold purple",
            format: "on [$symbol$branch(:$topic)]($style) ",
            truncation_length: std::i64::MAX,
            truncation_symbol: "…",
            disabled: true,
        }
    }
}
Repository: starship/starship
File: src/modules/hg_branch.rs (Rust)

Code before:
            })
            .parse(None, Some(context))
    });

    module.set_segments(match parsed {
        Ok(segments) => segments,
        Err(error) => {
            log::warn!("Error in module `hg_branch`:\n{}", error);
            return None;
        }
    });

    Some(module)
}

fn get_hg_repo_root<'a>(ctx: &'a Context) -> Result<&'a Path, Error> {
    let dir = ctx.current_dir.as_path();
    let dev_id = dir.device_id();
    for root_dir in dir.ancestors() {
        if dev_id != root_dir.device_id() {
            break;
        }
        if root_dir
            .read_dir()?
            .any(|e| e.unwrap().file_name() == ".hg")
        {
            return Ok(root_dir);
        }
    }
    Err(Error::new(ErrorKind::Other, "No .hg found!"))
}

fn get_hg_branch_name(hg_root: &Path) -> Result<String, Error> {
    match read_file(hg_root.join(".hg").join("branch")) {
        Ok(b) => Ok(b.trim().to_string()),
        Err(e) => Err(e),
    }
}

fn get_hg_current_bookmark(hg_root: &Path) -> Result<String, Error> {
    read_file(hg_root.join(".hg").join("bookmarks.current"))
}

fn get_hg_topic_name(hg_root: &Path) -> Result<String, Error> {
    read_file(hg_root.join(".hg").join("topic"))
}

#[cfg(test)]
mod tests {
    use nu_ansi_term::{Color, Style};
    use std::fs;

Review comment (suggestion):
Please avoid `unwrap`, please just use this:
```suggestion
        if root_dir.join(".hg").is_dir() {
```

Code after:
            })
            .parse(None, Some(context))
    });

    module.set_segments(match parsed {
        Ok(segments) => segments,
        Err(error) => {
            log::warn!("Error in module `hg_branch`:\n{}", error);
            return None;
        }
    });

    Some(module)
}

fn get_hg_repo_root<'a>(ctx: &'a Context) -> Result<&'a Path, Error> {
    let dir = ctx.current_dir.as_path();
    let dev_id = dir.device_id();
    for root_dir in dir.ancestors() {
        if dev_id != root_dir.device_id() {
            break;
        }
        if root_dir.join(".hg").is_dir() {
            return Ok(root_dir);
        }
    }
    Err(Error::new(ErrorKind::Other, "No .hg found!"))
}

fn get_hg_branch_name(hg_root: &Path) -> Result<String, Error> {
    match read_file(hg_root.join(".hg").join("branch")) {
        Ok(b) => Ok(b.trim().to_string()),
        Err(e) => Err(e),
    }
}

fn get_hg_current_bookmark(hg_root: &Path) -> Result<String, Error> {
    read_file(hg_root.join(".hg").join("bookmarks.current"))
}

fn get_hg_topic_name(hg_root: &Path) -> Result<String, Error> {
    read_file(hg_root.join(".hg").join("topic"))
}

#[cfg(test)]
mod tests {
    use nu_ansi_term::{Color, Style};
    use std::fs;
    use std::io;
    use std::path::Path;
Repository: starship/starship
File: src/modules/utils/path.rs (Rust)

Code before:
        self.starts_with(other)
    }

    #[inline]
    fn normalised_equals(&self, other: &Path) -> bool {
        self == other
    }

    #[inline]
    fn without_prefix(&self) -> &Path {
        self
    }

    #[cfg(target_os = "linux")]
    fn device_id(&self) -> u64 {
        use std::os::linux::fs::MetadataExt;
        let m = self.metadata().unwrap();
        m.st_dev()
    }

    #[cfg(all(unix, not(target_os = "linux")))]
    fn device_id(&self) -> u64 {
        use std::os::unix::fs::MetadataExt;
        let m = self.metadata().unwrap();
        m.dev()
    }
}

#[cfg(test)]
#[cfg(windows)]
mod windows {
    use super::*;

    #[test]
    fn normalised_equals() {
        fn test_equals(a: &Path, b: &Path) {
            assert!(a.normalised_equals(b));
            assert!(b.normalised_equals(a));
        }

        // UNC paths
        let verbatim_unc = Path::new(r"\\?\UNC\server\share\sub\path");
        let unc = Path::new(r"\\server\share\sub\path");
        test_equals(verbatim_unc, verbatim_unc);
        test_equals(verbatim_unc, unc);
        test_equals(unc, unc);
        test_equals(unc, verbatim_unc);

        // Disk paths
        let verbatim_disk = Path::new(r"\\?\C:\test\path");
        let disk = Path::new(r"C:\test\path");

Review comment (suggestion):
Please avoid `unwrap`, instead please just return `Option<u64>` or maybe `unwrap_or(u64::MAX)`.

Code after:
        self.starts_with(other)
    }

    #[inline]
    fn normalised_equals(&self, other: &Path) -> bool {
        self == other
    }

    #[inline]
    fn without_prefix(&self) -> &Path {
        self
    }

    #[cfg(target_os = "linux")]
    fn device_id(&self) -> Option<u64> {
        use std::os::linux::fs::MetadataExt;
        match self.metadata() {
            Ok(m) => Some(m.st_dev()),
            Err(_) => None,
        }
    }

    #[cfg(all(unix, not(target_os = "linux")))]
    fn device_id(&self) -> Option<u64> {
        use std::os::unix::fs::MetadataExt;
        match self.metadata() {
            Ok(m) => Some(m.dev()),
            Err(_) => None,
        }
    }
}

#[cfg(test)]
#[cfg(windows)]
mod windows {
    use super::*;

    #[test]
    fn normalised_equals() {
        fn test_equals(a: &Path, b: &Path) {
            assert!(a.normalised_equals(b));
            assert!(b.normalised_equals(a));
        }

        // UNC paths
        let verbatim_unc = Path::new(r"\\?\UNC\server\share\sub\path");
        let unc = Path::new(r"\\server\share\sub\path");
        test_equals(verbatim_unc, verbatim_unc);
        test_equals(verbatim_unc, unc);
        test_equals(unc, unc);
        test_equals(unc, verbatim_unc);
Repository: starship/starship
File: src/modules/fossil_branch.rs (Rust)

Code before:
use super::{Context, Module, ModuleConfig};

use crate::configs::fossil_branch::FossilBranchConfig;
use crate::formatter::StringFormatter;
use crate::modules::utils::truncate::truncate_text;

/// Creates a module with the Fossil branch of the check-out in the current directory
///
/// Will display the branch name if the current directory is a Fossil check-out
pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
    let is_checkout = context
        .try_begin_scan()?
        .set_files(&[".fslckout"])
        .is_match();

    if !is_checkout {
        return None;
    }

    let mut module = context.new_module("fossil_branch");
    let config = FossilBranchConfig::try_load(module.config);

    // As we default to disabled=true, we have to check here after loading our config module,
    // before it was only checking against whatever is in the config starship.toml
    if config.disabled {
        return None;
    };

    let len = if config.truncation_length <= 0 {
        log::warn!(
            "\"truncation_length\" should be a positive value, found {}",
            config.truncation_length
        );
        std::usize::MAX
    } else {
        config.truncation_length as usize
    };

    let truncated_branch_name = {
        let output = context.exec_cmd("fossil", &["branch", "current"])?.stdout;
        truncate_text(output.trim(), len, config.truncation_symbol)
    };

    let parsed = StringFormatter::new(config.format).and_then(|formatter| {

Review comment (refactor):
Please move this after the `config.disabled` check, because it should be cheaper.

Code after:
use super::{Context, Module, ModuleConfig};

use crate::configs::fossil_branch::FossilBranchConfig;
use crate::formatter::StringFormatter;
use crate::modules::utils::truncate::truncate_text;

/// Creates a module with the Fossil branch of the check-out in the current directory
///
/// Will display the branch name if the current directory is a Fossil check-out
pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
    let mut module = context.new_module("fossil_branch");
    let config = FossilBranchConfig::try_load(module.config);

    // As we default to disabled=true, we have to check here after loading our config module,
    // before it was only checking against whatever is in the config starship.toml
    if config.disabled {
        return None;
    };

    let is_checkout = context
        .try_begin_scan()?
        .set_files(&[".fslckout"])
        .is_match();

    if !is_checkout {
        return None;
    }

    let len = if config.truncation_length <= 0 {
        log::warn!(
            "\"truncation_length\" should be a positive value, found {}",
            config.truncation_length
        );
        std::usize::MAX
    } else {
        config.truncation_length as usize
    };

    let truncated_branch_name = {
        let output = context.exec_cmd("fossil", &["branch", "current"])?.stdout;
        truncate_text(output.trim(), len, config.truncation_symbol)
    };

    let parsed = StringFormatter::new(config.format).and_then(|formatter| {
Repository: starship/starship
File: src/configs/git_status.rs (Rust)

Code before:
    pub staged: &'a str,
    pub untracked: &'a str,
    pub typechange: &'a str,
    pub ignore_submodules: bool,
    pub disabled: bool,
    #[serde(skip_serializing_if = "Option::is_none")]
    pub windows_starship: Option<&'a str>,
}

impl<'a> Default for GitStatusConfig<'a> {
    fn default() -> Self {
        GitStatusConfig {
            format: "([\\[$all_status$ahead_behind\\]]($style) )",
            style: "red bold",
            stashed: "\\$",
            ahead: "⇡",
            behind: "⇣",
            up_to_date: "",
            diverged: "⇕",
            conflicted: "=",
            deleted: "✘",
            renamed: "»",
            modified: "!",
            staged: "+",
            untracked: "?",
            typechange: "⇢",
            ignore_submodules: false,
            disabled: false,
            windows_starship: None,
        }
    }
}


Review comment (suggestion):
For consistency, I think I would prefer calling this `typechanged`. Please also disable it by default by replacing the default to an empty string or `None` to avoid confusing users with additional symbols.

Code after:
    pub staged: &'a str,
    pub untracked: &'a str,
    pub typechanged: &'a str,
    pub ignore_submodules: bool,
    pub disabled: bool,
    #[serde(skip_serializing_if = "Option::is_none")]
    pub windows_starship: Option<&'a str>,
}

impl<'a> Default for GitStatusConfig<'a> {
    fn default() -> Self {
        GitStatusConfig {
            format: "([\\[$all_status$ahead_behind\\]]($style) )",
            style: "red bold",
            stashed: "\\$",
            ahead: "⇡",
            behind: "⇣",
            up_to_date: "",
            diverged: "⇕",
            conflicted: "=",
            deleted: "✘",
            renamed: "»",
            modified: "!",
            staged: "+",
            untracked: "?",
            typechanged: "",
            ignore_submodules: false,
            disabled: false,
            windows_starship: None,
        }
    }
}
Repository: starship/starship
File: src/modules/fossil_metrics.rs (Rust)

Code before:
/// Represents the parsed output from a Fossil diff with the --numstat option enabled.
#[derive(Debug, PartialEq)]
struct FossilDiff<'a> {
    added: &'a str,
    deleted: &'a str,
}

impl<'a> FossilDiff<'a> {
    /// Parses the output of `fossil diff --numstat` as a `FossilDiff` struct.
    pub fn parse(diff_numstat: &'a str, only_nonzero_diffs: bool) -> Self {
        // Fossil formats the last line of the output as "%10d %10d TOTAL over %d changed files\n"
        // where the 1st and 2nd placeholders are the number of added and deleted lines respectively
        let total_line = diff_numstat.lines().last().unwrap_or("");
        let mut split = total_line.split_whitespace();

        let zero = if only_nonzero_diffs { "" } else { "0" };

        let added = match split.next() {
            Some("0") | None => zero,
            Some(s) => s,
        };

        let deleted = match split.next() {
            Some("0") | None => zero,
            Some(s) => s,
        };

        Self { added, deleted }
    }
}

#[cfg(test)]
mod tests {
    use std::io;
    use std::path::Path;

    use nu_ansi_term::{Color, Style};

    use crate::test::{fixture_repo, FixtureProvider, ModuleRenderer};

    use super::FossilDiff;

    enum Expect<'a> {
        Empty,
        Added(Option<&'a str>),
        AddedStyle(Style),
        Deleted(Option<&'a str>),
        DeletedStyle(Style),
    }

    #[test]

Review comment (suggestion):
Since this operation isn't supported on some versions, perhaps try sanity-checking the returned values.

Code after:
}

impl<'a> FossilDiff<'a> {
    /// Parses the output of `fossil diff --numstat` as a `FossilDiff` struct.
    pub fn parse(diff_numstat: &'a str, only_nonzero_diffs: bool) -> Self {
        // Fossil formats the last line of the output as "%10d %10d TOTAL over %d changed files\n"
        // where the 1st and 2nd placeholders are the number of added and deleted lines respectively
        let re = Regex::new(r"^\s*(\d+)\s+(\d+) TOTAL over \d+ changed files$").unwrap();

        let (added, deleted) = diff_numstat
            .lines()
            .last()
            .and_then(|s| re.captures(s))
            .and_then(|caps| {
                let added = match caps.get(1)?.as_str() {
                    "0" if only_nonzero_diffs => "",
                    s => s,
                };

                let deleted = match caps.get(2)?.as_str() {
                    "0" if only_nonzero_diffs => "",
                    s => s,
                };

                Some((added, deleted))
            })
            .unwrap_or_default();

        Self { added, deleted }
    }
}

#[cfg(test)]
mod tests {
    use std::io;
    use std::path::Path;

    use nu_ansi_term::{Color, Style};

    use crate::test::{fixture_repo, FixtureProvider, ModuleRenderer};

    use super::FossilDiff;

    enum Expect<'a> {
        Empty,
        Added(Option<&'a str>),
        AddedStyle(Style),
        Deleted(Option<&'a str>),
        DeletedStyle(Style),
    }
Repository: starship/starship
File: src/init/starship.bash (Shell)

Code before:
        _PRESERVED_PROMPT_COMMAND="$PROMPT_COMMAND"
        PROMPT_COMMAND="starship_precmd"
    fi
fi

# Ensure that $COLUMNS gets set
shopt -s checkwinsize

# Set up the start time and STARSHIP_SHELL, which controls shell-specific sequences
STARSHIP_START_TIME=$(::STARSHIP:: time)
export STARSHIP_SHELL="bash"

# Set up the session key that will be used to store logs
STARSHIP_SESSION_KEY="$RANDOM$RANDOM$RANDOM$RANDOM$RANDOM"; # Random generates a number b/w 0 - 32767
STARSHIP_SESSION_KEY="${STARSHIP_SESSION_KEY}0000000000000000" # Pad it to 16+ chars.
export STARSHIP_SESSION_KEY=${STARSHIP_SESSION_KEY:0:16}; # Trim to 16-digits if excess.

# Set the continuation prompt
PS2="$(::STARSHIP:: prompt --continuation)"

if [[ ${BLE_VERSION-} ]]; then
    # Calculate number of newlines in the prompt string
    p_string="$(::STARSHIP:: prompt)"
    nlns_count="${p_string//[!$'\n']/}"
    for ((c=1;c<=${#nlns_count};c++)); do nlns="\n${nlns}"; done
fi


Review comment (suggestion):
```suggestion
```

If we need to keep these variables, we only need to keep `nlns` in the global scope, and the other variables can be made `local` after putting the code inside a function. The variable name `nlns` can conflict with a possible user variable name, so should be prefixed as e.g. `_starship_nlns`.

Code after:
    if [[ -z "$dbg_trap" ]]; then
        trap 'starship_preexec "$_"' DEBUG
    elif [[ "$dbg_trap" != 'starship_preexec "$_"' && "$dbg_trap" != 'starship_preexec_all "$_"' ]]; then
        starship_preexec_all() {
            local PREV_LAST_ARG=$1 ; $dbg_trap; starship_preexec; : "$PREV_LAST_ARG";
        }
        trap 'starship_preexec_all "$_"' DEBUG
    fi

    # Finally, prepare the precmd function and set up the start time. We will avoid to
    # add multiple instances of the starship function and keep other user functions if any.
    if [[ -z "$PROMPT_COMMAND" ]]; then
        PROMPT_COMMAND="starship_precmd"
    elif [[ "$PROMPT_COMMAND" != *"starship_precmd"* ]]; then
        # Appending to PROMPT_COMMAND breaks exit status ($?) checking.
        # Prepending to PROMPT_COMMAND breaks "command duration" module.
        # So, we are preserving the existing PROMPT_COMMAND
        # which will be executed later in the starship_precmd function
        _PRESERVED_PROMPT_COMMAND="$PROMPT_COMMAND"
        PROMPT_COMMAND="starship_precmd"
    fi
fi

# Ensure that $COLUMNS gets set
shopt -s checkwinsize

# Set up the start time and STARSHIP_SHELL, which controls shell-specific sequences
STARSHIP_START_TIME=$(::STARSHIP:: time)
export STARSHIP_SHELL="bash"

# Set up the session key that will be used to store logs
STARSHIP_SESSION_KEY="$RANDOM$RANDOM$RANDOM$RANDOM$RANDOM"; # Random generates a number b/w 0 - 32767
STARSHIP_SESSION_KEY="${STARSHIP_SESSION_KEY}0000000000000000" # Pad it to 16+ chars.
export STARSHIP_SESSION_KEY=${STARSHIP_SESSION_KEY:0:16}; # Trim to 16-digits if excess.

# Set the continuation prompt
PS2="$(::STARSHIP:: prompt --continuation)"
Repository: starship/starship
File: src/modules/shlvl.rs (Rust)

Code before:
use super::{Context, Module};

use crate::config::ModuleConfig;
use crate::configs::shlvl::ShLvlConfig;
use crate::formatter::StringFormatter;
use std::borrow::Cow;
use std::convert::TryInto;

const SHLVL_ENV_VAR: &str = "SHLVL";

pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
    let props = &context.properties;
    let shlvl = props
        .shlvl
        .unwrap_or(context.get_env(SHLVL_ENV_VAR)?.parse::<i64>().ok()?);

    let mut module = context.new_module("shlvl");
    let config: ShLvlConfig = ShLvlConfig::try_load(module.config);

    // As we default to disabled=true, we have to check here after loading our config module,
    // before it was only checking against whatever is in the config starship.toml
    if config.disabled || shlvl < config.threshold {
        return None;
    }

    let shlvl_str = &shlvl.to_string();

    let repeat_count = if config.repeat {
        shlvl.try_into().unwrap_or(1)
    } else {
        1
    };
    let symbol = if repeat_count != 1 {
        Cow::Owned(config.symbol.repeat(repeat_count))
    } else {
        Cow::Borrowed(config.symbol)
    };

    let parsed = StringFormatter::new(config.format).and_then(|formatter| {
        formatter

Review comment (suggestion):
```suggestion
        .or_else(|| context.get_env(SHLVL_ENV_VAR)?.parse::<i64>().ok())?;
```

Code after:
use super::{Context, Module};

use crate::config::ModuleConfig;
use crate::configs::shlvl::ShLvlConfig;
use crate::formatter::StringFormatter;
use std::borrow::Cow;
use std::convert::TryInto;

const SHLVL_ENV_VAR: &str = "SHLVL";

pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
    let props = &context.properties;
    let shlvl = props
        .shlvl
        .or_else(|| context.get_env(SHLVL_ENV_VAR)?.parse::<i64>().ok())?;

    let mut module = context.new_module("shlvl");
    let config: ShLvlConfig = ShLvlConfig::try_load(module.config);

    // As we default to disabled=true, we have to check here after loading our config module,
    // before it was only checking against whatever is in the config starship.toml
    if config.disabled || shlvl < config.threshold {
        return None;
    }

    let shlvl_str = &shlvl.to_string();

    let mut repeat_count: usize = if config.repeat {
        shlvl.try_into().unwrap_or(1)
    } else {
        1
    };

    if config.repeat_offset > 0 {
        repeat_count =
            repeat_count.saturating_sub(config.repeat_offset.try_into().unwrap_or(usize::MAX));
        if repeat_count == 0 {
            return None;
        }
    }
Repository: starship/starship
File: src/configs/fennel.rs (Rust)

Code before:
use serde::{Deserialize, Serialize};

#[derive(Clone, Deserialize, Serialize)]
#[cfg_attr(
    feature = "config-schema",
    derive(schemars::JsonSchema),
    schemars(deny_unknown_fields)
)]
#[serde(default)]
pub struct FennelConfig<'a> {
    pub format: &'a str,
    pub version_format: &'a str,
    pub symbol: &'a str,
    pub style: &'a str,
    pub disabled: bool,
    pub detect_extensions: Vec<&'a str>,
    pub detect_files: Vec<&'a str>,
    pub detect_folders: Vec<&'a str>,
}

impl<'a> Default for FennelConfig<'a> {
    fn default() -> Self {
        FennelConfig {
            format: "via [$symbol($version )]($style)",
            version_format: "v${raw}",
            symbol: "🧅 ",
            style: "bold green",
            disabled: true,
            detect_extensions: vec!["fnl"],
            detect_files: vec![],
            detect_folders: vec![],
        }
    }
}


Review comment (suggestion):
I would prefer not to break compatibility with older configs with this change:
```suggestion
    #[serde(alias = "detect_extentions")]
    pub detect_extensions: Vec<&'a str>,
```

Code after:
use serde::{Deserialize, Serialize};

#[derive(Clone, Deserialize, Serialize)]
#[cfg_attr(
    feature = "config-schema",
    derive(schemars::JsonSchema),
    schemars(deny_unknown_fields)
)]
#[serde(default)]
pub struct FennelConfig<'a> {
    pub format: &'a str,
    pub version_format: &'a str,
    pub symbol: &'a str,
    pub style: &'a str,
    pub disabled: bool,
    #[serde(alias = "detect_extentions")] // TODO: remove it after breaking change releases
    pub detect_extensions: Vec<&'a str>,
    pub detect_files: Vec<&'a str>,
    pub detect_folders: Vec<&'a str>,
}

impl<'a> Default for FennelConfig<'a> {
    fn default() -> Self {
        FennelConfig {
            format: "via [$symbol($version )]($style)",
            version_format: "v${raw}",
            symbol: "🧅 ",
            style: "bold green",
            disabled: true,
            detect_extensions: vec!["fnl"],
            detect_files: vec![],
            detect_folders: vec![],
        }
    }
}