ronantakizawa/codereview-bench huggingface dataset:
Repository: alacritty/alacritty
File: font/src/ft/fc/pattern.rs (Rust)

Code before:

            let result = FcPatternGetCharSet(
                self.as_ptr(),
                b"charset\0".as_ptr() as *mut c_char,
                0,
                &mut charset,
            );

            if result == FcResultMatch {
                Some(&*(charset as *const CharSetRef))
            } else {
                None
            }
        }
    }

    pub fn file(&self, index: usize) -> Option<PathBuf> {
        unsafe { self.get_string(b"file\0").nth(index) }.map(From::from)
    }

    pub fn ft_face_location(&self, index: usize) -> Option<FTFaceLocation> {
        if let (Some(path), Some(index)) = (self.file(index), self.index().next()) {
            Some(FTFaceLocation::new(path, index))
        } else {
            None
        }
    }

    pub fn config_substitute(&mut self, config: &ConfigRef, kind: MatchKind) {
        unsafe {
            FcConfigSubstitute(config.as_ptr(), self.as_ptr(), kind as u32);
        }
    }

    pub fn default_substitute(&mut self) {
        unsafe {
            FcDefaultSubstitute(self.as_ptr());
        }
    }
}


Review comment (suggestion):
```suggestion
        match (self.file(index), self.index().next()) {
            (Some(path), Some(index)) => Some(FtFaceLocation::new(path, index)),
            _ => None,
        }
```

Code after:

            let result = FcPatternGetCharSet(
                self.as_ptr(),
                b"charset\0".as_ptr() as *mut c_char,
                0,
                &mut charset,
            );

            if result == FcResultMatch {
                Some(&*(charset as *const CharSetRef))
            } else {
                None
            }
        }
    }

    pub fn file(&self, index: usize) -> Option<PathBuf> {
        unsafe { self.get_string(b"file\0").nth(index) }.map(From::from)
    }

    pub fn ft_face_location(&self, index: usize) -> Option<FTFaceLocation> {
        match (self.file(index), self.index().next()) {
            (Some(path), Some(index)) => Some(FTFaceLocation::new(path, index)),
            _ => None,
        }
    }

    pub fn config_substitute(&mut self, config: &ConfigRef, kind: MatchKind) {
        unsafe {
            FcConfigSubstitute(config.as_ptr(), self.as_ptr(), kind as u32);
        }
    }

    pub fn default_substitute(&mut self) {
        unsafe {
            FcDefaultSubstitute(self.as_ptr());
        }
    }
}
Repository: alacritty/alacritty
File: alacritty_terminal/src/config/mod.rs (Rust)

Code before:

#[serde(default)]
#[derive(Deserialize, Copy, Clone, Debug, Default, PartialEq)]
pub struct Cursor {
    #[serde(deserialize_with = "failure_default")]
    pub style: CursorStyle,
    #[serde(deserialize_with = "option_explicit_none")]
    pub vi_mode_style: Option<CursorStyle>,
    #[serde(deserialize_with = "failure_default")]
    thickness: CursorThickness,
    #[serde(deserialize_with = "failure_default")]
    unfocused_hollow: DefaultTrueBool,
}

impl Cursor {
    #[inline]
    pub fn unfocused_hollow(self) -> bool {
        self.unfocused_hollow.0
    }

    #[inline]
    pub fn thickness(self) -> f32 {
        self.thickness.0
    }
}
/// Wrapper around f32 that represents a cursor thickness between 0.0 and 1.0
#[derive(Clone, Copy, Debug, PartialEq)]
struct CursorThickness(f32);

impl CursorThickness {
    pub fn new(value: f32) -> Self {
        CursorThickness(if value < 0.0 {
            0.0
        } else if value > 1.0 {
            1.0
        } else {
            value
        })
    }
}

impl Default for CursorThickness {
    fn default() -> Self {
        Self(0.15)
    }
}

impl<'de> Deserialize<'de> for CursorThickness {
    fn deserialize<D>(deserializer: D) -> ::std::result::Result<Self, D::Error>
    where
        D: Deserializer<'de>,

Review comment (suggestion):
```suggestion

/// Wrapper around f32 that represents a cursor thickness between 0.0 and 1.0.
```

Code after:
}

#[serde(default)]
#[derive(Deserialize, Copy, Clone, Debug, PartialEq)]
pub struct Cursor {
    #[serde(deserialize_with = "failure_default")]
    pub style: CursorStyle,
    #[serde(deserialize_with = "option_explicit_none")]
    pub vi_mode_style: Option<CursorStyle>,
    #[serde(deserialize_with = "deserialize_cursor_thickness")]
    thickness: Percentage,
    #[serde(deserialize_with = "failure_default")]
    unfocused_hollow: DefaultTrueBool,
}

impl Cursor {
    #[inline]
    pub fn unfocused_hollow(self) -> bool {
        self.unfocused_hollow.0
    }

    #[inline]
    pub fn thickness(self) -> f64 {
        self.thickness.0 as f64
    }
}

impl Default for Cursor {
    fn default() -> Self {
        Self {
            style: Default::default(),
            vi_mode_style: Default::default(),
            thickness: Percentage::new(DEFAULT_CURSOR_THICKNESS),
            unfocused_hollow: Default::default(),
        }
    }
}

pub fn deserialize_cursor_thickness<'a, D>(deserializer: D) -> Result<Percentage, D::Error>
where
    D: Deserializer<'a>,
{
    Ok(Percentage::deserialize(Value::deserialize(deserializer)?)
        .unwrap_or_else(|_| Percentage::new(DEFAULT_CURSOR_THICKNESS)))
}

#[derive(Clone, Debug, Deserialize, PartialEq, Eq)]
pub struct Shell<'a> {
    pub program: Cow<'a, str>,

    #[serde(default, deserialize_with = "failure_default")]
Repository: alacritty/alacritty
File: alacritty_terminal/src/config/mod.rs (Rust)

Code before:
}

impl Cursor {
    #[inline]
    pub fn unfocused_hollow(self) -> bool {
        self.unfocused_hollow.0
    }

    #[inline]
    pub fn thickness(self) -> f64 {
        self.thickness.0 as f64
    }
}

impl Default for Cursor {
    fn default() -> Self {
        Self {
            style: Default::default(),
            vi_mode_style: Default::default(),
            thickness: Percentage::new(DEFAULT_CURSOR_THICKNESS),
            unfocused_hollow: Default::default(),
        }
    }
}

pub fn default_thickness<'a, D>(deserializer: D) -> Result<Percentage, D::Error>
where
    D: Deserializer<'a>,
{
    Ok(Percentage::deserialize(Value::deserialize(deserializer)?)
        .unwrap_or_else(|_| Percentage::new(DEFAULT_CURSOR_THICKNESS)))
}

#[derive(Clone, Debug, Deserialize, PartialEq, Eq)]
pub struct Shell<'a> {
    pub program: Cow<'a, str>,

    #[serde(default, deserialize_with = "failure_default")]
    pub args: Vec<String>,
}

impl<'a> Shell<'a> {
    pub fn new<S>(program: S) -> Shell<'a>
    where
        S: Into<Cow<'a, str>>,
    {
        Shell { program: program.into(), args: Vec::new() }
    }

    pub fn new_with_args<S>(program: S, args: Vec<String>) -> Shell<'a>
    where

Review comment (suggestion):
The naming here isn't really great, since usually that's more the naming we use for default values, instead of deserializers.

So it should be something like `deserialize_cursor_thickness`.

Code after:
}

impl Cursor {
    #[inline]
    pub fn unfocused_hollow(self) -> bool {
        self.unfocused_hollow.0
    }

    #[inline]
    pub fn thickness(self) -> f64 {
        self.thickness.0 as f64
    }
}

impl Default for Cursor {
    fn default() -> Self {
        Self {
            style: Default::default(),
            vi_mode_style: Default::default(),
            thickness: Percentage::new(DEFAULT_CURSOR_THICKNESS),
            unfocused_hollow: Default::default(),
        }
    }
}

pub fn deserialize_cursor_thickness<'a, D>(deserializer: D) -> Result<Percentage, D::Error>
where
    D: Deserializer<'a>,
{
    Ok(Percentage::deserialize(Value::deserialize(deserializer)?)
        .unwrap_or_else(|_| Percentage::new(DEFAULT_CURSOR_THICKNESS)))
}

#[derive(Clone, Debug, Deserialize, PartialEq, Eq)]
pub struct Shell<'a> {
    pub program: Cow<'a, str>,

    #[serde(default, deserialize_with = "failure_default")]
    pub args: Vec<String>,
}

impl<'a> Shell<'a> {
    pub fn new<S>(program: S) -> Shell<'a>
    where
        S: Into<Cow<'a, str>>,
    {
        Shell { program: program.into(), args: Vec::new() }
    }

    pub fn new_with_args<S>(program: S, args: Vec<String>) -> Shell<'a>
    where
Repository: alacritty/alacritty
File: alacritty/src/main.rs (Rust)

Code before:
//! Alacritty - The GPU Enhanced Terminal.

#![deny(clippy::all, clippy::if_not_else, clippy::enum_glob_use, clippy::wrong_pub_self_convention)]
#![cfg_attr(feature =  "cargo-clippy", deny(dead_code))]
#![cfg_attr(all(test, feature = "bench"), feature(test))]
// With the default subsystem, 'console', windows creates an additional console
// window for the program.
// This is silently ignored on non-windows systems.
// See https://msdn.microsoft.com/en-us/library/4cc7ya5b.aspx for more details.
#![windows_subsystem = "windows"]

#[cfg(not(any(feature = "x11", feature = "wayland", target_os = "macos", windows)))]
compile_error!(r#"at least one of the "x11"/"wayland" features must be enabled"#);

#[cfg(target_os = "macos")]
use std::env;
use std::error::Error;
use std::fs;
use std::io::{self, Write};
use std::sync::Arc;

use glutin::event_loop::EventLoop as GlutinEventLoop;
use log::{error, info};
#[cfg(windows)]
use winapi::um::wincon::{AttachConsole, FreeConsole, ATTACH_PARENT_PROCESS};

use alacritty_terminal::event_loop::{self, EventLoop, Msg};
use alacritty_terminal::sync::FairMutex;
use alacritty_terminal::term::Term;

Review comment (suggestion):
```suggestion
#![cfg_attr(feature =  "cargo-clippy", deny(warnings))]
```

Code after:
//! Alacritty - The GPU Enhanced Terminal.

#![warn(rust_2018_idioms, future_incompatible)]
#![deny(clippy::all, clippy::if_not_else, clippy::enum_glob_use, clippy::wrong_pub_self_convention)]
#![cfg_attr(feature = "cargo-clippy", deny(warnings))]
#![cfg_attr(all(test, feature = "bench"), feature(test))]
// With the default subsystem, 'console', windows creates an additional console
// window for the program.
// This is silently ignored on non-windows systems.
// See https://msdn.microsoft.com/en-us/library/4cc7ya5b.aspx for more details.
#![windows_subsystem = "windows"]

#[cfg(not(any(feature = "x11", feature = "wayland", target_os = "macos", windows)))]
compile_error!(r#"at least one of the "x11"/"wayland" features must be enabled"#);

#[cfg(target_os = "macos")]
use std::env;
use std::error::Error;
use std::fs;
use std::io::{self, Write};
use std::sync::Arc;

use glutin::event_loop::EventLoop as GlutinEventLoop;
use log::{error, info};
#[cfg(windows)]
use winapi::um::wincon::{AttachConsole, FreeConsole, ATTACH_PARENT_PROCESS};

use alacritty_terminal::event_loop::{self, EventLoop, Msg};
use alacritty_terminal::sync::FairMutex;
Repository: alacritty/alacritty
File: alacritty_terminal/src/lib.rs (Rust)

Code before:
//! Alacritty - The GPU Enhanced Terminal.

#![deny(clippy::all, clippy::if_not_else, clippy::enum_glob_use, clippy::wrong_pub_self_convention)]
#![cfg_attr(feature =  "cargo-clippy", deny(dead_code))]
#![cfg_attr(all(test, feature = "bench"), feature(test))]

pub mod ansi;
pub mod config;
pub mod event;
pub mod event_loop;
pub mod grid;
pub mod index;
pub mod selection;
pub mod sync;
pub mod term;
pub mod thread;
pub mod tty;
pub mod vi_mode;

pub use crate::grid::Grid;
pub use crate::term::Term;


Review comment (suggestion):
```suggestion
#![cfg_attr(feature =  "cargo-clippy", deny(warnings))]
```

Code after:
//! Alacritty - The GPU Enhanced Terminal.

#![warn(rust_2018_idioms, future_incompatible)]
#![deny(clippy::all, clippy::if_not_else, clippy::enum_glob_use, clippy::wrong_pub_self_convention)]
#![cfg_attr(feature = "cargo-clippy", deny(warnings))]
#![cfg_attr(all(test, feature = "bench"), feature(test))]

pub mod ansi;
pub mod config;
pub mod event;
pub mod event_loop;
pub mod grid;
pub mod index;
pub mod selection;
pub mod sync;
pub mod term;
pub mod thread;
pub mod tty;
pub mod vi_mode;

pub use crate::grid::Grid;
pub use crate::term::Term;
Repository: alacritty/alacritty
File: alacritty/src/display/content.rs (Rust)

Code before:
        }
    }

    /// Get the RGB color from a cell's background color.
    #[inline]
    fn compute_bg_rgb(content: &mut RenderableContent<'_>, bg: Color) -> Rgb {
        match bg {
            Color::Spec(rgb) => rgb,
            Color::Named(ansi) => content.color(ansi as usize),
            Color::Indexed(idx) => content.color(idx as usize),
        }
    }

    /// Compute background alpha based on cell's original color.
    ///
    /// Since an RGB color matching the background should not be transparent, this is computed
    /// using the named input color, rather than checking the RGB of the background after its color
    /// is computed.
    #[inline]
    fn compute_bg_alpha(content: &mut RenderableContent<'_>, bg: Color) -> f32 {
        if bg == Color::Named(NamedColor::Background) {
            0.
        } else if content.config.ui_config.colors.opaque_background_colors {
            1.
        } else {
            content.config.ui_config.background_opacity()
        }
    }
}

/// Cursor storing all information relevant for rendering.
#[derive(Debug, Eq, PartialEq, Copy, Clone)]
pub struct RenderableCursor {
    shape: CursorShape,
    cursor_color: Rgb,
    text_color: Rgb,
    is_wide: bool,
    point: Point<usize>,
}

impl RenderableCursor {
    pub fn color(&self) -> Rgb {
        self.cursor_color
    }

    pub fn shape(&self) -> CursorShape {
        self.shape
    }

    pub fn is_wide(&self) -> bool {
        self.is_wide

Review comment (refactor):
In theory we could take a reference to ui_config as argument to this method, just to keep things minimal and reusable in theory. But it doesn't matter *that* much.

Code after:
        }
    }

    /// Get the RGB color from a cell's background color.
    #[inline]
    fn compute_bg_rgb(content: &mut RenderableContent<'_>, bg: Color) -> Rgb {
        match bg {
            Color::Spec(rgb) => rgb,
            Color::Named(ansi) => content.color(ansi as usize),
            Color::Indexed(idx) => content.color(idx as usize),
        }
    }

    /// Compute background alpha based on cell's original color.
    ///
    /// Since an RGB color matching the background should not be transparent, this is computed
    /// using the named input color, rather than checking the RGB of the background after its color
    /// is computed.
    #[inline]
    fn compute_bg_alpha(config: &UiConfig, bg: Color) -> f32 {
        if bg == Color::Named(NamedColor::Background) {
            0.
        } else if config.colors.transparent_background_colors {
            config.window_opacity()
        } else {
            1.
        }
    }
}

/// Cursor storing all information relevant for rendering.
#[derive(Debug, Eq, PartialEq, Copy, Clone)]
pub struct RenderableCursor {
    shape: CursorShape,
    cursor_color: Rgb,
    text_color: Rgb,
    is_wide: bool,
    point: Point<usize>,
}

impl RenderableCursor {
    pub fn color(&self) -> Rgb {
        self.cursor_color
    }

    pub fn shape(&self) -> CursorShape {
        self.shape
    }

    pub fn is_wide(&self) -> bool {
        self.is_wide
Repository: alacritty/alacritty
File: alacritty/src/display/content.rs (Rust)

Code before:
                content.color(idx)
            },
        }
    }

    /// Get the RGB color from a cell's background color.
    #[inline]
    fn compute_bg_rgb(content: &mut RenderableContent<'_>, bg: Color) -> Rgb {
        match bg {
            Color::Spec(rgb) => rgb,
            Color::Named(ansi) => content.color(ansi as usize),
            Color::Indexed(idx) => content.color(idx as usize),
        }
    }

    /// Compute background alpha based on cell's original color.
    ///
    /// Since an RGB color matching the background should not be transparent, this is computed
    /// using the named input color, rather than checking the RGB of the background after its color
    /// is computed.
    #[inline]
    fn compute_bg_alpha(config: &UiConfig, bg: Color) -> f32 {
        if bg == Color::Named(NamedColor::Background) {
            0.
        } else if config.colors.transparent_background_colors {
            1.
        } else {
            config.window_opacity()
        }
    }
}

/// Cursor storing all information relevant for rendering.
#[derive(Debug, Eq, PartialEq, Copy, Clone)]
pub struct RenderableCursor {
    shape: CursorShape,
    cursor_color: Rgb,
    text_color: Rgb,
    is_wide: bool,
    point: Point<usize>,
}

impl RenderableCursor {
    pub fn color(&self) -> Rgb {
        self.cursor_color
    }

    pub fn shape(&self) -> CursorShape {
        self.shape
    }


Review comment (bug):
This is wrong, isn't it? Looks like you just changed the name from `opaque` to `transparent` but you didn't invert the behavior.

Code after:
                content.color(idx)
            },
        }
    }

    /// Get the RGB color from a cell's background color.
    #[inline]
    fn compute_bg_rgb(content: &mut RenderableContent<'_>, bg: Color) -> Rgb {
        match bg {
            Color::Spec(rgb) => rgb,
            Color::Named(ansi) => content.color(ansi as usize),
            Color::Indexed(idx) => content.color(idx as usize),
        }
    }

    /// Compute background alpha based on cell's original color.
    ///
    /// Since an RGB color matching the background should not be transparent, this is computed
    /// using the named input color, rather than checking the RGB of the background after its color
    /// is computed.
    #[inline]
    fn compute_bg_alpha(config: &UiConfig, bg: Color) -> f32 {
        if bg == Color::Named(NamedColor::Background) {
            0.
        } else if config.colors.transparent_background_colors {
            config.window_opacity()
        } else {
            1.
        }
    }
}

/// Cursor storing all information relevant for rendering.
#[derive(Debug, Eq, PartialEq, Copy, Clone)]
pub struct RenderableCursor {
    shape: CursorShape,
    cursor_color: Rgb,
    text_color: Rgb,
    is_wide: bool,
    point: Point<usize>,
}

impl RenderableCursor {
    pub fn color(&self) -> Rgb {
        self.cursor_color
    }

    pub fn shape(&self) -> CursorShape {
        self.shape
    }
Repository: alacritty/alacritty
File: alacritty/src/config/bindings.rs (Rust)

Code before:
pub enum Action {
    /// Write an escape sequence.
    #[config(skip)]
    Esc(String),

    /// Run given command.
    #[config(skip)]
    Command(Program),

    /// Regex keyboard hints.
    #[config(skip)]
    Hint(Hint),

    /// Move vi mode cursor.
    #[config(skip)]
    ViMotion(ViMotion),

    /// Perform vi mode action.
    #[config(skip)]
    Vi(ViAction),

    /// Perform search mode action.
    #[config(skip)]
    Search(SearchAction),

    /// Perform mouse action.
    #[config(skip)]
    Mouse(MouseAction),

    /// Paste contents of system clipboard.
    Paste,

    /// Store current selection into clipboard.
    Copy,

    #[cfg(not(any(target_os = "macos", windows)))]
    /// Store current selection into selection buffer.
    CopySelection,

    /// Paste contents of selection buffer.
    PasteSelection,

    /// Increase font size.
    IncreaseFontSize,

    /// Decrease font size.
    DecreaseFontSize,

    /// Reset font size to the config value.
    ResetFontSize,


Review comment (suggestion):
```suggestion
    /// Perform mouse binding exclusive action.
```

Code after:
pub enum Action {
    /// Write an escape sequence.
    #[config(skip)]
    Esc(String),

    /// Run given command.
    #[config(skip)]
    Command(Program),

    /// Regex keyboard hints.
    #[config(skip)]
    Hint(Hint),

    /// Move vi mode cursor.
    #[config(skip)]
    ViMotion(ViMotion),

    /// Perform vi mode action.
    #[config(skip)]
    Vi(ViAction),

    /// Perform search mode action.
    #[config(skip)]
    Search(SearchAction),

    /// Perform mouse binding exclusive action.
    #[config(skip)]
    Mouse(MouseAction),

    /// Paste contents of system clipboard.
    Paste,

    /// Store current selection into clipboard.
    Copy,

    #[cfg(not(any(target_os = "macos", windows)))]
    /// Store current selection into selection buffer.
    CopySelection,

    /// Paste contents of selection buffer.
    PasteSelection,

    /// Increase font size.
    IncreaseFontSize,

    /// Decrease font size.
    DecreaseFontSize,

    /// Reset font size to the config value.
    ResetFontSize,
Repository: alacritty/alacritty
File: alacritty/src/config/bindings.rs (Rust)

Code before:
    Open,
}

/// Search mode specific actions.
#[allow(clippy::enum_variant_names)]
#[derive(ConfigDeserialize, Debug, Copy, Clone, PartialEq, Eq)]
pub enum SearchAction {
    /// Move the focus to the next search match.
    SearchFocusNext,
    /// Move the focus to the previous search match.
    SearchFocusPrevious,
    /// Confirm the active search.
    SearchConfirm,
    /// Cancel the active search.
    SearchCancel,
    /// Reset the search regex.
    SearchClear,
    /// Delete the last word in the search regex.
    SearchDeleteWord,
    /// Go to the previous regex in the search history.
    SearchHistoryPrevious,
    /// Go to the next regex in the search history.
    SearchHistoryNext,
}

/// Mouse specific actions.
#[allow(clippy::enum_variant_names)]
#[derive(ConfigDeserialize, Debug, Copy, Clone, PartialEq, Eq)]
pub enum MouseAction {
    /// Expand the selection to the current mouse position.
    ExpandSelection,
}

macro_rules! bindings {
    (
        KeyBinding;
        $(
            $key:ident
            $(,$mods:expr)*
            $(,+$mode:expr)*
            $(,~$notmode:expr)*
            ;$action:expr
        );*
        $(;)*
    ) => {{
        bindings!(
            KeyBinding;
            $(
                Key::Keycode($key)
                $(,$mods)*
                $(,+$mode)*

Review comment (suggestion):
```suggestion
/// Mouse binding specific actions.
```

Code after:
    Open,
}

/// Search mode specific actions.
#[allow(clippy::enum_variant_names)]
#[derive(ConfigDeserialize, Debug, Copy, Clone, PartialEq, Eq)]
pub enum SearchAction {
    /// Move the focus to the next search match.
    SearchFocusNext,
    /// Move the focus to the previous search match.
    SearchFocusPrevious,
    /// Confirm the active search.
    SearchConfirm,
    /// Cancel the active search.
    SearchCancel,
    /// Reset the search regex.
    SearchClear,
    /// Delete the last word in the search regex.
    SearchDeleteWord,
    /// Go to the previous regex in the search history.
    SearchHistoryPrevious,
    /// Go to the next regex in the search history.
    SearchHistoryNext,
}

/// Mouse binding specific actions.
#[derive(ConfigDeserialize, Debug, Copy, Clone, PartialEq, Eq)]
pub enum MouseAction {
    /// Expand the selection to the current mouse cursor position.
    ExpandSelection,
}

macro_rules! bindings {
    (
        KeyBinding;
        $(
            $key:ident
            $(,$mods:expr)*
            $(,+$mode:expr)*
            $(,~$notmode:expr)*
            ;$action:expr
        );*
        $(;)*
    ) => {{
        bindings!(
            KeyBinding;
            $(
                Key::Keycode($key)
                $(,$mods)*
                $(,+$mode)*
                $(,~$notmode)*