ronantakizawa/codereview-bench huggingface dataset:
Repository: neovim/neovim
File: src/nvim/ui_client.c (C)

Code before:

/// Restarts the embedded server without killing the UI.
void ui_client_event_restart(Array args)
{
  // 1. Client-side server detach.
  ui_client_detach();

  // 2. Close ui client channel (auto kills the `nvim --embed` server due to self-exit).
  const char *error;
  bool success = channel_close(ui_client_channel_id, kChannelPartAll, &error);
  if (!success) {
    ELOG("%s", error);
    return;
  }

  // 3. Get v:argv.
  typval_T *tv = get_vim_var_tv(VV_ARGV);
  if (tv->v_type != VAR_LIST || tv->vval.v_list == NULL) {
    ELOG("failed to get vim var typval");
    return;
  }
  list_T *l = tv->vval.v_list;
  int argc = tv_list_len(l);

  // Assert to be positive for safe conversion to size_t.
  assert(argc >= 0);

  char **argv = xmalloc(sizeof(char *) * ((size_t)argc + 1));
  listitem_T *li = tv_list_first(l);
  for (int i = 0; i < argc && li != NULL; i++, li = TV_LIST_ITEM_NEXT(l, li)) {
    if (TV_LIST_ITEM_TV(li)->v_type == VAR_STRING && TV_LIST_ITEM_TV(li)->vval.v_string != NULL) {
      argv[i] = TV_LIST_ITEM_TV(li)->vval.v_string;
    } else {
      argv[i] = "";
    }
  }
  argv[argc] = NULL;

  // 4. Start a new `nvim --embed` server.
  uint64_t rv = ui_client_start_server(argc, argv);
  if (!rv) {
    ELOG("failed to start nvim server");
    return;
  }

  // 5. Client-side server re-attach.
  ui_client_channel_id = rv;
  ui_client_attach(tui_width, tui_height, tui_term, tui_rgb);

  ILOG("restarted server id=%" PRId64, rv);
}

Review comment (suggestion):
Change this to be `assert(argc > 0)` instead. `argc` can never be zero

Code after:

/// Restarts the embedded server without killing the UI.
void ui_client_event_restart(Array args)
{
  // 1. Client-side server detach.
  ui_client_detach();

  // 2. Close ui client channel (auto kills the `nvim --embed` server due to self-exit).
  const char *error;
  bool success = channel_close(ui_client_channel_id, kChannelPartAll, &error);
  if (!success) {
    ELOG("%s", error);
    return;
  }

  // 3. Get v:argv.
  typval_T *tv = get_vim_var_tv(VV_ARGV);
  if (tv->v_type != VAR_LIST || tv->vval.v_list == NULL) {
    ELOG("failed to get vim var typval");
    return;
  }
  list_T *l = tv->vval.v_list;
  int argc = tv_list_len(l);

  // Assert to be positive for safe conversion to size_t.
  assert(argc > 0);

  char **argv = xmalloc(sizeof(char *) * ((size_t)argc + 1));
  listitem_T *li = tv_list_first(l);
  for (int i = 0; i < argc && li != NULL; i++, li = TV_LIST_ITEM_NEXT(l, li)) {
    if (TV_LIST_ITEM_TV(li)->v_type == VAR_STRING && TV_LIST_ITEM_TV(li)->vval.v_string != NULL) {
      argv[i] = TV_LIST_ITEM_TV(li)->vval.v_string;
    } else {
      argv[i] = "";
    }
  }
  argv[argc] = NULL;

  // 4. Start a new `nvim --embed` server.
  uint64_t rv = ui_client_start_server(argc, argv);
  if (!rv) {
    ELOG("failed to start nvim server");
    goto cleanup;
  }

  // 5. Client-side server re-attach.
  ui_client_channel_id = rv;
  ui_client_attach(tui_width, tui_height, tui_term, tui_rgb);

  ILOG("restarted server id=%" PRId64, rv);
cleanup:
Repository: neovim/neovim
File: src/nvim/ui_client.c (C)

Code before:
    grid_line_buf_char = xmalloc(grid_line_buf_size * sizeof(schar_T));
    grid_line_buf_attr = xmalloc(grid_line_buf_size * sizeof(sattr_T));
  }
}

void ui_client_event_grid_line(Array args)
  FUNC_ATTR_NORETURN
{
  abort();  // unreachable
}

void ui_client_event_raw_line(GridLineEvent *g)
{
  int grid = g->args[0];
  int row = g->args[1];
  int startcol = g->args[2];
  Integer endcol = startcol + g->coloff;
  Integer clearcol = endcol + g->clear_width;
  LineFlags lineflags = g->wrap ? kLineFlagWrap : 0;

  tui_raw_line(tui, grid, row, startcol, endcol, clearcol, g->cur_attr, lineflags,
               (const schar_T *)grid_line_buf_char, grid_line_buf_attr);
}

/// Restarts the embedded server without killing the UI.
void ui_client_event_restart(Array args)
{
  // 1. Client-side server detach.
  ui_client_detach();

  // 2. Close ui client channel (auto kills the `nvim --embed` server due to self-exit).
  const char *error;
  bool success = channel_close(ui_client_channel_id, kChannelPartAll, &error);
  if (!success) {
    ELOG("%s", error);
    return;
  }

  // 3. Get v:argv.
  typval_T *tv = get_vim_var_tv(VV_ARGV);
  if (tv->v_type != VAR_LIST || tv->vval.v_list == NULL) {
    ELOG("failed to get vim var typval");
    return;
  }
  list_T *l = tv->vval.v_list;
  int argc = tv_list_len(l);

  // Assert to be positive for safe conversion to size_t.
  assert(argc >= 0);

  char **argv = xmalloc(sizeof(char *) * ((size_t)argc + 1));

Review comment (suggestion):
Unused argument `Array args` can specify `void` instead

Code after:
    grid_line_buf_char = xmalloc(grid_line_buf_size * sizeof(schar_T));
    grid_line_buf_attr = xmalloc(grid_line_buf_size * sizeof(sattr_T));
  }
}

void ui_client_event_grid_line(Array args)
  FUNC_ATTR_NORETURN
{
  abort();  // unreachable
}

void ui_client_event_raw_line(GridLineEvent *g)
{
  int grid = g->args[0];
  int row = g->args[1];
  int startcol = g->args[2];
  Integer endcol = startcol + g->coloff;
  Integer clearcol = endcol + g->clear_width;
  LineFlags lineflags = g->wrap ? kLineFlagWrap : 0;

  tui_raw_line(tui, grid, row, startcol, endcol, clearcol, g->cur_attr, lineflags,
               (const schar_T *)grid_line_buf_char, grid_line_buf_attr);
}

/// Restarts the embedded server without killing the UI.
void ui_client_event_restart(Array args)
{
  // 1. Client-side server detach.
  ui_client_detach();

  // 2. Close ui client channel (auto kills the `nvim --embed` server due to self-exit).
  const char *error;
  bool success = channel_close(ui_client_channel_id, kChannelPartAll, &error);
  if (!success) {
    ELOG("%s", error);
    return;
  }

  // 3. Get v:argv.
  typval_T *tv = get_vim_var_tv(VV_ARGV);
  if (tv->v_type != VAR_LIST || tv->vval.v_list == NULL) {
    ELOG("failed to get vim var typval");
    return;
  }
  list_T *l = tv->vval.v_list;
  int argc = tv_list_len(l);

  // Assert to be positive for safe conversion to size_t.
  assert(argc > 0);

  char **argv = xmalloc(sizeof(char *) * ((size_t)argc + 1));
Repository: neovim/neovim
File: runtime/lua/vim/_system.lua (Lua)

Code before:
  if type(input) == 'string' or type(input) == 'table' then
    towrite = input
  end

  return assert(uv.new_pipe(false)), towrite
end

--- @return table<string,string>
local function base_env()
  local env = vim.fn.environ() --- @type table<string,string>
  env['NVIM'] = vim.v.servername
  env['NVIM_LISTEN_ADDRESS'] = nil
  return env
end

--- uv.spawn will completely overwrite the environment
--- when we just want to modify the existing one, so
--- make sure to prepopulate it with the current env.
--- @param env? table<string,string|number>
--- @param clear_env? boolean
--- @return string[]?
local function setup_env(env, clear_env)
  if clear_env then
    return env
  end

  --- @type table<string,string|number>
  env = vim.tbl_extend('force', base_env(), env or {})

  local renv = {} --- @type string[]
  for k, v in pairs(env) do
    renv[#renv + 1] = string.format('%s=%s', k, tostring(v))
  end

  return renv
end

local is_win = vim.fn.has('win32') == 1

local M = {}

--- @param cmd string
--- @param opts uv.spawn.options
--- @param on_exit fun(code: integer, signal: integer)
--- @param on_error fun()
--- @return uv.uv_process_t, integer
local function spawn(cmd, opts, on_exit, on_error)
  if is_win then
    local cmd1 = vim.fn.exepath(cmd)
    if cmd1 ~= '' then
      cmd = cmd1

Review comment (suggestion):
In the old code if `env` was `nil` and `clear_env` was `true`, this would return `nil`, and this is what Gitsigns was relying on.

Code after:
  end

  local towrite --- @type string|string[]?
  if type(input) == 'string' or type(input) == 'table' then
    towrite = input
  end

  return assert(uv.new_pipe(false)), towrite
end

--- @return table<string,string>
local function base_env()
  local env = vim.fn.environ() --- @type table<string,string>
  env['NVIM'] = vim.v.servername
  env['NVIM_LISTEN_ADDRESS'] = nil
  return env
end

--- uv.spawn will completely overwrite the environment
--- when we just want to modify the existing one, so
--- make sure to prepopulate it with the current env.
--- @param env? table<string,string|number>
--- @param clear_env? boolean
--- @return string[]?
local function setup_env(env, clear_env)
  if not clear_env then
    --- @type table<string,string|number>
    env = vim.tbl_extend('force', base_env(), env or {})
  end

  local renv = {} --- @type string[]
  for k, v in pairs(env or {}) do
    renv[#renv + 1] = string.format('%s=%s', k, tostring(v))
  end

  return renv
end

local is_win = vim.fn.has('win32') == 1

local M = {}

--- @param cmd string
--- @param opts uv.spawn.options
--- @param on_exit fun(code: integer, signal: integer)
--- @param on_error fun()
--- @return uv.uv_process_t, integer
local function spawn(cmd, opts, on_exit, on_error)
  if is_win then
    local cmd1 = vim.fn.exepath(cmd)
    if cmd1 ~= '' then
Repository: neovim/neovim
File: runtime/lua/vim/diagnostic.lua (Lua)

Code before:
---
--- ```lua
--- vim.diagnostic.set(ns, 0, diagnostics, { virtual_text = false })
--- ```
---
--- then virtual text will not be enabled for those diagnostics.
---
---@param opts vim.diagnostic.Opts? When omitted or `nil`, retrieve the current
---       configuration. Otherwise, a configuration table (see |vim.diagnostic.Opts|).
---@param namespace integer? Update the options for the given namespace.
---                          When omitted, update the global diagnostic options.
---@return vim.diagnostic.Opts? : Current diagnostic config if {opts} is omitted.
function M.config(opts, namespace)
  vim.validate('opts', opts, 'table', true)
  vim.validate('namespace', namespace, 'number', true)

  local t --- @type vim.diagnostic.Opts
  if namespace then
    local ns = M.get_namespace(namespace)
    t = ns.opts
  else
    t = global_diagnostic_options
  end

  if not opts then
    -- Return current config
    return vim.deepcopy(t, true)
  end

  for k, v in
    pairs(opts --[[@as table<any,any>]])
  do
    t[k] = v
  end

  if namespace then
    for bufnr, v in pairs(diagnostic_cache) do
      if v[namespace] then
        M.show(namespace, bufnr)
      end
    end
  else
    for bufnr, v in pairs(diagnostic_cache) do
      for ns in pairs(v) do
        M.show(ns, bufnr)
      end
    end
  end
end

--- Set diagnostics for the given namespace and buffer.

Review comment (suggestion):
```suggestion
  if opts.jump and opts.jump.float then ---@diagnostic disable-line
```

Code after:
---
--- then virtual text will not be enabled for those diagnostics.
---
---@param opts vim.diagnostic.Opts? When omitted or `nil`, retrieve the current
---       configuration. Otherwise, a configuration table (see |vim.diagnostic.Opts|).
---@param namespace integer? Update the options for the given namespace.
---                          When omitted, update the global diagnostic options.
---@return vim.diagnostic.Opts? : Current diagnostic config if {opts} is omitted.
function M.config(opts, namespace)
  vim.validate('opts', opts, 'table', true)
  vim.validate('namespace', namespace, 'number', true)

  local t --- @type vim.diagnostic.Opts
  if namespace then
    local ns = M.get_namespace(namespace)
    t = ns.opts
  else
    t = global_diagnostic_options
  end

  if not opts then
    -- Return current config
    return vim.deepcopy(t, true)
  end

  if opts.jump and opts.jump.float ~= nil then ---@diagnostic disable-line
    vim.deprecate('opts.jump.float', 'opts.jump.on_jump', '0.14')

    local float_opts = opts.jump.float ---@type table|boolean
    if float_opts then
      float_opts = type(float_opts) == 'table' and float_opts or {}

      opts.jump.on_jump = function(_, bufnr)
        M.open_float(vim.tbl_extend('keep', float_opts, {
          bufnr = bufnr,
          scope = 'cursor',
          focus = false,
        }))
      end
    end

    opts.jump.float = nil ---@diagnostic disable-line
  end

  for k, v in
    pairs(opts --[[@as table<any,any>]])
  do
    t[k] = v
  end

  if namespace then
Repository: neovim/neovim
File: runtime/lua/vim/lsp/buf.lua (Lua)

Code before:
          hl[1] = hl[1] + 1
          hl[3] = hl[3] + 1
        end
      end

      config._update_win = update_win

      local buf, win = util.open_floating_preview(lines, 'markdown', config)

      if hl then
        vim.api.nvim_buf_clear_namespace(buf, sig_help_ns, 0, -1)
        vim.hl.range(
          buf,
          sig_help_ns,
          'LspSignatureActiveParameter',
          { hl[1], hl[2] },
          { hl[3], hl[4] }
        )
      end
      return buf, win
    end

    local fbuf, fwin = show_signature()

    if can_cycle then
      vim.keymap.set('n', '<Plug>(LspCycleSignature)', function()
        show_signature(fwin)
      end, {
        buffer = fbuf,
        desc = 'Cycle next signature',
      })
    end
  end)
end

--- @deprecated
--- Retrieves the completion items at the current cursor position. Can only be
--- called in Insert mode.
---
---@param context table (context support not yet implemented) Additional information
--- about the context in which a completion was triggered (how it was triggered,
--- and by which trigger character, if applicable)
---
---@see vim.lsp.protocol.CompletionTriggerKind
function M.completion(context)
  vim.depends('vim.lsp.buf.completion', 'vim.lsp.completion.trigger', '0.12')
  return lsp.buf_request(
    0,
    ms.textDocument_completion,
    client_positional_params({
      context = context,

Review comment (suggestion):
Let's following the same "dot" convention as we do for namespaces, autocmds, etc.
```suggestion
      vim.keymap.set('n', '<Plug>(nvim.lsp.ctrl-s)', function()
```

Code after:
      config._update_win = update_win

      local buf, win = util.open_floating_preview(lines, 'markdown', config)

      if hl then
        vim.api.nvim_buf_clear_namespace(buf, sig_help_ns, 0, -1)
        vim.hl.range(
          buf,
          sig_help_ns,
          'LspSignatureActiveParameter',
          { hl[1], hl[2] },
          { hl[3], hl[4] }
        )
      end
      return buf, win
    end

    local fbuf, fwin = show_signature()

    if can_cycle then
      vim.keymap.set('n', '<Plug>(nvim.lsp.ctrl-s)', function()
        show_signature(fwin)
      end, {
        buffer = fbuf,
        desc = 'Cycle next signature',
      })
      if vim.fn.hasmapto('<Plug>(nvim.lsp.ctrl-s)', 'n') == 0 then
        vim.keymap.set('n', '<C-s>', '<Plug>(nvim.lsp.ctrl-s)', {
          buffer = fbuf,
          desc = 'Cycle next signature',
        })
      end
    end
  end)
end

--- @deprecated
--- Retrieves the completion items at the current cursor position. Can only be
--- called in Insert mode.
---
---@param context table (context support not yet implemented) Additional information
--- about the context in which a completion was triggered (how it was triggered,
--- and by which trigger character, if applicable)
---
---@see vim.lsp.protocol.CompletionTriggerKind
function M.completion(context)
  vim.depends('vim.lsp.buf.completion', 'vim.lsp.completion.trigger', '0.12')
  return lsp.buf_request(
    0,
    ms.textDocument_completion,
    client_positional_params({
Repository: neovim/neovim
File: src/nvim/api/command.c (C)

Code before:
#include "nvim/cmdexpand_defs.h"
#include "nvim/ex_cmds_defs.h"
#include "nvim/ex_docmd.h"
#include "nvim/ex_eval.h"
#include "nvim/garray.h"
#include "nvim/garray_defs.h"
#include "nvim/globals.h"
#include "nvim/lua/executor.h"
#include "nvim/macros_defs.h"
#include "nvim/mbyte.h"
#include "nvim/memory.h"
#include "nvim/memory_defs.h"
#include "nvim/ops.h"
#include "nvim/pos_defs.h"
#include "nvim/regexp.h"
#include "nvim/strings.h"
#include "nvim/types_defs.h"
#include "nvim/usercmd.h"
#include "nvim/vim_defs.h"
#include "nvim/window.h"

#ifdef INCLUDE_GENERATED_DECLARATIONS
# include "api/command.c.generated.h"
#endif

/// Parse arguments for mapping commands, preserving whitespace in RHS
/// @param arg_str  The argument string to parse
/// @param arena    Arena allocator
/// @return Array with at most 2 elements: [lhs, rhs]
static Array parse_mapping_args(const char *arg_str, Arena *arena)
{
  Array args = arena_array(arena, 2);  // Allocate for at most 2 arguments

  char *lhs_start = (char *)arg_str;
  char *lhs_end = skiptowhite(lhs_start);
  size_t lhs_len = (size_t)(lhs_end - lhs_start);

  // Add the LHS (first argument)
  ADD_C(args, STRING_OBJ(cstrn_as_string(lhs_start, lhs_len)));

  // Add the RHS (second argument) if it exists, preserving all whitespace
  char *rhs_start = skipwhite(lhs_end);
  if (*rhs_start != NUL) {
    size_t rhs_len = strlen(rhs_start);
    ADD_C(args, STRING_OBJ(cstrn_as_string(rhs_start, rhs_len)));
  }

  return args;
}

/// Parse command line.

Review comment (suggestion):
```suggestion
/// Parse arguments for :map/:abbrev commands, preserving whitespace in RHS.
```

Code after:
#include "nvim/cmdexpand_defs.h"
#include "nvim/ex_cmds_defs.h"
#include "nvim/ex_docmd.h"
#include "nvim/ex_eval.h"
#include "nvim/garray.h"
#include "nvim/garray_defs.h"
#include "nvim/globals.h"
#include "nvim/lua/executor.h"
#include "nvim/macros_defs.h"
#include "nvim/mbyte.h"
#include "nvim/memory.h"
#include "nvim/memory_defs.h"
#include "nvim/ops.h"
#include "nvim/pos_defs.h"
#include "nvim/regexp.h"
#include "nvim/strings.h"
#include "nvim/types_defs.h"
#include "nvim/usercmd.h"
#include "nvim/vim_defs.h"
#include "nvim/window.h"

#ifdef INCLUDE_GENERATED_DECLARATIONS
# include "api/command.c.generated.h"
#endif

/// Parse arguments for :map/:abbrev commands, preserving whitespace in RHS.
/// @param arg_str  The argument string to parse
/// @param arena    Arena allocator
/// @return Array with at most 2 elements: [lhs, rhs]
static Array parse_map_cmd(const char *arg_str, Arena *arena)
{
  Array args = arena_array(arena, 2);

  char *lhs_start = (char *)arg_str;
  char *lhs_end = skiptowhite(lhs_start);
  size_t lhs_len = (size_t)(lhs_end - lhs_start);

  // Add the LHS (first argument)
  ADD_C(args, STRING_OBJ(cstrn_as_string(lhs_start, lhs_len)));

  // Add the RHS (second argument) if it exists, preserving all whitespace
  char *rhs_start = skipwhite(lhs_end);
  if (*rhs_start != NUL) {
    size_t rhs_len = strlen(rhs_start);
    ADD_C(args, STRING_OBJ(cstrn_as_string(rhs_start, rhs_len)));
  }

  return args;
}

/// Parse command line.
Repository: neovim/neovim
File: runtime/lua/vim/shared.lua (Lua)

Code before:
--- once.
---
--- Based on https://stackoverflow.com/questions/12394841/safely-remove-items-from-an-array-table-while-iterating/53038524#53038524
---
---@param t any[]
---@param first integer
---@param last integer
function vim._list_remove(t, first, last)
  local n = #t
  for i = 0, n - first do
    t[first + i] = t[last + 1 + i]
    t[last + 1 + i] = nil
  end
end

--- Trim whitespace (Lua pattern "%s") from both sides of a string.
---
---@see |lua-pattern|s
---@see https://www.lua.org/pil/20.2.html
---@param s string String to trim
---@return string String with whitespace removed from its beginning and end
function vim.trim(s)
  vim.validate('s', s, 'string')
  return s:gsub('^%s+', ''):match('^.*%S') or ''
end

--- Escapes magic chars in |lua-pattern|s.
---
---@see https://github.com/rxi/lume
---@param s string String to escape
---@return string %-escaped pattern string
function vim.pesc(s)
  vim.validate('s', s, 'string')
  return (s:gsub('[%(%)%.%%%+%-%*%?%[%]%^%$]', '%%%1'))
end

--- Tests if `s` starts with `prefix`.
---
---@param s string String
---@param prefix string Prefix to match
---@return boolean `true` if `prefix` is a prefix of `s`
function vim.startswith(s, prefix)
  vim.validate('s', s, 'string')
  vim.validate('prefix', prefix, 'string')
  return s:sub(1, #prefix) == prefix
end

--- Tests if `s` ends with `suffix`.
---
---@param s string String
---@param suffix string Suffix to match

Review comment (suggestion):
Can you add a comment here explaining why we don't use just `match`. This I'll prevent anyone changing this in the future.

Code after:
--- once.
---
--- Based on https://stackoverflow.com/questions/12394841/safely-remove-items-from-an-array-table-while-iterating/53038524#53038524
---
---@param t any[]
---@param first integer
---@param last integer
function vim._list_remove(t, first, last)
  local n = #t
  for i = 0, n - first do
    t[first + i] = t[last + 1 + i]
    t[last + 1 + i] = nil
  end
end

--- Trim whitespace (Lua pattern "%s") from both sides of a string.
---
---@see |lua-pattern|s
---@see https://www.lua.org/pil/20.2.html
---@param s string String to trim
---@return string String with whitespace removed from its beginning and end
function vim.trim(s)
  vim.validate('s', s, 'string')
  -- `s:match('^%s*(.*%S)')` is slow for long whitespace strings,
  -- so we are forced to split it into two parts to prevent this
  return s:gsub('^%s+', ''):match('^.*%S') or ''
end

--- Escapes magic chars in |lua-pattern|s.
---
---@see https://github.com/rxi/lume
---@param s string String to escape
---@return string %-escaped pattern string
function vim.pesc(s)
  vim.validate('s', s, 'string')
  return (s:gsub('[%(%)%.%%%+%-%*%?%[%]%^%$]', '%%%1'))
end

--- Tests if `s` starts with `prefix`.
---
---@param s string String
---@param prefix string Prefix to match
---@return boolean `true` if `prefix` is a prefix of `s`
function vim.startswith(s, prefix)
  vim.validate('s', s, 'string')
  vim.validate('prefix', prefix, 'string')
  return s:sub(1, #prefix) == prefix
end

--- Tests if `s` ends with `suffix`.
---