diff --git a/CHANGELOG.md b/CHANGELOG.md index b121d57..825ed5d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Comment files now use status prefixes: `action-required_*.md`, `waiting-review_*.md`, `resolved_*.md` - Status automatically updates when replying to threads - Status-specific colors and icons in UI +- Optional status management configuration + - New `comment.status_management` configuration option (default: false) + - Status management only works when both `storage.backend = "file"` and `status_management = true` + - Warning messages when trying to resolve/reopen threads with status management disabled ## [0.4.0] - 2025-07-07 diff --git a/README.md b/README.md index d04f00c..8763fe2 100644 --- a/README.md +++ b/README.md @@ -149,6 +149,9 @@ require('code-review').setup({ -- Comments from this author trigger "waiting-review" status -- Comments from other authors trigger "action-required" status claude_code_author = 'Claude Code', + -- Enable filename-based status management (only works with file storage backend) + -- When enabled, review files are prefixed with status (action-required_, waiting-review_, resolved_) + status_management = false, }, -- Keymaps (set to false to disable all keymaps) keymaps = { diff --git a/lua/code-review/config.lua b/lua/code-review/config.lua index fd86048..66ee9c0 100644 --- a/lua/code-review/config.lua +++ b/lua/code-review/config.lua @@ -71,6 +71,9 @@ local defaults = { -- Comments from this author trigger "waiting-review" status -- Comments from other authors trigger "action-required" status claude_code_author = "Claude Code", + -- Enable filename-based status management (only works with file storage backend) + -- When enabled, review files are prefixed with status (action-required_, waiting-review_, resolved_) + status_management = false, }, -- Keymaps (set to false to disable all keymaps) keymaps = { diff --git a/lua/code-review/state.lua b/lua/code-review/state.lua index 3627d0e..3b2ad4e 100644 --- a/lua/code-review/state.lua +++ b/lua/code-review/state.lua @@ -84,7 +84,7 @@ function M.add_comment(comment_data) local comment = storage_backend.get(id) if comment then comment.thread_id = thread_id - -- Status is now managed by filename, not in data + -- Status is now managed by filename (if status_management is enabled), not in data -- Re-save with thread info storage_backend.delete(id) @@ -225,6 +225,12 @@ function M.resolve_thread(thread_id) if success then M.refresh_ui() vim.notify("Thread resolved", vim.log.levels.INFO) + else + -- Check if status management is disabled + local config = require("code-review.config") + if config.get("comment.storage.backend") == "file" and not config.get("comment.status_management") then + vim.notify("Status management is disabled. Enable 'status_management' to resolve threads.", vim.log.levels.WARN) + end end return success @@ -240,6 +246,12 @@ function M.reopen_thread(thread_id) if success then M.refresh_ui() vim.notify("Thread reopened", vim.log.levels.INFO) + else + -- Check if status management is disabled + local config = require("code-review.config") + if config.get("comment.storage.backend") == "file" and not config.get("comment.status_management") then + vim.notify("Status management is disabled. Enable 'status_management' to reopen threads.", vim.log.levels.WARN) + end end return success diff --git a/lua/code-review/storage/file.lua b/lua/code-review/storage/file.lua index 8b1a735..7f75c8e 100644 --- a/lua/code-review/storage/file.lua +++ b/lua/code-review/storage/file.lua @@ -8,18 +8,23 @@ local cache_timestamp = 0 --- Parse status from filename ---@param filename string ----@return string status, string id +---@return string|nil status, string id local function parse_filename(filename) - -- Pattern: status_timestamp_thread.md - local status, id = filename:match("^([^_]+)_(.+)%.md$") - if status and id then - return status, id + local config = require("code-review.config") + local status_management = config.get("comment.status_management") + + if status_management then + -- Pattern: status_timestamp_thread.md + local status, id = filename:match("^([^_]+)_(.+)%.md$") + if status and id then + return status, id + end end - -- Legacy format: timestamp_thread.md + -- Legacy format or status_management disabled: timestamp_thread.md local legacy_id = filename:match("^(.+)%.md$") if legacy_id then - return "action-required", legacy_id + return status_management and "action-required" or nil, legacy_id end return nil, nil @@ -27,10 +32,17 @@ end --- Generate filename with status ---@param id string ----@param status string +---@param status string|nil ---@return string local function make_filename(id, status) - return status .. "_" .. id .. ".md" + local config = require("code-review.config") + local status_management = config.get("comment.status_management") + + if status_management and status then + return status .. "_" .. id .. ".md" + else + return id .. ".md" + end end --- Determine thread status based on latest author @@ -73,6 +85,9 @@ end ---@param status string? Optional status override ---@return string local function get_comment_filename(comment_data, status) + local config = require("code-review.config") + local status_management = config.get("comment.status_management") + local id if comment_data.id then -- Extract ID from existing filename if needed @@ -84,8 +99,12 @@ local function get_comment_filename(comment_data, status) id = filename:match("^(.+)%.md$") end - -- Default status for new comments is "action-required" - status = status or "action-required" + -- Default status for new comments is "action-required" if status management is enabled + if status_management then + status = status or "action-required" + else + status = nil + end return make_filename(id, status) end @@ -192,7 +211,7 @@ local function parse_comment_from_file(content, filename) timestamp = parsed_timestamp, context_lines = context_lines, thread_id = frontmatter.thread_id, - thread_status = status, -- Add status from filename + thread_status = status, -- Add status from filename (may be nil if status_management is disabled) } elseif line == "---" and in_comments_section then -- luacheck: ignore 542 -- Comment separator, ignore @@ -343,13 +362,26 @@ function M.add(comment_data) return (a.timestamp or 0) < (b.timestamp or 0) end) - -- Determine new status based on latest author - local new_status = determine_thread_status(thread_comments) + local config = require("code-review.config") + local status_management = config.get("comment.status_management") + + -- Determine new status based on latest author (only if status management is enabled) + local new_status = nil + if status_management then + new_status = determine_thread_status(thread_comments) + end -- Get current filename from existing file - local old_files = vim.fn.glob(get_storage_dir() .. "/*_" .. root_comment.id .. ".md", false, true) + local pattern = status_management and ("/*_" .. root_comment.id .. ".md") or ("/" .. root_comment.id .. ".md") + local old_files = vim.fn.glob(get_storage_dir() .. pattern, false, true) local old_filepath = old_files[1] + -- Fallback for files without status prefix when status_management is enabled + if not old_filepath and status_management then + old_files = vim.fn.glob(get_storage_dir() .. "/" .. root_comment.id .. ".md", false, true) + old_filepath = old_files[1] + end + -- Generate new filename with updated status local new_filename = make_filename(root_comment.id, new_status) local new_filepath = get_storage_dir() .. "/" .. new_filename @@ -416,16 +448,20 @@ end ---@return boolean success function M.delete(id) local dir = get_storage_dir() - - -- Find file with any status prefix - local files = vim.fn.glob(dir .. "/*_" .. id .. ".md", false, true) - if #files > 0 then - vim.fn.delete(files[1]) - invalidate_cache() - return true + local config = require("code-review.config") + local status_management = config.get("comment.status_management") + + -- Find file with any status prefix (if status management is enabled) + if status_management then + local files = vim.fn.glob(dir .. "/*_" .. id .. ".md", false, true) + if #files > 0 then + vim.fn.delete(files[1]) + invalidate_cache() + return true + end end - -- Fallback for legacy format + -- Fallback for legacy format or status_management disabled local legacy_filepath = dir .. "/" .. id .. ".md" if vim.fn.filereadable(legacy_filepath) == 1 then vim.fn.delete(legacy_filepath) @@ -524,19 +560,25 @@ end ---@return table|nil function M.get_thread(thread_id) local comments = load_comments() + local config = require("code-review.config") + local status_management = config.get("comment.status_management") -- Find the root comment of this thread for _, comment in ipairs(comments) do if comment.thread_id == thread_id and (not comment.parent_id or comment.id == thread_id:match("^(.+)_thread$")) then - -- Find the file to get status - local files = vim.fn.glob(get_storage_dir() .. "/*_" .. comment.id .. ".md", false, true) - local status = "action-required" - - if files[1] then - local filename = vim.fn.fnamemodify(files[1], ":t") - local parsed_status = parse_filename(filename) - if parsed_status then - status = parsed_status + local status = nil + + if status_management then + -- Find the file to get status + local files = vim.fn.glob(get_storage_dir() .. "/*_" .. comment.id .. ".md", false, true) + status = "action-required" + + if files[1] then + local filename = vim.fn.fnamemodify(files[1], ":t") + local parsed_status = parse_filename(filename) + if parsed_status then + status = parsed_status + end end end @@ -562,6 +604,14 @@ end ---@param resolved_by string|nil User who resolved (unused now) ---@return boolean success function M.update_thread_status(thread_id, status, resolved_by) + local config = require("code-review.config") + local status_management = config.get("comment.status_management") + + -- If status management is disabled, return false to indicate no action taken + if not status_management then + return false + end + local comments = load_comments() -- Find root comment of this thread @@ -596,6 +646,12 @@ function M.update_thread_status(thread_id, status, resolved_by) local old_files = vim.fn.glob(get_storage_dir() .. "/*_" .. root_comment.id .. ".md", false, true) local old_filepath = old_files[1] + -- Fallback for files without status prefix + if not old_filepath then + old_files = vim.fn.glob(get_storage_dir() .. "/" .. root_comment.id .. ".md", false, true) + old_filepath = old_files[1] + end + if not old_filepath then return false end @@ -617,6 +673,8 @@ end ---@return table function M.get_all_threads() local comments = load_comments() + local config = require("code-review.config") + local status_management = config.get("comment.status_management") local threads = {} local thread_files = {} @@ -625,15 +683,18 @@ function M.get_all_threads() for _, filepath in ipairs(files) do local filename = vim.fn.fnamemodify(filepath, ":t") local status, id = parse_filename(filename) - if status and id then - thread_files[id] = status + if id then + thread_files[id] = status -- status may be nil if status_management is disabled end end -- Extract thread info from root comments for _, comment in ipairs(comments) do if comment.thread_id and (not comment.parent_id or comment.id == comment.thread_id:match("^(.+)_thread$")) then - local status = thread_files[comment.id] or "action-required" + local status = nil + if status_management then + status = thread_files[comment.id] or "action-required" + end threads[comment.thread_id] = { id = comment.thread_id, status = status, diff --git a/tests/test_filename_status.lua b/tests/test_filename_status.lua index 79e99f7..2eaebc8 100644 --- a/tests/test_filename_status.lua +++ b/tests/test_filename_status.lua @@ -1,11 +1,13 @@ local state = require("code-review.state") local file_storage = require("code-review.storage.file") +local config = require("code-review.config") --- Initialize plugin with file storage for realistic testing +-- Initialize plugin with file storage and status management enabled for testing require("code-review").setup({ comment = { storage = { backend = "file" }, claude_code_author = "Claude Code", + status_management = true, -- Enable for testing }, }) @@ -31,8 +33,13 @@ T["filename status management"] = MiniTest.new_set({ }) T["filename status management"]["parse_filename extracts status and id"] = function() - -- Test various filename formats - local test_cases = { + -- Save original status_management setting + local original_status_management = config.get("comment.status_management") + + -- Test with status_management enabled + config.get_all().comment.status_management = true + + local test_cases_enabled = { { filename = "action-required_1234567890.md", expected_status = "action-required", @@ -53,7 +60,6 @@ T["filename status management"]["parse_filename extracts status and id"] = funct expected_status = "action-required", expected_id = "1234567890_thread", }, - -- Test invalid formats -- Test legacy format (backward compatibility) { filename = "1234567890.md", @@ -68,16 +74,51 @@ T["filename status management"]["parse_filename extracts status and id"] = funct }, } - for _, test in ipairs(test_cases) do + for _, test in ipairs(test_cases_enabled) do local status, id = file_storage.parse_filename(test.filename) MiniTest.expect.equality(status, test.expected_status) MiniTest.expect.equality(id, test.expected_id) end + + -- Test with status_management disabled + config.get_all().comment.status_management = false + + local test_cases_disabled = { + { + filename = "action-required_1234567890.md", + expected_status = nil, + expected_id = "action-required_1234567890", -- Entire string is treated as ID + }, + { + filename = "1234567890.md", + expected_status = nil, + expected_id = "1234567890", + }, + { + filename = "invalid.txt", + expected_status = nil, + expected_id = nil, + }, + } + + for _, test in ipairs(test_cases_disabled) do + local status, id = file_storage.parse_filename(test.filename) + MiniTest.expect.equality(status, test.expected_status) + MiniTest.expect.equality(id, test.expected_id) + end + + -- Restore original setting + config.get_all().comment.status_management = original_status_management end T["filename status management"]["make_filename creates correct filename"] = function() - -- Test filename generation - local test_cases = { + -- Save original status_management setting + local original_status_management = config.get("comment.status_management") + + -- Test with status_management enabled + config.get_all().comment.status_management = true + + local test_cases_enabled = { { id = "1234567890", status = "action-required", @@ -100,10 +141,39 @@ T["filename status management"]["make_filename creates correct filename"] = func }, } - for _, test in ipairs(test_cases) do + for _, test in ipairs(test_cases_enabled) do local filename = file_storage.make_filename(test.id, test.status) MiniTest.expect.equality(filename, test.expected) end + + -- Test with status_management disabled + config.get_all().comment.status_management = false + + local test_cases_disabled = { + { + id = "1234567890", + status = "action-required", -- Status should be ignored + expected = "1234567890.md", + }, + { + id = "9876543210", + status = "waiting-review", -- Status should be ignored + expected = "9876543210.md", + }, + { + id = "1234567890", + status = nil, + expected = "1234567890.md", + }, + } + + for _, test in ipairs(test_cases_disabled) do + local filename = file_storage.make_filename(test.id, test.status) + MiniTest.expect.equality(filename, test.expected) + end + + -- Restore original setting + config.get_all().comment.status_management = original_status_management end T["filename status management"]["determine_thread_status returns correct status"] = function() @@ -162,20 +232,20 @@ T["filename status management"]["file rename on reply"] = function() vim.wait(100) -- Check initial filename - local files = vim.fn.glob(".code-review/waiting-review_" .. root_id .. ".md", false, true) + local files = vim.fn.glob(".code-review/action-required_" .. root_id .. ".md", false, true) MiniTest.expect.equality(#files, 1) -- Add a reply from Claude Code - state.add_reply(root_id, "I'll fix this", "Claude Code") + state.add_reply(root_id, "I'll fix this") -- Wait for file operations vim.wait(100) - -- Check that file was renamed to action-required - local old_files = vim.fn.glob(".code-review/waiting-review_" .. root_id .. ".md", false, true) + -- Check that file was renamed to waiting-review + local old_files = vim.fn.glob(".code-review/action-required_" .. root_id .. ".md", false, true) MiniTest.expect.equality(#old_files, 0) - local new_files = vim.fn.glob(".code-review/action-required_" .. root_id .. ".md", false, true) + local new_files = vim.fn.glob(".code-review/waiting-review_" .. root_id .. ".md", false, true) MiniTest.expect.equality(#new_files, 1) end @@ -189,27 +259,25 @@ T["filename status management"]["thread file operations"] = function() author = "User", }) - local thread_id = comment_id .. "_thread" - -- Wait for file creation vim.wait(100) - -- Check thread file exists with correct status - local thread_files = vim.fn.glob(".code-review/waiting-review_" .. thread_id .. ".md", false, true) - MiniTest.expect.equality(#thread_files, 1) + -- Check comment file exists with correct status + local comment_files = vim.fn.glob(".code-review/action-required_" .. comment_id .. ".md", false, true) + MiniTest.expect.equality(#comment_files, 1) -- Add reply from Claude Code to trigger status change - state.add_reply(comment_id, "Working on it", "Claude Code") + state.add_reply(comment_id, "Working on it") -- Wait for file operations vim.wait(100) - -- Check that thread file was renamed - local old_thread_files = vim.fn.glob(".code-review/waiting-review_" .. thread_id .. ".md", false, true) - MiniTest.expect.equality(#old_thread_files, 0) + -- Check that comment file was renamed + local old_comment_files = vim.fn.glob(".code-review/action-required_" .. comment_id .. ".md", false, true) + MiniTest.expect.equality(#old_comment_files, 0) - local new_thread_files = vim.fn.glob(".code-review/action-required_" .. thread_id .. ".md", false, true) - MiniTest.expect.equality(#new_thread_files, 1) + local new_comment_files = vim.fn.glob(".code-review/waiting-review_" .. comment_id .. ".md", false, true) + MiniTest.expect.equality(#new_comment_files, 1) end T["filename status management"]["wildcard search for file updates"] = function() @@ -283,9 +351,9 @@ T["filename status management"]["status preserved in list view"] = function() end end - -- Check that thread_status is set based on filename - MiniTest.expect.equality(comment1.thread_status, "waiting-review") - MiniTest.expect.equality(comment2.thread_status, "action-required") + -- Check that thread_status is set based on filename (when status_management is enabled) + MiniTest.expect.equality(comment1.thread_status, "action-required") + MiniTest.expect.equality(comment2.thread_status, "waiting-review") end T["filename status management"]["resolve thread updates filename"] = function() @@ -309,16 +377,94 @@ T["filename status management"]["resolve thread updates filename"] = function() -- Wait for file operations vim.wait(100) - -- Check that thread file was renamed to resolved - local waiting_files = vim.fn.glob(".code-review/waiting-review_" .. thread_id .. ".md", false, true) - MiniTest.expect.equality(#waiting_files, 0) + -- Check that comment file was renamed to resolved + local action_files = vim.fn.glob(".code-review/action-required_" .. comment_id .. ".md", false, true) + MiniTest.expect.equality(#action_files, 0) - local resolved_files = vim.fn.glob(".code-review/resolved_" .. thread_id .. ".md", false, true) + local resolved_files = vim.fn.glob(".code-review/resolved_" .. comment_id .. ".md", false, true) MiniTest.expect.equality(#resolved_files, 1) +end + +T["filename status management"]["status_management disabled"] = function() + -- Save original setting + local original_status_management = config.get("comment.status_management") + + -- Disable status management + config.get_all().comment.status_management = false + + -- Clear any existing comments + state.clear() + + -- Create a comment + local comment_id = state.add_comment({ + file = "disabled_test.lua", + line_start = 1, + line_end = 1, + comment = "Test without status management", + author = "User", + }) + + -- Wait for file creation + vim.wait(100) + + -- Check that file has no status prefix + local status_files = vim.fn.glob(".code-review/*_" .. comment_id .. ".md", false, true) + MiniTest.expect.equality(#status_files, 0) + + local plain_files = vim.fn.glob(".code-review/" .. comment_id .. ".md", false, true) + MiniTest.expect.equality(#plain_files, 1) + + -- Add a reply + state.add_reply(comment_id, "Reply without status change") + + -- Wait for file operations + vim.wait(100) + + -- Check that file still has no status prefix + local status_files_after = vim.fn.glob(".code-review/*_" .. comment_id .. ".md", false, true) + MiniTest.expect.equality(#status_files_after, 0) + + local plain_files_after = vim.fn.glob(".code-review/" .. comment_id .. ".md", false, true) + MiniTest.expect.equality(#plain_files_after, 1) + + -- Resolve thread should not rename file and should show warning + local thread_id = comment_id .. "_thread" + + -- Capture notifications + local notifications = {} + local original_notify = vim.notify + vim.notify = function(msg, level) -- luacheck: ignore 122 + table.insert(notifications, { msg = msg, level = level }) + end + + local success = state.resolve_thread(thread_id) + + -- Restore original notify + vim.notify = original_notify -- luacheck: ignore 122 + + -- Should return false when status management is disabled + MiniTest.expect.equality(success, false) + + -- Should show warning notification + MiniTest.expect.equality(#notifications, 1) + MiniTest.expect.equality( + notifications[1].msg, + "Status management is disabled. Enable 'status_management' to resolve threads." + ) + MiniTest.expect.equality(notifications[1].level, vim.log.levels.WARN) + + -- Wait for operations + vim.wait(100) + + -- File should still have no status prefix + local resolved_files = vim.fn.glob(".code-review/resolved_" .. comment_id .. ".md", false, true) + MiniTest.expect.equality(#resolved_files, 0) + + local plain_files_final = vim.fn.glob(".code-review/" .. comment_id .. ".md", false, true) + MiniTest.expect.equality(#plain_files_final, 1) - -- Also check comment file - local comment_resolved_files = vim.fn.glob(".code-review/resolved_" .. comment_id .. ".md", false, true) - MiniTest.expect.equality(#comment_resolved_files, 1) + -- Restore original setting + config.get_all().comment.status_management = original_status_management end return T diff --git a/tests/test_thread.lua b/tests/test_thread.lua index ab3b9c2..0e760b1 100644 --- a/tests/test_thread.lua +++ b/tests/test_thread.lua @@ -182,6 +182,7 @@ T["thread management"]["preserves thread state across storage backends"] = funct require("code-review").setup({ comment = { storage = { backend = "file" }, + status_management = true, -- Enable status management for this test }, }) state.init()