ronantakizawa/codereview-bench huggingface dataset:
Repository: helix-editor/helix
File: helix-stdx/src/env.rs (Rust)

Code before:
use std::{
    ffi::OsStr,
    path::{Path, PathBuf},
    sync::RwLock,
};

static CWD: RwLock<Option<PathBuf>> = RwLock::new(None);

// Get the current working directory.
// This information is managed internally as the call to std::env::current_dir
// might fail if the cwd has been deleted.
pub fn current_working_dir() -> PathBuf {
    if let Some(path) = &*CWD.read().unwrap() {
        return path.clone();
    }

    // implementation of crossplatform pwd -L
    // we want pwd -L so that symlinked directories are handeled correctly
    let mut cwd = std::env::current_dir().expect("Couldn't determine current working directory");

    let pwd = std::env::var_os("PWD");
    #[cfg(windows)]
    let pwd = pwd.or_else(|| std::env::var_os("CD"));

    if let Some(pwd) = pwd.map(PathBuf::from) {
        if pwd.canonicalize().ok().as_ref() == Some(&cwd) {
            cwd = pwd;
        }
    }
    let mut dst = CWD.write().unwrap();
    *dst = Some(cwd.clone());

    cwd
}

pub fn set_current_working_dir(path: impl AsRef<Path>) -> std::io::Result<()> {
    let path = crate::path::canonicalize(path);
    std::env::set_current_dir(&path)?;
    let mut cwd = CWD.write().unwrap();
    *cwd = Some(path);
    Ok(())
}


Review comment (suggestion):
```suggestion
    // we want pwd -L so that symlinked directories are handled correctly
```

Code after:
use std::{
    ffi::OsStr,
    path::{Path, PathBuf},
    sync::RwLock,
};

static CWD: RwLock<Option<PathBuf>> = RwLock::new(None);

// Get the current working directory.
// This information is managed internally as the call to std::env::current_dir
// might fail if the cwd has been deleted.
pub fn current_working_dir() -> PathBuf {
    if let Some(path) = &*CWD.read().unwrap() {
        return path.clone();
    }

    // implementation of crossplatform pwd -L
    // we want pwd -L so that symlinked directories are handled correctly
    let mut cwd = std::env::current_dir().expect("Couldn't determine current working directory");

    let pwd = std::env::var_os("PWD");
    #[cfg(windows)]
    let pwd = pwd.or_else(|| std::env::var_os("CD"));

    if let Some(pwd) = pwd.map(PathBuf::from) {
        if pwd.canonicalize().ok().as_ref() == Some(&cwd) {
            cwd = pwd;
        }
    }
    let mut dst = CWD.write().unwrap();
    *dst = Some(cwd.clone());

    cwd
}

pub fn set_current_working_dir(path: impl AsRef<Path>) -> std::io::Result<()> {
    let path = crate::path::canonicalize(path);
    std::env::set_current_dir(&path)?;
    let mut cwd = CWD.write().unwrap();
    *cwd = Some(path);
    Ok(())
}
Repository: helix-editor/helix
File: helix-tui/src/backend/crossterm.rs (Rust)

Code before:
    },
    terminal::{self, Clear, ClearType},
    Command,
};
use helix_view::{
    editor::Config as EditorConfig,
    graphics::{Color, CursorKind, Modifier, Rect, UnderlineStyle},
};
use once_cell::sync::OnceCell;
use std::{
    fmt,
    io::{self, Write},
};
use termini::TermInfo;

fn term_program() -> Option<String> {
    // Some terminals don't set $TERM_PROGRAM
    match std::env::var("TERM_PROGRAM") {
        Err(_) => std::env::var("TERM").ok(),
        Ok(term_program) => Some(term_program),
    }
}
fn vte_version() -> Option<usize> {
    std::env::var("VTE_VERSION").ok()?.parse().ok()
}
fn reset_cursor_approach(terminfo: TermInfo, fallback: &str) -> String {
    let mut reset_str: String = Default::default();
    if let termini::Value::Utf8String(reset_str_se) = terminfo
        .extended_cap("Se")
        .unwrap_or(termini::Value::Utf8String(fallback))
    {
        reset_str.push_str(reset_str_se);
    }

    reset_str.push_str(
        terminfo
            .utf8_string_cap(termini::StringCapability::CursorNormal)
            .unwrap(),
    );

    match term_program().as_deref() {
        // some terminals don't reset the cursor with either cnorm or Se
        Some("tmux") => {
            reset_str.push_str(fallback);
            reset_str
        }
        _ => reset_str,
    }
}

/// Describes terminal capabilities like extended underline, truecolor, etc.

Review comment (security):
```suggestion
fn reset_cursor_escape_sequence(terminfo: TermInfo) -> String {
```
 Passing the fallback as an argument is a bit qeord/unnecessary just put it into the function 

Code after:
    },
    terminal::{self, Clear, ClearType},
    Command,
};
use helix_view::{
    editor::Config as EditorConfig,
    graphics::{Color, CursorKind, Modifier, Rect, UnderlineStyle},
};
use once_cell::sync::OnceCell;
use std::{
    fmt,
    io::{self, Write},
};
use termini::TermInfo;

fn term_program() -> Option<String> {
    // Some terminals don't set $TERM_PROGRAM
    match std::env::var("TERM_PROGRAM") {
        Err(_) => std::env::var("TERM").ok(),
        Ok(term_program) => Some(term_program),
    }
}
fn vte_version() -> Option<usize> {
    std::env::var("VTE_VERSION").ok()?.parse().ok()
}
fn reset_cursor_approach(terminfo: TermInfo) -> String {
    let mut reset_str = "\x1B[0 q".to_string();

    if let Some(termini::Value::Utf8String(se_str)) = terminfo.extended_cap("Se") {
        reset_str.push_str(se_str);
    };

    reset_str.push_str(
        terminfo
            .utf8_string_cap(termini::StringCapability::CursorNormal)
            .unwrap_or(""),
    );

    reset_str
}

/// Describes terminal capabilities like extended underline, truecolor, etc.
#[derive(Clone, Debug)]
struct Capabilities {
    /// Support for undercurled, underdashed, etc.
    has_extended_underlines: bool,
    /// Support for resetting the cursor style back to normal.
    reset_cursor_command: String,
}

impl Default for Capabilities {
Repository: helix-editor/helix
File: helix-tui/src/backend/crossterm.rs (Rust)

Code before:
    graphics::{Color, CursorKind, Modifier, Rect, UnderlineStyle},
};
use once_cell::sync::OnceCell;
use std::{
    fmt,
    io::{self, Write},
};
use termini::TermInfo;

fn term_program() -> Option<String> {
    // Some terminals don't set $TERM_PROGRAM
    match std::env::var("TERM_PROGRAM") {
        Err(_) => std::env::var("TERM").ok(),
        Ok(term_program) => Some(term_program),
    }
}
fn vte_version() -> Option<usize> {
    std::env::var("VTE_VERSION").ok()?.parse().ok()
}
fn reset_cursor_approach(terminfo: TermInfo) -> String {
    let Some(termini::Value::Utf8String(se_str)) = terminfo.extended_cap("Se") else {
        return "".to_string();
    };

    format!(
        "{}{:?}{}",
        "\x1B[0 q",
        se_str,
        terminfo
            .utf8_string_cap(termini::StringCapability::CursorNormal)
            .unwrap()
    )
}

/// Describes terminal capabilities like extended underline, truecolor, etc.
#[derive(Clone, Debug)]
struct Capabilities {
    /// Support for undercurled, underdashed, etc.
    has_extended_underlines: bool,
    /// Support for resetting the cursor style back to normal.
    reset_cursor_command: String,
}

impl Default for Capabilities {
    fn default() -> Self {
        Self {
            has_extended_underlines: false,
            reset_cursor_command: "\x1B[0 q".to_string(),
        }
    }
}

Review comment (bug):
`"{:?}"` is wrong here, that is why the cursor resetting was working in tmux for me. I accidentally left the `"{:?}"` while trying to figure out the types because I'm a rust noob xD

Code after:
    graphics::{Color, CursorKind, Modifier, Rect, UnderlineStyle},
};
use once_cell::sync::OnceCell;
use std::{
    fmt,
    io::{self, Write},
};
use termini::TermInfo;

fn term_program() -> Option<String> {
    // Some terminals don't set $TERM_PROGRAM
    match std::env::var("TERM_PROGRAM") {
        Err(_) => std::env::var("TERM").ok(),
        Ok(term_program) => Some(term_program),
    }
}
fn vte_version() -> Option<usize> {
    std::env::var("VTE_VERSION").ok()?.parse().ok()
}
fn reset_cursor_approach(terminfo: TermInfo) -> String {
    let mut reset_str = "\x1B[0 q".to_string();

    if let Some(termini::Value::Utf8String(se_str)) = terminfo.extended_cap("Se") {
        reset_str.push_str(se_str);
    };

    reset_str.push_str(
        terminfo
            .utf8_string_cap(termini::StringCapability::CursorNormal)
            .unwrap_or(""),
    );

    reset_str
}

/// Describes terminal capabilities like extended underline, truecolor, etc.
#[derive(Clone, Debug)]
struct Capabilities {
    /// Support for undercurled, underdashed, etc.
    has_extended_underlines: bool,
    /// Support for resetting the cursor style back to normal.
    reset_cursor_command: String,
}

impl Default for Capabilities {
    fn default() -> Self {
        Self {
            has_extended_underlines: false,
            reset_cursor_command: "\x1B[0 q".to_string(),
        }
    }
Repository: helix-editor/helix
File: helix-tui/src/backend/crossterm.rs (Rust)

Code before:
    editor::Config as EditorConfig,
    graphics::{Color, CursorKind, Modifier, Rect, UnderlineStyle},
};
use once_cell::sync::OnceCell;
use std::{
    fmt,
    io::{self, Write},
};
use termini::TermInfo;

fn term_program() -> Option<String> {
    // Some terminals don't set $TERM_PROGRAM
    match std::env::var("TERM_PROGRAM") {
        Err(_) => std::env::var("TERM").ok(),
        Ok(term_program) => Some(term_program),
    }
}
fn vte_version() -> Option<usize> {
    std::env::var("VTE_VERSION").ok()?.parse().ok()
}
fn reset_cursor_approach(terminfo: TermInfo) -> String {
    let Some(termini::Value::Utf8String(se_str)) = terminfo.extended_cap("Se") else {
        return "".to_string();
    };

    format!(
        "{}{}{}",
        "\x1B[0 q",
        se_str,
        terminfo
            .utf8_string_cap(termini::StringCapability::CursorNormal)
            .unwrap()
    )
}

/// Describes terminal capabilities like extended underline, truecolor, etc.
#[derive(Clone, Debug)]
struct Capabilities {
    /// Support for undercurled, underdashed, etc.
    has_extended_underlines: bool,
    /// Support for resetting the cursor style back to normal.
    reset_cursor_command: String,
}

impl Default for Capabilities {
    fn default() -> Self {
        Self {
            has_extended_underlines: false,
            reset_cursor_command: "\x1B[0 q".to_string(),
        }
    }

Review comment (suggestion):
If there's no `Se` capability we don't end up emitting the fallback? I thought the intent was to always use the fallback.

Code after:
    editor::Config as EditorConfig,
    graphics::{Color, CursorKind, Modifier, Rect, UnderlineStyle},
};
use once_cell::sync::OnceCell;
use std::{
    fmt,
    io::{self, Write},
};
use termini::TermInfo;

fn term_program() -> Option<String> {
    // Some terminals don't set $TERM_PROGRAM
    match std::env::var("TERM_PROGRAM") {
        Err(_) => std::env::var("TERM").ok(),
        Ok(term_program) => Some(term_program),
    }
}
fn vte_version() -> Option<usize> {
    std::env::var("VTE_VERSION").ok()?.parse().ok()
}
fn reset_cursor_approach(terminfo: TermInfo) -> String {
    let mut reset_str = "\x1B[0 q".to_string();

    if let Some(termini::Value::Utf8String(se_str)) = terminfo.extended_cap("Se") {
        reset_str.push_str(se_str);
    };

    reset_str.push_str(
        terminfo
            .utf8_string_cap(termini::StringCapability::CursorNormal)
            .unwrap_or(""),
    );

    reset_str
}

/// Describes terminal capabilities like extended underline, truecolor, etc.
#[derive(Clone, Debug)]
struct Capabilities {
    /// Support for undercurled, underdashed, etc.
    has_extended_underlines: bool,
    /// Support for resetting the cursor style back to normal.
    reset_cursor_command: String,
}

impl Default for Capabilities {
    fn default() -> Self {
        Self {
            has_extended_underlines: false,
            reset_cursor_command: "\x1B[0 q".to_string(),
        }
Repository: helix-editor/helix
File: helix-tui/src/backend/crossterm.rs (Rust)

Code before:
    editor::Config as EditorConfig,
    graphics::{Color, CursorKind, Modifier, Rect, UnderlineStyle},
};
use once_cell::sync::OnceCell;
use std::{
    fmt,
    io::{self, Write},
};
use termini::TermInfo;

fn term_program() -> Option<String> {
    // Some terminals don't set $TERM_PROGRAM
    match std::env::var("TERM_PROGRAM") {
        Err(_) => std::env::var("TERM").ok(),
        Ok(term_program) => Some(term_program),
    }
}
fn vte_version() -> Option<usize> {
    std::env::var("VTE_VERSION").ok()?.parse().ok()
}
fn reset_cursor_approach(terminfo: TermInfo) -> String {
    let mut reset_str = "\x1B[0 q".to_string();

    if let termini::Value::Utf8String(se_str) = terminfo
        .extended_cap("Se")
        .unwrap_or(termini::Value::Utf8String(""))
    {
        reset_str.push_str(se_str);
    };

    reset_str.push_str(
        terminfo
            .utf8_string_cap(termini::StringCapability::CursorNormal)
            .unwrap(),
    );

    return reset_str;
}

/// Describes terminal capabilities like extended underline, truecolor, etc.
#[derive(Clone, Debug)]
struct Capabilities {
    /// Support for undercurled, underdashed, etc.
    has_extended_underlines: bool,
    /// Support for resetting the cursor style back to normal.
    reset_cursor_command: String,
}

impl Default for Capabilities {
    fn default() -> Self {
        Self {

Review comment (suggestion):
```suggestion
    if let Some(termini::Value::Utf8String(se_str)) = terminfo.extended_cap("Se")
```

Code after:
    editor::Config as EditorConfig,
    graphics::{Color, CursorKind, Modifier, Rect, UnderlineStyle},
};
use once_cell::sync::OnceCell;
use std::{
    fmt,
    io::{self, Write},
};
use termini::TermInfo;

fn term_program() -> Option<String> {
    // Some terminals don't set $TERM_PROGRAM
    match std::env::var("TERM_PROGRAM") {
        Err(_) => std::env::var("TERM").ok(),
        Ok(term_program) => Some(term_program),
    }
}
fn vte_version() -> Option<usize> {
    std::env::var("VTE_VERSION").ok()?.parse().ok()
}
fn reset_cursor_approach(terminfo: TermInfo) -> String {
    let mut reset_str = "\x1B[0 q".to_string();

    if let Some(termini::Value::Utf8String(se_str)) = terminfo.extended_cap("Se") {
        reset_str.push_str(se_str);
    };

    reset_str.push_str(
        terminfo
            .utf8_string_cap(termini::StringCapability::CursorNormal)
            .unwrap_or(""),
    );

    reset_str
}

/// Describes terminal capabilities like extended underline, truecolor, etc.
#[derive(Clone, Debug)]
struct Capabilities {
    /// Support for undercurled, underdashed, etc.
    has_extended_underlines: bool,
    /// Support for resetting the cursor style back to normal.
    reset_cursor_command: String,
}

impl Default for Capabilities {
    fn default() -> Self {
        Self {
            has_extended_underlines: false,
            reset_cursor_command: "\x1B[0 q".to_string(),
        }
Repository: helix-editor/helix
File: helix-term/tests/test/commands.rs (Rust)

Code before:
    ))
    .await?;

    test((
        INPUT,
        ":lang rust<ret>2mrm{",
        r#"\
fn foo() {
    if let Some(_) = None {
        todo!{"f#[|o]#o)"};
    }
}
"#,
    ))
    .await?;

    test((
        indoc! {"\
            #[a|]#
            b
            c
            d
            e
            f
            "},
        "4xs\\n<ret>r,",
        "a#[,|]#b#(,|)#c#(,|)#d#(,|)#e\nf\n",
    ))
    .await?;

    Ok(())
}


Review comment (suggestion):
I think it would be clearer here to see what we're intending to test here if we start with the selection on the newlines rather than using `4xs\\n<ret>` to form a selection. I believe you can format it like:

```
a#[
|]#b#[
|]#c#[
```

etc

Code after:
    ))
    .await?;

    test((
        INPUT,
        ":lang rust<ret>2mrm{",
        r#"\
fn foo() {
    if let Some(_) = None {
        todo!{"f#[|o]#o)"};
    }
}
"#,
    ))
    .await?;

    test((
        indoc! {"\
            #[a
            b
            c
            d
            e|]#
            f
            "},
        "s\\n<ret>r,",
        "a#[,|]#b#(,|)#c#(,|)#d#(,|)#e\nf\n",
    ))
    .await?;

    Ok(())
}
Repository: helix-editor/helix
File: helix-term/src/commands.rs (Rust)

Code before:
}

fn shell_prompt(cx: &mut Context, prompt: Cow<'static, str>, behavior: ShellBehavior) {
    ui::prompt(
        cx,
        prompt,
        Some('|'),
        ui::completers::filename,
        move |cx, input: &str, event: PromptEvent| {
            if event != PromptEvent::Validate {
                return;
            }
            if input.is_empty() {
                return;
            }

            shell(cx, input, &behavior);
        },
    );
}

fn suspend(cx: &mut Context) {
    #[cfg(not(windows))]
    cx.block_try_flush_writes();
    signal_hook::low_level::raise(signal_hook::consts::signal::SIGTSTP).unwrap();
}

fn add_newline_above(cx: &mut Context) {
    add_newline_impl(cx, Open::Above);
}

fn add_newline_below(cx: &mut Context) {
    add_newline_impl(cx, Open::Below)
}

fn add_newline_impl(cx: &mut Context, open: Open) {
    let count = cx.count();
    let (view, doc) = current!(cx.editor);
    let selection = doc.selection(view.id);
    let text = doc.text();
    let slice = text.slice(..);

    let changes = selection.into_iter().map(|range| {
        let (start, end) = range.line_range(slice);
        let line = match open {
            Open::Above => start,
            Open::Below => end + 1,
        };
        let pos = text.line_to_char(line);
        (
            pos,

Review comment (suggestion):
```suggestion
fn suspend(cx: &mut Context) {
    #[cfg(not(windows))]
    {
        cx.block_try_flush_writes();
        signal_hook::low_level::raise(signal_hook::consts::signal::SIGTSTP).unwrap();
    }
}
```

Code after:

fn shell_prompt(cx: &mut Context, prompt: Cow<'static, str>, behavior: ShellBehavior) {
    ui::prompt(
        cx,
        prompt,
        Some('|'),
        ui::completers::filename,
        move |cx, input: &str, event: PromptEvent| {
            if event != PromptEvent::Validate {
                return;
            }
            if input.is_empty() {
                return;
            }

            shell(cx, input, &behavior);
        },
    );
}

fn suspend(_cx: &mut Context) {
    #[cfg(not(windows))]
    {
        _cx.block_try_flush_writes().ok();
        signal_hook::low_level::raise(signal_hook::consts::signal::SIGTSTP).unwrap();
    }
}

fn add_newline_above(cx: &mut Context) {
    add_newline_impl(cx, Open::Above);
}

fn add_newline_below(cx: &mut Context) {
    add_newline_impl(cx, Open::Below)
}

fn add_newline_impl(cx: &mut Context, open: Open) {
    let count = cx.count();
    let (view, doc) = current!(cx.editor);
    let selection = doc.selection(view.id);
    let text = doc.text();
    let slice = text.slice(..);

    let changes = selection.into_iter().map(|range| {
        let (start, end) = range.line_range(slice);
        let line = match open {
            Open::Above => start,
            Open::Below => end + 1,
        };
        let pos = text.line_to_char(line);
        (
Repository: helix-editor/helix
File: helix-term/src/commands.rs (Rust)

Code before:

    // after replace cursor may be out of bounds, do this to
    // make sure cursor is in view and update scroll as well
    view.ensure_cursor_in_view(doc, config.scrolloff);
}

fn shell_prompt(cx: &mut Context, prompt: Cow<'static, str>, behavior: ShellBehavior) {
    ui::prompt(
        cx,
        prompt,
        Some('|'),
        ui::completers::filename,
        move |cx, input: &str, event: PromptEvent| {
            if event != PromptEvent::Validate {
                return;
            }
            if input.is_empty() {
                return;
            }

            shell(cx, input, &behavior);
        },
    );
}

fn suspend(cx: &mut Context) {
    #[cfg(not(windows))]
    {
        cx.block_try_flush_writes();
        signal_hook::low_level::raise(signal_hook::consts::signal::SIGTSTP).unwrap();
    }
}

fn add_newline_above(cx: &mut Context) {
    add_newline_impl(cx, Open::Above);
}

fn add_newline_below(cx: &mut Context) {
    add_newline_impl(cx, Open::Below)
}

fn add_newline_impl(cx: &mut Context, open: Open) {
    let count = cx.count();
    let (view, doc) = current!(cx.editor);
    let selection = doc.selection(view.id);
    let text = doc.text();
    let slice = text.slice(..);

    let changes = selection.into_iter().map(|range| {
        let (start, end) = range.line_range(slice);
        let line = match open {

Review comment (suggestion):
As @poliorcetics said, let's keep this as `_cx` and use `_cx.block_try_flush_writes()` below to avoid a compiler warning when compiling for Windows

Code after:
    // after replace cursor may be out of bounds, do this to
    // make sure cursor is in view and update scroll as well
    view.ensure_cursor_in_view(doc, config.scrolloff);
}

fn shell_prompt(cx: &mut Context, prompt: Cow<'static, str>, behavior: ShellBehavior) {
    ui::prompt(
        cx,
        prompt,
        Some('|'),
        ui::completers::filename,
        move |cx, input: &str, event: PromptEvent| {
            if event != PromptEvent::Validate {
                return;
            }
            if input.is_empty() {
                return;
            }

            shell(cx, input, &behavior);
        },
    );
}

fn suspend(_cx: &mut Context) {
    #[cfg(not(windows))]
    {
        _cx.block_try_flush_writes().ok();
        signal_hook::low_level::raise(signal_hook::consts::signal::SIGTSTP).unwrap();
    }
}

fn add_newline_above(cx: &mut Context) {
    add_newline_impl(cx, Open::Above);
}

fn add_newline_below(cx: &mut Context) {
    add_newline_impl(cx, Open::Below)
}

fn add_newline_impl(cx: &mut Context, open: Open) {
    let count = cx.count();
    let (view, doc) = current!(cx.editor);
    let selection = doc.selection(view.id);
    let text = doc.text();
    let slice = text.slice(..);

    let changes = selection.into_iter().map(|range| {
        let (start, end) = range.line_range(slice);
        let line = match open {
            Open::Above => start,