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

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

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

    Some(module)
}

fn get_haxe_version(context: &Context) -> Option<String> {
    if let Some(version) = get_haxerc_version(context) {
        Some(version)
    } else {
        let cmd_output = context.exec_cmd("haxe", &["--version"])?;
        parse_haxe_version(cmd_output.stdout.as_str())
    }
}

fn get_haxerc_version(context: &Context) -> Option<String> {
    let file_contents = context.read_file_from_pwd(".haxerc");
    if let Some(raw_json) = file_contents {
        let package_json: json::Value = json::from_str(&raw_json).ok()?;

        let raw_version = package_json.get("version")?.as_str()?;
        if raw_version == "null" {
            return None;
        };
        if raw_version.contains('/') || raw_version.contains('\\') {
            return None;
        }

        Some(raw_version.to_string())
    } else {
        None
    }
}

fn parse_haxe_version(raw_version: &str) -> Option<String> {
    let re = Regex::new(HAXERC_VERSION_PATTERN).ok()?;
    let captures = re.captures(raw_version)?;
    let version = &captures["version"];

    Some(version.to_string())
}

#[cfg(test)]

Review comment (suggestion):
This should just use `?`:
```suggestion
    let raw_json = context.read_file_from_pwd(".haxerc")?;
```

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

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

    Some(module)
}

fn get_haxe_version(context: &Context) -> Option<String> {
    get_haxerc_version(context).or_else(|| {
        let cmd_output = context.exec_cmd("haxe", &["--version"])?;
        parse_haxe_version(cmd_output.stdout.as_str())
    })
}

fn get_haxerc_version(context: &Context) -> Option<String> {
    let raw_json = context.read_file_from_pwd(".haxerc")?;
    let package_json: json::Value = json::from_str(&raw_json).ok()?;

    let raw_version = package_json.get("version")?.as_str()?;
    if raw_version.contains('/') || raw_version.contains('\\') {
        return None;
    }
    Some(raw_version.to_string())
}

fn parse_haxe_version(raw_version: &str) -> Option<String> {
    let re = Regex::new(HAXERC_VERSION_PATTERN).ok()?;
    if !re.is_match(raw_version) {
        return None;
    }
    Some(raw_version.trim().to_string())
}

#[cfg(test)]
mod tests {
    use super::parse_haxe_version;
    use crate::{test::ModuleRenderer, utils::CommandOutput};
    use nu_ansi_term::Color;
    use serde_json as json;
    use std::fs::File;
    use std::io;
    use std::io::Write;
    use tempfile::TempDir;
Repository: starship/starship
File: src/configs/guix_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 GuixShellConfig<'a> {
    pub format: &'a str,
    pub symbol: &'a str,
    pub style: &'a str,
    pub disabled: bool,
}

/* The trailing double spaces in `symbol` are needed to work around issues with
multiwidth emoji support in some shells. Please do not file a PR to change this
unless you can show that your changes do not affect this workaround.  */
impl<'a> Default for GuixShellConfig<'a> {
    fn default() -> Self {
        GuixShellConfig {
            format: "via [$symbol]($style) ",
            symbol: " ",
            style: "yellow bold",
            disabled: false,
        }
    }
}


Review comment (refactor):
Depending on the symbol you settle on, please remove this:
```suggestion
```

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 GuixShellConfig<'a> {
    pub format: &'a str,
    pub symbol: &'a str,
    pub style: &'a str,
    pub disabled: bool,
}

impl<'a> Default for GuixShellConfig<'a> {
    fn default() -> Self {
        GuixShellConfig {
            format: "via [$symbol]($style) ",
            symbol: "🐃 ",
            style: "yellow bold",
            disabled: false,
        }
    }
}
Repository: starship/starship
File: src/configs/gradle.rs (Rust)

Code before:
#[serde(default)]
pub struct GradleConfig<'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>,
    pub strategy: GradleVersionStrategy,
}

#[derive(Clone, Deserialize, Serialize)]
#[cfg_attr(
    feature = "config-schema",
    derive(schemars::JsonSchema),
    schemars(deny_unknown_fields)
)]
#[serde(try_from = "String")]
pub enum GradleVersionStrategy {
    #[serde(rename = "properties")]
    WrapperProperties,
    #[serde(rename = "executable")]
    Executable,
}

pub struct InvalidGradleVersionStrategy(String);

impl Display for InvalidGradleVersionStrategy {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        write!(f, "{} is not a valid gradle version strategy. Available strategies are: `properties` and `executable`", self.0)
    }
}

impl TryFrom<String> for GradleVersionStrategy {
    type Error = InvalidGradleVersionStrategy;

    fn try_from(value: String) -> Result<Self, Self::Error> {
        let strategy = match value.as_str() {
            "properties" => GradleVersionStrategy::WrapperProperties,
            "executable" => GradleVersionStrategy::Executable,
            _ => return Err(InvalidGradleVersionStrategy(value)),
        };
        Ok(strategy)
    }
}

impl<'a> Default for GradleConfig<'a> {
    fn default() -> Self {
        GradleConfig {

Review comment (suggestion):
Instead of renaming every option, please use `rename_all=pascal_case`:

```suggestion
#[serde(try_from = "String")]
pub enum GradleVersionStrategy {
    #[serde(rename = "properties")]
    WrapperProperties,
    #[serde(rename = "executable")]
    Executable,
}
```

Code after:
    pub format: &'a str,
    pub version_format: &'a str,
    pub symbol: &'a str,
    pub style: &'a str,
    pub disabled: bool,
    pub recursive: bool,
    pub detect_extensions: Vec<&'a str>,
    pub detect_files: Vec<&'a str>,
    pub detect_folders: Vec<&'a str>,
}

impl<'a> Default for GradleConfig<'a> {
    fn default() -> Self {
        GradleConfig {
            format: "via [$symbol($version )]($style)",
            version_format: "v${raw}",
            symbol: "🅶 ",
            style: "bold bright-cyan",
            disabled: false,
            recursive: false,
            detect_extensions: vec!["gradle", "gradle.kts"],
            detect_files: vec![],
            detect_folders: vec!["gradle"],
        }
    }
}
Repository: starship/starship
File: src/configs/gradle.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 GradleConfig<'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>,
    pub strategy: GradleVersionStrategy,
}

#[derive(Clone, Deserialize, Serialize)]
#[cfg_attr(
    feature = "config-schema",
    derive(schemars::JsonSchema),
    schemars(deny_unknown_fields)
)]
#[serde(try_from = "String")]
pub enum GradleVersionStrategy {
    #[serde(rename = "properties")]
    WrapperProperties,
    #[serde(rename = "executable")]
    Executable,
}

pub struct InvalidGradleVersionStrategy(String);

impl Display for InvalidGradleVersionStrategy {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        write!(f, "{} is not a valid gradle version strategy. Available strategies are: `properties` and `executable`", self.0)
    }
}

impl TryFrom<String> for GradleVersionStrategy {
    type Error = InvalidGradleVersionStrategy;

    fn try_from(value: String) -> Result<Self, Self::Error> {
        let strategy = match value.as_str() {
            "properties" => GradleVersionStrategy::WrapperProperties,
            "executable" => GradleVersionStrategy::Executable,
            _ => return Err(InvalidGradleVersionStrategy(value)),

Review comment (suggestion):
This doesn't seem to do anything:
```suggestion
```

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

impl<'a> Default for GradleConfig<'a> {
    fn default() -> Self {
        GradleConfig {
            format: "via [$symbol($version )]($style)",
            version_format: "v${raw}",
            symbol: "🅶 ",
            style: "bold bright-cyan",
            disabled: false,
            recursive: false,
            detect_extensions: vec!["gradle", "gradle.kts"],
            detect_files: vec![],
            detect_folders: vec!["gradle"],
        }
    }
}
Repository: starship/starship
File: src/modules/package.rs (Rust)

Code before:
    #[test]
    fn test_extract_gradle_version_without_version() -> io::Result<()> {
        let config_name = "build.gradle";
        let config_content = "plugins {
    id 'java'
    id 'test.plugin' version '0.2.0'
}
java {
    sourceCompatibility = JavaVersion.VERSION_1_8
    targetCompatibility = JavaVersion.VERSION_1_8
}";

        let project_dir = create_project_dir()?;
        fill_config(&project_dir, config_name, Some(config_content))?;
        expect_output(&project_dir, None, None);
        project_dir.close()
    }
    #[test]
    fn test_extract_grade_version_from_properties() -> io::Result<()> {
        let config_name = "gradle.properties";
        let config_content = "
            version=1.2.3
            ";
        let project_dir = create_project_dir()?;
        fill_config(&project_dir, config_name, Some(config_content))?;
        expect_output(&project_dir, None, None);
        project_dir.close()
    }

    #[test]
    fn test_extract_mix_version() -> io::Result<()> {
        let config_name = "mix.exs";
        let config_content = "defmodule MyApp.MixProject do
  use Mix.Project

  def project do
    [
      app: :my_app,
      version: \"1.2.3\",
      elixir: \"~> 1.10\",
      start_permanent: Mix.env() == :prod,
      deps: deps()
    ]
  end

  # Run \"mix help compile.app\" to learn about applications.
  def application do
    [extra_applications: [:logger]]
  end

  # Run \"mix help deps\" to learn about dependencies.

Review comment (refactor):
The test should show that the version number can be extracted:
```suggestion
        expect_output(&project_dir, Some("v1.2.3"), None);
```

Code after:
java {
    sourceCompatibility = JavaVersion.VERSION_1_8
    targetCompatibility = JavaVersion.VERSION_1_8
}";

        let project_dir = create_project_dir()?;
        fill_config(&project_dir, config_name, Some(config_content))?;
        expect_output(&project_dir, None, None);
        project_dir.close()
    }
    #[test]
    fn test_extract_grade_version_from_properties() -> io::Result<()> {
        let config_name = "gradle.properties";
        let config_content = "
            version=1.2.3
            ";
        let project_dir = create_project_dir()?;
        fill_config(&project_dir, config_name, Some(config_content))?;
        expect_output(&project_dir, Some("v1.2.3"), None);
        project_dir.close()
    }

    #[test]
    fn test_extract_mix_version() -> io::Result<()> {
        let config_name = "mix.exs";
        let config_content = "defmodule MyApp.MixProject do
  use Mix.Project

  def project do
    [
      app: :my_app,
      version: \"1.2.3\",
      elixir: \"~> 1.10\",
      start_permanent: Mix.env() == :prod,
      deps: deps()
    ]
  end

  # Run \"mix help compile.app\" to learn about applications.
  def application do
    [extra_applications: [:logger]]
  end

  # Run \"mix help deps\" to learn about dependencies.
  defp deps do
    []
  end
end";

        let project_dir = create_project_dir()?;
        fill_config(&project_dir, config_name, Some(config_content))?;
Repository: starship/starship
File: src/configs/opa.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 OpaConfig<'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 OpaConfig<'a> {
    fn default() -> Self {
        OpaConfig {
            format: "via [$symbol($version )]($style)",
            version_format: "v${raw}",
            symbol: "🪖 ",
            style: "bold blue",
            disabled: false,
            detect_extensions: vec!["rego"],
            detect_files: vec![],
            detect_folders: vec![],
        }
    }
}


Review comment (suggestion):
VS-Code and iTerm require two spaces this for proper spacing, or are there any other options? The emoji was only standardized recently:
```suggestion
            symbol: "🪖  ",
```

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 OpaConfig<'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 OpaConfig<'a> {
    fn default() -> Self {
        OpaConfig {
            format: "via [$symbol($version )]($style)",
            version_format: "v${raw}",
            symbol: "🪖  ",
            style: "bold blue",
            disabled: false,
            detect_extensions: vec!["rego"],
            detect_files: vec![],
            detect_folders: vec![],
        }
    }
}
Repository: starship/starship
File: src/modules/cmd_duration.rs (Rust)

Code before:

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

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

    if config.show_notifications && config.min_time_to_notify as u128 <= elapsed {
        if cfg!(target_os = "linux") {
            let in_graphical_session = ["DISPLAY", "WAYLAND_DISPLAY", "MIR_SOCKET"]
                .iter()
                .map(|&var| context.get_env(var).filter(|val| !val.is_empty()))
                .any(|o| o.is_some());

            if !in_graphical_session {
                return module;
            };
        }

        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() {

Review comment (refactor):
Not a blocker but I think you can use a [find_map](https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.find_map) here to simplify this a bit. 

Code after:

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

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

    if config.show_notifications && config.min_time_to_notify as u128 <= elapsed {
        if cfg!(target_os = "linux") {
            let in_graphical_session = ["DISPLAY", "WAYLAND_DISPLAY", "MIR_SOCKET"]
                .iter()
                .find_map(|&var| context.get_env(var).filter(|val| !val.is_empty()))
                .is_some();

            if !in_graphical_session {
                return module;
            };
        }

        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() {
Repository: starship/starship
File: src/configs/buf.rs (Rust)

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

#[derive(Clone, Deserialize, Serialize)]
#[cfg_attr(
    feature = "config-schema",
    derive(schemars::JsonSchema),
    schemars(deny_unknown_fields)
)]
#[serde(default)]
pub struct BufConfig<'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 BufConfig<'a> {
    fn default() -> Self {
        if cfg!(windows) {
            let version_str = os_info::get().version().to_string();
            let mut nums_of_version = version_str.split('.');

            // Gets either version 11 or not
            let version = nums_of_version.next().unwrap().parse::<i32>().unwrap();

            if version != 11 {
                return BufConfig {
                    format: "with [$symbol($version )]($style)",
                    version_format: "v${raw}",
                    symbol: "🐃 ",
                    style: "bold blue",
                    disabled: false,
                    detect_extensions: vec![],
                    detect_files: vec!["buf.yaml", "buf.gen.yaml", "buf.work.yaml"],
                    detect_folders: vec![],
                };
            }
        }

        BufConfig {
            format: "with [$symbol($version )]($style)",
            version_format: "v${raw}",
            symbol: "🦬 ",
            style: "bold blue",

Review comment (suggestion):
I'm reluctant to have it conditionally show one emoji or the other. That behavior will need to be documented and can lead to people reporting inconsistent behavior  across systems for not very much gain.

I'd personally prefer it if we could find a symbol to represent Buf that works across all systems.

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 BufConfig<'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 BufConfig<'a> {
    fn default() -> Self {
        BufConfig {
            format: "with [$symbol($version )]($style)",
            version_format: "v${raw}",
            symbol: "🐃 ",
            style: "bold blue",
            disabled: false,
            detect_extensions: vec![],
            detect_files: vec!["buf.yaml", "buf.gen.yaml", "buf.work.yaml"],
            detect_folders: vec![],
        }
    }
}
Repository: starship/starship
File: src/modules/utils/path.rs (Rust)

Code before:
#[cfg(not(windows))]
impl PathExt for Path {
    #[inline]
    fn normalised_starts_with(&self, other: &Path) -> bool {
        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")))]
    pub 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);

Review comment (suggestion):
```suggestion
    fn device_id(&self) -> u64 {
```

Code after:
    }
}

// NOTE: Windows path prefixes are only parsed on Windows.
// On other platforms, we can fall back to the non-normalized versions of these routines.
#[cfg(not(windows))]
impl PathExt for Path {
    #[inline]
    fn normalised_starts_with(&self, other: &Path) -> bool {
        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));