ronantakizawa/codereview-bench huggingface dataset:
Repository: sharkdp/fd
File: src/cli.rs (Rust)

Code before:
            .filter(|&m| m > 0)
            .or_else(|| self.max_one_result.then(|| 1))
    }

    #[cfg(feature = "completions")]
    pub fn gen_completions(&self) -> anyhow::Result<Option<Shell>> {
        self.gen_completions
            .map(|maybe_shell| match maybe_shell {
                Some(sh) => Ok(sh),
                None => {
                    Shell::from_env().ok_or_else(|| anyhow!("Unable to get shell from environment"))
                }
            })
            .transpose()
    }
}

#[derive(Copy, Clone, PartialEq, Eq, ValueEnum)]
pub enum FileType {
    #[value(alias = "f")]
    File,
    #[value(alias = "d")]
    Directory,
    #[value(alias = "l")]
    Symlink,
    /// Filter file which are executable by the current effective user
    #[value(alias = "x")]
    Executable,
    #[value(alias = "e")]
    Empty,
    #[value(alias = "s")]
    Socket,
    #[value(alias = "p")]
    Pipe,
}

#[derive(Copy, Clone, PartialEq, Eq, Debug, ValueEnum)]
pub enum ColorWhen {
    /// show colors if the output goes to an interactive console (default)
    Auto,
    /// always use colorized output
    Always,
    /// do not use colorized output
    Never,
}

impl ColorWhen {
    pub fn as_str(&self) -> &'static str {
        use ColorWhen::*;
        match *self {
            Auto => "auto",

Review comment (suggestion):
This is not the filter itself. Maybe just
```suggestion
    /// A files which is executable by the current effective user
```

Also, what is the current *effective* user? Is that a known term? Or could it simply be
```suggestion
    /// A file which is executable by the current user
```

Code after:
            .filter(|&m| m > 0)
            .or_else(|| self.max_one_result.then(|| 1))
    }

    #[cfg(feature = "completions")]
    pub fn gen_completions(&self) -> anyhow::Result<Option<Shell>> {
        self.gen_completions
            .map(|maybe_shell| match maybe_shell {
                Some(sh) => Ok(sh),
                None => {
                    Shell::from_env().ok_or_else(|| anyhow!("Unable to get shell from environment"))
                }
            })
            .transpose()
    }
}

#[derive(Copy, Clone, PartialEq, Eq, ValueEnum)]
pub enum FileType {
    #[value(alias = "f")]
    File,
    #[value(alias = "d")]
    Directory,
    #[value(alias = "l")]
    Symlink,
    /// A file which is executable by the current effective user
    #[value(alias = "x")]
    Executable,
    #[value(alias = "e")]
    Empty,
    #[value(alias = "s")]
    Socket,
    #[value(alias = "p")]
    Pipe,
}

#[derive(Copy, Clone, PartialEq, Eq, Debug, ValueEnum)]
pub enum ColorWhen {
    /// show colors if the output goes to an interactive console (default)
    Auto,
    /// always use colorized output
    Always,
    /// do not use colorized output
    Never,
}

impl ColorWhen {
    pub fn as_str(&self) -> &'static str {
        use ColorWhen::*;
        match *self {
            Auto => "auto",
Repository: sharkdp/fd
File: src/filesystem.rs (Rust)

Code before:
    env::current_dir().map(|path_buf| path_buf.join(path))
}

pub fn absolute_path(path: &Path) -> io::Result<PathBuf> {
    let path_buf = path_absolute_form(path)?;

    #[cfg(windows)]
    let path_buf = Path::new(
        path_buf
            .as_path()
            .to_string_lossy()
            .trim_start_matches(r"\\?\"),
    )
    .to_path_buf();

    Ok(path_buf)
}

pub fn is_existing_directory(path: &Path) -> bool {
    // Note: we do not use `.exists()` here, as `.` always exists, even if
    // the CWD has been deleted.
    path.is_dir() && (path.file_name().is_some() || path.normalize().is_ok())
}

/// Return true if the path point to a file executable by the current effective user
pub fn is_executable(path: &Path) -> bool {
    path.executable()
}

pub fn is_empty(entry: &dir_entry::DirEntry) -> bool {
    if let Some(file_type) = entry.file_type() {
        if file_type.is_dir() {
            if let Ok(mut entries) = fs::read_dir(entry.path()) {
                entries.next().is_none()
            } else {
                false
            }
        } else if file_type.is_file() {
            entry.metadata().map(|m| m.len() == 0).unwrap_or(false)
        } else {
            false
        }
    } else {
        false
    }
}

#[cfg(any(unix, target_os = "redox"))]
pub fn is_socket(ft: fs::FileType) -> bool {
    ft.is_socket()
}

Review comment (refactor):
This whole helper function is now completely unnecessary. I think we should simply use `.executable()` at the usage site?

Code after:

pub fn absolute_path(path: &Path) -> io::Result<PathBuf> {
    let path_buf = path_absolute_form(path)?;

    #[cfg(windows)]
    let path_buf = Path::new(
        path_buf
            .as_path()
            .to_string_lossy()
            .trim_start_matches(r"\\?\"),
    )
    .to_path_buf();

    Ok(path_buf)
}

pub fn is_existing_directory(path: &Path) -> bool {
    // Note: we do not use `.exists()` here, as `.` always exists, even if
    // the CWD has been deleted.
    path.is_dir() && (path.file_name().is_some() || path.normalize().is_ok())
}

pub fn is_empty(entry: &dir_entry::DirEntry) -> bool {
    if let Some(file_type) = entry.file_type() {
        if file_type.is_dir() {
            if let Ok(mut entries) = fs::read_dir(entry.path()) {
                entries.next().is_none()
            } else {
                false
            }
        } else if file_type.is_file() {
            entry.metadata().map(|m| m.len() == 0).unwrap_or(false)
        } else {
            false
        }
    } else {
        false
    }
}

#[cfg(any(unix, target_os = "redox"))]
pub fn is_socket(ft: fs::FileType) -> bool {
    ft.is_socket()
}

#[cfg(windows)]
pub fn is_socket(_: fs::FileType) -> bool {
    false
}

#[cfg(any(unix, target_os = "redox"))]
Repository: sharkdp/fd
File: tests/tests.rs (Rust)

Code before:
    te.assert_output(&["--no-ignore-vcs", "foo"], "inner/foo");

    te.assert_output(&["--no-ignore", "foo"], "inner/foo");
}

/// Don't require git to respect gitignore (--no-require-git)
#[test]
fn test_respect_ignore_files() {
    let te = TestEnv::new(DEFAULT_DIRS, DEFAULT_FILES);

    // Not in a git repo anymore
    fs::remove_dir(te.test_root().join(".git")).unwrap();

    // don't respect gitignore because we're not in a git repo
    te.assert_output(
        &["foo"],
        "a.foo
        gitignored.foo
        one/b.foo
        one/two/c.foo
        one/two/C.Foo2
        one/two/three/d.foo
        one/two/three/directory_foo/",
    );

    // respect gitignore because we set `--ignore-vcs`
    te.assert_output(
        &["--no-require-git", "foo"],
        "a.foo
        one/b.foo
        one/two/c.foo
        one/two/C.Foo2
        one/two/three/d.foo
        one/two/three/directory_foo/",
    );

    // make sure overriding works
    te.assert_output(
        &["--no-require-git", "--require-git", "foo"],
        "a.foo
        gitignored.foo
        one/b.foo
        one/two/c.foo
        one/two/C.Foo2
        one/two/three/d.foo
        one/two/three/directory_foo/",
    );

    te.assert_output(
        &["--no-require-git", "--no-ignore", "foo"],
        "a.foo

Review comment (suggestion):
```suggestion
    // respect gitignore because we set `--no-require-git`
```

Code after:
    te.assert_output(&["--no-ignore-vcs", "foo"], "inner/foo");

    te.assert_output(&["--no-ignore", "foo"], "inner/foo");
}

/// Don't require git to respect gitignore (--no-require-git)
#[test]
fn test_respect_ignore_files() {
    let te = TestEnv::new(DEFAULT_DIRS, DEFAULT_FILES);

    // Not in a git repo anymore
    fs::remove_dir(te.test_root().join(".git")).unwrap();

    // don't respect gitignore because we're not in a git repo
    te.assert_output(
        &["foo"],
        "a.foo
        gitignored.foo
        one/b.foo
        one/two/c.foo
        one/two/C.Foo2
        one/two/three/d.foo
        one/two/three/directory_foo/",
    );

    // respect gitignore because we set `--no-require-git`
    te.assert_output(
        &["--no-require-git", "foo"],
        "a.foo
        one/b.foo
        one/two/c.foo
        one/two/C.Foo2
        one/two/three/d.foo
        one/two/three/directory_foo/",
    );

    // make sure overriding works
    te.assert_output(
        &["--no-require-git", "--require-git", "foo"],
        "a.foo
        gitignored.foo
        one/b.foo
        one/two/c.foo
        one/two/C.Foo2
        one/two/three/d.foo
        one/two/three/directory_foo/",
    );

    te.assert_output(
        &["--no-require-git", "--no-ignore", "foo"],
        "a.foo
Repository: sharkdp/fd
File: scripts/create-deb.sh (Shell)

Code before:
#!/bin/bash
COPYRIGHT_YEARS="2018 - "$(date "+%Y")
MAINTAINER="David Peter <mail@david-peter.de>"
REPO="https://github.com/sharkdp/fd"
DPKG_STAGING="${CICD_INTERMEDIATES_DIR:-.}/debian-package"
DPKG_DIR="${DPKG_STAGING}/dpkg"
mkdir -p "${DPKG_DIR}"

if [[ -z "$TARGET" ]] then
  TARGET="$(rustc -vV | sed -n 's|host: \(.*\)|\1|p'
fi
case "$TARGET" in
  *-musl*)
    DPKG_BASENAME=fd-musl
    DPKG_CONFLICTS=fd
    ;;
  *)
    DPKG_BASENAME=fd
    DPKG_CONFLICTS=fd-musl
    ;;
esac

if [[ -z "$DPKG_VERSION" ]] then
  DPKG_VERSION=$(cargo metadata --no-deps --format-version 1 | jq -r .packages[0].version)
fi

unset DPKG_ARCH
case "${TARGET}" in
  aarch64-*-linux-*) DPKG_ARCH=arm64 ;;
  arm-*-linux-*hf) DPKG_ARCH=armhf ;;
  i686-*-linux-*) DPKG_ARCH=i686 ;;
  x86_64-*-linux-*) DPKG_ARCH=amd64 ;;
  *) DPKG_ARCH=notset ;;
esac;

Review comment (suggestion):
```suggestion
if [[ -z "$TARGET" ]]; then
```

And similarly below

Code after:
#!/bin/bash
COPYRIGHT_YEARS="2018 - "$(date "+%Y")
MAINTAINER="David Peter <mail@david-peter.de>"
REPO="https://github.com/sharkdp/fd"
DPKG_STAGING="${CICD_INTERMEDIATES_DIR:-.}/debian-package"
DPKG_DIR="${DPKG_STAGING}/dpkg"
mkdir -p "${DPKG_DIR}"

if [[ -z "$TARGET" ]]; then
  TARGET="$(rustc -vV | sed -n 's|host: \(.*\)|\1|p')"
fi

case "$TARGET" in
  *-musl*)
    DPKG_BASENAME=fd-musl
    DPKG_CONFLICTS="fd, fd-find"
    ;;
  *)
    DPKG_BASENAME=fd
    DPKG_CONFLICTS="fd-musl, fd-find"
    ;;
esac

if [[ -z "$DPKG_VERSION" ]]; then
  DPKG_VERSION=$(cargo metadata --no-deps --format-version 1 | jq -r .packages[0].version)
fi

unset DPKG_ARCH
case "${TARGET}" in
  aarch64-*-linux-*) DPKG_ARCH=arm64 ;;
  arm-*-linux-*hf) DPKG_ARCH=armhf ;;
  i686-*-linux-*) DPKG_ARCH=i686 ;;
  x86_64-*-linux-*) DPKG_ARCH=amd64 ;;
  *) DPKG_ARCH=notset ;;
Repository: sharkdp/fd
File: src/filter/time.rs (Rust)

Code before:
            Some(UNIX_EPOCH + Duration::from_secs(timestamp_secs))
        }
    }

    pub fn before(s: &str) -> Option<TimeFilter> {
        TimeFilter::from_str(s).map(TimeFilter::Before)
    }

    pub fn after(s: &str) -> Option<TimeFilter> {
        TimeFilter::from_str(s).map(TimeFilter::After)
    }

    pub fn applies_to(&self, t: &SystemTime) -> bool {
        match self {
            TimeFilter::Before(limit) => t < limit,
            TimeFilter::After(limit) => t > limit,
        }
    }
}

#[cfg(test)]
mod tests {
    use super::*;
    use std::time::Duration;

    fn set_test_time(time: Zoned) -> SystemTime {
        TESTTIME.with_borrow_mut(|t| *t = Some(time.clone()));
        time.into()
    }

    #[test]
    fn is_time_filter_applicable() {
        let local_tz = TimeZone::system();
        let ref_time = set_test_time(
            local_tz
                .to_ambiguous_zoned("2010-10-10 10:10:10".parse::<DateTime>().unwrap())
                .later()
                .unwrap(),
        );

        assert!(TimeFilter::after("1min").unwrap().applies_to(&ref_time));
        assert!(!TimeFilter::before("1min").unwrap().applies_to(&ref_time));

        let t1m_ago = ref_time - Duration::from_secs(60);
        assert!(!TimeFilter::after("30sec").unwrap().applies_to(&t1m_ago));
        assert!(TimeFilter::after("2min").unwrap().applies_to(&t1m_ago));

        assert!(TimeFilter::before("30sec").unwrap().applies_to(&t1m_ago));
        assert!(!TimeFilter::before("2min").unwrap().applies_to(&t1m_ago));

        let t10s_before = "2010-10-10 10:10:00";

Review comment (suggestion):
To avoid leaking state between tests, I think an API like this would be better:

```rust
fn with_test_time(time: Zoned, f: impl FnOnce()) -> SystemTime
```

And have it reset TESTTIME to `None` with a `Drop` handler.

Code after:
            Some(UNIX_EPOCH + Duration::from_secs(timestamp_secs))
        }
    }

    pub fn before(s: &str) -> Option<TimeFilter> {
        TimeFilter::from_str(s).map(TimeFilter::Before)
    }

    pub fn after(s: &str) -> Option<TimeFilter> {
        TimeFilter::from_str(s).map(TimeFilter::After)
    }

    pub fn applies_to(&self, t: &SystemTime) -> bool {
        match self {
            TimeFilter::Before(limit) => t < limit,
            TimeFilter::After(limit) => t > limit,
        }
    }
}

#[cfg(test)]
mod tests {
    use super::*;
    use std::time::Duration;

    struct TestTime(SystemTime);

    impl TestTime {
        fn new(time: Zoned) -> Self {
            TESTTIME.with_borrow_mut(|t| *t = Some(time.clone()));
            TestTime(time.into())
        }

        fn set(&mut self, time: Zoned) {
            TESTTIME.with_borrow_mut(|t| *t = Some(time.clone()));
            self.0 = time.into();
        }

        fn timestamp(&self) -> SystemTime {
            self.0
        }
    }

    impl Drop for TestTime {
        fn drop(&mut self) {
            // Stop using manually set times
            TESTTIME.with_borrow_mut(|t| *t = None);
        }
    }

    #[test]
Repository: sharkdp/fd
File: tests/tests.rs (Rust)

Code before:
                test a.foo
                test b.foo
                test C.Foo2
                test c.foo
                test d.foo
                test directory_foo",
            abs_path = &abs_path
        ),
    );

    te.assert_output(
        &[
            "e1", "--exec", "echo", "{.}", ";", "--exec", "echo", "{/}", ";", "--exec", "echo",
            "{//}", ";", "--exec", "echo", "{/.}",
        ],
        "e1 e2
        e1 e2
        .
        e1 e2",
    );

    // We use printf here because we need to suppress a newline and
    // echo -n is not POSIX-compliant.
    te.assert_output(
        &[
            "foo", "--exec", "printf", "{/}: ", ";", "--exec", "echo", "{//}",
        ],
        "a.foo: .
        b.foo: ./one
        C.Foo2: ./one/two
        c.foo: ./one/two
        d.foo: ./one/two/three
        directory_foo: ./one/two/three",
    );
}

#[test]
fn test_exec_batch() {
    let (te, abs_path) = get_test_env_with_abs_path(DEFAULT_DIRS, DEFAULT_FILES);
    let te = te.normalize_line(true);

    // TODO Test for windows
    if !cfg!(windows) {
        te.assert_output(
            &["--absolute-path", "foo", "--exec-batch", "echo"],
            &format!(
                "{abs_path}/a.foo {abs_path}/one/b.foo {abs_path}/one/two/C.Foo2 {abs_path}/one/two/c.foo {abs_path}/one/two/three/d.foo {abs_path}/one/two/three/directory_foo",
                abs_path = &abs_path
            ),
        );


Review comment (suggestion):
```suggestion
            "foo", "--exec", "printf", "%s", "{/}: ", ";", "--exec", "printf", "%s\\n", "{//}",
```

Code after:
                test a.foo
                test b.foo
                test C.Foo2
                test c.foo
                test d.foo
                test directory_foo",
            abs_path = &abs_path
        ),
    );

    te.assert_output(
        &[
            "e1", "--exec", "echo", "{.}", ";", "--exec", "echo", "{/}", ";", "--exec", "echo",
            "{//}", ";", "--exec", "echo", "{/.}",
        ],
        "e1 e2
        e1 e2
        .
        e1 e2",
    );

    // We use printf here because we need to suppress a newline and
    // echo -n is not POSIX-compliant.
    te.assert_output(
        &[
            "foo", "--exec", "printf", "%s", "{/}: ", ";", "--exec", "printf", "%s\\n", "{//}",
        ],
        "a.foo: .
        b.foo: ./one
        C.Foo2: ./one/two
        c.foo: ./one/two
        d.foo: ./one/two/three
        directory_foo: ./one/two/three",
    );
}

#[test]
fn test_exec_batch() {
    let (te, abs_path) = get_test_env_with_abs_path(DEFAULT_DIRS, DEFAULT_FILES);
    let te = te.normalize_line(true);

    // TODO Test for windows
    if !cfg!(windows) {
        te.assert_output(
            &["--absolute-path", "foo", "--exec-batch", "echo"],
            &format!(
                "{abs_path}/a.foo {abs_path}/one/b.foo {abs_path}/one/two/C.Foo2 {abs_path}/one/two/c.foo {abs_path}/one/two/three/d.foo {abs_path}/one/two/three/directory_foo",
                abs_path = &abs_path
            ),
        );
Repository: shreyashankar/gpt3-sandbox
File: examples/run_analogies_app.py (Python)

Code before:
import os, sys
sys.path.append(os.path.dirname(os.path.dirname(os.path.realpath(__file__))))

from api.demo_web_app import demo_web_app
from api.GPT import GPT, Example
from api.UIConfig import UIConfig

# Construct GPT object and show some examples
gpt = GPT(engine="davinci",
          temperature=0.5,
          max_tokens=100)

gpt.add_example(Example('Neural networks are like', 'genetic algorithms in that both are systems that learn from experience.'))
gpt.add_example(Example('Social media is like', 'a market in that both are systems that coordinate the actions of many individuals.'))
gpt.add_example(Example('A2E is like', 'lipofuscin in that both are byproducts of the normal operation of a system.'))
gpt.add_example(Example('Haskell is like', 'LISP in that both are functional languages.'))
gpt.add_example(Example('Quaternions are like', 'matrices in that both are used to represent rotations in three dimensions.'))
gpt.add_example(Example('Quaternions are like', 'octonions in that both are examples of non-commutative algebra.'))

# Define UI configuration
config = UIConfig(description="Analogies generator",
                  buttonText="Generate",
                  placeholder="Memes are like")

demo_web_app(gpt, config)


Review comment (suggestion):
can you try changing this to 
```
from api import demo_web_app
from api import GPT, Example, UIConfig
```

Code after:
import os, sys
sys.path.append(os.path.dirname(os.path.dirname(os.path.realpath(__file__))))

from api import demo_web_app
from api import GPT, Example, UIConfig

# Construct GPT object and show some examples
gpt = GPT(engine="davinci",
          temperature=0.5,
          max_tokens=100)

gpt.add_example(Example('Neural networks are like', 'genetic algorithms in that both are systems that learn from experience.'))
gpt.add_example(Example('Social media is like', 'a market in that both are systems that coordinate the actions of many individuals.'))
gpt.add_example(Example('A2E is like', 'lipofuscin in that both are byproducts of the normal operation of a system.'))
gpt.add_example(Example('Haskell is like', 'LISP in that both are functional languages.'))
gpt.add_example(Example('Quaternions are like', 'matrices in that both are used to represent rotations in three dimensions.'))
gpt.add_example(Example('Quaternions are like', 'octonions in that both are examples of non-commutative algebra.'))

# Define UI configuration
config = UIConfig(description="Analogies generator",
                  buttonText="Generate",
                  placeholder="Memes are like")

demo_web_app(gpt, config)
Repository: shreyashankar/gpt3-sandbox
File: api/gpt.py (Python)

Code before:
"""Creates the Example and GPT classes for a user to interface with the OpenAI API."""

import openai


def set_openai_key(key):
    """Sets OpenAI key."""
    openai.api_key = key

class Example():
    """Stores an input, output pair and formats it to prime the model."""

    def __init__(self, inp, out, input_prefix = "input: ", input_suffix = "\n", output_prefix = "output: ", output_suffix = "\n"):
        self.input = inp
        self.output = out
        self.input_prefix = input_prefix
        self.input_suffix = input_suffix
        self.output_prefix = output_prefix
        self.output_suffix = output_suffix

    def get_input(self):
        """Returns the input of the example."""
        return self.input

    def get_output(self):
        """Returns the intended output of the example."""
        return self.output

    def format(self):
        """Formats the input, output pair."""
        return self.input_prefix + f"{self.input}" + self.input_suffix + self.output_prefix + f"{self.output}\n"
        + self.output_suffix


class GPT:
    """The main class for a user to interface with the OpenAI API.
    A user can add examples and set parameters of the API request."""


Review comment (suggestion):
To minimize redundancy, I think it might be better to declare the prefixes and suffixes in the `GPT` class, and remove the `format` method from this class entirely. See my comment in the `GPT` class for an idea of how to format the example in there.

Code after:
"""Creates the Example and GPT classes for a user to interface with the OpenAI API."""

import openai


def set_openai_key(key):
    """Sets OpenAI key."""
    openai.api_key = key

class Example():
    """Stores an input, output pair it to prime the model."""

    def __init__(self, inp, out):
        self.input = inp
        self.output = out

    def get_input(self):
        """Returns the input of the example."""
        return self.input

    def get_output(self):
        """Returns the intended output of the example."""
        return self.output

class TextFormat():
    """Formats text by prefixing and suffixing with the provided parameters."""

    def __init__(self, prefix, suffix):
        self.prefix = prefix
        self.suffix = suffix

    def get_prefix(self):
        """Returns the prefix of this format."""
        return self.prefix

    def get_suffix(self):
        """Returns the suffix of this format."""
        return self.suffix