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

Code before:
  nvim_command(cstr_as_string(quit_cmd), &err);
  if (ERROR_SET(&err)) {
    emsg(err.msg);  // Could not exit
    api_clear_error(&err);
    return;
  }
  if (!exiting) {
    ELOG("':%s' did not quit the server, quitting manually", quit_cmd);
    getout(0);
  }
}

/// ":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) {
      ex_win_close(eap->forceit, win, NULL);
      break;
    }
  }
}

/// Close window "win" and take care of handling closing the last window for a
/// modified buffer.

Review comment (suggestion):
```suggestion
    ELOG("restart: \"%s\" did not quit the server, quitting manually", quit_cmd);
```

Code after:
  if ((cmdmod.cmod_flags & CMOD_CONFIRM) && check_changed_any(false, false)) {
    return;
  }
  restarting = true;
  nvim_command(cstr_as_string(quit_cmd), &err);
  if (ERROR_SET(&err)) {
    emsg(err.msg);  // Could not exit
    api_clear_error(&err);
    not_restarting();
    return;
  }
  if (!exiting) {
    emsg("restart failed: +cmd did not quit the server");
    not_restarting();
  }
}

/// ":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) {
      ex_win_close(eap->forceit, win, NULL);
      break;
    }
Repository: neovim/neovim
File: src/nvim/ex_docmd.c (C)

Code before:
  }

  return OK;
}

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

/// ":restart": restart the Nvim server (using ":qall").
/// ":restart +cmd": restart the Nvim server using ":cmd".
static void ex_restart(exarg_T *eap)
{
  char *quit_cmd = (eap->do_ecmd_cmd) ? eap->do_ecmd_cmd : "qall";
  Error err = ERROR_INIT;
  if (quit_cmd[vim_strsize(quit_cmd) - 1] != '!' && check_changed_any(false,
                                                                      false)) {
    return;
  }
  restarting = true;
  nvim_command(cstr_as_string(quit_cmd), &err);
  if (ERROR_SET(&err)) {
    emsg(err.msg);  // Could not exit
    api_clear_error(&err);
    not_restarting();
    return;
  }
  if (!exiting) {
    emsg("':cmd' did not quit the server, abandoning restart");
    not_restarting();
  }
}

/// ":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()) {

Review comment (suggestion):
```suggestion
  if (quit_cmd[strlen(quit_cmd) - 1] != '!' && check_changed_any(false,
```

Code after:
  }

  return OK;
}

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

/// ":restart": restart the Nvim server (using ":qall!").
/// ":restart +cmd": restart the Nvim server using ":cmd".
static void ex_restart(exarg_T *eap)
{
  char *quit_cmd = (eap->do_ecmd_cmd) ? eap->do_ecmd_cmd : "qall!";
  Error err = ERROR_INIT;
  if ((cmdmod.cmod_flags & CMOD_CONFIRM) && check_changed_any(false, false)) {
    return;
  }
  restarting = true;
  nvim_command(cstr_as_string(quit_cmd), &err);
  if (ERROR_SET(&err)) {
    emsg(err.msg);  // Could not exit
    api_clear_error(&err);
    not_restarting();
    return;
  }
  if (!exiting) {
    emsg("restart failed: +cmd did not quit the server");
    not_restarting();
  }
}

/// ":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) {
Repository: neovim/neovim
File: src/nvim/ex_docmd.c (C)

Code before:
  if (!eap->forceit && check_changed_any(false, false)) {
    not_exiting();
    return;
  }
  getout(0);
}

/// ":restart": restart the Nvim server (using ":qall!").
/// ":restart +cmd": restart the Nvim server using ":cmd".
static void ex_restart(exarg_T *eap)
{
  char *quit_cmd = (eap->do_ecmd_cmd) ? eap->do_ecmd_cmd : "qall!";
  Error err = ERROR_INIT;
  if ((cmdmod.cmod_flags & CMOD_CONFIRM) && check_changed_any(false, false)) {
    return;
  }
  restarting = true;
  nvim_command(cstr_as_string(quit_cmd), &err);
  if (ERROR_SET(&err)) {
    emsg(err.msg);  // Could not exit
    api_clear_error(&err);
    not_restarting();
    return;
  }
  if (!exiting) {
    emsg("':cmd' did not quit the server, abandoning restart");
    not_restarting();
  }
}

/// ":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;
      }

Review comment (bug):
```suggestion
    emsg("restart failed: +cmd did not quit the server");
```

Code after:
  if (!eap->forceit && check_changed_any(false, false)) {
    not_exiting();
    return;
  }
  getout(0);
}

/// ":restart": restart the Nvim server (using ":qall!").
/// ":restart +cmd": restart the Nvim server using ":cmd".
static void ex_restart(exarg_T *eap)
{
  char *quit_cmd = (eap->do_ecmd_cmd) ? eap->do_ecmd_cmd : "qall!";
  Error err = ERROR_INIT;
  if ((cmdmod.cmod_flags & CMOD_CONFIRM) && check_changed_any(false, false)) {
    return;
  }
  restarting = true;
  nvim_command(cstr_as_string(quit_cmd), &err);
  if (ERROR_SET(&err)) {
    emsg(err.msg);  // Could not exit
    api_clear_error(&err);
    not_restarting();
    return;
  }
  if (!exiting) {
    emsg("restart failed: +cmd did not quit the server");
    not_restarting();
  }
}

/// ":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;
      }
Repository: neovim/neovim
File: src/nvim/ex_docmd.c (C)

Code before:
  if (!eap->forceit && check_changed_any(false, false)) {
    not_exiting();
    return;
  }
  getout(0);
}

/// ":restart": restart the Nvim server (using ":qall!").
/// ":restart +cmd": restart the Nvim server using ":cmd".
static void ex_restart(exarg_T *eap)
{
  char *quit_cmd = (eap->do_ecmd_cmd) ? eap->do_ecmd_cmd : "qall!";
  Error err = ERROR_INIT;
  if ((cmdmod.cmod_flags & CMOD_CONFIRM) && check_changed_any(false, false)) {
    return;
  }
  restarting = true;
  nvim_command(cstr_as_string(quit_cmd), &err);
  if (ERROR_SET(&err)) {
    emsg(err.msg);  // Could not exit
    api_clear_error(&err);
    not_restarting();
    return;
  }
  if (!exiting) {
    emsg("restart failed: +cmd did not quit the server, abandoning restart");
    not_restarting();
  }
}

/// ":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;
      }

Review comment (bug):
this is redundant

```suggestion
    emsg("restart failed: +cmd did not quit the server");
```

Code after:
  if (!eap->forceit && check_changed_any(false, false)) {
    not_exiting();
    return;
  }
  getout(0);
}

/// ":restart": restart the Nvim server (using ":qall!").
/// ":restart +cmd": restart the Nvim server using ":cmd".
static void ex_restart(exarg_T *eap)
{
  char *quit_cmd = (eap->do_ecmd_cmd) ? eap->do_ecmd_cmd : "qall!";
  Error err = ERROR_INIT;
  if ((cmdmod.cmod_flags & CMOD_CONFIRM) && check_changed_any(false, false)) {
    return;
  }
  restarting = true;
  nvim_command(cstr_as_string(quit_cmd), &err);
  if (ERROR_SET(&err)) {
    emsg(err.msg);  // Could not exit
    api_clear_error(&err);
    not_restarting();
    return;
  }
  if (!exiting) {
    emsg("restart failed: +cmd did not quit the server");
    not_restarting();
  }
}

/// ":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;
      }
Repository: neovim/neovim
File: runtime/lua/vim/lsp/util.lua (Lua)

Code before:
  if client_id then
    if enable == vim.g[var] then
      vim.g[client_var] = nil
    else
      vim.g[client_var] = enable
    end
  elseif bufnr then
    if enable == vim.g[var] then
      vim.b[bufnr][var] = nil
    else
      vim.b[bufnr][var] = enable
    end
  else
    vim.g[var] = enable
    for _, it_bufnr in ipairs(api.nvim_list_bufs()) do
      if api.nvim_buf_is_loaded(it_bufnr) and vim.b[it_bufnr][var] == enable then
        vim.b[it_bufnr][var] = nil
      end
    end
    for _, it_client in ipairs(vim.lsp.get_clients()) do
      local it_client_var = ('__lsp_%s_client_%d_enabled'):format(feature, it_client.id)
      if vim.g[it_client_var] and vim.g[it_client_var] == enable then
        vim.g[it_client_var] = nil
      end
    end
  end
end

M._get_line_byte_from_position = get_line_byte_from_position

---@nodoc
---@type table<integer,integer>
M.buf_versions = setmetatable({}, {
  __index = function(t, bufnr)
    return rawget(t, bufnr) or 0
  end,
})

return M


Review comment (suggestion):
For `enable()`, passing both `bufnr` and `client_id` at the same time seems ambiguous, so it is set here that these two values cannot be passed simultaneously.

Code after:
  filter = filter or {}
  local bufnr = filter.bufnr
  local client_id = filter.client_id

  local var = make_enable_var(feature)
  local client_var = make_enable_var(feature, client_id)
  return vim.F.if_nil(client_id and vim.g[client_var], vim.g[var])
    and vim.F.if_nil(bufnr and vim.b[bufnr][var], vim.g[var])
end

---@param feature 'semantic_tokens'
---@param enable? boolean
---@param filter? vim.lsp.enable.Filter
function M._enable(feature, enable, filter)
  vim.validate('feature', feature, 'string')
  vim.validate('enable', enable, 'boolean', true)
  vim.validate('filter', filter, 'table', true)

  enable = enable == nil or enable
  filter = filter or {}
  local bufnr = filter.bufnr
  local client_id = filter.client_id
  assert(
    not (bufnr and client_id),
    'Only one of `bufnr` or `client_id` filters can be specified at a time.'
  )

  local var = make_enable_var(feature)
  local client_var = make_enable_var(feature, client_id)

  if client_id then
    if enable == vim.g[var] then
      vim.g[client_var] = nil
    else
      vim.g[client_var] = enable
    end
  elseif bufnr then
    if enable == vim.g[var] then
      vim.b[bufnr][var] = nil
    else
      vim.b[bufnr][var] = enable
    end
  else
    vim.g[var] = enable
    for _, it_bufnr in ipairs(api.nvim_list_bufs()) do
      if api.nvim_buf_is_loaded(it_bufnr) and vim.b[it_bufnr][var] == enable then
        vim.b[it_bufnr][var] = nil
      end
    end
    for _, it_client in ipairs(vim.lsp.get_clients()) do
      local it_client_var = make_enable_var(feature, it_client.id)
Repository: neovim/neovim
File: runtime/lua/vim/lsp/util.lua (Lua)

Code before:
          client.offset_encoding
        ),
      }, opts.handler, bufnr)
    end
  end
end

---@class vim.lsp.enable.Filter
---@inlinedoc
---
--- Buffer number, or 0 for current buffer, or nil for all.
---@field bufnr? integer
---
--- Client ID, or nil for all
---@field client_id? integer

---@param feature string
---@param filter? vim.lsp.enable.Filter
function M._is_enabled(feature, filter)
  vim.validate('filter', filter, 'table', true)

  filter = filter or {}
  local bufnr = filter.bufnr
  local client_id = filter.client_id

  local var = ('__lsp_%s_enabled'):format(feature)
  local client_var = ('__lsp_%s_client_%d_enabled'):format(feature, client_id or 0)
  return vim.F.if_nil(client_id and vim.g[client_var], vim.g[var])
    and vim.F.if_nil(bufnr and vim.b[bufnr][var], vim.g[var])
end

---@param feature string
---@param enable? boolean
---@param filter? vim.lsp.enable.Filter
function M._enable(feature, enable, filter)
  vim.validate('enable', enable, 'boolean', true)
  vim.validate('filter', filter, 'table', true)

  enable = enable == nil or enable
  filter = filter or {}
  local bufnr = filter.bufnr
  local client_id = filter.client_id
  assert(
    not (bufnr and client_id),
    'Only one of `bufnr` or `client_id` filters can be specified at a time.'
  )

  local var = ('__lsp_%s_enabled'):format(feature)
  local client_var = ('__lsp_%s_client_%d_enabled'):format(feature, client_id or 0)

  if client_id then

Review comment (refactor):
Can these `('__lsp...'):format(...)` calls be extracted into a single function that just takes the feature and an optional client id? Just so that the strings are only constructed in one place, to prevent risk of typos 

Code after:
        })
        for _, client in ipairs(clients) do
          client:request(method, {
            textDocument = textDocument,
            range = make_line_range_params(bufnr, first - 1, last - 1, client.offset_encoding),
          }, opts.handler, bufnr)
        end
      end
    end
  else
    for _, client in ipairs(clients) do
      client:request(method, {
        textDocument = textDocument,
        range = make_line_range_params(
          bufnr,
          0,
          api.nvim_buf_line_count(bufnr) - 1,
          client.offset_encoding
        ),
      }, opts.handler, bufnr)
    end
  end
end

---@param feature string
---@param client_id? integer
local function make_enable_var(feature, client_id)
  return ('_lsp_enabled_%s%s'):format(feature, client_id and ('_client_%d'):format(client_id) or '')
end

---@class vim.lsp.enable.Filter
---@inlinedoc
---
--- Buffer number, or 0 for current buffer, or nil for all.
---@field bufnr? integer
---
--- Client ID, or nil for all
---@field client_id? integer

---@param feature string
---@param filter? vim.lsp.enable.Filter
function M._is_enabled(feature, filter)
  vim.validate('feature', feature, 'string')
  vim.validate('filter', filter, 'table', true)

  filter = filter or {}
  local bufnr = filter.bufnr
  local client_id = filter.client_id

  local var = make_enable_var(feature)
  local client_var = make_enable_var(feature, client_id)
Repository: neovim/neovim
File: runtime/lua/vim/lsp/util.lua (Lua)

Code before:
          client:request(method, {
            textDocument = textDocument,
            range = make_line_range_params(bufnr, first - 1, last - 1, client.offset_encoding),
          }, opts.handler, bufnr)
        end
      end
    end
  else
    for _, client in ipairs(clients) do
      client:request(method, {
        textDocument = textDocument,
        range = make_line_range_params(
          bufnr,
          0,
          api.nvim_buf_line_count(bufnr) - 1,
          client.offset_encoding
        ),
      }, opts.handler, bufnr)
    end
  end
end

---@param feature string
---@param client_id? integer
local function make_enable_var(feature, client_id)
  return ('_lsp_%s%s_enabled'):format(
    feature,
    client_id and ('_client_%d'):format(client_id) or nil
  )
end

---@class vim.lsp.enable.Filter
---@inlinedoc
---
--- Buffer number, or 0 for current buffer, or nil for all.
---@field bufnr? integer
---
--- Client ID, or nil for all
---@field client_id? integer

---@param feature string
---@param filter? vim.lsp.enable.Filter
function M._is_enabled(feature, filter)
  vim.validate('feature', feature, 'string')
  vim.validate('filter', filter, 'table', true)

  filter = filter or {}
  local bufnr = filter.bufnr
  local client_id = filter.client_id

  local var = make_enable_var(feature)

Review comment (suggestion):


```suggestion
  return ('_lsp_enable_%s%s'):format(
```

Code after:
  if opts.only_visible then
    for _, window in ipairs(api.nvim_list_wins()) do
      if api.nvim_win_get_buf(window) == bufnr then
        local first = vim.fn.line('w0', window)
        local last = vim.fn.line('w$', window)
        M._cancel_requests({
          bufnr = bufnr,
          clients = clients,
          method = method,
          type = 'pending',
        })
        for _, client in ipairs(clients) do
          client:request(method, {
            textDocument = textDocument,
            range = make_line_range_params(bufnr, first - 1, last - 1, client.offset_encoding),
          }, opts.handler, bufnr)
        end
      end
    end
  else
    for _, client in ipairs(clients) do
      client:request(method, {
        textDocument = textDocument,
        range = make_line_range_params(
          bufnr,
          0,
          api.nvim_buf_line_count(bufnr) - 1,
          client.offset_encoding
        ),
      }, opts.handler, bufnr)
    end
  end
end

---@param feature string
---@param client_id? integer
local function make_enable_var(feature, client_id)
  return ('_lsp_enabled_%s%s'):format(feature, client_id and ('_client_%d'):format(client_id) or '')
end

---@class vim.lsp.enable.Filter
---@inlinedoc
---
--- Buffer number, or 0 for current buffer, or nil for all.
---@field bufnr? integer
---
--- Client ID, or nil for all
---@field client_id? integer

---@param feature string
---@param filter? vim.lsp.enable.Filter
Repository: neovim/neovim
File: runtime/lua/vim/lsp/util.lua (Lua)

Code before:
            range = make_line_range_params(bufnr, first - 1, last - 1, client.offset_encoding),
          }, opts.handler, bufnr)
        end
      end
    end
  else
    for _, client in ipairs(clients) do
      client:request(method, {
        textDocument = textDocument,
        range = make_line_range_params(
          bufnr,
          0,
          api.nvim_buf_line_count(bufnr) - 1,
          client.offset_encoding
        ),
      }, opts.handler, bufnr)
    end
  end
end

---@param feature string
---@param client_id? integer
local function make_enable_var(feature, client_id)
  return ('_lsp_%s%s_enabled'):format(
    feature,
    client_id and ('_client_%d'):format(client_id) or nil
  )
end

---@class vim.lsp.enable.Filter
---@inlinedoc
---
--- Buffer number, or 0 for current buffer, or nil for all.
---@field bufnr? integer
---
--- Client ID, or nil for all
---@field client_id? integer

---@param feature string
---@param filter? vim.lsp.enable.Filter
function M._is_enabled(feature, filter)
  vim.validate('feature', feature, 'string')
  vim.validate('filter', filter, 'table', true)

  filter = filter or {}
  local bufnr = filter.bufnr
  local client_id = filter.client_id

  local var = make_enable_var(feature)
  local client_var = make_enable_var(feature, client_id)
  return vim.F.if_nil(client_id and vim.g[client_var], vim.g[var])

Review comment (suggestion):
`nil` will print "nil", should it be empty something like "allclients" ?
```suggestion
    client_id and ('_client_%d'):format(client_id) or "allclients"
```

Code after:
      if api.nvim_win_get_buf(window) == bufnr then
        local first = vim.fn.line('w0', window)
        local last = vim.fn.line('w$', window)
        M._cancel_requests({
          bufnr = bufnr,
          clients = clients,
          method = method,
          type = 'pending',
        })
        for _, client in ipairs(clients) do
          client:request(method, {
            textDocument = textDocument,
            range = make_line_range_params(bufnr, first - 1, last - 1, client.offset_encoding),
          }, opts.handler, bufnr)
        end
      end
    end
  else
    for _, client in ipairs(clients) do
      client:request(method, {
        textDocument = textDocument,
        range = make_line_range_params(
          bufnr,
          0,
          api.nvim_buf_line_count(bufnr) - 1,
          client.offset_encoding
        ),
      }, opts.handler, bufnr)
    end
  end
end

---@param feature string
---@param client_id? integer
local function make_enable_var(feature, client_id)
  return ('_lsp_enabled_%s%s'):format(feature, client_id and ('_client_%d'):format(client_id) or '')
end

---@class vim.lsp.enable.Filter
---@inlinedoc
---
--- Buffer number, or 0 for current buffer, or nil for all.
---@field bufnr? integer
---
--- Client ID, or nil for all
---@field client_id? integer

---@param feature string
---@param filter? vim.lsp.enable.Filter
function M._is_enabled(feature, filter)
  vim.validate('feature', feature, 'string')