ronantakizawa/codereview-bench huggingface dataset:
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: 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,
        }
    }
}


Review comment (suggestion):
Just remembered that this is required for `Option`-values in the config:
```suggestion
    #[serde(skip_serializing_if = "Option::is_none")]
    pub pipestatus_segment_format: Option<&'a str>,
```

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/print.rs (Rust)

Code before:
            }
            match context.target {
                Target::Main => (lf, modules),
                Target::Right => (rf, modules),
                Target::Continuation => (cf, modules),
            }
        }
        _ => (StringFormatter::raw(">"), BTreeSet::new()),
    }
}

#[cfg(feature = "config-schema")]
pub fn print_schema() {
    let schema = schemars::schema_for!(crate::configs::FullConfig);
    println!("{}", serde_json::to_string_pretty(&schema).unwrap());
}

#[derive(clap::ValueEnum, Clone, Debug)]
pub enum Preset {
    BracketedSegments,
    NerdFontSymbols,
    NoRuntimeVersions,
    PastelPowerline,
    PlainTextSymbols,
    PurePreset,
}

#[derive(RustEmbed)]
#[folder = "docs/.vuepress/public/presets/toml/"]
struct Presets;

pub fn preset_command(name: Option<Preset>, list: bool) {
    if list {
        println!("{}", preset_list());
        return;
    }
    let variant = name.expect("name argument must be specified");
    println!("{}", get_preset(variant));
}

fn get_preset(variant: Preset) -> String {
    let path = format!(
        "{}.toml",
        variant
            .to_possible_value()
            .expect("Failed to convert to possible value")
            .get_name()
    );
    let file = Presets::get(&path).expect("Failed to get preset file");
    std::str::from_utf8(file.data.as_ref())
        .expect("Failed to convert preset from utf-8")

Review comment (suggestion):
I actually meant handling the validation via a type that manually implement the `clap::ValueEnum` trait, by looking at the values in `Presets`, instead of the manual validation you had before.
If the implementation does require presets to be added to the code manually, please don't use `rust-embed` and update `CONTRIBUTING.md`.

Code after:
            }
            match context.target {
                Target::Main => (lf, modules),
                Target::Right => (rf, modules),
                Target::Continuation => (cf, modules),
            }
        }
        _ => (StringFormatter::raw(">"), BTreeSet::new()),
    }
}

#[cfg(feature = "config-schema")]
pub fn print_schema() {
    let schema = schemars::schema_for!(crate::configs::FullConfig);
    println!("{}", serde_json::to_string_pretty(&schema).unwrap());
}

#[derive(Clone, Debug)]
pub struct Preset(pub &'static str);

impl ValueEnum for Preset {
    fn value_variants<'a>() -> &'a [Self] {
        shadow::get_preset_list()
    }

    fn to_possible_value<'a>(&self) -> Option<clap::PossibleValue<'a>> {
        Some(PossibleValue::new(self.0))
    }
}

pub fn preset_command(name: Option<Preset>, list: bool) {
    if list {
        println!("{}", preset_list());
        return;
    }
    let variant = name.expect("name argument must be specified");
    shadow::print_preset_content(variant.0);
}

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

#[cfg(test)]
mod test {
    use super::*;
    use crate::config::StarshipConfig;
    use crate::test::default_context;
Repository: starship/starship
File: build.rs (Rust)

Code before:
use std::fs::{self, File};
use std::io::Write;

use shadow_rs::SdResult;

fn main() -> SdResult<()> {
    shadow_rs::new().map_err(|err| err.to_string())?;
    shadow_rs::new_hook(gen_presets_hook)?;

    #[cfg(windows)]
    {
        let mut res = winres::WindowsResource::new();
        res.set_manifest_file("starship.exe.manifest")
            .set_icon("media/icon.ico");
        res.compile()?;
    }

    Ok(())
}

fn gen_presets_hook(mut file: &File) -> SdResult<()> {
    let paths = fs::read_dir("docs/.vuepress/public/presets/toml")?;

    let mut presets = String::new();
    let mut hashmap_assignments = String::new();
    for path in paths {
        let unwrapped = path?;
        let file_name = unwrapped.file_name();
        let full_path = dunce::canonicalize(unwrapped.path())?;
        let full_path = full_path.to_str().expect("failed to convert to string");
        let name = file_name
            .to_str()
            .and_then(|v| v.strip_suffix(".toml"))
            .expect("Failed to process filename");
        presets.push_str(format!("print::Preset(\"{}\"),\n", name).as_str());
        hashmap_assignments.push_str(
            format!(
                r#"
    let preset = String::from_utf8_lossy(include_bytes!(
        r"{}"
    ))
    .to_string();
    preset_map.insert(String::from("{}"), preset);
"#,
                full_path, name
            )
            .as_str(),
        );

Review comment (suggestion):
To ensure this gets rerun on changes:
```suggestion
    println!("cargo:rerun-if-changed=docs/.vuepress/public/presets/toml");
    let paths = fs::read_dir("docs/.vuepress/public/presets/toml")?;
```

Code after:
use std::fs::{self, File};
use std::io::Write;

use shadow_rs::SdResult;

fn main() -> SdResult<()> {
    shadow_rs::new().map_err(|err| err.to_string())?;
    shadow_rs::new_hook(gen_presets_hook)?;

    #[cfg(windows)]
    {
        let mut res = winres::WindowsResource::new();
        res.set_manifest_file("starship.exe.manifest")
            .set_icon("media/icon.ico");
        res.compile()?;
    }

    Ok(())
}

fn gen_presets_hook(mut file: &File) -> SdResult<()> {
    println!("cargo:rerun-if-changed=docs/.vuepress/public/presets/toml");
    let paths = fs::read_dir("docs/.vuepress/public/presets/toml")?;

    let mut presets = String::new();
    let mut match_arms = String::new();
    for path in paths {
        let unwrapped = path?;
        let file_name = unwrapped.file_name();
        let full_path = dunce::canonicalize(unwrapped.path())?;
        let full_path = full_path.to_str().expect("failed to convert to string");
        let name = file_name
            .to_str()
            .and_then(|v| v.strip_suffix(".toml"))
            .expect("Failed to process filename");
        presets.push_str(format!("print::Preset(\"{}\"),\n", name).as_str());
        match_arms.push_str(
            format!(
                r#"
"{name}" => {{
        let mut stdout = io::stdout().lock();
        let _ = stdout.write_all(include_bytes!(r"{full_path}"));
}}
"#
            )
            .as_str(),
        );
    }
Repository: starship/starship
File: build.rs (Rust)

Code before:
    Ok(())
}

fn gen_presets_hook(mut file: &File) -> SdResult<()> {
    let paths = fs::read_dir("docs/.vuepress/public/presets/toml")?;

    let mut presets = String::new();
    let mut hashmap_assignments = String::new();
    for path in paths {
        let unwrapped = path?;
        let file_name = unwrapped.file_name();
        let full_path = dunce::canonicalize(unwrapped.path())?;
        let full_path = full_path.to_str().expect("failed to convert to string");
        let name = file_name
            .to_str()
            .and_then(|v| v.strip_suffix(".toml"))
            .expect("Failed to process filename");
        presets.push_str(format!("print::Preset(\"{}\"),\n", name).as_str());
        hashmap_assignments.push_str(
            format!(
                r#"
    let preset = String::from_utf8_lossy(include_bytes!(
        r"{}"
    ))
    .to_string();
    preset_map.insert(String::from("{}"), preset);
"#,
                full_path, name
            )
            .as_str(),
        );
    }

    let content: String = format!(
        r#"
use crate::print;
use std::collections::HashMap;

pub fn get_preset_list<'a>() -> &'a [print::Preset] {{
    &[
        {}
    ]
}}

pub fn get_preset_content(name: String) -> Option<String> {{
    let mut preset_map: HashMap<String, String> = HashMap::new();

    {}

    preset_map.get(&name).cloned()
}}

Review comment (suggestion):
I'd prefer just using a `match` instead of a `HashMap` here. And instead of `String`, please just print the bytes as-is.

Code after:
    Ok(())
}

fn gen_presets_hook(mut file: &File) -> SdResult<()> {
    println!("cargo:rerun-if-changed=docs/.vuepress/public/presets/toml");
    let paths = fs::read_dir("docs/.vuepress/public/presets/toml")?;

    let mut presets = String::new();
    let mut match_arms = String::new();
    for path in paths {
        let unwrapped = path?;
        let file_name = unwrapped.file_name();
        let full_path = dunce::canonicalize(unwrapped.path())?;
        let full_path = full_path.to_str().expect("failed to convert to string");
        let name = file_name
            .to_str()
            .and_then(|v| v.strip_suffix(".toml"))
            .expect("Failed to process filename");
        presets.push_str(format!("print::Preset(\"{}\"),\n", name).as_str());
        match_arms.push_str(
            format!(
                r#"
"{name}" => {{
        let mut stdout = io::stdout().lock();
        let _ = stdout.write_all(include_bytes!(r"{full_path}"));
}}
"#
            )
            .as_str(),
        );
    }

    writeln!(
        file,
        r#"
use std::io::{{self, Write}};
use crate::print;

pub fn get_preset_list<'a>() -> &'a [print::Preset] {{
    &[
        {presets}
    ]
}}

pub fn print_preset_content(name: &str) {{
    match name {{
    {match_arms}
    _ => {{}}
    }}
}}
"#
Repository: starship/starship
File: build.rs (Rust)

Code before:
    preset_map.insert(String::from("{}"), preset);
"#,
                full_path, name
            )
            .as_str(),
        );
    }

    let content: String = format!(
        r#"
use crate::print;
use std::collections::HashMap;

pub fn get_preset_list<'a>() -> &'a [print::Preset] {{
    &[
        {}
    ]
}}

pub fn get_preset_content(name: String) -> Option<String> {{
    let mut preset_map: HashMap<String, String> = HashMap::new();

    {}

    preset_map.get(&name).cloned()
}}
"#,
        presets, hashmap_assignments,
    );
    writeln!(file, "{}", content)?;
    Ok(())
}


Review comment (suggestion):
For these statements, perhaps use the names directly (`{presets}` instead of `{}`) instead of arguments to make this more readable. Also, `format!` and `writeln!` are redundant here.

Code after:
}}
"#
            )
            .as_str(),
        );
    }

    writeln!(
        file,
        r#"
use std::io::{{self, Write}};
use crate::print;

pub fn get_preset_list<'a>() -> &'a [print::Preset] {{
    &[
        {presets}
    ]
}}

pub fn print_preset_content(name: &str) {{
    match name {{
    {match_arms}
    _ => {{}}
    }}
}}
"#
    )?;
    Ok(())
}
Repository: starship/starship
File: src/print.rs (Rust)

Code before:
use ansi_term::ANSIStrings;
use clap::{PossibleValue, ValueEnum};
use rayon::prelude::*;
use std::collections::BTreeSet;
use std::fmt::{self, Debug, Write as FmtWrite};
use std::io::{self, Write};
use std::time::Duration;
use terminal_size::terminal_size;
use unicode_segmentation::UnicodeSegmentation;
use unicode_width::UnicodeWidthChar;

use crate::configs::PROMPT_ORDER;
use crate::context::{Context, Properties, Shell, Target};
use crate::formatter::{StringFormatter, VariableHolder};
use crate::module::Module;
use crate::module::ALL_MODULES;
use crate::modules;
use crate::segment::Segment;

pub struct Grapheme<'a>(pub &'a str);

shadow!(build);

impl<'a> Grapheme<'a> {
    pub fn width(&self) -> usize {
        self.0
            .chars()
            .filter_map(UnicodeWidthChar::width)
            .max()
            .unwrap_or(0)
    }
}

pub trait UnicodeWidthGraphemes {
    fn width_graphemes(&self) -> usize;
}

impl<T> UnicodeWidthGraphemes for T
where
    T: AsRef<str>,
{
    fn width_graphemes(&self) -> usize {
        self.as_ref()
            .graphemes(true)
            .map(Grapheme)
            .map(|g| g.width())
            .sum()

Review comment (suggestion):
`lib.rs` already has this. Please try re-using it.

Code after:
use ansi_term::ANSIStrings;
use clap::{PossibleValue, ValueEnum};
use rayon::prelude::*;
use std::collections::BTreeSet;
use std::fmt::{self, Debug, Write as FmtWrite};
use std::io::{self, Write};
use std::time::Duration;
use terminal_size::terminal_size;
use unicode_segmentation::UnicodeSegmentation;
use unicode_width::UnicodeWidthChar;

use crate::configs::PROMPT_ORDER;
use crate::context::{Context, Properties, Shell, Target};
use crate::formatter::{StringFormatter, VariableHolder};
use crate::module::Module;
use crate::module::ALL_MODULES;
use crate::modules;
use crate::segment::Segment;
use crate::shadow;

pub struct Grapheme<'a>(pub &'a str);

impl<'a> Grapheme<'a> {
    pub fn width(&self) -> usize {
        self.0
            .chars()
            .filter_map(UnicodeWidthChar::width)
            .max()
            .unwrap_or(0)
    }
}

pub trait UnicodeWidthGraphemes {
    fn width_graphemes(&self) -> usize;
}

impl<T> UnicodeWidthGraphemes for T
where
    T: AsRef<str>,
{
    fn width_graphemes(&self) -> usize {
        self.as_ref()
            .graphemes(true)
            .map(Grapheme)
            .map(|g| g.width())
            .sum()
    }
Repository: starship/starship
File: build.rs (Rust)

Code before:

    Ok(())
}

fn gen_presets_hook(mut file: &File) -> SdResult<()> {
    println!("cargo:rerun-if-changed=docs/.vuepress/public/presets/toml");
    let paths = fs::read_dir("docs/.vuepress/public/presets/toml")?;

    let mut presets = String::new();
    let mut match_arms = String::new();
    for path in paths {
        let unwrapped = path?;
        let file_name = unwrapped.file_name();
        let full_path = dunce::canonicalize(unwrapped.path())?;
        let full_path = full_path.to_str().expect("failed to convert to string");
        let name = file_name
            .to_str()
            .and_then(|v| v.strip_suffix(".toml"))
            .expect("Failed to process filename");
        presets.push_str(format!("print::Preset(\"{}\"),\n", name).as_str());
        match_arms.push_str(
            format!(
                r#"
"{name}" => {{
        let mut stdout = io::stdout().lock();
        stdout.write_all(include_bytes!(r"{full_path}")).expect("Failed to print preset");
}}
"#
            )
            .as_str(),
        );
    }

    writeln!(
        file,
        r#"
use std::io::{{self, Write}};
use crate::print;

pub fn get_preset_list<'a>() -> &'a [print::Preset] {{
    &[
        {presets}
    ]
}}

pub fn print_preset_content(name: &str) {{
    match name {{
    {match_arms}
    _ => {{}}
    }}
}}

Review comment (suggestion):
`expect` makes for poor error messages (the other instances in this PR should never be hit, so they should be fine). I think doing nothing is preferable here, as printing an error message when something like `head` is used is unexpected. (#885)

```suggestion
        let _ = stdout.write_all(include_bytes!(r"{full_path}"));
```

Code after:

    Ok(())
}

fn gen_presets_hook(mut file: &File) -> SdResult<()> {
    println!("cargo:rerun-if-changed=docs/.vuepress/public/presets/toml");
    let paths = fs::read_dir("docs/.vuepress/public/presets/toml")?;

    let mut presets = String::new();
    let mut match_arms = String::new();
    for path in paths {
        let unwrapped = path?;
        let file_name = unwrapped.file_name();
        let full_path = dunce::canonicalize(unwrapped.path())?;
        let full_path = full_path.to_str().expect("failed to convert to string");
        let name = file_name
            .to_str()
            .and_then(|v| v.strip_suffix(".toml"))
            .expect("Failed to process filename");
        presets.push_str(format!("print::Preset(\"{}\"),\n", name).as_str());
        match_arms.push_str(
            format!(
                r#"
"{name}" => {{
        let mut stdout = io::stdout().lock();
        let _ = stdout.write_all(include_bytes!(r"{full_path}"));
}}
"#
            )
            .as_str(),
        );
    }

    writeln!(
        file,
        r#"
use std::io::{{self, Write}};
use crate::print;

pub fn get_preset_list<'a>() -> &'a [print::Preset] {{
    &[
        {presets}
    ]
}}

pub fn print_preset_content(name: &str) {{
    match name {{
    {match_arms}
    _ => {{}}
    }}
}}
Repository: starship/starship
File: src/configs/starship_root.rs (Rust)

Code before:
use serde::{Deserialize, Serialize};
use std::collections::HashMap;

#[derive(Clone, Serialize, Deserialize, Debug)]
#[cfg_attr(feature = "config-schema", derive(schemars::JsonSchema))]
#[serde(default)]
pub struct StarshipRootConfig {
    #[serde(rename = "$schema")]
    schema: String,
    pub format: String,
    pub right_format: String,
    pub continuation_prompt: String,
    pub scan_timeout: u64,
    pub command_timeout: u64,
    pub add_newline: bool,
    pub palette: Option<String>,
    pub palettes: HashMap<String, Palette>,
}

pub type Palette = HashMap<String, String>;

// List of default prompt order
// NOTE: If this const value is changed then Default prompt order subheading inside
// prompt heading of config docs needs to be updated according to changes made here.
pub const PROMPT_ORDER: &[&str] = &[
    "username",
    "hostname",
    "localip",
    "shlvl",
    "singularity",
    "kubernetes",
    "directory",
    "vcsh",
    "git_branch",
    "git_commit",
    "git_state",
    "git_metrics",
    "git_status",
    "hg_branch",
    "docker_context",
    "package",
    // ↓ Toolchain version modules ↓
    // (Let's keep these sorted alphabetically)

Review comment (suggestion):
Serializing `None` as TOML can cause issues, because it doesn't have a native type for that.
```suggestion
    #[serde(skip_serializing_if = "Option::is_none")]
    pub palettes: HashMap<String, Palette>,
```

Code after:
use serde::{Deserialize, Serialize};
use std::collections::HashMap;

#[derive(Clone, Serialize, Deserialize, Debug)]
#[cfg_attr(feature = "config-schema", derive(schemars::JsonSchema))]
#[serde(default)]
pub struct StarshipRootConfig {
    #[serde(rename = "$schema")]
    schema: String,
    pub format: String,
    pub right_format: String,
    pub continuation_prompt: String,
    pub scan_timeout: u64,
    pub command_timeout: u64,
    pub add_newline: bool,
    #[serde(skip_serializing_if = "Option::is_none")]
    pub palette: Option<String>,
    pub palettes: HashMap<String, Palette>,
}

pub type Palette = HashMap<String, String>;

// List of default prompt order
// NOTE: If this const value is changed then Default prompt order subheading inside
// prompt heading of config docs needs to be updated according to changes made here.
pub const PROMPT_ORDER: &[&str] = &[
    "username",
    "hostname",
    "localip",
    "shlvl",
    "singularity",
    "kubernetes",
    "directory",
    "vcsh",
    "git_branch",
    "git_commit",
    "git_state",
    "git_metrics",
    "git_status",
    "hg_branch",
    "docker_context",
    "package",
    // ↓ Toolchain version modules ↓