ronantakizawa/codereview-bench huggingface dataset:
Repository: sst/opentui
File: packages/react/examples/animation.tsx (TypeScript)

Code before:
    cpu: 0,
    memory: 0,
    network: 0,
    disk: 0,
  })

  const timeline = useTimeline({
    duration: 3000,
    loop: false,
  })

  timeline.add(
    stats,
    {
      cpu: 85,
      memory: 70,
      network: 95,
      disk: 60,
      duration: 3000,
      ease: "linear",
      onUpdate: (values) => {
        setAnimatedStats({ ...values.targets[0] })
      },
    },
    0,
  )

  const statsMap = [
    { name: "CPU", key: "cpu", color: "#6a5acd" },
    { name: "Memory", key: "memory", color: "#4682b4" },
    { name: "Network", key: "network", color: "#20b2aa" },
    { name: "Disk", key: "disk", color: "#daa520" },
  ]

  return (
    <box
      title="System Monitor"
      style={{
        margin: 1,
        padding: 1,
        border: true,
        marginLeft: 2,
        marginRight: 2,
        borderStyle: "single",
        borderColor: "#4a4a4a",
      }}
    >
      {statsMap.map((stat) => (
        <box key={stat.key}>
          <box flexDirection="row" justifyContent="space-between">
            <text>{stat.name}</text>

Review comment (suggestion):
```suggestion
useEffect(() => {
  timeline.add(
    stats,
    {
      cpu: 85,
      memory: 70,
      network: 95,
      disk: 60,
      duration: 3000,
      ease: "linear",
      onUpdate: (values) => {
        setAnimatedStats({ ...values.targets[0] })
      },
    },
    0,
  )
  },[])
```

Code after:
    cpu: 0,
    memory: 0,
    network: 0,
    disk: 0,
  })

  const timeline = useTimeline({
    duration: 3000,
    loop: false,
  })

  useEffect(() => {
    timeline.add(
      stats,
      {
        cpu: 85,
        memory: 70,
        network: 95,
        disk: 60,
        duration: 3000,
        ease: "linear",
        onUpdate: (values) => {
          setAnimatedStats({ ...values.targets[0] })
        },
      },
      0,
    )
  }, [])

  const statsMap = [
    { name: "CPU", key: "cpu", color: "#6a5acd" },
    { name: "Memory", key: "memory", color: "#4682b4" },
    { name: "Network", key: "network", color: "#20b2aa" },
    { name: "Disk", key: "disk", color: "#daa520" },
  ]

  return (
    <box
      title="System Monitor"
      style={{
        margin: 1,
        padding: 1,
        border: true,
        marginLeft: 2,
        marginRight: 2,
        borderStyle: "single",
        borderColor: "#4a4a4a",
      }}
    >
      {statsMap.map((stat) => (
        <box key={stat.key}>
Repository: sst/opentui
File: packages/react/examples/animation.tsx (TypeScript)

Code before:
import { TextAttributes } from "@opentui/core"
import { render, useTimeline } from "@opentui/react"
import { useState } from "react"

type Stats = {
  cpu: number
  memory: number
  network: number
  disk: number
}

export const App = () => {
  const [stats, setAnimatedStats] = useState<Stats>({
    cpu: 0,
    memory: 0,
    network: 0,
    disk: 0,
  })

  const timeline = useTimeline({
    duration: 3000,
    loop: false,
  })

  timeline.add(
    stats,
    {
      cpu: 85,

Review comment (suggestion):
```suggestion
import { useState, useEffect } from "react"
```

Code after:
import { TextAttributes } from "@opentui/core"
import { render, useTimeline } from "@opentui/react"
import { useEffect, useState } from "react"

type Stats = {
  cpu: number
  memory: number
  network: number
  disk: number
}

export const App = () => {
  const [stats, setAnimatedStats] = useState<Stats>({
    cpu: 0,
    memory: 0,
    network: 0,
    disk: 0,
  })

  const timeline = useTimeline({
    duration: 3000,
    loop: false,
  })

  useEffect(() => {
    timeline.add(
      stats,
      {
Repository: sst/opentui
File: packages/core/src/renderables/Box.ts (TypeScript)

Code before:
      this._border = value
      this.borderSides = getBorderSides(value)
      this.applyYogaBorders()
      this.requestRender()
    }
  }

  public get borderStyle(): BorderStyle {
    return this._borderStyle
  }

  public set borderStyle(value: BorderStyle) {
    let _value = value ?? this._defaultOptions.borderStyle
    if (this._borderStyle !== _value) {
      this._borderStyle = _value
      this._customBorderChars = undefined
      this.requestRender()
    }
  }

  public get borderColor(): RGBA {
    return this._borderColor
  }

  public set borderColor(value: RGBA | string) {
    const newColor = parseColor(value ?? this._defaultOptions.borderColor)
    if (this._borderColor !== newColor) {
      this._borderColor = newColor
      this.requestRender()
    }
  }

  public get focusedBorderColor(): RGBA {
    return this._focusedBorderColor
  }

  public set focusedBorderColor(value: RGBA | string) {
    const newColor = parseColor(value ?? this._defaultOptions.focusedBorderColor)
    if (this._focusedBorderColor !== newColor) {
      this._focusedBorderColor = newColor
      if (this._focused) {
        this.requestRender()
      }
    }
  }

  public get title(): string | undefined {
    return this._title
  }

  public set title(value: string | undefined) {

Review comment (suggestion):
`!this._border` condition required for scenarios where value === defaultValue so this condition never fires

Code after:
    if (this._backgroundColor !== newColor) {
      this._backgroundColor = newColor
      this.requestRender()
    }
  }

  public get border(): boolean | BorderSides[] {
    return this._border
  }

  public set border(value: boolean | BorderSides[]) {
    if (this._border !== value) {
      this._border = value
      this.borderSides = getBorderSides(value)
      this.applyYogaBorders()
      this.requestRender()
    }
  }

  public get borderStyle(): BorderStyle {
    return this._borderStyle
  }

  public set borderStyle(value: BorderStyle) {
    let _value = value ?? this._defaultOptions.borderStyle
    if (this._borderStyle !== _value || !this._border) {
      this._borderStyle = _value
      this._customBorderChars = undefined
      this.initializeBorder()
      this.requestRender()
    }
  }

  public get borderColor(): RGBA {
    return this._borderColor
  }

  public set borderColor(value: RGBA | string) {
    const newColor = parseColor(value ?? this._defaultOptions.borderColor)
    if (this._borderColor !== newColor) {
      this._borderColor = newColor
      this.initializeBorder()
      this.requestRender()
    }
  }

  public get focusedBorderColor(): RGBA {
    return this._focusedBorderColor
  }

  public set focusedBorderColor(value: RGBA | string) {
Repository: starship/starship
File: src/configs/nim.rs (Rust)

Code before:
use crate::config::{ModuleConfig, RootModuleConfig, SegmentConfig};

use ansi_term::{Color, Style};
use starship_module_config_derive::ModuleConfig;

#[derive(Clone, ModuleConfig)]
pub struct NimConfig<'a> {
    pub symbol: SegmentConfig<'a>,
    pub version: SegmentConfig<'a>,
    pub style: Style,
    pub disabled: bool,
}

impl<'a> RootModuleConfig<'a> for NimConfig<'a> {
    fn new() -> Self {
        NimConfig {
            symbol: SegmentConfig::new("👑 v"),
            version: SegmentConfig::default(),
            style: Color::Yellow.bold(),
            disabled: false,
        }
    }
}


Review comment (suggestion):
The prefix shouldn't be part of a symbol:
```suggestion
            symbol: SegmentConfig::new("👑 "),
```

Code after:
use crate::config::{ModuleConfig, RootModuleConfig, SegmentConfig};

use ansi_term::{Color, Style};
use starship_module_config_derive::ModuleConfig;

#[derive(Clone, ModuleConfig)]
pub struct NimConfig<'a> {
    pub symbol: SegmentConfig<'a>,
    pub version: SegmentConfig<'a>,
    pub style: Style,
    pub disabled: bool,
}

impl<'a> RootModuleConfig<'a> for NimConfig<'a> {
    fn new() -> Self {
        NimConfig {
            symbol: SegmentConfig::new("👑 "),
            version: SegmentConfig::default(),
            style: Color::Yellow.bold(),
            disabled: false,
        }
    }
}
Repository: starship/starship
File: src/modules/nim.rs (Rust)

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

use crate::configs::nim::NimConfig;
use crate::utils;

/// Creates a module with the current Nim version
///
/// Will display the Nim version if any of the following criteria are met:
///     - The current directory contains a file with extension `.nim`, `.nims`, or `.nimble`
///     - The current directory contains a `nim.cfg` file
pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
    let is_nim_project = context
        .try_begin_scan()?
        .set_files(&["nim.cfg"])
        .set_extensions(&["nim", "nims", "nimble"])
        .is_match();

    if !is_nim_project {
        return None;
    }
    let nim_version_output = utils::exec_cmd("nim", &["--version"])?.stdout;
    let nim_version = parse_nim_version(&nim_version_output)?;

    let mut module = context.new_module("nim");
    let config = NimConfig::try_load(module.config);


Review comment (suggestion):
```suggestion
use super::{Context, Module, RootModuleConfig};
```

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

use crate::configs::nim::NimConfig;
use crate::utils;

/// Creates a module with the current Nim version
///
/// Will display the Nim version if any of the following criteria are met:
///     - The current directory contains a file with extension `.nim`, `.nims`, or `.nimble`
///     - The current directory contains a `nim.cfg` file
pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
    let is_nim_project = context
        .try_begin_scan()?
        .set_files(&["nim.cfg"])
        .set_extensions(&["nim", "nims", "nimble"])
        .is_match();

    if !is_nim_project {
        return None;
    }

    let nim_version_output = utils::exec_cmd("nim", &["--version"])?.stdout;
    let formatted_nim_version = format!("v{}", parse_nim_version(&nim_version_output)?);

    let mut module = context.new_module("nim");
    let config = NimConfig::try_load(module.config);
Repository: starship/starship
File: src/modules/nim.rs (Rust)

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

use crate::configs::nim::NimConfig;
use crate::utils;

/// Creates a module with the current Nim version
///
/// Will display the Nim version if any of the following criteria are met:
///     - The current directory contains a file with extension `.nim`, `.nims`, or `.nimble`
///     - The current directory contains a `nim.cfg` file
pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
    let is_nim_project = context
        .try_begin_scan()?
        .set_files(&["nim.cfg"])
        .set_extensions(&["nim", "nims", "nimble"])
        .is_match();

    if !is_nim_project {
        return None;
    }
    let nim_version_output = utils::exec_cmd("nim", &["--version"])?.stdout;
    let nim_version = parse_nim_version(&nim_version_output)?;

    let mut module = context.new_module("nim");
    let config = NimConfig::try_load(module.config);

    module.set_style(config.style);

    module.create_segment("symbol", &config.symbol);
    module.create_segment("version", &SegmentConfig::new(nim_version));

    Some(module)
}

fn parse_nim_version(version_cmd_output: &str) -> Option<&str> {
    version_cmd_output
        .lines()
        // First line has the version
        .next()?
        .split(' ')
        .find(|&s| s.chars().all(|c| c >= '0' && c <= '9' || c == '.'))
}

#[cfg(test)]
mod tests {
    use super::parse_nim_version;
    use crate::modules::utils::test::render_module;

Review comment (suggestion):
```suggestion
    let nim_version = format!("v{}", parse_nim_version(&nim_version_output)?);
```

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

use crate::configs::nim::NimConfig;
use crate::utils;

/// Creates a module with the current Nim version
///
/// Will display the Nim version if any of the following criteria are met:
///     - The current directory contains a file with extension `.nim`, `.nims`, or `.nimble`
///     - The current directory contains a `nim.cfg` file
pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
    let is_nim_project = context
        .try_begin_scan()?
        .set_files(&["nim.cfg"])
        .set_extensions(&["nim", "nims", "nimble"])
        .is_match();

    if !is_nim_project {
        return None;
    }

    let nim_version_output = utils::exec_cmd("nim", &["--version"])?.stdout;
    let formatted_nim_version = format!("v{}", parse_nim_version(&nim_version_output)?);

    let mut module = context.new_module("nim");
    let config = NimConfig::try_load(module.config);

    module.set_style(config.style);

    module.create_segment("symbol", &config.symbol);
    module.create_segment("version", &SegmentConfig::new(&formatted_nim_version));

    Some(module)
}

fn parse_nim_version(version_cmd_output: &str) -> Option<&str> {
    version_cmd_output
        .lines()
        // First line has the version
        .next()?
        .split(' ')
        .find(|&s| s.chars().all(|c| c >= '0' && c <= '9' || c == '.'))
}

#[cfg(test)]
mod tests {
    use super::parse_nim_version;
Repository: starship/starship
File: src/modules/nim.rs (Rust)

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

use crate::configs::nim::NimConfig;
use crate::utils;

/// Creates a module with the current Nim version
///
/// Will display the Nim version if any of the following criteria are met:
///     - The current directory contains a file with extension `.nim`, `.nims`, or `.nimble`
///     - The current directory contains a `nim.cfg` file
pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
    let is_nim_project = context
        .try_begin_scan()?
        .set_files(&["nim.cfg"])
        .set_extensions(&["nim", "nims", "nimble"])
        .is_match();

    if !is_nim_project {
        return None;
    }
    let nim_version_output = utils::exec_cmd("nim", &["--version"])?.stdout;
    let nim_version = parse_nim_version(&nim_version_output)?;

    let mut module = context.new_module("nim");
    let config = NimConfig::try_load(module.config);

    module.set_style(config.style);

    module.create_segment("symbol", &config.symbol);
    module.create_segment("version", &SegmentConfig::new(nim_version));

    Some(module)
}

fn parse_nim_version(version_cmd_output: &str) -> Option<&str> {
    version_cmd_output
        .lines()
        // First line has the version
        .next()?
        .split(' ')
        .find(|&s| s.chars().all(|c| c >= '0' && c <= '9' || c == '.'))
}

#[cfg(test)]
mod tests {

Review comment (suggestion):
Pad with a blank line: 
```suggestion
    }

```

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

use crate::configs::nim::NimConfig;
use crate::utils;

/// Creates a module with the current Nim version
///
/// Will display the Nim version if any of the following criteria are met:
///     - The current directory contains a file with extension `.nim`, `.nims`, or `.nimble`
///     - The current directory contains a `nim.cfg` file
pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
    let is_nim_project = context
        .try_begin_scan()?
        .set_files(&["nim.cfg"])
        .set_extensions(&["nim", "nims", "nimble"])
        .is_match();

    if !is_nim_project {
        return None;
    }

    let nim_version_output = utils::exec_cmd("nim", &["--version"])?.stdout;
    let formatted_nim_version = format!("v{}", parse_nim_version(&nim_version_output)?);

    let mut module = context.new_module("nim");
    let config = NimConfig::try_load(module.config);

    module.set_style(config.style);

    module.create_segment("symbol", &config.symbol);
    module.create_segment("version", &SegmentConfig::new(&formatted_nim_version));

    Some(module)
}

fn parse_nim_version(version_cmd_output: &str) -> Option<&str> {
    version_cmd_output
        .lines()
        // First line has the version
        .next()?
        .split(' ')
        .find(|&s| s.chars().all(|c| c >= '0' && c <= '9' || c == '.'))
}

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

Code before:
                module.get_suffix().set_value(config.suffix);

                if let Some(symbol) = config.symbol {
                    module.create_segment("symbol", &symbol);
                }

                module.create_segment("value", &SegmentConfig::new(&shlvl_num.to_string()));

                Some(module)
            } else {
                None
            }
        }
        None => None,
    }
}

fn get_shlvl_value() -> Option<i64> {
    match env::var_os(SHLVL_ENV_VAR) {
        Some(os_value) => match os_value.into_string() {
            Ok(str_value) => match str_value.parse::<i64>() {
                Ok(value) => Some(value),
                _ => None,
            },
            _ => None,
        },
        None => None,
    }
}


Review comment (suggestion):
Ok, a few notes. If you are going to convert from OsString to String, you might as well use `env::var` rather than `env::var_os`, which does that conversion for you.

I believe you should be able to write the whole function as:
`env::var(SHLVL_ENV_VAR).ok()?.parse::<i64>().ok()`

(But I didn't try to compile this, I might have missed something subtle.)

Code after:
                "symbol" => Some(config.symbol),
                _ => None,
            })
            .map_style(|variable| match variable {
                "style" => Some(Ok(config.style)),
                _ => None,
            })
            .map(|variable| match variable {
                "shlvl" => Some(Ok(shlvl_str)),
                _ => None,
            })
            .parse(None)
    });

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

    Some(module)
}

fn get_shlvl_value() -> Option<i64> {
    env::var(SHLVL_ENV_VAR).ok()?.parse::<i64>().ok()
}
Repository: starship/starship
File: src/modules/shlvl.rs (Rust)

Code before:
use std::env;

use super::{Context, Module, SegmentConfig};

use crate::config::RootModuleConfig;
use crate::configs::shlvl::ShLvlConfig;

const SHLVL_ENV_VAR: &str = "SHLVL";

pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
    let shlvl = get_shlvl_value();

    match shlvl {
        Some(shlvl_num) => {
            let mut module = context.new_module("shlvl");
            let config: ShLvlConfig = ShLvlConfig::try_load(module.config);

            if config.disabled {
                return None;
            }

            if config.threshold <= shlvl_num {
                module.set_style(config.style);

                module.get_prefix().set_value(config.prefix);
                module.get_suffix().set_value(config.suffix);

                if let Some(symbol) = config.symbol {
                    module.create_segment("symbol", &symbol);
                }

                module.create_segment("value", &SegmentConfig::new(&shlvl_num.to_string()));

                Some(module)
            } else {
                None

Review comment (suggestion):
You can remove the match below if you use the `?` operator, i.e.:
`let shlvl = get_shlvl_value()?;`

Code after:
use std::env;

use super::{Context, Module};

use crate::config::RootModuleConfig;
use crate::configs::shlvl::ShLvlConfig;
use crate::formatter::StringFormatter;

const SHLVL_ENV_VAR: &str = "SHLVL";

pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
    let shlvl = get_shlvl_value()?;

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

    if config.disabled || shlvl < config.threshold {
        return None;
    }

    let shlvl_str = &shlvl.to_string();

    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 {
                "shlvl" => Some(Ok(shlvl_str)),
                _ => None,
            })
Repository: starship/starship
File: src/modules/shlvl.rs (Rust)

Code before:
use std::env;

use super::{Context, Module, SegmentConfig};

use crate::config::RootModuleConfig;
use crate::configs::shlvl::ShLvlConfig;

const SHLVL_ENV_VAR: &str = "SHLVL";

pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
    let shlvl = get_shlvl_value()?;

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

    if config.disabled {
        return None;
    }

    if config.threshold <= shlvl {
        module.set_style(config.style);

        module.get_prefix().set_value(config.prefix);
        module.get_suffix().set_value(config.suffix);

        if let Some(symbol) = config.symbol {
            module.create_segment("symbol", &symbol);
        }

        module.create_segment("value", &SegmentConfig::new(&shlvl.to_string()));

        Some(module)
    } else {
        None
    }
}

fn get_shlvl_value() -> Option<i64> {
    env::var(SHLVL_ENV_VAR).ok()?.parse::<i64>().ok()
}


Review comment (suggestion):
```suggestion
    if config.disabled || shlvl < config.threshold {
```

Use early return to avoid indenting the main portion of the function.

Code after:
use std::env;

use super::{Context, Module};

use crate::config::RootModuleConfig;
use crate::configs::shlvl::ShLvlConfig;
use crate::formatter::StringFormatter;

const SHLVL_ENV_VAR: &str = "SHLVL";

pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
    let shlvl = get_shlvl_value()?;

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

    if config.disabled || shlvl < config.threshold {
        return None;
    }

    let shlvl_str = &shlvl.to_string();

    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 {
                "shlvl" => Some(Ok(shlvl_str)),
                _ => None,
            })
            .parse(None)
    });

    module.set_segments(match parsed {
        Ok(segments) => segments,