ronantakizawa/codereview-bench huggingface dataset:
Repository: starship/starship
File: src/init/starship.ps1 (PowerShell)

Code before:
        # $? is a read-only or constant variable so we can't directly override it.
        # In order to propagate up its original boolean value we will take an action
        # which will produce the desired value.
        #
        # This has to be the very last thing that happens in the prompt function
        # since every PowerShell command sets the $? variable.
        if ($global:? -ne $origDollarQuestion) {
            if ($origDollarQuestion) {
                 # Simple command which will execute successfully and set $? = True without any other side affects.
                1+1
            } else {
                # Write-Error will set $? to False.
                # ErrorAction Ignore will prevent the error from being added to the $Error collection.
                Write-Error '' -ErrorAction 'Ignore'
            }
        }

    }

    # Invoke Starship and set continuation prompt
    Set-PSReadLineOption -ContinuationPrompt (
        Invoke-Native -Executable ::STARSHIP:: -Arguments @(
            "prompt",
            "--continuation"
        )
    )

    # Disable virtualenv prompt, it breaks starship
    $ENV:VIRTUAL_ENV_DISABLE_PROMPT=1

    $ENV:STARSHIP_SHELL = "powershell"

    # Set up the session key that will be used to store logs
    $ENV:STARSHIP_SESSION_KEY = -join ((48..57) + (65..90) + (97..122) | Get-Random -Count 16 | ForEach-Object { [char]$_ })

    Export-ModuleMember
}


Review comment (security):
Please move this after `STARSHIP_SHELL` and `STARSHIP_SESSION_KEY`, or the continuation prompt could get escaped for a different shell.

Code after:
        # $? is a read-only or constant variable so we can't directly override it.
        # In order to propagate up its original boolean value we will take an action
        # which will produce the desired value.
        #
        # This has to be the very last thing that happens in the prompt function
        # since every PowerShell command sets the $? variable.
        if ($global:? -ne $origDollarQuestion) {
            if ($origDollarQuestion) {
                 # Simple command which will execute successfully and set $? = True without any other side affects.
                1+1
            } else {
                # Write-Error will set $? to False.
                # ErrorAction Ignore will prevent the error from being added to the $Error collection.
                Write-Error '' -ErrorAction 'Ignore'
            }
        }

    }

    # Disable virtualenv prompt, it breaks starship
    $ENV:VIRTUAL_ENV_DISABLE_PROMPT=1

    $ENV:STARSHIP_SHELL = "powershell"

    # Set up the session key that will be used to store logs
    $ENV:STARSHIP_SESSION_KEY = -join ((48..57) + (65..90) + (97..122) | Get-Random -Count 16 | ForEach-Object { [char]$_ })

    # Invoke Starship and set continuation prompt
    Set-PSReadLineOption -ContinuationPrompt (
        Invoke-Native -Executable ::STARSHIP:: -Arguments @(
            "prompt",
            "--continuation"
        )
    )

    Export-ModuleMember
}
Repository: starship/starship
File: src/modules/kubernetes.rs (Rust)

Code before:
use crate::utils;

fn get_kube_context(filename: path::PathBuf) -> Option<String> {
    let contents = utils::read_file(filename).ok()?;

    let yaml_docs = YamlLoader::load_from_str(&contents).ok()?;
    if yaml_docs.is_empty() {
        return None;
    }
    let conf = &yaml_docs[0];

    let current_ctx = conf["current-context"].as_str()?;

    if current_ctx.is_empty() {
        return None;
    }
    Some(current_ctx.to_string())
}

fn get_kube_ctx_component(
    filename: path::PathBuf,
    current_ctx: String,
    component: &str,
) -> Option<String> {
    let contents = utils::read_file(filename).ok()?;

    let yaml_docs = YamlLoader::load_from_str(&contents).ok()?;
    if yaml_docs.is_empty() {
        return None;
    }
    let conf = &yaml_docs[0];

    let ctx_component = conf["contexts"].as_vec().and_then(|contexts| {
        contexts
            .iter()
            .filter_map(|ctx| Some((ctx, ctx["name"].as_str()?)))
            .find(|(_, name)| *name == current_ctx)
            .and_then(|(ctx, _)| ctx["context"][component].as_str())
    })?;

    if ctx_component.is_empty() {
        return None;
    }
    Some(ctx_component.to_owned())
}

fn get_kube_context_name<'a>(config: &'a KubernetesConfig, kube_ctx: &'a str) -> Cow<'a, str> {
    if let Some(val) = config.context_aliases.get(kube_ctx) {
        return Cow::Borrowed(val);
    }


Review comment (performance):
I think it might be better from a performance state of view to change this to return a struct that contains the 3 values or rather an Option of each one if they are set to avoid reading the kubectl files multiple times. 

Code after:
use crate::utils;

struct KubeCtxComponents {
    user: Option<String>,
    namespace: Option<String>,
    cluster: Option<String>,
}

fn get_kube_context(filename: path::PathBuf) -> Option<String> {
    let contents = utils::read_file(filename).ok()?;

    let yaml_docs = YamlLoader::load_from_str(&contents).ok()?;
    if yaml_docs.is_empty() {
        return None;
    }
    let conf = &yaml_docs[0];

    let current_ctx = conf["current-context"].as_str()?;

    if current_ctx.is_empty() {
        return None;
    }
    Some(current_ctx.to_string())
}

fn get_kube_ctx_component(
    filename: path::PathBuf,
    current_ctx: String,
) -> Option<KubeCtxComponents> {
    let contents = utils::read_file(filename).ok()?;

    let yaml_docs = YamlLoader::load_from_str(&contents).ok()?;
    if yaml_docs.is_empty() {
        return None;
    }
    let conf = &yaml_docs[0];

    let ctx_yaml = conf["contexts"].as_vec().and_then(|contexts| {
        contexts
            .iter()
            .filter_map(|ctx| Some((ctx, ctx["name"].as_str()?)))
            .find(|(_, name)| *name == current_ctx)
    });

    let ctx_components = KubeCtxComponents {
        user: ctx_yaml
            .and_then(|(ctx, _)| ctx["context"]["user"].as_str())
            .and_then(|s| {
                if s.is_empty() {
                    return None;
                }
Repository: starship/starship
File: src/configs/haskell.rs (Rust)

Code before:
use crate::config::ModuleConfig;

use serde::Serialize;
use starship_module_config_derive::ModuleConfig;

#[derive(Clone, ModuleConfig, Serialize)]
pub struct HaskellConfig<'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 HaskellConfig<'a> {
    fn default() -> Self {
        HaskellConfig {
            format: "via [$symbol($version )]($style)",
            version_format: "v${raw}",
            symbol: " ",
            style: "bold purple",
            disabled: false,
            detect_extensions: vec!["hs", "cabal", "hs-boot"],
            detect_files: vec!["stack.yaml", "cabal.project"],
            detect_folders: vec![],
        }
    }
}


Review comment (suggestion):
I can't actually see this symbol in the web browser due to font issues. Is this supposed to be a nerdfont symbol? If so, we'll need to change it over to unicode.

NerdFont symbols can go in the NerdFont preset if you'd like to preserve it.

Code after:
use crate::config::ModuleConfig;

use serde::Serialize;
use starship_module_config_derive::ModuleConfig;

#[derive(Clone, ModuleConfig, Serialize)]
pub struct HaskellConfig<'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 HaskellConfig<'a> {
    fn default() -> Self {
        HaskellConfig {
            format: "via [$symbol($version )]($style)",
            version_format: "v${raw}",
            symbol: "λ ",
            style: "bold purple",
            disabled: false,
            detect_extensions: vec!["hs", "cabal", "hs-boot"],
            detect_files: vec!["stack.yaml", "cabal.project"],
            detect_folders: vec![],
        }
    }
}
Repository: starship/starship
File: src/configs/git_branch.rs (Rust)

Code before:
use starship_module_config_derive::ModuleConfig;

#[derive(Clone, ModuleConfig, Serialize)]
pub struct GitBranchConfig<'a> {
    pub format: &'a str,
    pub symbol: &'a str,
    pub style: &'a str,
    pub truncation_length: i64,
    pub truncation_symbol: &'a str,
    pub only_attached: bool,
    pub always_show_remote: bool,
    pub ignore_branches: Vec<&'a str>,
    pub disabled: bool,
}

impl<'a> Default for GitBranchConfig<'a> {
    fn default() -> Self {
        GitBranchConfig {
            format: "on [$symbol$branch]($style)(:[$remote]($style)) ",
            symbol: " ",
            style: "bold purple",
            truncation_length: std::i64::MAX,
            truncation_symbol: "…",
            only_attached: false,
            always_show_remote: false,
            ignore_branches: vec![""],
            disabled: false,
        }
    }
}


Review comment (suggestion):
I think this should be empty rather than an array containing an empty string.

```suggestion
            ignore_branches: vec![],
```


Code after:
use starship_module_config_derive::ModuleConfig;

#[derive(Clone, ModuleConfig, Serialize)]
pub struct GitBranchConfig<'a> {
    pub format: &'a str,
    pub symbol: &'a str,
    pub style: &'a str,
    pub truncation_length: i64,
    pub truncation_symbol: &'a str,
    pub only_attached: bool,
    pub always_show_remote: bool,
    pub ignore_branches: Vec<&'a str>,
    pub disabled: bool,
}

impl<'a> Default for GitBranchConfig<'a> {
    fn default() -> Self {
        GitBranchConfig {
            format: "on [$symbol$branch]($style)(:[$remote]($style)) ",
            symbol: " ",
            style: "bold purple",
            truncation_length: std::i64::MAX,
            truncation_symbol: "…",
            only_attached: false,
            always_show_remote: false,
            ignore_branches: vec![],
            disabled: false,
        }
    }
}
Repository: starship/starship
File: src/configs/hostname.rs (Rust)

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

#[derive(Clone, Deserialize, Serialize)]
#[serde(default)]
pub struct HostnameConfig<'a> {
    pub ssh_only: bool,
    pub ssh_symbol: &'a str,
    pub trim_at: &'a str,
    pub format: &'a str,
    pub style: &'a str,
    pub disabled: bool,
}

impl<'a> Default for HostnameConfig<'a> {
    fn default() -> Self {
        HostnameConfig {
            ssh_only: true,
            ssh_symbol: "🌐 ",
            trim_at: ".",
            format: "[$ssh_symbol($hostname)]($style) in ",
            style: "green dimmed bold",
            disabled: false,
        }
    }
}


Review comment (suggestion):
I'm not sure you need the `(` and `)` here
```suggestion
            format: "[$ssh_symbol$hostname]($style) in ",
```

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

#[derive(Clone, Deserialize, Serialize)]
#[cfg_attr(feature = "config-schema", derive(schemars::JsonSchema))]
#[serde(default)]
pub struct HostnameConfig<'a> {
    pub ssh_only: bool,
    pub ssh_symbol: &'a str,
    pub trim_at: &'a str,
    pub format: &'a str,
    pub style: &'a str,
    pub disabled: bool,
}

impl<'a> Default for HostnameConfig<'a> {
    fn default() -> Self {
        HostnameConfig {
            ssh_only: true,
            ssh_symbol: "🌐 ",
            trim_at: ".",
            format: "[$ssh_symbol$hostname]($style) in ",
            style: "green dimmed bold",
            disabled: false,
        }
    }
}
Repository: starship/starship
File: src/modules/cmd_duration.rs (Rust)

Code before:
        }
    });

    Some(undistract_me(module, &config, elapsed))
}

#[cfg(not(feature = "notify"))]
fn undistract_me<'a, 'b>(
    module: Module<'a>,
    _config: &'b CmdDurationConfig,
    _elapsed: u128,
) -> Module<'a> {
    module
}

#[cfg(feature = "notify")]
fn undistract_me<'a, 'b>(
    module: Module<'a>,
    config: &'b CmdDurationConfig,
    elapsed: u128,
) -> Module<'a> {
    use ansi_term::{unstyle, ANSIStrings};
    #[cfg(feature = "notify")]
    use notify_rust::{Notification, Timeout};

    #[cfg(feature = "notify")]
    if config.show_notifications && config.min_time_to_notify as u128 <= elapsed {
        let body = format!(
            "Command execution {}",
            unstyle(&ANSIStrings(&module.ansi_strings()))
        );

        let timeout = match config.notification_timeout {
            Some(v) => Timeout::Milliseconds(v),
            None => Timeout::Default,
        };

        let mut notification = Notification::new();
        notification
            .summary("Command finished")
            .body(&body)
            .icon("utilities-terminal")
            .timeout(timeout);

        if let Err(err) = notification.show() {
            log::trace!("Cannot show notification: {}", err);
        }
    }

    module
}

Review comment (suggestion):
LGTM, but please remove the inner `cfg`s:

```suggestion
    use notify_rust::{Notification, Timeout};

```

Code after:
        }
    });

    Some(undistract_me(module, &config, elapsed))
}

#[cfg(not(feature = "notify"))]
fn undistract_me<'a, 'b>(
    module: Module<'a>,
    _config: &'b CmdDurationConfig,
    _elapsed: u128,
) -> Module<'a> {
    module
}

#[cfg(feature = "notify")]
fn undistract_me<'a, 'b>(
    module: Module<'a>,
    config: &'b CmdDurationConfig,
    elapsed: u128,
) -> Module<'a> {
    use ansi_term::{unstyle, ANSIStrings};
    use notify_rust::{Notification, Timeout};

    if config.show_notifications && config.min_time_to_notify as u128 <= elapsed {
        let body = format!(
            "Command execution {}",
            unstyle(&ANSIStrings(&module.ansi_strings()))
        );

        let timeout = match config.notification_timeout {
            Some(v) => Timeout::Milliseconds(v),
            None => Timeout::Default,
        };

        let mut notification = Notification::new();
        notification
            .summary("Command finished")
            .body(&body)
            .icon("utilities-terminal")
            .timeout(timeout);

        if let Err(err) = notification.show() {
            log::trace!("Cannot show notification: {}", err);
        }
    }

    module
}

#[cfg(test)]
Repository: starship/starship
File: src/modules/daml.rs (Rust)

Code before:
use super::{Context, Module, ModuleConfig};
use crate::configs::daml::DamlConfig;
use crate::formatter::{StringFormatter, VersionFormatter};

const DAML_SDK_VERSION: &str = "sdk-version";
const DAML_SDK_VERSION_ENV: &str = "DAML_SDK_VERSION";
const DAML_YAML: &str = "daml.yaml";

/// Creates a module with the current Daml version
pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
    let mut module = context.new_module("daml");
    let config: DamlConfig = DamlConfig::try_load(module.config);

    let is_daml_project = context.try_begin_scan()?.set_files(&[DAML_YAML]).is_match();

    if !is_daml_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 daml_sdk_version = get_daml_sdk_version(context)?;
                    VersionFormatter::format_module_version(
                        module.get_name(),
                        &daml_sdk_version,
                        config.version_format,
                    )
                    .map(Ok)
                }

Review comment (suggestion):
Please use `detect_`.... options like most other modules.

Code after:
use super::{Context, Module, ModuleConfig};
use crate::configs::daml::DamlConfig;
use crate::formatter::{StringFormatter, VersionFormatter};

const DAML_SDK_VERSION: &str = "sdk-version";
const DAML_SDK_VERSION_ENV: &str = "DAML_SDK_VERSION";
const DAML_YAML: &str = "daml.yaml";

/// Creates a module with the current Daml version
pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
    let mut module = context.new_module("daml");
    let config: DamlConfig = DamlConfig::try_load(module.config);

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

    if !is_daml_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 daml_sdk_version = get_daml_sdk_version(context)?;
                    VersionFormatter::format_module_version(
                        module.get_name(),
Repository: starship/starship
File: src/configs/status.rs (Rust)

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

#[derive(Clone, Deserialize, Serialize)]
#[cfg_attr(feature = "config-schema", derive(schemars::JsonSchema))]
#[serde(default)]
pub struct StatusConfig<'a> {
    pub format: &'a str,
    pub symbol: &'a str,
    pub success_symbol: &'a str,
    pub not_executable_symbol: &'a str,
    pub not_found_symbol: &'a str,
    pub sigint_symbol: &'a str,
    pub signal_symbol: &'a str,
    pub style: &'a str,
    pub map_symbol: bool,
    pub recognize_signal_code: bool,
    pub pipestatus: bool,
    pub pipestatus_separator: &'a str,
    pub pipestatus_format: &'a str,
    pub pipestatus_segment_format: &'a str,
    pub disabled: bool,
}

impl<'a> Default for StatusConfig<'a> {
    fn default() -> Self {
        StatusConfig {
            format: "[$symbol$status]($style) ",
            symbol: "✖",
            success_symbol: "",
            not_executable_symbol: "🚫",
            not_found_symbol: "🔍",
            sigint_symbol: "🧱",
            signal_symbol: "⚡",
            style: "bold red",
            map_symbol: false,
            recognize_signal_code: true,
            pipestatus: false,
            pipestatus_separator: "|",
            pipestatus_format:
                "\\[$pipestatus\\] => [$symbol$common_meaning$signal_name$maybe_int]($style)",
            pipestatus_segment_format: "",
            disabled: true,
        }
    }
}

Review comment (suggestion):
`Option<&'a str>` might be better here to make it more obvious that this is optional.

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

#[derive(Clone, Deserialize, Serialize)]
#[cfg_attr(feature = "config-schema", derive(schemars::JsonSchema))]
#[serde(default)]
pub struct StatusConfig<'a> {
    pub format: &'a str,
    pub symbol: &'a str,
    pub success_symbol: &'a str,
    pub not_executable_symbol: &'a str,
    pub not_found_symbol: &'a str,
    pub sigint_symbol: &'a str,
    pub signal_symbol: &'a str,
    pub style: &'a str,
    pub map_symbol: bool,
    pub recognize_signal_code: bool,
    pub pipestatus: bool,
    pub pipestatus_separator: &'a str,
    pub pipestatus_format: &'a str,
    #[serde(skip_serializing_if = "Option::is_none")]
    pub pipestatus_segment_format: Option<&'a str>,
    pub disabled: bool,
}

impl<'a> Default for StatusConfig<'a> {
    fn default() -> Self {
        StatusConfig {
            format: "[$symbol$status]($style) ",
            symbol: "✖",
            success_symbol: "",
            not_executable_symbol: "🚫",
            not_found_symbol: "🔍",
            sigint_symbol: "🧱",
            signal_symbol: "⚡",
            style: "bold red",
            map_symbol: false,
            recognize_signal_code: true,
            pipestatus: false,
            pipestatus_separator: "|",
            pipestatus_format:
                "\\[$pipestatus\\] => [$symbol$common_meaning$signal_name$maybe_int]($style)",
            pipestatus_segment_format: None,
            disabled: true,
        }
    }
Repository: starship/starship
File: src/modules/status.rs (Rust)

Code before:
                    map_symbol = true
                    pipestatus = true
                    pipestatus_separator = "|"
                    pipestatus_format = "PSF $int$common_meaning$signal_name=$pipestatus"
                    disabled = false
                })
                .status(main_exit_code)
                .pipestatus(pipe_exit_code)
                .collect();
            assert_eq!(expected, actual);
        }
    }

    #[test]
    fn pipestatus_segment_format() {
        let pipe_exit_code = &[0, 1];
        let main_exit_code = 1;

        let expected = Some("[0]|[1] => [1]".to_string());
        let actual = ModuleRenderer::new("status")
            .config(toml::toml! {
                [status]
                format = "\\($status\\)"
                pipestatus = true
                pipestatus_separator = "|"
                pipestatus_format = "$pipestatus => \\[$status\\]"
                pipestatus_segment_format = "\\[$status\\]"
                disabled = false
            })
            .status(main_exit_code)
            .pipestatus(pipe_exit_code)
            .collect();
        assert_eq!(expected, actual);
    }
}


Review comment (suggestion):
Please use a different style for the final `$status` in `pipestatus_format`
```suggestion
        let expected = Some("[0]|[1] => <1>".to_string());
        let actual = ModuleRenderer::new("status")
            .config(toml::toml! {
                [status]
                format = "\\($status\\)"
                pipestatus = true
                pipestatus_separator = "|"
                pipestatus_format = "$pipestatus => <$status>"
```

Code after:
                    pipestatus_format = "PSF $int$common_meaning$signal_name=$pipestatus"
                    disabled = false
                })
                .status(main_exit_code)
                .pipestatus(pipe_exit_code)
                .collect();
            assert_eq!(expected, actual);
        }
    }

    #[test]
    fn pipestatus_segment_format() {
        let pipe_exit_code = &[0, 1];
        let main_exit_code = 1;

        let expected = Some("[0]|[1] => <1>".to_string());
        let actual = ModuleRenderer::new("status")
            .config(toml::toml! {
                [status]
                format = "\\($status\\)"
                pipestatus = true
                pipestatus_separator = "|"
                pipestatus_format = "$pipestatus => <$status>"
                pipestatus_segment_format = "\\[$status\\]"
                disabled = false
            })
            .status(main_exit_code)
            .pipestatus(pipe_exit_code)
            .collect();
        assert_eq!(expected, actual);
    }
}