Skip to content

Commit 1952ba2

Browse files
committed
fix(terminal): add buffer validation guards and improve tests
Addresses CodeRabbit feedback by adding pcall protection around buffer option API calls and enhancing test quality with explicit assertions. - Add pcall wrapper for nvim_get_option_value to prevent invalid buffer errors - Replace hardcoded test values with dynamic calculations - Improve test assertions with explicit configuration checks - Add clearer comments for buffer name validation patterns
1 parent 1617a92 commit 1952ba2

2 files changed

Lines changed: 19 additions & 9 deletions

File tree

lua/claude-code/terminal.lua

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,11 @@ local function is_valid_terminal_buffer(bufnr)
271271
return false
272272
end
273273

274-
local buftype = vim.api.nvim_get_option_value('buftype', {buf = bufnr})
274+
local buftype = nil
275+
pcall(function()
276+
buftype = vim.api.nvim_get_option_value('buftype', {buf = bufnr})
277+
end)
278+
275279
local terminal_job_id = nil
276280
pcall(function()
277281
terminal_job_id = vim.b[bufnr].terminal_job_id

tests/spec/terminal_spec.lua

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,8 @@ describe('terminal module', function()
350350
-- Extract buffer name from the file command and check it doesn't have invalid chars
351351
local buffer_name = cmd:match('file (.+)')
352352
if buffer_name then
353-
assert.is_nil(buffer_name:match('[^%w%-_]'), 'Buffer name should not contain special characters')
353+
-- Buffer names should only contain alphanumeric, hyphen, and underscore
354+
assert.is_nil(buffer_name:match('[^%w%-_]'), 'Buffer name should only contain [a-zA-Z0-9_-]')
354355
end
355356
break
356357
end
@@ -668,9 +669,11 @@ describe('terminal module', function()
668669
local expected_height = math.floor(editor_height * 0.5) -- 50% of 38
669670
assert.are.equal(expected_width, nvim_open_win_config.width)
670671
assert.are.equal(expected_height, nvim_open_win_config.height)
671-
-- Verify percentage calculations are independent of hardcoded values
672-
assert.are.equal(96, expected_width)
673-
assert.are.equal(19, expected_height) -- floor(38 * 0.5) = 19
672+
-- Verify percentage calculations match expected formula
673+
local mock_columns = 120
674+
local mock_editor_height = 38
675+
assert.are.equal(math.floor(mock_columns * 0.8), expected_width)
676+
assert.are.equal(math.floor(mock_editor_height * 0.5), expected_height)
674677
end)
675678

676679
it('should center floating window when position is "center"', function()
@@ -717,10 +720,13 @@ describe('terminal module', function()
717720

718721
-- Should open floating window with existing buffer
719722
assert.is_true(nvim_open_win_called, 'nvim_open_win should be called')
720-
-- Validate the window was created successfully
721-
assert.is_not_nil(nvim_open_win_config)
722-
-- In the reuse case, the buffer validation happens inside create_float
723-
-- This test primarily ensures the floating window path is taken correctly
723+
-- Verify floating window configuration is correct
724+
assert.is_not_nil(nvim_open_win_config, 'Window config should be provided')
725+
assert.are.equal('editor', nvim_open_win_config.relative)
726+
assert.are.equal('none', nvim_open_win_config.border)
727+
728+
-- Verify existing buffer is reused (buffer 42 from instances)
729+
-- The specific buffer reuse logic is tested implicitly through the toggle function
724730
end)
725731

726732
it('should handle out-of-bounds dimensions gracefully', function()

0 commit comments

Comments
 (0)