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

Code before:
            log::error!("Failed to parse Windows path:\n{:?}", e);

            return None;
        }
    };

    log::trace!("Windows path: {}", winpath);

    // In Windows or Linux dir?
    if winpath.starts_with(r"\\wsl") {
        log::trace!("Not a Windows path");
        return None;
    }

    // Get foreign starship to use WSL config
    // https://devblogs.microsoft.com/commandline/share-environment-vars-between-wsl-and-windows/
    let wslenv = env::var("WSLENV").map_or_else(
        |_| "STARSHIP_CONFIG/wp".to_string(),
        |e| e + ":STARSHIP_CONFIG/wp",
    );

    let out = match create_command(starship_exe)
        .map(|mut c| {
            if let Some(config_path_os) = context.get_config_path_os() {
                c.env("STARSHIP_CONFIG", config_path_os);
            }
            c.env("WSLENV", wslenv)
                .args(["module", "git_status", "--path", winpath]);
            c
        })
        .and_then(|mut c| c.output())
    {
        Ok(r) => r,
        Err(e) => {
            log::error!("Failed to run Git Status module on Windows:\n{}", e);

            return None;
        }
    };

    match String::from_utf8(out.stdout) {
        Ok(r) => Some(r),
        Err(e) => {
            log::error!(
                "Failed to parse Windows Git Status module status output:\n{}",
                e
            );

            None
        }
    }

Review comment (bug):
If config file resolutions fails on Linux, it should also fail on Windows, by defaulting to `/dev/null` as before.

Code after:
        Err(e) => {
            log::error!("Failed to parse Windows path:\n{:?}", e);

            return None;
        }
    };

    log::trace!("Windows path: {}", winpath);

    // In Windows or Linux dir?
    if winpath.starts_with(r"\\wsl") {
        log::trace!("Not a Windows path");
        return None;
    }

    // Get foreign starship to use WSL config
    // https://devblogs.microsoft.com/commandline/share-environment-vars-between-wsl-and-windows/
    let wslenv = env::var("WSLENV").map_or_else(
        |_| "STARSHIP_CONFIG/wp".to_string(),
        |e| e + ":STARSHIP_CONFIG/wp",
    );

    let out = match create_command(starship_exe)
        .map(|mut c| {
            c.env(
                "STARSHIP_CONFIG",
                context
                    .get_config_path_os()
                    .unwrap_or_else(|| OsString::from("/dev/null")),
            )
            .env("WSLENV", wslenv)
            .args(["module", "git_status", "--path", winpath]);
            c
        })
        .and_then(|mut c| c.output())
    {
        Ok(r) => r,
        Err(e) => {
            log::error!("Failed to run Git Status module on Windows:\n{}", e);

            return None;
        }
    };

    match String::from_utf8(out.stdout) {
        Ok(r) => Some(r),
        Err(e) => {
            log::error!(
                "Failed to parse Windows Git Status module status output:\n{}",
                e
            );
Repository: starship/starship
File: src/configs/solidity.rs (Rust)

Code before:
#[cfg_attr(
    feature = "config-schema",
    derive(schemars::JsonSchema),
    schemars(deny_unknown_fields)
)]
#[serde(default)]

pub struct SolidityConfig<'a> {
    pub format: &'a str,
    pub version_format: &'a str,
    pub disabled: bool,
    pub style: &'a str,
    pub symbol: &'a str,
    pub compiler: VecOr<&'a str>,
    pub detect_extensions: Vec<&'a str>,
    pub detect_files: Vec<&'a str>,
    pub detect_folders: Vec<&'a str>,
}

impl<'a> Default for SolidityConfig<'a> {
    fn default() -> Self {
        SolidityConfig {
            format: "via [$symbol($version)]($style)",
            symbol: "S ",
            style: "bold blue",
            compiler: VecOr(vec!["solc", "solcjs"]),
            version_format: "v${major}.${minor}.${patch}",
            disabled: false,
            detect_extensions: vec!["sol"],
            detect_files: vec![],
            detect_folders: vec![],
        }
    }
}


Review comment (performance):
Looking at the performance, please exclude `solcjs` from the default compiler list.

Code after:
#[cfg_attr(
    feature = "config-schema",
    derive(schemars::JsonSchema),
    schemars(deny_unknown_fields)
)]
#[serde(default)]

pub struct SolidityConfig<'a> {
    pub format: &'a str,
    pub version_format: &'a str,
    pub disabled: bool,
    pub style: &'a str,
    pub symbol: &'a str,
    pub compiler: VecOr<&'a str>,
    pub detect_extensions: Vec<&'a str>,
    pub detect_files: Vec<&'a str>,
    pub detect_folders: Vec<&'a str>,
}

impl<'a> Default for SolidityConfig<'a> {
    fn default() -> Self {
        SolidityConfig {
            format: "via [$symbol($version)]($style)",
            symbol: "S ",
            style: "bold blue",
            compiler: VecOr(vec!["solc"]),
            version_format: "v${major}.${minor}.${patch}",
            disabled: false,
            detect_extensions: vec!["sol"],
            detect_files: vec![],
            detect_folders: vec![],
        }
    }
}
Repository: starship/starship
File: src/print.rs (Rust)

Code before:

fn preset_list() -> String {
    Preset::value_variants()
        .iter()
        .map(|v| format!("{}\n", v.0))
        .collect()
}

#[cfg(test)]
mod test {
    use super::*;
    use crate::test::default_context;
    use crate::utils;

    #[test]
    fn main_prompt() {
        let mut context = default_context().set_config(toml::toml! {
                add_newline=false
                format="$character"
                [character]
                format=">\n>"
        });

        context.root_config.format = "$character".to_string();
        context.target = Target::Main;
        context.root_config.add_newline = false;

        let expected = String::from(">\n>");
        let actual = get_prompt(context);
        assert_eq!(expected, actual);
    }

    #[test]
    fn right_prompt() {
        let mut context = default_context().set_config(toml::toml! {
                right_format="$character"
                [character]
                format=">\n>"
        });

        context.root_config.right_format = "$character".to_string();
        context.target = Target::Right;

        let expected = String::from(">>"); // should strip new lines
        let actual = get_prompt(context);
        assert_eq!(expected, actual);
    }

    #[test]
    fn custom_prompt() {
        let mut context = default_context().set_config(toml::toml! {

Review comment (suggestion):
`set_config()` can handle `root_config` now here and elsewhere:
```suggestion
        context.target = Target::Main;
```

Code after:

fn preset_list() -> String {
    Preset::value_variants()
        .iter()
        .map(|v| format!("{}\n", v.0))
        .collect()
}

#[cfg(test)]
mod test {
    use super::*;
    use crate::test::default_context;
    use crate::utils;

    #[test]
    fn main_prompt() {
        let mut context = default_context().set_config(toml::toml! {
                add_newline=false
                format="$character"
                [character]
                format=">\n>"
        });
        context.target = Target::Main;

        let expected = String::from(">\n>");
        let actual = get_prompt(context);
        assert_eq!(expected, actual);
    }

    #[test]
    fn right_prompt() {
        let mut context = default_context().set_config(toml::toml! {
                right_format="$character"
                [character]
                format=">\n>"
        });
        context.target = Target::Right;

        let expected = String::from(">>"); // should strip new lines
        let actual = get_prompt(context);
        assert_eq!(expected, actual);
    }

    #[test]
    fn custom_prompt() {
        let mut context = default_context().set_config(toml::toml! {
                add_newline = false
                [profiles]
                test="0_0$character"
                [character]
                format=">>"
Repository: starship/starship
File: src/configs/starship_root.rs (Rust)

Code before:
    "guix_shell",
    "nix_shell",
    "conda",
    "meson",
    "spack",
    "memory_usage",
    "aws",
    "gcloud",
    "openstack",
    "azure",
    "env_var",
    "crystal",
    "custom",
    "sudo",
    "cmd_duration",
    "line_break",
    "jobs",
    #[cfg(feature = "battery")]
    "battery",
    "time",
    "status",
    "container",
    "os",
    "shell",
    "character",
    "direnv",
];

// On changes please also update `Default` for the `FullConfig` struct in `mod.rs`
impl Default for StarshipRootConfig {
    fn default() -> Self {
        Self {
            schema: "https://starship.rs/config-schema.json".to_string(),
            format: "$all".to_string(),
            right_format: String::new(),
            continuation_prompt: "[∙](bright-black) ".to_string(),
            profiles: Default::default(),
            scan_timeout: 30,
            command_timeout: 500,
            add_newline: true,
            palette: None,
            palettes: HashMap::default(),
        }
    }
}


Review comment (suggestion):
Is there a particular reason, why the `[direnv]` module is shown after the `[character]` module? I would be confused, if the `[character]` module wouldn't be the last.

Code after:
    "guix_shell",
    "nix_shell",
    "conda",
    "meson",
    "spack",
    "memory_usage",
    "aws",
    "gcloud",
    "openstack",
    "azure",
    "direnv",
    "env_var",
    "crystal",
    "custom",
    "sudo",
    "cmd_duration",
    "line_break",
    "jobs",
    #[cfg(feature = "battery")]
    "battery",
    "time",
    "status",
    "container",
    "os",
    "shell",
    "character",
];

// On changes please also update `Default` for the `FullConfig` struct in `mod.rs`
impl Default for StarshipRootConfig {
    fn default() -> Self {
        Self {
            schema: "https://starship.rs/config-schema.json".to_string(),
            format: "$all".to_string(),
            right_format: String::new(),
            continuation_prompt: "[∙](bright-black) ".to_string(),
            profiles: Default::default(),
            scan_timeout: 30,
            command_timeout: 500,
            add_newline: true,
            palette: None,
            palettes: HashMap::default(),
        }
    }
}
Repository: starship/starship
File: src/modules/direnv.rs (Rust)

Code before:
            log_error(e);

            return None;
        }
    };

    let parsed = StringFormatter::new(config.format).and_then(|formatter| {
        formatter
            .map_style(|variable| match variable {
                "style" => Some(Ok(config.style)),
                _ => None,
            })
            .map(|variable| match variable {
                "rc_path" => Some(Ok(state.rc_path.to_string_lossy())),
                "allowed" => Some(Ok(Cow::from(state.allowed.to_string()))),
                "loaded" => state
                    .loaded
                    .then_some("loaded")
                    .or(Some("not loaded"))
                    .map(Cow::from)
                    .map(Ok),
                _ => None,
            })
            .parse(None, Some(context))
    });

    module.set_segments(match parsed {
        Ok(segments) => segments,
        Err(e) => {
            log_error(e);

            return None;
        }
    });

    Some(module)
}

fn log_error(error: impl Display) {
    log::warn!("Error in module `direnv`:\n{}", error);
}

struct DirenvState {
    pub rc_path: PathBuf,
    pub allowed: AllowStatus,
    pub loaded: bool,
}

impl FromStr for DirenvState {
    type Err = Cow<'static, str>;


Review comment (suggestion):
I think it should be possible to customize the different `allowed`/`loaded`-strings (or symbols).

Code after:
        Err(e) => {
            log::warn!("{e}");

            return None;
        }
    };

    let parsed = StringFormatter::new(config.format).and_then(|formatter| {
        formatter
            .map_style(|variable| match variable {
                "style" => Some(Ok(config.style)),
                _ => None,
            })
            .map(|variable| match variable {
                "symbol" => Some(Ok(Cow::from(config.symbol))),
                "rc_path" => Some(Ok(state.rc_path.to_string_lossy())),
                "allowed" => Some(Ok(match state.allowed {
                    AllowStatus::Allowed => Cow::from(config.allowed_msg),
                    AllowStatus::Denied => Cow::from(config.denied_msg),
                })),
                "loaded" => state
                    .loaded
                    .then_some(config.loaded_msg)
                    .or(Some(config.unloaded_msg))
                    .map(Cow::from)
                    .map(Ok),
                _ => None,
            })
            .parse(None, Some(context))
    });

    module.set_segments(match parsed {
        Ok(segments) => segments,
        Err(e) => {
            log::warn!("{e}");

            return None;
        }
    });

    Some(module)
}

struct DirenvState {
    pub rc_path: PathBuf,
    pub allowed: AllowStatus,
    pub loaded: bool,
}

impl FromStr for DirenvState {
    type Err = Cow<'static, str>;
Repository: starship/starship
File: src/configs/shell.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 ShellConfig<'a> {
    pub format: &'a str,
    pub bash_indicator: &'a str,
    pub fish_indicator: &'a str,
    pub zsh_indicator: &'a str,
    pub powershell_indicator: &'a str,
    pub powershellcore_indicator: &'a str,
    pub ion_indicator: &'a str,
    pub elvish_indicator: &'a str,
    pub tcsh_indicator: &'a str,
    pub nu_indicator: &'a str,
    pub xonsh_indicator: &'a str,
    pub cmd_indicator: &'a str,
    pub unknown_indicator: &'a str,
    pub style: &'a str,
    pub disabled: bool,
}

impl<'a> Default for ShellConfig<'a> {
    fn default() -> Self {
        ShellConfig {
            format: "[$indicator]($style) ",
            bash_indicator: "bsh",
            fish_indicator: "fsh",
            zsh_indicator: "zsh",
            powershell_indicator: "psh",
            powershellcore_indicator: "pwsh",
            ion_indicator: "ion",
            elvish_indicator: "esh",
            tcsh_indicator: "tsh",
            nu_indicator: "nu",
            xonsh_indicator: "xsh",

Review comment (suggestion):
```suggestion
    #[serde(skip_serializing_if = "Option::is_none")]
    pub powershellcore_indicator: Option<&'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 ShellConfig<'a> {
    pub format: &'a str,
    pub bash_indicator: &'a str,
    pub fish_indicator: &'a str,
    pub zsh_indicator: &'a str,
    pub powershell_indicator: &'a str,
    #[serde(skip_serializing_if = "Option::is_none")]
    pub pwsh_indicator: Option<&'a str>,
    pub ion_indicator: &'a str,
    pub elvish_indicator: &'a str,
    pub tcsh_indicator: &'a str,
    pub nu_indicator: &'a str,
    pub xonsh_indicator: &'a str,
    pub cmd_indicator: &'a str,
    pub unknown_indicator: &'a str,
    pub style: &'a str,
    pub disabled: bool,
}

impl<'a> Default for ShellConfig<'a> {
    fn default() -> Self {
        ShellConfig {
            format: "[$indicator]($style) ",
            bash_indicator: "bsh",
            fish_indicator: "fsh",
            zsh_indicator: "zsh",
            powershell_indicator: "psh",
            pwsh_indicator: None,
            ion_indicator: "ion",
            elvish_indicator: "esh",
            tcsh_indicator: "tsh",
            nu_indicator: "nu",
Repository: starship/starship
File: src/configs/shell.rs (Rust)

Code before:
    pub format: &'a str,
    pub bash_indicator: &'a str,
    pub fish_indicator: &'a str,
    pub zsh_indicator: &'a str,
    pub powershell_indicator: &'a str,
    pub powershellcore_indicator: &'a str,
    pub ion_indicator: &'a str,
    pub elvish_indicator: &'a str,
    pub tcsh_indicator: &'a str,
    pub nu_indicator: &'a str,
    pub xonsh_indicator: &'a str,
    pub cmd_indicator: &'a str,
    pub unknown_indicator: &'a str,
    pub style: &'a str,
    pub disabled: bool,
}

impl<'a> Default for ShellConfig<'a> {
    fn default() -> Self {
        ShellConfig {
            format: "[$indicator]($style) ",
            bash_indicator: "bsh",
            fish_indicator: "fsh",
            zsh_indicator: "zsh",
            powershell_indicator: "psh",
            powershellcore_indicator: "pwsh",
            ion_indicator: "ion",
            elvish_indicator: "esh",
            tcsh_indicator: "tsh",
            nu_indicator: "nu",
            xonsh_indicator: "xsh",
            cmd_indicator: "cmd",
            unknown_indicator: "",
            style: "white bold",
            disabled: true,
        }
    }
}


Review comment (suggestion):
```suggestion
            powershellcore_indicator: None,
```

Code after:
    pub format: &'a str,
    pub bash_indicator: &'a str,
    pub fish_indicator: &'a str,
    pub zsh_indicator: &'a str,
    pub powershell_indicator: &'a str,
    #[serde(skip_serializing_if = "Option::is_none")]
    pub pwsh_indicator: Option<&'a str>,
    pub ion_indicator: &'a str,
    pub elvish_indicator: &'a str,
    pub tcsh_indicator: &'a str,
    pub nu_indicator: &'a str,
    pub xonsh_indicator: &'a str,
    pub cmd_indicator: &'a str,
    pub unknown_indicator: &'a str,
    pub style: &'a str,
    pub disabled: bool,
}

impl<'a> Default for ShellConfig<'a> {
    fn default() -> Self {
        ShellConfig {
            format: "[$indicator]($style) ",
            bash_indicator: "bsh",
            fish_indicator: "fsh",
            zsh_indicator: "zsh",
            powershell_indicator: "psh",
            pwsh_indicator: None,
            ion_indicator: "ion",
            elvish_indicator: "esh",
            tcsh_indicator: "tsh",
            nu_indicator: "nu",
            xonsh_indicator: "xsh",
            cmd_indicator: "cmd",
            unknown_indicator: "",
            style: "white bold",
            disabled: true,
        }
    }
}
Repository: starship/starship
File: src/modules/username.rs (Rust)

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

use crate::configs::username::UsernameConfig;
use crate::formatter::StringFormatter;
#[cfg(not(test))]
use whoami::username;

#[cfg(test)]
use tests::username;

/// Creates a module with the current user's username
pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
    let mut username = username();
    let mut module = context.new_module("username");
    let config: UsernameConfig = UsernameConfig::try_load(module.config);

    let is_root = is_root_user();
    if cfg!(target_os = "windows") && is_root {
        username = "Administrator".to_string();
    }
    let show_username = config.show_always
        || is_root // [1]
        || !is_login_user(context, &username) // [2]
        || is_ssh_session(context); // [3]

    if !show_username {
        return None;
    }

    let parsed = StringFormatter::new(config.format).and_then(|formatter| {
        formatter
            .map_style(|variable| match variable {
                "style" => {
                    let module_style = if is_root {

Review comment (suggestion):
I would prefer handling this with a small wrapper function, and it seems like a good idea to keep around the possibility of overriding the username in the tests:
```rust
fn username() -> String {
    #[cfg(test)]
    if let Some(user) = context.get_env("USER") {
        return user;
    };
    whoami::username()
}

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

use crate::configs::username::UsernameConfig;
use crate::formatter::StringFormatter;
#[cfg(test)]
const USERNAME_ENV_VAR: &str = "USERNAME";

/// Creates a module with the current user's username
pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
    #[cfg(test)]
    let mut username = context.get_env(USERNAME_ENV_VAR)?;
    #[cfg(not(test))]
    let mut username = whoami::username();

    let mut module = context.new_module("username");
    let config: UsernameConfig = UsernameConfig::try_load(module.config);

    let is_root = is_root_user();
    if cfg!(target_os = "windows") && is_root {
        username = "Administrator".to_string();
    }
    let show_username = config.show_always
        || is_root // [1]
        || !is_login_user(context, &username) // [2]
        || is_ssh_session(context); // [3]

    if !show_username {
        return None;
    }

    let parsed = StringFormatter::new(config.format).and_then(|formatter| {
        formatter
            .map_style(|variable| match variable {
                "style" => {
Repository: starship/starship
File: src/modules/typst.rs (Rust)

Code before:
    let config = TypstConfig::try_load(module.config);

    let is_hs_project = context
        .try_begin_scan()?
        .set_files(&config.detect_files)
        .set_extensions(&config.detect_extensions)
        .set_folders(&config.detect_folders)
        .is_match();

    if !is_hs_project {
        return None;
    }

    let parsed = StringFormatter::new(config.format).and_then(|formatter| {
        formatter
            .map_meta(|var, _| match var {
                "symbol" => Some(config.symbol),
                _ => None,
            })
            .map_style(|variable| match variable {
                "style" => Some(Ok(config.style)),
                _ => None,
            })
            .map(|variable| match variable {
                "version" => get_version(context).map(Ok),
                "typst_version" => get_typst_config(context).map(Ok),
                _ => None,
            })
            .parse(None, Some(context))
    });

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

    Some(module)
}

fn get_typst_config(context: &Context) -> Option<String> {
    Some(
        context
            .exec_cmd("typst", &["--version"])?
            .stdout
            .trim()
            .to_string()
            .as_str()[6..11]
            .trim()

Review comment (suggestion):
Is an extra variable for this necessary?
```suggestion
```

Code after:
    let config = TypstConfig::try_load(module.config);

    let is_typst_project = context
        .try_begin_scan()?
        .set_files(&config.detect_files)
        .set_extensions(&config.detect_extensions)
        .set_folders(&config.detect_folders)
        .is_match();

    if !is_typst_project {
        return None;
    }

    let parsed = StringFormatter::new(config.format).and_then(|formatter| {
        formatter
            .map_meta(|var, _| match var {
                "symbol" => Some(config.symbol),
                _ => None,
            })
            .map_style(|variable| match variable {
                "style" => Some(Ok(config.style)),
                _ => None,
            })
            .map(|variable| match variable {
                "version" => {
                    let version = get_typst_config(context)?;
                    VersionFormatter::format_module_version(
                        module.get_name(),
                        &version,
                        config.version_format,
                    )
                    .map(Ok)
                }
                _ => None,
            })
            .parse(None, Some(context))
    });

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

    Some(module)
}

fn get_typst_config(context: &Context) -> Option<String> {
    context