ronantakizawa/codereview-bench huggingface dataset:
Repository: httpie/httpie
File: httpie/uploads.py (Python)

Code before:

CallbackT = Callable[[bytes], bytes]


def _wrap_function_with_callback(
    func: Callable[..., Any],
    callback: CallbackT
) -> Callable[..., Any]:
    @functools.wraps(func)
    def wrapped(*args, **kwargs):
        chunk = func(*args, **kwargs)
        callback(chunk)
        return chunk
    return wrapped


def is_stdin(file: IO) -> bool:
    try:
        file_no = file.fileno()
    except Exception:
        return False
    else:
        return file_no == sys.stdin.fileno()


READ_THRESHOLD = float(os.getenv("HTTPIE_STDIN_READ_THRESHOLD", 10.0))


def trigger_stdin_warning_thread(env: Environment, file: IO) -> None:
    # Windows unfortunately does not support select() operation
    # on regular files, like stdin in our use case.
    # https://docs.python.org/3/library/select.html#select.select
    if os.name == 'nt':
        return None

    import select
    import threading

    def worker():
        can_read, _, _ = select.select([file], [], [], READ_THRESHOLD)
        if not can_read:
            env.stderr.write(
                f"> no stdin data read in {READ_THRESHOLD}s "
                f"(perhaps you want to --ignore-stdin)\n"
            )
            env.stderr.write("> https://httpie.io/docs/cli/best-practices\n")

    thread = threading.Thread(
        target=worker
    )
    thread.start()

Review comment (suggestion):
I wonder: Should make this configurable (`--xxx=60`) & disableable  (`--xxx=0`)?

Let’s also describe this in the docs.

 Also, single quotes (many places).

Code after:


CallbackT = Callable[[bytes], bytes]


def _wrap_function_with_callback(
    func: Callable[..., Any],
    callback: CallbackT
) -> Callable[..., Any]:
    @functools.wraps(func)
    def wrapped(*args, **kwargs):
        chunk = func(*args, **kwargs)
        callback(chunk)
        return chunk
    return wrapped


def is_stdin(file: IO) -> bool:
    try:
        file_no = file.fileno()
    except Exception:
        return False
    else:
        return file_no == sys.stdin.fileno()


READ_THRESHOLD = float(os.getenv("HTTPIE_STDIN_READ_WARN_THRESHOLD", 10.0))


def observe_stdin_for_data_thread(env: Environment, file: IO) -> None:
    # Windows unfortunately does not support select() operation
    # on regular files, like stdin in our use case.
    # https://docs.python.org/3/library/select.html#select.select
    if is_windows:
        return None

    # If the user configures READ_THRESHOLD to be 0, then
    # disable this warning.
    if READ_THRESHOLD == 0:
        return None

    import select
    import threading

    def worker():
        can_read, _, _ = select.select([file], [], [], READ_THRESHOLD)
        if not can_read:
            env.stderr.write(
                f'> warning: no stdin data read in {READ_THRESHOLD}s '
                f'(perhaps you want to --ignore-stdin)\n'
                f'> See: https://httpie.io/docs/cli/best-practices\n'
Repository: httpie/httpie
File: tests/test_cli_ui.py (Python)

Code before:
    http [METHOD] URL {REQUEST_ITEM_MSG}

error:
    the following arguments are required: URL

For more information try with --help or visit the https://pie.co/docs
"""

NAKED_HELP_MESSAGE_PRETTY_WITH_NO_ARG = f"""\
usage:
    http [--pretty {{all,colors,format,none}}] [METHOD] URL {REQUEST_ITEM_MSG}

error:
    argument --pretty: expected one argument

For more information try with --help or visit the https://pie.co/docs
"""

NAKED_HELP_MESSAGE_PRETTY_WITH_INVALID_ARG = f"""\
usage:
    http [--pretty {{all,colors,format,none}}] [METHOD] URL {REQUEST_ITEM_MSG}

error:
    argument --pretty: invalid choice: '$invalid' (choose from 'all', 'colors', 'format', 'none')

For more information try with --help or visit the https://pie.co/docs
"""


PREDEFINED_TERMINAL_SIZE = (160, 80)


@pytest.fixture(scope="function")
def ignore_terminal_size(monkeypatch):
    """Some tests wrap/crop the output depending on the
    size of the executed terminal, which might not be consistent
    through all runs.

    This fixture ensures every run uses the same exact configuration.
    """

    def fake_terminal_size(*args, **kwargs):
        return os.terminal_size(PREDEFINED_TERMINAL_SIZE)

    # Setting COLUMNS as an env var is required for 3.8<
    monkeypatch.setitem(os.environ, 'COLUMNS', str(PREDEFINED_TERMINAL_SIZE[0]))
    monkeypatch.setattr(shutil, 'get_terminal_size', fake_terminal_size)


@pytest.mark.parametrize(
    'args, expected_msg', [

Review comment (suggestion):
```suggestion
For more information try with --help or visit the https://pie.co/docs/cli
```

Code after:
    http [METHOD] URL {REQUEST_ITEM_MSG}

error:
    the following arguments are required: URL

for more information:
    run 'http --help' or visit https://httpie.io/docs/cli

"""

NAKED_HELP_MESSAGE_PRETTY_WITH_NO_ARG = f"""\
usage:
    http [--pretty {{all,colors,format,none}}] [METHOD] URL {REQUEST_ITEM_MSG}

error:
    argument --pretty: expected one argument

for more information:
    run 'http --help' or visit https://httpie.io/docs/cli

"""

NAKED_HELP_MESSAGE_PRETTY_WITH_INVALID_ARG = f"""\
usage:
    http [--pretty {{all,colors,format,none}}] [METHOD] URL {REQUEST_ITEM_MSG}

error:
    argument --pretty: invalid choice: '$invalid' (choose from 'all', 'colors', 'format', 'none')

for more information:
    run 'http --help' or visit https://httpie.io/docs/cli

"""


PREDEFINED_TERMINAL_SIZE = (160, 80)


@pytest.fixture(scope="function")
def ignore_terminal_size(monkeypatch):
    """Some tests wrap/crop the output depending on the
    size of the executed terminal, which might not be consistent
    through all runs.

    This fixture ensures every run uses the same exact configuration.
    """

    def fake_terminal_size(*args, **kwargs):
        return os.terminal_size(PREDEFINED_TERMINAL_SIZE)

    # Setting COLUMNS as an env var is required for 3.8<
Repository: httpie/httpie
File: httpie/cli/argparser.py (Python)

Code before:
            self.args.download_resume = False
            return
        if not self.args.download:
            if self.args.download_resume:
                self.error('--continue only works with --download')
        if self.args.download_resume and not (
                self.args.download and self.args.output_file):
            self.error('--continue requires --output to be specified')

    def _process_format_options(self):
        format_options = self.args.format_options or []
        parsed_options = PARSED_DEFAULT_FORMAT_OPTIONS
        for options_group in format_options:
            parsed_options = parse_format_options(options_group, defaults=parsed_options)
        self.args.format_options = parsed_options

    def error(self, message):
        """Prints a usage message incorporating the message to stderr and
        exits."""
        self.print_usage(sys.stderr)
        args = {'prog': self.prog, 'message': message}
        self.exit(
            2,
            '\nerror:\n    %(message)s\n\n'
            'For more information try with --help '
            'or visit the https://pie.co/docs' % args
        )


Review comment (suggestion):
```suggestion
            'or visit the https://pie.co/docs/cli' % args
```

Code after:
        if self.args.offline:
            self.args.download = False
            self.args.download_resume = False
            return
        if not self.args.download:
            if self.args.download_resume:
                self.error('--continue only works with --download')
        if self.args.download_resume and not (
                self.args.download and self.args.output_file):
            self.error('--continue requires --output to be specified')

    def _process_format_options(self):
        format_options = self.args.format_options or []
        parsed_options = PARSED_DEFAULT_FORMAT_OPTIONS
        for options_group in format_options:
            parsed_options = parse_format_options(options_group, defaults=parsed_options)
        self.args.format_options = parsed_options

    def error(self, message):
        """Prints a usage message incorporating the message to stderr and
        exits."""
        self.print_usage(sys.stderr)
        self.exit(
            2,
            dedent(
                f'''
                error:
                    {message}

                for more information:
                    run '{self.prog} --help' or visit https://httpie.io/docs/cli
                '''
            )
        )
Repository: httpie/httpie
File: httpie/uploads.py (Python)

Code before:

CallbackT = Callable[[bytes], bytes]


def _wrap_function_with_callback(
    func: Callable[..., Any],
    callback: CallbackT
) -> Callable[..., Any]:
    @functools.wraps(func)
    def wrapped(*args, **kwargs):
        chunk = func(*args, **kwargs)
        callback(chunk)
        return chunk
    return wrapped


def is_stdin(file: IO) -> bool:
    try:
        file_no = file.fileno()
    except Exception:
        return False
    else:
        return file_no == sys.stdin.fileno()


READ_THRESHOLD = float(os.getenv("HTTPIE_STDIN_READ_WARN_THRESHOLD", 10.0))


def observe_stdin_for_data_thread(env: Environment, file: IO, read_event: threading.Event) -> None:
    # Windows unfortunately does not support select() operation
    # on regular files, like stdin in our use case.
    # https://docs.python.org/3/library/select.html#select.select
    if is_windows:
        return None

    # If the user configures READ_THRESHOLD to be 0, then
    # disable this warning.
    if READ_THRESHOLD == 0:
        return None

    def worker(event: threading.Event) -> None:
        time.sleep(READ_THRESHOLD)
        if not event.is_set():
            env.stderr.write(
                f'> warning: no stdin data read in {READ_THRESHOLD}s '
                f'(perhaps you want to --ignore-stdin)\n'
                f'> See: https://httpie.io/docs/cli/best-practices\n'
            )

    thread = threading.Thread(
        target=worker,

Review comment (suggestion):
```suggestion
READ_THRESHOLD = float(os.getenv('HTTPIE_STDIN_READ_WARN_THRESHOLD', 10.0))
```

Code after:

CallbackT = Callable[[bytes], bytes]


def _wrap_function_with_callback(
    func: Callable[..., Any],
    callback: CallbackT
) -> Callable[..., Any]:
    @functools.wraps(func)
    def wrapped(*args, **kwargs):
        chunk = func(*args, **kwargs)
        callback(chunk)
        return chunk
    return wrapped


def is_stdin(file: IO) -> bool:
    try:
        file_no = file.fileno()
    except Exception:
        return False
    else:
        return file_no == sys.stdin.fileno()


READ_THRESHOLD = float(os.getenv('HTTPIE_STDIN_READ_WARN_THRESHOLD', 10.0))


def observe_stdin_for_data_thread(env: Environment, file: IO, read_event: threading.Event) -> None:
    # Windows unfortunately does not support select() operation
    # on regular files, like stdin in our use case.
    # https://docs.python.org/3/library/select.html#select.select
    if is_windows:
        return None

    # If the user configures READ_THRESHOLD to be 0, then
    # disable this warning.
    if READ_THRESHOLD == 0:
        return None

    def worker(event: threading.Event) -> None:
        time.sleep(READ_THRESHOLD)
        if not event.is_set():
            env.stderr.write(
                f'> warning: no stdin data read in {READ_THRESHOLD}s '
                f'(perhaps you want to --ignore-stdin)\n'
                f'> See: https://httpie.io/docs/cli/best-practices\n'
            )

    thread = threading.Thread(
        target=worker,
Repository: iamkun/dayjs
File: src/index.js (JavaScript)

Code before:
        case 'd':
          return String(this.$W)
        case 'dddd':
          return C.WEEKS[this.$W]
        case 'H':
          return String(this.$H)
        case 'HH':
          return Utils.padStart(this.$H, 2, '0')
        case 'h':
        case 'hh':
          if (this.$H === 0) return 12
          return Utils.padStart(this.$H < 13 ? this.$H : this.$H - 12, match === 'hh' ? 2 : 1, '0')
        case 'a':
          return this.$H < 12 ? 'am' : 'pm'
        case 'A':
          return this.$H < 12 ? 'AM' : 'PM'
        case 'm':
          return String(this.$m)
        case 'mm':
          return Utils.padStart(this.$m, 2, '0')
        case 's':
          return String(this.$s)
        case 'ss':
          return Utils.padStart(this.$s, 2, '0')
        case 'Z':
          return `${this.$zoneStr.slice(0, -2)}:${this.$zoneStr.slice(-2)}`
        default: // 'ZZ'
          return this.$zoneStr
      }
    })
  }

  diff(input, units, float) {
    const unit = Utils.prettyUnit(units)
    const that = input instanceof Dayjs ? input : new Dayjs(input)
    const diff = this - that
    let result = Utils.monthDiff(this, that)
    switch (unit) {
      case C.Y:
        result /= 12
        break
      case C.M:
        break
      case C.Q:
        result /= 3
        break
      case C.W:
        result = diff / C.MILLISECONDS_A_WEEK
        break
      case C.D:
        result = diff / C.MILLISECONDS_A_DAY

Review comment (suggestion):
this.$zoneStr = padZoneStr -> return '+08:00'
and here 
case 'Z':
   return this.$zoneStr
default: 
  return this.$zoneStr.replace(':','')

Code after:
        case 'd':
          return String(this.$W)
        case 'dddd':
          return C.WEEKS[this.$W]
        case 'H':
          return String(this.$H)
        case 'HH':
          return Utils.padStart(this.$H, 2, '0')
        case 'h':
        case 'hh':
          if (this.$H === 0) return 12
          return Utils.padStart(this.$H < 13 ? this.$H : this.$H - 12, match === 'hh' ? 2 : 1, '0')
        case 'a':
          return this.$H < 12 ? 'am' : 'pm'
        case 'A':
          return this.$H < 12 ? 'AM' : 'PM'
        case 'm':
          return String(this.$m)
        case 'mm':
          return Utils.padStart(this.$m, 2, '0')
        case 's':
          return String(this.$s)
        case 'ss':
          return Utils.padStart(this.$s, 2, '0')
        case 'Z':
          return this.$zoneStr
        default: // 'ZZ'
          return this.$zoneStr.replace(':', '')
      }
    })
  }

  diff(input, units, float) {
    const unit = Utils.prettyUnit(units)
    const that = input instanceof Dayjs ? input : new Dayjs(input)
    const diff = this - that
    let result = Utils.monthDiff(this, that)
    switch (unit) {
      case C.Y:
        result /= 12
        break
      case C.M:
        break
      case C.Q:
        result /= 3
        break
      case C.W:
        result = diff / C.MILLISECONDS_A_WEEK
        break
      case C.D:
        result = diff / C.MILLISECONDS_A_DAY
Repository: iamkun/dayjs
File: src/plugin/buddhistEra/index.js (JavaScript)

Code before:
import { FORMAT_DEFAULT } from '../../constant'

export default (o, c) => { // locale needed later
  const proto = c.prototype
  const oldFormat = proto.format
  // extend en locale here
  proto.format = function (formatStr, localeObject) {
    const locale = localeObject || this.$locale()
    const str = formatStr || FORMAT_DEFAULT
    const result = str.replace(/BBBB|BB/g, (match) => {
      switch (match) {
        case 'BB':
          return (this.$y + 543) % 100
        default: // BBBB
          return this.$y + 543
      }
    })
    return oldFormat.bind(this)(result, locale)
  }
}


Review comment (suggestion):
You could add 543 as a variable so we avoid the repetition and the 'magic number'.
```
const yearBias = 453;
...
return (this.$y + yearBias) % 100
```

Also applicable on the test file.

Code after:
import { FORMAT_DEFAULT } from '../../constant'

export default (o, c) => { // locale needed later
  const proto = c.prototype
  const oldFormat = proto.format
  // extend en locale here
  proto.format = function (formatStr, localeObject) {
    const yearBias = 543
    const utils = this.$utils()
    const locale = localeObject || this.$locale()
    const str = formatStr || FORMAT_DEFAULT
    const result = str.replace(/BBBB|BB/g, (match) => {
      switch (match) {
        case 'BB':
          return utils.padStart(String(this.$y + yearBias).slice(-2), 2, '0')
        default: // BBBB
          return utils.padStart(String(this.$y + yearBias), 4, '0')
      }
    })
    return oldFormat.bind(this)(result, locale)
  }
}
Repository: iamkun/dayjs
File: src/plugin/buddhistEra/index.js (JavaScript)

Code before:
import { FORMAT_DEFAULT } from '../../constant'

export default (o, c) => { // locale needed later
  const proto = c.prototype
  const oldFormat = proto.format
  // extend en locale here
  proto.format = function (formatStr, localeObject) {
    const yearBias = 543
    const locale = localeObject || this.$locale()
    const str = formatStr || FORMAT_DEFAULT
    const result = str.replace(/BBBB|BB/g, (match) => {
      switch (match) {
        case 'BB':
          return (this.$y + yearBias) % 100
        default: // BBBB
          return this.$y + yearBias
      }
    })
    return oldFormat.bind(this)(result, locale)
  }
}


Review comment (suggestion):
@saknarak we should check and add the leading zero using `padStart`, consider this case `2000 % 100`

Code after:
import { FORMAT_DEFAULT } from '../../constant'

export default (o, c) => { // locale needed later
  const proto = c.prototype
  const oldFormat = proto.format
  // extend en locale here
  proto.format = function (formatStr, localeObject) {
    const yearBias = 543
    const utils = this.$utils()
    const locale = localeObject || this.$locale()
    const str = formatStr || FORMAT_DEFAULT
    const result = str.replace(/BBBB|BB/g, (match) => {
      switch (match) {
        case 'BB':
          return utils.padStart(String(this.$y + yearBias).slice(-2), 2, '0')
        default: // BBBB
          return utils.padStart(String(this.$y + yearBias), 4, '0')
      }
    })
    return oldFormat.bind(this)(result, locale)
  }
}
Repository: iamkun/dayjs
File: src/plugin/buddhistEra/index.js (JavaScript)

Code before:
import { FORMAT_DEFAULT } from '../../constant'

export default (o, c) => { // locale needed later
  const proto = c.prototype
  const oldFormat = proto.format
  // extend en locale here
  proto.format = function (formatStr, localeObject) {
    const yearBias = 543
    const utils = this.$utils()
    const locale = localeObject || this.$locale()
    const str = formatStr || FORMAT_DEFAULT
    const result = str.replace(/BBBB|BB/g, (match) => {
      switch (match) {
        case 'BB':
          return utils.padStart(String((this.$y + yearBias) % 100), 2, '0')
        default: // BBBB
          return utils.padStart(String(this.$y + yearBias), 4, '0')
      }
    })
    return oldFormat.bind(this)(result, locale)
  }
}


Review comment (suggestion):
@saknarak we don't need this in four-digits year right? e.g. year 998 should be 998 not 0998

and also come up with a new idea, `String(this.$y + yearBias).slice(-2)` seems easier to understand. 

Code after:
import { FORMAT_DEFAULT } from '../../constant'

export default (o, c) => { // locale needed later
  const proto = c.prototype
  const oldFormat = proto.format
  // extend en locale here
  proto.format = function (formatStr, localeObject) {
    const yearBias = 543
    const utils = this.$utils()
    const locale = localeObject || this.$locale()
    const str = formatStr || FORMAT_DEFAULT
    const result = str.replace(/BBBB|BB/g, (match) => {
      switch (match) {
        case 'BB':
          return utils.padStart(String(this.$y + yearBias).slice(-2), 2, '0')
        default: // BBBB
          return utils.padStart(String(this.$y + yearBias), 4, '0')
      }
    })
    return oldFormat.bind(this)(result, locale)
  }
}
Repository: iamkun/dayjs
File: src/locale/zh-tw.js (JavaScript)

Code before:
import dayjs from 'dayjs'

const locale = {
  name: 'zh-tw',
  weekdays: '星期日_星期一_星期二_星期三_星期四_星期五_星期六'.split('_'),
  weekdaysShort: '周日_周一_周二_周三_周四_周五_周六'.split('_'),
  weekdaysMin: '日_一_二_三_四_五_六'.split('_'),
  months: '一月_二月_三月_四月_五月_六月_七月_八月_九月_十月_十一月_十二月'.split('_'),
  monthsShort: '1月_2月_3月_4月_5月_6月_7月_8月_9月_10月_11月_12月'.split('_'),
  ordinal: n => n,
  relativeTime: {
    future: '%s內',
    past: '%s前',
    s: '幾秒',
    m: '1 分鐘',
    mm: '%d 分鐘',
    h: '1 小時',
    hh: '%d 小時',
    d: '1 天',
    dd: '%d 天',
    M: '1 個月',
    MM: '%d 個月',
    y: '1 年',
    yy: '%d 年'
  }
}

dayjs.locale(locale, null, true)

export default locale


Review comment (suggestion):
Not sure if this should be `週日_週一_週二_週三_週四_週五_週六` ? I founded in wikipedia.

Code after:
import dayjs from 'dayjs'

const locale = {
  name: 'zh-tw',
  weekdays: '星期日_星期一_星期二_星期三_星期四_星期五_星期六'.split('_'),
  weekdaysShort: '週日_週一_週二_週三_週四_週五_週六'.split('_'),
  weekdaysMin: '日_一_二_三_四_五_六'.split('_'),
  months: '一月_二月_三月_四月_五月_六月_七月_八月_九月_十月_十一月_十二月'.split('_'),
  monthsShort: '1月_2月_3月_4月_5月_6月_7月_8月_9月_10月_11月_12月'.split('_'),
  ordinal: n => n,
  relativeTime: {
    future: '%s內',
    past: '%s前',
    s: '幾秒',
    m: '1 分鐘',
    mm: '%d 分鐘',
    h: '1 小時',
    hh: '%d 小時',
    d: '1 天',
    dd: '%d 天',
    M: '1 個月',
    MM: '%d 個月',
    y: '1 年',
    yy: '%d 年'
  }
}

dayjs.locale(locale, null, true)

export default locale
Repository: iamkun/dayjs
File: src/plugin/isBetween/index.js (JavaScript)

Code before:
export default (o, c, d) => {
  const proto = c.prototype
  proto.isBetween = function (a, b) {
    const dayA = d(a)
    const dayB = d(b)

    if (this.isAfter(dayA) && this.isBefore(dayB)) {
      return true
    } else if (this.isBefore(dayA) && this.isAfter(dayB)) {
      return true
    }

    return false
  }
}



Review comment (suggestion):
```
return (isAfter && isBefore) ||   (isBefore && isAfter)
```

it's easier.

Code after:
export default (o, c, d) => {
  const proto = c.prototype
  proto.isBetween = function (a, b) {
    const dA = d(a)
    const dB = d(b)

    return (this.isAfter(dA) && this.isBefore(dB)) || (this.isBefore(dA) && this.isAfter(dB))
  }
}
Repository: iamkun/dayjs
File: src/plugin/weekOfYear/index.js (JavaScript)

Code before:
import { MS, Y, D, W } from '../../constant'

export default (o, c, d) => {
  const proto = c.prototype
  proto.week = function (week = 0) {
    if (week > 0) {
      // eslint-disable-next-line prefer-template
      if (week > 52) throw new Error('In one year 52 weeks, the resulting value: ' + week)
      return this.add((week - this.week()) * 7, 'day')
    }
    const endOfYear = this.endOf(Y)
    if (endOfYear.day() !== 6 && this.month() === 11 && (31 - this.date()) <= endOfYear.day()) {
      return 1
    }
    const startOfYear = d(this.$d).startOf(Y)
    const compareDay = startOfYear.subtract(startOfYear.day(), D).subtract(1, MS)
    const diffInWeek = this.diff(compareDay, W, true)
    return Math.ceil(diffInWeek)
  }
  proto.weeks = function (week = 0) {
    return this.week(week)
  }
}


Review comment (suggestion):
Instead of checking if `week` is positive, you should check if it's undefined.

Momentjs allows set a negative week

``` javascript
console.log(moment('2018-01-20').week(-1).format()); // "2017-12-23T00:00:00+01:00"
```

So I think that we should as well

Code after:
import { MS, Y, D, W } from '../../constant'

export default (o, c, d) => {
  const proto = c.prototype
  proto.week = function (week = null) {
    if (week !== null) {
      return this.add((week - this.week()) * 7, 'day')
    }
    const endOfYear = this.endOf(Y)
    if (endOfYear.day() !== 6 && this.month() === 11 && (31 - this.date()) <= endOfYear.day()) {
      return 1
    }
    const startOfYear = d(this.$d).startOf(Y)
    const compareDay = startOfYear.subtract(startOfYear.day(), D).subtract(1, MS)
    const diffInWeek = this.diff(compareDay, W, true)
    return Math.ceil(diffInWeek)
  }
  proto.weeks = function (week = null) {
    return this.week(week)
  }
}