ronantakizawa/codereview-bench huggingface dataset:
Repository: neovim/neovim
File: src/nvim/strings.h (C/C++)

Code before:
static inline char *strappend(char *const dst, const char *const src)
  FUNC_ATTR_ALWAYS_INLINE FUNC_ATTR_NONNULL_ALL
    FUNC_ATTR_NONNULL_RET FUNC_ATTR_WARN_UNUSED_RESULT
{
  const size_t src_len = strlen(src);
  return (char *)memmove(dst, src, src_len) + src_len;
}

#ifdef HAVE_STRCASECMP
# define STRICMP(d, s)      strcasecmp((char *)(d), (char *)(s))
#else
# ifdef HAVE_STRICMP
#  define STRICMP(d, s)     stricmp((char *)(d), (char *)(s))
# else
#  define STRICMP(d, s)     vim_stricmp((char *)(d), (char *)(s))
# endif
#endif

#ifdef HAVE_STRNCASECMP
# define STRNICMP(d, s, n)  strncasecmp((char *)(d), (char *)(s), (size_t)(n))
#else
# ifdef HAVE_STRNICMP
#  define STRNICMP(d, s, n) strnicmp((char *)(d), (char *)(s), (size_t)(n))
# else
#  define STRNICMP(d, s, n) vim_strnicmp((char *)(d), (char *)(s), (size_t)(n))
# endif
#endif


Review comment (suggestion):
I moved this here from `klib/kvec.h` since `kv_do_printf` is declared in `strings.h.generated.h`. So any file that wants to use `kv_printf` needs to depend on `strings.h`, not (just) `kvec.h`.

Code after:
{
  const size_t src_len = strlen(src);
  return (char *)memmove(dst, src, src_len) + src_len;
}

#ifdef HAVE_STRCASECMP
# define STRICMP(d, s)      strcasecmp((char *)(d), (char *)(s))
#else
# ifdef HAVE_STRICMP
#  define STRICMP(d, s)     stricmp((char *)(d), (char *)(s))
# else
#  define STRICMP(d, s)     vim_stricmp((char *)(d), (char *)(s))
# endif
#endif

#ifdef HAVE_STRNCASECMP
# define STRNICMP(d, s, n)  strncasecmp((char *)(d), (char *)(s), (size_t)(n))
#else
# ifdef HAVE_STRNICMP
#  define STRNICMP(d, s, n) strnicmp((char *)(d), (char *)(s), (size_t)(n))
# else
#  define STRNICMP(d, s, n) vim_strnicmp((char *)(d), (char *)(s), (size_t)(n))
# endif
#endif

#define kv_printf(v, ...) kv_do_printf(&(v), __VA_ARGS__)
Repository: neovim/neovim
File: src/nvim/api/win_config.c (C)

Code before:
          hl_ids[5] = hl_blend;
          hl_ids[6] = hl_through;
        }
        return;
      }
    }
    api_set_error(err, kErrorTypeValidation, "invalid border style \"%s\"", str.data);
  }
}

static void generate_api_error(win_T *wp, const char *attribute, Error *err)
{
  if (wp != NULL && wp->w_floating) {
    api_set_error(err, kErrorTypeValidation,
                  "Missing 'relative' field when reconfiguring floating window %d",
                  wp->handle);
  } else {
    api_set_error(err, kErrorTypeValidation, "non-float cannot have '%s'", attribute);
  }
}

bool parse_winborder(WinConfig *fconfig, Error *err)
{
  if (!fconfig) {
    return false;
  }
  Object style = OBJECT_INIT;

  if (strchr(p_winborder, ',')) {
    Array border_chars = ARRAY_DICT_INIT;
    char *p = p_winborder;
    char part[MAX_SCHAR_SIZE] = { 0 };
    int count = 0;

    while (*p != NUL) {
      if (count >= 8) {
        api_free_array(border_chars);
        return false;
      }

      size_t part_len = copy_option_part(&p, part, sizeof(part), ",");
      if (part_len == 0 || part[0] == NUL) {
        api_free_array(border_chars);
        return false;
      }

      String str = cstr_to_string(part);
      ADD(border_chars, STRING_OBJ(str));
      count++;
    }


Review comment (suggestion):
```suggestion
/// Parses a border style name or custom (comma-separated) style. 
bool parse_winborder(WinConfig *fconfig, Error *err)
```

Code after:
          hl_ids[2] = hl_through;
          hl_ids[3] = hl_blend;
          hl_ids[4] = hl_blend;
          hl_ids[5] = hl_blend;
          hl_ids[6] = hl_through;
        }
        return;
      }
    }
    api_set_error(err, kErrorTypeValidation, "invalid border style \"%s\"", str.data);
  }
}

static void generate_api_error(win_T *wp, const char *attribute, Error *err)
{
  if (wp != NULL && wp->w_floating) {
    api_set_error(err, kErrorTypeValidation,
                  "Missing 'relative' field when reconfiguring floating window %d",
                  wp->handle);
  } else {
    api_set_error(err, kErrorTypeValidation, "non-float cannot have '%s'", attribute);
  }
}

/// Parses a border style name or custom (comma-separated) style.
bool parse_winborder(WinConfig *fconfig, Error *err)
{
  if (!fconfig) {
    return false;
  }
  Object style = OBJECT_INIT;

  if (strchr(p_winborder, ',')) {
    Array border_chars = ARRAY_DICT_INIT;
    char *p = p_winborder;
    char part[MAX_SCHAR_SIZE] = { 0 };
    int count = 0;

    while (*p != NUL) {
      if (count >= 8) {
        api_free_array(border_chars);
        return false;
      }

      size_t part_len = copy_option_part(&p, part, sizeof(part), ",");
      if (part_len == 0 || part[0] == NUL) {
        api_free_array(border_chars);
        return false;
      }

      String str = cstr_to_string(part);
Repository: neovim/neovim
File: runtime/lua/vim/lsp.lua (Lua)

Code before:
  if not config.root_dir and opts._root_markers then
    validate('root_markers', opts._root_markers, 'table')
    config = vim.deepcopy(config)

    for _, marker in ipairs(opts._root_markers) do
      local root = vim.fs.root(bufnr, marker)
      if root ~= nil then
        config.root_dir = root
        break
      end
    end
  end

  if
    not config.root_dir
    and (not config.workspace_folders or #config.workspace_folders == 0)
    and config.workspace_required
  then
    log.info(
      ('skipping config "%s": workspace_required=true, no workspace found'):format(config.name)
    )
    return
  end

  for _, client in pairs(all_clients) do
    if not client.is_stopping and reuse_client(client, config) then
      if opts.attach == false then
        return client.id
      end

      if lsp.buf_attach_client(bufnr, client.id) then
        return client.id
      end
      return
    end
  end

  local client_id, err = create_and_init_client(config)
  if err then
    if not opts.silent then
      vim.notify(err, vim.log.levels.WARN)
    end
    return
  end

  if opts.attach == false then
    return client_id
  end

  if client_id and lsp.buf_attach_client(bufnr, client_id) then
    return client_id

Review comment (suggestion):
does checking `client:is_stopped()` work instead? https://github.com/neovim/neovim/blob/03d378fda68fc5942660707cdd2314c0e12fb0a0/runtime/lua/vim/lsp/client.lua#L938-L939

if not, then `Client:is_stopped()` will need to be updated to check `is_stopping`, assuming that new field is actually needed.

Code after:
---
---
--- To ensure a language server is only started for languages it can handle,
--- make sure to call |vim.lsp.start()| within a |FileType| autocmd.
--- Either use |:au|, |nvim_create_autocmd()| or put the call in a
--- `ftplugin/<filetype_name>.lua` (See |ftplugin-name|)
---
--- @since 10
---
--- @param config vim.lsp.ClientConfig Configuration for the server.
--- @param opts vim.lsp.start.Opts? Optional keyword arguments.
--- @return integer? client_id
function lsp.start(config, opts)
  opts = opts or {}
  local reuse_client = opts.reuse_client or reuse_client_default
  local bufnr = vim._resolve_bufnr(opts.bufnr)

  if not config.root_dir and opts._root_markers then
    validate('root_markers', opts._root_markers, 'table')
    config = vim.deepcopy(config)

    for _, marker in ipairs(opts._root_markers) do
      local root = vim.fs.root(bufnr, marker)
      if root ~= nil then
        config.root_dir = root
        break
      end
    end
  end

  if
    not config.root_dir
    and (not config.workspace_folders or #config.workspace_folders == 0)
    and config.workspace_required
  then
    log.info(
      ('skipping config "%s": workspace_required=true, no workspace found'):format(config.name)
    )
    return
  end

  for _, client in pairs(all_clients) do
    if reuse_client(client, config) then
      if opts.attach == false then
        return client.id
      end

      if lsp.buf_attach_client(bufnr, client.id) then
        return client.id
      end
      return
Repository: neovim/neovim
File: runtime/lua/vim/lsp.lua (Lua)

Code before:
--- even if it has side effects.
---
---@generic T: function
---@param fn (T) Function to run
---@return T
local function once(fn)
  local value --- @type function
  local ran = false
  return function(...)
    if not ran then
      value = fn(...) --- @type function
      ran = true
    end
    return value
  end
end

--- @param client vim.lsp.Client
--- @param config vim.lsp.ClientConfig
--- @return boolean
local function reuse_client_default(client, config)
  if client:is_stopped() then
    return false
  end

  if client.name ~= config.name then
    return false
  end

  local config_folders = lsp._get_workspace_folders(config.workspace_folders or config.root_dir)

  if not config_folders or not next(config_folders) then
    -- Reuse if the client was configured with no workspace folders
    local client_config_folders =
      lsp._get_workspace_folders(client.config.workspace_folders or client.config.root_dir)
    return not client_config_folders or not next(client_config_folders)
  end

  for _, config_folder in ipairs(config_folders) do
    local found = false
    for _, client_folder in ipairs(client.workspace_folders or {}) do
      if config_folder.uri == client_folder.uri then
        found = true
        break
      end
    end
    if not found then
      return false
    end
  end


Review comment (suggestion):
```suggestion
  if client.name ~= config.name or client:is_stopped() then
```

Code after:
--- even if it has side effects.
---
---@generic T: function
---@param fn (T) Function to run
---@return T
local function once(fn)
  local value --- @type function
  local ran = false
  return function(...)
    if not ran then
      value = fn(...) --- @type function
      ran = true
    end
    return value
  end
end

--- @param client vim.lsp.Client
--- @param config vim.lsp.ClientConfig
--- @return boolean
local function reuse_client_default(client, config)
  if client.name ~= config.name or client:is_stopped() then
    return false
  end

  local config_folders = lsp._get_workspace_folders(config.workspace_folders or config.root_dir)

  if not config_folders or not next(config_folders) then
    -- Reuse if the client was configured with no workspace folders
    local client_config_folders =
      lsp._get_workspace_folders(client.config.workspace_folders or client.config.root_dir)
    return not client_config_folders or not next(client_config_folders)
  end

  for _, config_folder in ipairs(config_folders) do
    local found = false
    for _, client_folder in ipairs(client.workspace_folders or {}) do
      if config_folder.uri == client_folder.uri then
        found = true
        break
      end
    end
    if not found then
      return false
    end
  end

  return true
end

--- Reset defaults set by `set_defaults`.
Repository: neovim/neovim
File: src/nvim/ex_docmd.c (C)

Code before:
      || curbuf_locked()
      || (wp->w_buffer->b_nwindows == 1 && wp->w_buffer->b_locked > 0)) {
    return true;
  }

  if (quit_all
      || (check_more(false, forceit) == OK && only_one_window())) {
    apply_autocmds(EVENT_EXITPRE, NULL, NULL, false, curbuf);
    // Refuse to quit when locked or when the window was closed or the
    // buffer in the last window is being closed (can only happen in
    // autocommands).
    if (!win_valid(wp)
        || curbuf_locked()
        || (curbuf->b_nwindows == 1 && curbuf->b_locked > 0)) {
      return true;
    }
  }

  return false;
}

/// ":quit": quit current window, quit Vim if the last window is closed.
/// ":{nr}quit": quit window {nr}
static void ex_quit(exarg_T *eap)
{
  if (curwin == cmdwin_win) {
    cmdwin_result = Ctrl_C;
    return;
  }
  // Don't quit while editing the command line.
  if (text_locked()) {
    text_locked_msg();
    return;
  }

  win_T *wp;

  if (eap->addr_count > 0) {
    linenr_T wnr = eap->line2;

    for (wp = firstwin; wp->w_next != NULL; wp = wp->w_next) {
      if (--wnr <= 0) {
        break;
      }
    }
  } else {
    wp = curwin;
  }

  // Refuse to quit when locked.
  if (curbuf_locked()) {

Review comment (suggestion):
Can use this to close other windows when in the messages window, which can allow for `cmdwin_old_curwin` to be closed or the cmdwin to be left as the only window; that can cause strangeness.

E.g: this causes E199, which shouldn't happen:

```vim
only!
lua require"vim._extui".enable{}
echom "foo"
call feedkeys("q:", "nt")
au CmdwinEnter * ++once mess | 1q! | qa! | au SafeState * ++once au SafeState * ++once mes
```



Code after:
      || curbuf_locked()
      || (wp->w_buffer->b_nwindows == 1 && wp->w_buffer->b_locked > 0)) {
    return true;
  }

  if (quit_all
      || (check_more(false, forceit) == OK && only_one_window())) {
    apply_autocmds(EVENT_EXITPRE, NULL, NULL, false, curbuf);
    // Refuse to quit when locked or when the window was closed or the
    // buffer in the last window is being closed (can only happen in
    // autocommands).
    if (!win_valid(wp)
        || curbuf_locked()
        || (curbuf->b_nwindows == 1 && curbuf->b_locked > 0)) {
      return true;
    }
  }

  return false;
}

/// ":quit": quit current window, quit Vim if the last window is closed.
/// ":{nr}quit": quit window {nr}
static void ex_quit(exarg_T *eap)
{
  if (cmdwin_type != 0) {
    cmdwin_result = Ctrl_C;
    return;
  }
  // Don't quit while editing the command line.
  if (text_locked()) {
    text_locked_msg();
    return;
  }

  win_T *wp;

  if (eap->addr_count > 0) {
    linenr_T wnr = eap->line2;

    for (wp = firstwin; wp->w_next != NULL; wp = wp->w_next) {
      if (--wnr <= 0) {
        break;
      }
    }
  } else {
    wp = curwin;
  }

  // Refuse to quit when locked.
  if (curbuf_locked()) {
Repository: neovim/neovim
File: src/nvim/ex_docmd.c (C)

Code before:
  if (before_quit_autocmds(curwin, true, eap->forceit)) {
    return FAIL;
  }

  return OK;
}

/// ":qall": try to quit all windows
static void ex_quit_all(exarg_T *eap)
{
  if (before_quit_all(eap) == FAIL) {
    return;
  }
  exiting = true;
  if (eap->forceit || !check_changed_any(false, false)) {
    getout(0);
  }
  not_exiting();
}

/// ":close": close current window, unless it is the last one
static void ex_close(exarg_T *eap)
{
  win_T *win = NULL;
  int winnr = 0;
  if (curwin == cmdline_win) {
    cmdwin_result = Ctrl_C;
  } else if (!text_locked() && !curbuf_locked()) {
    if (eap->addr_count == 0) {
      ex_win_close(eap->forceit, curwin, NULL);
    } else {
      FOR_ALL_WINDOWS_IN_TAB(wp, curtab) {
        winnr++;
        if (winnr == eap->line2) {
          win = wp;
          break;
        }
      }
      if (win == NULL) {
        win = lastwin;
      }
      ex_win_close(eap->forceit, win, NULL);
    }
  }
}

/// ":pclose": Close any preview window.
static void ex_pclose(exarg_T *eap)
{
  FOR_ALL_WINDOWS_IN_TAB(win, curtab) {
    if (win->w_p_pvw) {

Review comment (suggestion):
Same here:

```vim
only!
lua require"vim._extui".enable{}
echom "foo"
call feedkeys("q:", "nt")
au CmdwinEnter * ++once mess | 1close! | qa! | au SafeState * ++once au SafeState * ++once mes
```

Code after:
  if (before_quit_autocmds(curwin, true, eap->forceit)) {
    return FAIL;
  }

  return OK;
}

/// ":qall": try to quit all windows
static void ex_quit_all(exarg_T *eap)
{
  if (before_quit_all(eap) == FAIL) {
    return;
  }
  exiting = true;
  if (eap->forceit || !check_changed_any(false, false)) {
    getout(0);
  }
  not_exiting();
}

/// ":close": close current window, unless it is the last one
static void ex_close(exarg_T *eap)
{
  win_T *win = NULL;
  int winnr = 0;
  if (cmdwin_type != 0) {
    cmdwin_result = Ctrl_C;
  } else if (!text_locked() && !curbuf_locked()) {
    if (eap->addr_count == 0) {
      ex_win_close(eap->forceit, curwin, NULL);
    } else {
      FOR_ALL_WINDOWS_IN_TAB(wp, curtab) {
        winnr++;
        if (winnr == eap->line2) {
          win = wp;
          break;
        }
      }
      if (win == NULL) {
        win = lastwin;
      }
      ex_win_close(eap->forceit, win, NULL);
    }
  }
}

/// ":pclose": Close any preview window.
static void ex_pclose(exarg_T *eap)
{
  FOR_ALL_WINDOWS_IN_TAB(win, curtab) {
    if (win->w_p_pvw) {
Repository: neovim/neovim
File: src/nvim/window.c (C)

Code before:
  // If 'switchbuf' contains "usetab": jump to first window in any tab page
  // containing "buf" if one exists.
  if (wp == NULL && (swb_flags & kOptSwbFlagUsetab)) {
    wp = buf_jump_open_tab(buf);
  }

  return wp;
}

// 'cmdheight' value explicitly set by the user: window commands are allowed to
// resize the topframe to values higher than this minimum, but not lower.
static OptInt min_set_ch = 1;

/// all CTRL-W window commands are handled here, called from normal_cmd().
///
/// @param xchar  extra char from ":wincmd gx" or NUL
void do_window(int nchar, int Prenum, int xchar)
{
  int type = FIND_DEFINE;
  char cbuf[40];

  int Prenum1 = Prenum == 0 ? 1 : Prenum;

#define CHECK_CMDWIN \
  do { \
    if (curwin == cmdwin_win && cmdwin_lock) { \
      emsg(_(e_cmdwin)); \
      return; \
    } \
  } while (0)

  switch (nchar) {
  // split current window in two parts, horizontally
  case 'S':
  case Ctrl_S:
  case 's':
    CHECK_CMDWIN;
    reset_VIsual_and_resel();  // stop Visual mode
    // When splitting the quickfix window open a new buffer in it,
    // don't replicate the quickfix buffer.
    if (bt_quickfix(curbuf)) {
      goto newwindow;
    }
    win_split(Prenum, 0);
    break;

  // split current window in two parts, vertically
  case Ctrl_V:
  case 'v':
    CHECK_CMDWIN;
    reset_VIsual_and_resel();  // stop Visual mode

Review comment (suggestion):
Likewise, this allows entering other windows once in the messages window with `<C-W>w` and the like.

Code after:
  // If 'switchbuf' contains "usetab": jump to first window in any tab page
  // containing "buf" if one exists.
  if (wp == NULL && (swb_flags & kOptSwbFlagUsetab)) {
    wp = buf_jump_open_tab(buf);
  }

  return wp;
}

// 'cmdheight' value explicitly set by the user: window commands are allowed to
// resize the topframe to values higher than this minimum, but not lower.
static OptInt min_set_ch = 1;

/// all CTRL-W window commands are handled here, called from normal_cmd().
///
/// @param xchar  extra char from ":wincmd gx" or NUL
void do_window(int nchar, int Prenum, int xchar)
{
  int type = FIND_DEFINE;
  char cbuf[40];

  int Prenum1 = Prenum == 0 ? 1 : Prenum;

#define CHECK_CMDWIN \
  do { \
    if (cmdwin_type != 0) { \
      emsg(_(e_cmdwin)); \
      return; \
    } \
  } while (0)

  switch (nchar) {
  // split current window in two parts, horizontally
  case 'S':
  case Ctrl_S:
  case 's':
    CHECK_CMDWIN;
    reset_VIsual_and_resel();  // stop Visual mode
    // When splitting the quickfix window open a new buffer in it,
    // don't replicate the quickfix buffer.
    if (bt_quickfix(curbuf)) {
      goto newwindow;
    }
    win_split(Prenum, 0);
    break;

  // split current window in two parts, vertically
  case Ctrl_V:
  case 'v':
    CHECK_CMDWIN;
    reset_VIsual_and_resel();  // stop Visual mode
Repository: neovim/neovim
File: test/functional/editor/ftplugin_spec.lua (Lua)

Code before:
  it('finds module in packpath/start', function()
    eq(
      stdpath('data') .. '/site/pack/packer/start/plugin-foo/lua/plugin-foo/init.lua',
      lua_includeexpr('plugin-foo')
    )
    eq(
      stdpath('data') .. '/site/pack/packer/start/plugin-foo/lua/plugin-foo/bar.lua',
      lua_includeexpr('plugin-foo.bar')
    )
  end)

  it('finds module in $VIMRUNTIME', function()
    command('edit ' .. repo_root)
    eq(vimruntime() .. '/lua/vim/_ftplugin/lua.lua', lua_includeexpr('vim._ftplugin.lua'))
    eq(vimruntime() .. '/lua/editorconfig.lua', lua_includeexpr('editorconfig'))
  end)

  it('finds module in runtimepath', function()
    eq(stdpath('config') .. '/lua/user-foo/init.lua', lua_includeexpr('user-foo'))
    eq(stdpath('config') .. '/lua/user-foo/bar.lua', lua_includeexpr('user-foo.bar'))
    command('set rtp+=' .. temp_dir)
    eq(temp_dir .. '/lua/runtime-foo/init.lua', lua_includeexpr('runtime-foo'))
    eq(temp_dir .. '/lua/runtime-foo/bar.lua', lua_includeexpr('runtime-foo.bar'))
  end)

  it('Non-Nvim-style Lua modules', function()
    command('cd ' .. temp_dir)
    eq(temp_dir .. '/general-foo/bar/init.lua', lua_includeexpr('general-foo.bar'))
    eq(temp_dir .. '/general-foo/bar/baz.lua', lua_includeexpr('general-foo.bar.baz'))
    command('cd -')
  end)
end)


Review comment (suggestion):
```suggestion
  it('non-Nvim-style Lua modules', function()
```

Code after:
  it('finds module in packpath/start', function()
    eq(
      stdpath('data') .. '/site/pack/packer/start/plugin-foo/lua/plugin-foo/init.lua',
      lua_includeexpr('plugin-foo')
    )
    eq(
      stdpath('data') .. '/site/pack/packer/start/plugin-foo/lua/plugin-foo/bar.lua',
      lua_includeexpr('plugin-foo.bar')
    )
  end)

  it('finds module in $VIMRUNTIME', function()
    command('edit ' .. repo_root)
    eq(vimruntime() .. '/lua/vim/_ftplugin/lua.lua', lua_includeexpr('vim._ftplugin.lua'))
    eq(vimruntime() .. '/lua/editorconfig.lua', lua_includeexpr('editorconfig'))
  end)

  it('finds module in runtimepath', function()
    eq(stdpath('config') .. '/lua/user-foo/init.lua', lua_includeexpr('user-foo'))
    eq(stdpath('config') .. '/lua/user-foo/bar.lua', lua_includeexpr('user-foo.bar'))
    command('set rtp+=' .. temp_dir)
    eq(temp_dir .. '/lua/runtime-foo/init.lua', lua_includeexpr('runtime-foo'))
    eq(temp_dir .. '/lua/runtime-foo/bar.lua', lua_includeexpr('runtime-foo.bar'))
  end)

  it('non-Nvim-style Lua modules', function()
    command('cd ' .. temp_dir)
    eq(temp_dir .. '/general-foo/bar/init.lua', lua_includeexpr('general-foo.bar'))
    eq(temp_dir .. '/general-foo/bar/baz.lua', lua_includeexpr('general-foo.bar.baz'))
    command('cd -')
  end)
end)