Skip to content

Commit a428ceb

Browse files
committed
refactor: improve code quality based on PR review feedback
- Enhanced error handling in diff.lua with proper return values - Optimized debug logging to avoid string operations when disabled - Documented timing configuration changes with rationale - Removed redundant comments that added no value - Added backward compatibility for test environments Change-Id: Ic9ac0aa77506114955b740b42ee3d1af0bc7fea0 Signed-off-by: Thomas Kosiewski <[email protected]>
1 parent 648c90a commit a428ceb

File tree

4 files changed

+42
-25
lines changed

4 files changed

+42
-25
lines changed

lua/claudecode/config.lua

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@ M.defaults = {
1818
}
1919

2020
--- @param config table The configuration table to validate.
21-
-- @return boolean true if the configuration is valid.
22-
-- @error string if any configuration option is invalid.
2321
function M.validate(config)
2422
assert(
2523
type(config.port_range) == "table"
@@ -63,7 +61,6 @@ function M.validate(config)
6361
end
6462

6563
--- @param user_config table|nil The user-provided configuration table.
66-
-- @return table The final, validated configuration table.
6764
function M.apply(user_config)
6865
local config = vim.deepcopy(M.defaults)
6966

lua/claudecode/diff.lua

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -399,11 +399,14 @@ end
399399
--- Apply accepted changes to the original file and reload open buffers
400400
-- @param diff_data table The diff state data
401401
-- @param final_content string The final content to write
402+
-- @return boolean success Whether the operation succeeded
403+
-- @return string|nil error Error message if operation failed
402404
function M._apply_accepted_changes(diff_data, final_content)
403405
local old_file_path = diff_data.old_file_path
404406
if not old_file_path then
405-
require("claudecode.logger").error("diff", "No old_file_path found in diff_data")
406-
return
407+
local error_msg = "No old_file_path found in diff_data"
408+
require("claudecode.logger").error("diff", error_msg)
409+
return false, error_msg
407410
end
408411

409412
require("claudecode.logger").debug("diff", "Writing accepted changes to file:", old_file_path)
@@ -415,14 +418,9 @@ function M._apply_accepted_changes(diff_data, final_content)
415418
require("claudecode.logger").debug("diff", "Creating parent directories for new file:", parent_dir)
416419
local mkdir_success, mkdir_err = pcall(vim.fn.mkdir, parent_dir, "p")
417420
if not mkdir_success then
418-
require("claudecode.logger").error(
419-
"diff",
420-
"Failed to create parent directories:",
421-
parent_dir,
422-
"error:",
423-
mkdir_err
424-
)
425-
return
421+
local error_msg = "Failed to create parent directories: " .. parent_dir .. " - " .. tostring(mkdir_err)
422+
require("claudecode.logger").error("diff", error_msg)
423+
return false, error_msg
426424
end
427425
require("claudecode.logger").debug("diff", "Successfully created parent directories:", parent_dir)
428426
end
@@ -433,8 +431,9 @@ function M._apply_accepted_changes(diff_data, final_content)
433431
local success, err = pcall(vim.fn.writefile, lines, old_file_path)
434432

435433
if not success then
436-
require("claudecode.logger").error("diff", "Failed to write file:", old_file_path, "error:", err)
437-
return
434+
local error_msg = "Failed to write file: " .. old_file_path .. " - " .. tostring(err)
435+
require("claudecode.logger").error("diff", error_msg)
436+
return false, error_msg
438437
end
439438

440439
require("claudecode.logger").debug("diff", "Successfully wrote changes to", old_file_path)
@@ -454,6 +453,8 @@ function M._apply_accepted_changes(diff_data, final_content)
454453
end
455454
end
456455
end
456+
457+
return true, nil
457458
end
458459

459460
--- Resolve diff as accepted with final content

lua/claudecode/init.lua

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ local default_config = {
4646
terminal_cmd = nil,
4747
log_level = "info",
4848
track_selection = true,
49-
visual_demotion_delay_ms = 50,
49+
visual_demotion_delay_ms = 50, -- Reduced from 200ms for better responsiveness in tree navigation
5050
diff_opts = {
5151
auto_close_on_accept = true,
5252
show_diff_stats = true,
@@ -294,17 +294,25 @@ function M._create_commands()
294294

295295
local broadcast_success = M.state.server.broadcast("at_mentioned", params)
296296
if broadcast_success then
297-
local message = "Broadcast success: Added " .. (is_directory and "directory" or "file") .. " " .. formatted_path
298-
if not is_directory and (start_line or end_line) then
299-
local range_info = ""
300-
if start_line and end_line then
301-
range_info = " (lines " .. start_line .. "-" .. end_line .. ")"
302-
elseif start_line then
303-
range_info = " (from line " .. start_line .. ")"
297+
if logger.is_level_enabled and logger.is_level_enabled("debug") then
298+
local message = "Broadcast success: Added " .. (is_directory and "directory" or "file") .. " " .. formatted_path
299+
if not is_directory and (start_line or end_line) then
300+
local range_info = ""
301+
if start_line and end_line then
302+
range_info = " (lines " .. start_line .. "-" .. end_line .. ")"
303+
elseif start_line then
304+
range_info = " (from line " .. start_line .. ")"
305+
end
306+
message = message .. range_info
304307
end
305-
message = message .. range_info
308+
logger.debug("command", message)
309+
elseif not logger.is_level_enabled then
310+
-- Fallback for tests or environments where logger isn't fully initialized
311+
logger.debug(
312+
"command",
313+
"Broadcast success: Added " .. (is_directory and "directory" or "file") .. " " .. formatted_path
314+
)
306315
end
307-
logger.debug("command", message)
308316
return true, nil
309317
else
310318
local error_msg = "Failed to broadcast " .. (is_directory and "directory" or "file") .. " " .. formatted_path

lua/claudecode/logger.lua

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,17 @@ function M.info(component, ...)
112112
end
113113
end
114114

115+
--- Check if a specific log level is enabled
116+
-- @param level_name string The level name ("error", "warn", "info", "debug", "trace")
117+
-- @return boolean Whether the level is enabled
118+
function M.is_level_enabled(level_name)
119+
local level_value = level_values[level_name]
120+
if not level_value then
121+
return false
122+
end
123+
return level_value <= current_log_level_value
124+
end
125+
115126
--- @param component string|nil Optional component/module name.
116127
-- @param ... any Varargs representing parts of the message.
117128
function M.debug(component, ...)

0 commit comments

Comments
 (0)