ronantakizawa/codereview-bench huggingface dataset:
Repository: neovim/neovim
File: test/functional/terminal/tui_spec.lua (Lua)

Code before:
    })
    tt.feed_data('iThis is server 2.\027')
    tt.feed_data(':connect ' .. server1 .. '\013')

    screen2:expect({
      any = [[Process exited]],
    })

    screen2:detach()

    local screen3 = tt.setup_child_nvim({
      '--remote-ui',
      '--server',
      server2,
    })
    screen3:expect([[
      This is server 2^.                                 |
      ~                                                 |*3
      {2:[No Name] [+]                   1,17           All}|
                                                        |
      {5:-- TERMINAL --}                                    |
    ]])

    screen3:detach()
  end)
  it('connect! connects to a different server and stops the current one', function()
    n.clear()
    finally(function()
      n.check_close()
    end)

    local server1 = new_pipename()
    local screen1 = tt.setup_child_nvim({
      '--listen',
      server1,
    })
    tt.feed_data('iThis is server 1')

    screen1:detach()

    local server2 = new_pipename()
    local screen2 = tt.setup_child_nvim({
      '--listen',
      server2,
    })
    tt.feed_data('\027:connect! ' .. server1 .. '\013')
    screen2:expect([[
      This is server 1^                                  |
      ~                                                 |*3
      [No Name] [+]                   1,17           All|
      -- INSERT --                                      |

Review comment (suggestion):
```suggestion
  it('"!" stops the current server', function()
```

Code after:
    n.clear()
    finally(function()
      n.check_close()
    end)

    local server1 = new_pipename()
    local screen = tt.setup_child_nvim({
      '--listen',
      server1,
      '-u',
      'NONE',
    })

    tt.feed_data(':connect\013')
    screen:expect([[
      ^                                                  |
      ~                                                 |*3
      [No Name]                       0,0-1          All|
      E471: Argument required                           |
      {5:-- TERMINAL --}                                    |
    ]])

    screen:detach()

    local server2 = new_pipename()
    local screen2 = tt.setup_child_nvim({
      '--listen',
      server2,
      '-u',
      'NONE',
    })
    tt.feed_data('iThis is server 2.\027')
    tt.feed_data(':connect ' .. server1 .. '\013')

    screen2:expect({
      any = [[Process exited]],
    })

    local server1_session = n.connect(server1)
    server1_session:request('nvim_command', 'qall!')

    screen2:detach()

    local server2_session = n.connect(server2)

    local screen3 = tt.setup_child_nvim({
      '--remote-ui',
      '--server',
      server2,
    })
    screen3:expect([[
Repository: neovim/neovim
File: test/functional/ui/cmdline2_spec.lua (Lua)

Code before:
local n = require('test.functional.testnvim')()
local Screen = require('test.functional.ui.screen')

local clear, exec, exec_lua, feed = n.clear, n.exec, n.exec_lua, n.feed

describe('cmdline2', function()
  local screen
  before_each(function()
    clear()
    screen = Screen.new()
    screen:add_extra_attr_ids({
      [100] = { foreground = Screen.colors.Magenta1, bold = true },
    })
    exec_lua(function()
      require('vim._extui').enable({})
    end)
  end)

  it("no crash for invalid grid after 'cmdheight' OptionSet", function()
    exec('tabnew | tabprev')
    feed(':set ch=0')
    screen:expect([[
      {5: }{100:2}{5: [No Name] }{24: [No Name] }{2:                            }{24:X}|
                                                           |
      {1:~                                                    }|*11
      {16::}{15:set} {16:ch}{15:=}0^                                            |
    ]])
    feed('<CR>')

Review comment (suggestion):
Before I forget, let's add a note like this here and in messages2_spec.lua.

```suggestion
-- Tests for ui2 (protocol-driven UI), which will replace the legacy UI.

local n = require('test.functional.testnvim')()
```

Code after:
-- Tests for (protocol-driven) ui2, intended to replace the legacy message grid UI.

local n = require('test.functional.testnvim')()
local Screen = require('test.functional.ui.screen')

local clear, exec, exec_lua, feed = n.clear, n.exec, n.exec_lua, n.feed

describe('cmdline2', function()
  local screen
  before_each(function()
    clear()
    screen = Screen.new()
    screen:add_extra_attr_ids({
      [100] = { foreground = Screen.colors.Magenta1, bold = true },
    })
    exec_lua(function()
      require('vim._extui').enable({})
    end)
  end)

  it("no crash for invalid grid after 'cmdheight' OptionSet", function()
    exec('tabnew | tabprev')
    feed(':set ch=0')
    screen:expect([[
      {5: }{100:2}{5: [No Name] }{24: [No Name] }{2:                            }{24:X}|
                                                           |
      {1:~                                                    }|*11
      {16::}{15:set} {16:ch}{15:=}0^                                            |
Repository: neovim/neovim
File: test/functional/plugin/lsp/on_type_formatting_spec.lua (Lua)

Code before:
          ---@param params lsp.DocumentOnTypeFormattingParams
          ---@param callback fun(err?: lsp.ResponseError, result?: lsp.TextEdit[])
          ['textDocument/onTypeFormatting'] = function(_, params, callback)
            callback(nil, {
              {
                newText = ';',
                range = {
                  start = params.position,
                  ['end'] = params.position,
                },
              },
            })
          end,
        },
      })

      vim.lsp.start({
        name = 'dummy2',
        cmd = _G.server2.cmd,
      })
      local win = vim.api.nvim_get_current_win()
      vim.api.nvim_win_set_cursor(win, { 2, 0 })
    end)
    feed('A =')
    exec_lua(function()
      vim.wait(1000, function()
        return #_G.server2.messages > 1 and #_G.server.messages > 1
      end)
    end)
    eq(
      {
        'int main() {',
        '  int hi =;;',
        '}',
      },
      exec_lua(function()
        return vim.api.nvim_buf_get_lines(0, 0, -1, false)
      end)
    )
  end)

  it('can be disabled', function()
    exec_lua(function()
      vim.lsp.on_type_formatting.enable(false, { client_id = _G.server_id })
      local win = vim.api.nvim_get_current_win()
      vim.api.nvim_win_set_cursor(win, { 2, 0 })
    end)
    feed('A = 5')
    eq(
      {
        'int main() {',

Review comment (refactor):
Nit: I would use our `retry` utility over `vim.wait`
https://github.com/neovim/neovim/blob/7a71235399066e722d54036742d3c5550f3dfd21/test/functional/terminal/clipboard_spec.lua?plain=1#L61-L63

Code after:
          ['textDocument/onTypeFormatting'] = function(_, params, callback)
            callback(nil, {
              {
                newText = ';',
                range = {
                  start = params.position,
                  ['end'] = params.position,
                },
              },
            })
          end,
        },
      })

      vim.lsp.start({
        name = 'dummy2',
        cmd = _G.server2.cmd,
      })
      local win = vim.api.nvim_get_current_win()
      vim.api.nvim_win_set_cursor(win, { 2, 0 })
    end)
    feed('A =')
    retry(nil, 100, function()
      eq(
        {
          'int main() {',
          '  int hi =;;',
          '}',
        },
        exec_lua(function()
          return vim.api.nvim_buf_get_lines(0, 0, -1, false)
        end)
      )
    end)
  end)

  it('can be disabled', function()
    exec_lua(function()
      vim.lsp.on_type_formatting.enable(false, { client_id = _G.server_id })
      local win = vim.api.nvim_get_current_win()
      vim.api.nvim_win_set_cursor(win, { 2, 0 })
    end)
    feed('A = 5')
    eq(
      {
        'int main() {',
        '  int hi = 5',
        '}',
      },
      exec_lua(function()
        return vim.api.nvim_buf_get_lines(0, 0, -1, false)
Repository: neovim/neovim
File: runtime/lua/vim/_defaults.lua (Lua)

Code before:
          end
        end)
      end
    end
  end

  vim.api.nvim_create_autocmd('VimEnter', {
    group = vim.api.nvim_create_augroup('nvim.exrc', {}),
    desc = 'Find exrc files in parent directories',
    callback = function()
      if not vim.o.exrc then
        return
      end
      local files = vim.fs.find({ '.nvim.lua', '.nvimrc', '.exrc' }, {
        type = 'file',
        upward = true,
        limit = math.huge,
        -- exrc in cwd already handled from C, thus start in parent directory.
        path = vim.fs.dirname((vim.uv.cwd())),
      })
      for _, file in ipairs(files) do
        local trusted = vim.secure.read(file) --[[@as string|nil]]
        if trusted then
          if vim.endswith(file, '.lua') then
            local chunk = assert(load(trusted, '@' .. file))
            chunk()
          else
            vim.api.nvim_exec2(trusted, {})
          end
        end
        -- If the user unset 'exrc' in the current exrc then stop searching
        if not vim.o.exrc then
          return
        end
      end
    end,
  })
end

--- Default options
do
  --- Default 'grepprg' to ripgrep if available.
  if vim.fn.executable('rg') == 1 then
    -- Use -uu to make ripgrep not check ignore files/skip dot-files
    vim.o.grepprg = 'rg --vimgrep -uu '
    vim.o.grepformat = '%f:%l:%c:%m'
  end
end


Review comment (suggestion):
`loadstring()` accepts a `chunkname` argument too so this could just be

```suggestion
            assert(loadstring(trusted, '@' .. file))()
```

Code after:
          end)

          if not timer:is_closing() then
            timer:close()
          end
        end)
      end
    end
  end

  vim.api.nvim_create_autocmd('VimEnter', {
    group = vim.api.nvim_create_augroup('nvim.exrc', {}),
    desc = 'Find exrc files in parent directories',
    callback = function()
      if not vim.o.exrc then
        return
      end
      local files = vim.fs.find({ '.nvim.lua', '.nvimrc', '.exrc' }, {
        type = 'file',
        upward = true,
        limit = math.huge,
        -- exrc in cwd already handled from C, thus start in parent directory.
        path = vim.fs.dirname((vim.uv.cwd())),
      })
      for _, file in ipairs(files) do
        local trusted = vim.secure.read(file) --[[@as string|nil]]
        if trusted then
          if vim.endswith(file, '.lua') then
            assert(loadstring(trusted, '@' .. file))()
          else
            vim.api.nvim_exec2(trusted, {})
          end
        end
        -- If the user unset 'exrc' in the current exrc then stop searching
        if not vim.o.exrc then
          return
        end
      end
    end,
  })
end

--- Default options
do
  --- Default 'grepprg' to ripgrep if available.
  if vim.fn.executable('rg') == 1 then
    -- Use -uu to make ripgrep not check ignore files/skip dot-files
    vim.o.grepprg = 'rg --vimgrep -uu '
    vim.o.grepformat = '%f:%l:%c:%m'
  end
end
Repository: neovim/neovim
File: runtime/lua/vim/iter.lua (Lua)

Code before:
--- -- 1
--- it2:next()
--- -- nil
--- ```
---
---@generic A
---
---@param f integer|fun(a:A, i:integer):boolean Either the number if values to
---                                             take or a predicate which takes
---                                             a value and it's index.
---@return Iter
function Iter:take(f)
  if type(f) ~= 'function' then
    local n = f
    f = function(_, i)
      return i <= n
    end
  end

  local next = self.next
  local i = 1
  self.next = function()
    local val = next(self)
    if f(val, i) then
      i = i + 1
      return val
    end
  end
  return self
end

---@private
function ArrayIter:take(n)
  if type(n) == 'function' then
    local inc = self._head < self._tail and 1 or -1
    for i = self._head, self._tail, inc do
      if not n(unpack(self._table[i]), i) then
        self._tail = i
        break
      end
    end
    return self
  end

  local inc = self._head < self._tail and n or -n
  local cmp = self._head < self._tail and math.min or math.max
  self._tail = cmp(self._tail, self._head + inc)
  return self
end

--- "Pops" a value from a |list-iterator| (gets the last value and decrements the tail).

Review comment (performance):
This is not quite right because `next()` can return multiple values. So you could pack it into a table first, and then unpack it. But this is pretty bad for performance. You'll note that the implementations of `filter()` and `map()` have a similar problem and they go to great contortions to address it.

Also we shouldn't pass the index to the function, we don't do that for any other method. If the user wants the index they can call `enumerate()` first which includes the index as one of the iterator values. Staying consistent makes everything more composable.

Simple (but possibly slow) implementation:

```suggestion
    local val = pack(next(self))
    if f(unpack(val)) then
      return unpack(val)
```

For a solution that avoids the `pack` and `unpack` dance you can look at what `map()` does.

Code after:
--- -- 1
--- it2:next()
--- -- nil
--- ```
---
---@param n integer|fun(...):boolean Number of values to take or a predicate.
---@return Iter
function Iter:take(n)
  local i = 0
  local f = n
  if type(n) ~= 'function' then
    f = function()
      return i < n
    end
  end

  local stop = false
  local function fn(...)
    if not stop and select(1, ...) ~= nil and f(...) then
      i = i + 1
      return ...
    else
      stop = true
    end
  end

  local next = self.next
  self.next = function()
    return fn(next(self))
  end
  return self
end

---@private
function ArrayIter:take(n)
  if type(n) == 'function' then
    local inc = self._head < self._tail and 1 or -1
    for i = self._head, self._tail, inc do
      if not n(unpack(self._table[i])) then
        self._tail = i
        break
      end
    end
    return self
  end

  local inc = self._head < self._tail and n or -n
  local cmp = self._head < self._tail and math.min or math.max
  self._tail = cmp(self._tail, self._head + inc)
  return self
end
Repository: neovim/neovim
File: runtime/lua/vim/iter.lua (Lua)

Code before:
--- Transforms an iterator to yield only the first n values, or the first
--- values satisfying a predicate.
---
--- Example:
---
--- ```lua
--- local it = vim.iter({ 1, 2, 3, 4 }):take(2)
--- it:next()
--- -- 1
--- it:next()
--- -- 2
--- it:next()
--- -- nil
---
--- local function pred(x) return x < 2 end
--- local it2 = vim.iter({ 1, 2, 3, 4 }):take(pred)
--- it2:next()
--- -- 1
--- it2:next()
--- -- nil
--- ```
---
---@param n integer|fun(...):boolean Number of values to take or a predicate.
---@return Iter
function Iter:take(n)
  if type(n) == 'function' then
    local stop = false
    local function fn(...)
      if not stop and select(1, ...) ~= nil and n(...) then
        return ...
      else
        stop = true
      end
    end

    local next = self.next
    self.next = function()
      return fn(next(self))
    end
    return self
  end

  local next = self.next
  local i = 0
  self.next = function()
    if i < n then
      i = i + 1
      return next(self)
    end
  end
  return self

Review comment (refactor):
Does it simplify the code if the "number" case is implemented by setting `n` to a predicate? Same for skip().

Code after:
--- Transforms an iterator to yield only the first n values, or all values
--- satisfying a predicate.
---
--- Example:
---
--- ```lua
--- local it = vim.iter({ 1, 2, 3, 4 }):take(2)
--- it:next()
--- -- 1
--- it:next()
--- -- 2
--- it:next()
--- -- nil
---
--- local function pred(x) return x < 2 end
--- local it2 = vim.iter({ 1, 2, 3, 4 }):take(pred)
--- it2:next()
--- -- 1
--- it2:next()
--- -- nil
--- ```
---
---@param n integer|fun(...):boolean Number of values to take or a predicate.
---@return Iter
function Iter:take(n)
  local i = 0
  local f = n
  if type(n) ~= 'function' then
    f = function()
      return i < n
    end
  end

  local stop = false
  local function fn(...)
    if not stop and select(1, ...) ~= nil and f(...) then
      i = i + 1
      return ...
    else
      stop = true
    end
  end

  local next = self.next
  self.next = function()
    return fn(next(self))
  end
  return self
end

---@private
Repository: neovim/neovim
File: runtime/lua/vim/health/health.lua (Lua)

Code before:
    )
  end

  for _, env_var in ipairs({
    'XTERM_VERSION',
    'VTE_VERSION',
    'TERM_PROGRAM',
    'COLORTERM',
    'SSH_TTY',
  }) do
    if vim.env[env_var] then
      health.info(string.format('$%s="%s"', env_var, vim.env[env_var]))
    end
  end
end

local function check_external_tools()
  health.start('External Tools')

  if vim.fn.executable('rg') == 1 then
    local rg_path = vim.fn.exepath('rg')
    local rg_job = vim.system({ rg_path, '-V' }):wait()
    if rg_job.code == 0 then
      health.ok(('%s (%s)'):format(vim.trim(rg_job.stdout), rg_path))
    else
      health.warn('ripgrep is installed but failed to run `rg -V`', { rg_job.stderr })
    end
  else
    health.warn('ripgrep not available')
  end
end

function M.check()
  check_config()
  check_runtime()
  check_performance()
  check_rplugin_manifest()
  check_terminal()
  check_tmux()
  check_external_tools()
end

return M


Review comment (suggestion):
off-topic: `wait()` really should include a `cmd` field that has the command definition, so that callers don't need to store it separately, or repeat it as done literally here with `rg -V`.

Code after:
    )
  end

  for _, env_var in ipairs({
    'XTERM_VERSION',
    'VTE_VERSION',
    'TERM_PROGRAM',
    'COLORTERM',
    'SSH_TTY',
  }) do
    if vim.env[env_var] then
      health.info(string.format('$%s="%s"', env_var, vim.env[env_var]))
    end
  end
end

local function check_external_tools()
  health.start('External Tools')

  if vim.fn.executable('rg') == 1 then
    local rg_path = vim.fn.exepath('rg')
    local rg_job = vim.system({ rg_path, '-V' }):wait()
    if rg_job.code == 0 then
      health.ok(('%s (%s)'):format(vim.trim(rg_job.stdout), rg_path))
    else
      health.warn('found `rg` but failed to run `rg -V`', { rg_job.stderr })
    end
  else
    health.warn('ripgrep not available')
  end
end

function M.check()
  check_config()
  check_runtime()
  check_performance()
  check_rplugin_manifest()
  check_terminal()
  check_tmux()
  check_external_tools()
end

return M
Repository: neovim/neovim
File: runtime/lua/vim/health/health.lua (Lua)

Code before:
    )
  end

  for _, env_var in ipairs({
    'XTERM_VERSION',
    'VTE_VERSION',
    'TERM_PROGRAM',
    'COLORTERM',
    'SSH_TTY',
  }) do
    if vim.env[env_var] then
      health.info(string.format('$%s="%s"', env_var, vim.env[env_var]))
    end
  end
end

local function check_external_tools()
  health.start('External Tools')

  if vim.fn.executable('rg') == 1 then
    local rg_path = vim.fn.exepath('rg')
    local rg_job = vim.system({ rg_path, '-V' }):wait()
    if rg_job.code == 0 then
      health.ok(('%s (%s)'):format(vim.trim(rg_job.stdout), rg_path))
    else
      health.warn('ripgrep is installed but failed to run `rg -V`', { rg_job.stderr })
    end
  else
    health.warn('ripgrep not available')
  end
end

function M.check()
  check_config()
  check_runtime()
  check_performance()
  check_rplugin_manifest()
  check_terminal()
  check_tmux()
  check_external_tools()
end

return M


Review comment (bug):


```suggestion
      health.warn('found `rg` but failed to run `rg -V`', { rg_job.stderr })
```

Code after:
    )
  end

  for _, env_var in ipairs({
    'XTERM_VERSION',
    'VTE_VERSION',
    'TERM_PROGRAM',
    'COLORTERM',
    'SSH_TTY',
  }) do
    if vim.env[env_var] then
      health.info(string.format('$%s="%s"', env_var, vim.env[env_var]))
    end
  end
end

local function check_external_tools()
  health.start('External Tools')

  if vim.fn.executable('rg') == 1 then
    local rg_path = vim.fn.exepath('rg')
    local rg_job = vim.system({ rg_path, '-V' }):wait()
    if rg_job.code == 0 then
      health.ok(('%s (%s)'):format(vim.trim(rg_job.stdout), rg_path))
    else
      health.warn('found `rg` but failed to run `rg -V`', { rg_job.stderr })
    end
  else
    health.warn('ripgrep not available')
  end
end

function M.check()
  check_config()
  check_runtime()
  check_performance()
  check_rplugin_manifest()
  check_terminal()
  check_tmux()
  check_external_tools()
end

return M
Repository: neovim/neovim
File: runtime/lua/vim/health/health.lua (Lua)

Code before:
local M = {}
local health = require('vim.health')

---Run a system command and return ok and its stdout.
---@param cmd string[]
---@return boolean
---@return string
local function system(cmd)
  local result = vim.system(cmd, { text = true }):wait()
  -- NOTE: `or ''` to please the type checker
  return result.code == 0, vim.trim(result.stdout or '')
end

local suggest_faq = 'https://github.com/neovim/neovim/blob/master/BUILD.md#building'

local function check_runtime()
  health.start('Runtime')
  -- Files from an old installation.
  local bad_files = {
    ['autoload/health/nvim.vim'] = false,
    ['autoload/health/provider.vim'] = false,
    ['autoload/man.vim'] = false,
    ['lua/provider/node/health.lua'] = false,
    ['lua/provider/perl/health.lua'] = false,
    ['lua/provider/python/health.lua'] = false,
    ['lua/provider/ruby/health.lua'] = false,
    ['plugin/health.vim'] = false,
    ['plugin/man.vim'] = false,
    ['queries/help/highlights.scm'] = false,
    ['queries/help/injections.scm'] = false,
    ['scripts.vim'] = false,
    ['syntax/syncolor.vim'] = false,
  }
  local bad_files_msg = ''
  for k, _ in pairs(bad_files) do

Review comment (suggestion):
don't need this comment. it's a common and idiomatic pattern (and the type checker is signaling a valid problem)

Code after:
local M = {}
local health = require('vim.health')

---Run a system command and return ok and its stdout and stderr combined.
---@param cmd string[]
---@return boolean
---@return string
local function system(cmd)
  local result = vim.system(cmd, { text = true }):wait()
  return result.code == 0, vim.trim(('%s\n%s'):format(result.stdout, result.stderr))
end

local suggest_faq = 'https://github.com/neovim/neovim/blob/master/BUILD.md#building'

local function check_runtime()
  health.start('Runtime')
  -- Files from an old installation.
  local bad_files = {
    ['autoload/health/nvim.vim'] = false,
    ['autoload/health/provider.vim'] = false,
    ['autoload/man.vim'] = false,
    ['lua/provider/node/health.lua'] = false,
    ['lua/provider/perl/health.lua'] = false,
    ['lua/provider/python/health.lua'] = false,
    ['lua/provider/ruby/health.lua'] = false,
    ['plugin/health.vim'] = false,
    ['plugin/man.vim'] = false,
    ['queries/help/highlights.scm'] = false,
    ['queries/help/injections.scm'] = false,
    ['scripts.vim'] = false,
    ['syntax/syncolor.vim'] = false,
  }
  local bad_files_msg = ''
  for k, _ in pairs(bad_files) do
    local path = ('%s/%s'):format(vim.env.VIMRUNTIME, k)