diff --git a/.claude/review-lessons.md b/.claude/review-lessons.md index d6ce5b3..07ddf39 100644 --- a/.claude/review-lessons.md +++ b/.claude/review-lessons.md @@ -24,6 +24,11 @@ - **対策**: 呼び出し側の規約に依存せず、関数冒頭で空入力を弾いてエラーcallbackする。配列をJSONエンコードする関数では空テーブルがオブジェクトになるLuaの仕様を常に考慮する - **該当箇所**: lua/fude/gh.lua +### エッジケース: 集約判定は部分集合ではなく論理的対象の全体に評価する (PR #163, 2026-07-23) +- **問題**: inline modeでresolvedコメントのboxを隠す際、行末フォールバックの`[resolved]`判定を「隠した部分集合(hidden_resolved)」だけで行っていたため、同一行に未解決コメントが混在しても`all_comments_resolved`が常にtrueになり、誤って`[resolved]`が表示された。「この行は全部resolvedか」という集約述語に行の一部だけを渡して意味が変わっていた +- **対策**: 集約述語(all_X / every_X 等)は、その述語が論理的に対象とする集合の全体(行の全コメント等)に対して評価する。フィルタ後の部分集合を渡すと述語の意味が壊れる。あわせて、要素を振り分けるフィルタ条件と集約述語は同じフィールド(`is_resolved`)で判定し、食い違いを構造的に排除する +- **該当箇所**: lua/fude/ui/extmarks.lua + ### テスト: 実クロックと比較されるfixture日付の時限爆弾 (PR #155, 2026-07-06) - **問題**: retention pruneを通る実ロードパスのテストで、fixtureの`saved_at`をハードコードした過去日付にしていたため、日付経過でretention window(30日)から外れテストが壊れた。動的な`os.date()`(現在時刻)への修正も「実行時刻依存で再現性が落ちる」とレビュー指摘を受けた - **対策**: 実時刻(`os.time()`)と比較される経路を通るfixtureのタイムスタンプは、固定の十分未来の日付(例: `2126-01-01T00:00:00Z`)を使う。過去日付のハードコードは時限爆弾、現在時刻の動的生成は再現性低下。時刻を注入できる純粋関数(`prune(t, now, days)`等)のテストは固定`now`を渡して書く diff --git a/CLAUDE.md b/CLAUDE.md index 8e18482..916a35a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -36,11 +36,11 @@ All plugin code lives under `lua/fude/`. The plugin entry point is `plugin/fude. - **`local/store.lua`** — Append-only JSONL event store for local (pre-PR) review sessions, persisted inside the worktree (`.fude/reviews/.jsonl` + `.fude/current.json` pointer; `M._dir` overrides the base dir for tests). One JSON event per line; kinds: `session`/`comment`/`reply`/`edit`/`move`/`resolve`/`reopen`/`delete`/`viewed`. Pure functions: `generate_uuid`, `make_session_id`, `serialize_event`, `parse_event_line`/`parse_events` (corrupt lines skipped), `materialize(events)` (rebuilds GitHub-compatible comment objects + thread resolved state + a per-path viewed map; replies inherit the root's path/line including moves; deleted comments are hidden but stay on disk as audit trail), `apply_outdated(comments, line_counts)` (file gone or line > EOF → `is_outdated`), `reanchor(comments, file_lines_map)` (context-based re-anchor: moves a comment to the unique location of its saved `context` block, re-propagates to replies, returns the moves to persist), and `build_*_event` constructors. IO: `append_event`, `read_events`, and the current-session pointer helpers `write_current(root, branch, session)`/`read_current(root, branch)`/`clear_current(root, branch)`. `.fude/current.json` is a **branch-keyed map** (`{ [branch] = session }`) so reviewing several branches in the same worktree doesn't collide; a legacy flat single-session pointer is migrated on read. - **`local/session.lua`** — Local review session lifecycle, parallel in shape to `init.lua` start/stop/reload. `start(base)` resolves the base ref (arg → `current.json` → default branch), computes the diff base from the scope, synthesizes `changed_files` from `git diff --name-status/--numstat` + untracked files (pure helpers: `status_word`, `resolve_rename_path`, `parse_name_status`, `parse_numstat`, `build_changed_files`), creates or resumes the session for the current branch (`.fude/current.json` is keyed by branch, so different branches in one worktree get separate sessions), sets state (`review_mode = "local"`, `local_session`), reuses `init.setup_review_autocmds`, registers local-only autocmds (BufWritePost re-anchor, BufEnter tracker sync), and starts the auto-reload timer. `stop()` tears down and clears `current.json` (the JSONL file is kept). `toggle(base)` stops an active local session or starts one (refuses while a GitHub review is active). `reload(silent)` re-reads git state and the JSONL synchronously. **Scope**: `local_session.scope` is `"base"` (merge-base with the base branch — whole branch diff), `"unpushed"` (the `@{upstream}` tracking ref — changes not yet pushed), or `"uncommitted"` (HEAD — staged + unstaged only). All three compare the working tree against a ref, so comments stay anchored (per-commit was deferred for exactly this reason — its right side is a commit, not the working tree). `resolve_scope_base(scope, base_ref, cwd)` returns the `git diff` base and the `git show` content ref (nil when the scope is unavailable). `scope_specs(session)` builds the adaptive list of available scopes with labels (base only on a non-base branch; unpushed only with an upstream; uncommitted always). `set_scope(scope)` re-derives the base, reloads changed files/comments, re-applies the gitsigns base and preview, and updates the statusline; `select_scope()` is the `vim.ui.select` picker. Comments are scope-independent (they anchor to the working tree). - **`local/tracker.lua`** — Extmark-based line tracking for local review comments (dedicated `fude_local_track` namespace so `refresh_extmarks` never clears it). `sync_buffer`/`sync_all` place invisible extmarks for root comments; `collect_moves(buf)` computes drifted positions from extmark rows; `on_buf_write(buf)` persists drift as `move` events via `local_sync.move_comments`. `teardown()` clears marks on session stop. Module-local registry, not `config.state`. -- **`comments/local_sync.lua`** — Local comment backend mirroring `comments/sync.lua`'s external shape (`load_comments`, `reply_to_comment`, `edit_comment`, `delete_comment` with identical callback signatures) plus `create_comment` (with best-effort `context` capture), `move_comments` (batch re-anchor), `toggle_resolved`, and `set_viewed` (local viewed state, no GitHub round-trip). `load_comments` also populates `state.viewed_files` from the materialized viewed map, and runs `store.reanchor` (reading current file lines, preferring loaded buffers) to recover drifted comments, persisting confident re-anchors as `move` events before `apply_outdated`. After `apply_outdated` it normalizes the local thread-level `resolved` flag onto the display-facing `is_resolved`, gated by `resolved.show` (mirroring how `sync.lua` only sets `is_resolved` at fetch time), so the whole display layer reads `is_resolved` alone; `resolved` stays as the toggle source of truth. All operations are synchronous JSONL appends followed by one re-materialize into `state.comments`/`state.comment_map`; `comments.lua` dispatches to this backend when `state.review_mode == "local"`. +- **`comments/local_sync.lua`** — Local comment backend mirroring `comments/sync.lua`'s external shape (`load_comments`, `reply_to_comment`, `edit_comment`, `delete_comment` with identical callback signatures) plus `create_comment` (with best-effort `context` capture), `move_comments` (batch re-anchor), `toggle_resolved`, and `set_viewed` (local viewed state, no GitHub round-trip). `load_comments` also populates `state.viewed_files` from the materialized viewed map, and runs `store.reanchor` (reading current file lines, preferring loaded buffers) to recover drifted comments, persisting confident re-anchors as `move` events before `apply_outdated`. After `apply_outdated` it normalizes the local thread-level `resolved` flag onto the display-facing `is_resolved`, gated by `resolved.show` (mirroring how `sync.lua` only sets `is_resolved` at fetch time), so the whole display layer reads `is_resolved` alone (the resolved-visibility toggle keys off `is_resolved`, so locally resolved threads are covered too). All operations are synchronous JSONL appends followed by one re-materialize into `state.comments`/`state.comment_map`; `comments.lua` dispatches to this backend when `state.review_mode == "local"`. - **`preview.lua`** — Manages the side-by-side diff preview window. Creates a scratch buffer with base branch content, opens it in a vsplit, and enables `diffthis` on both windows. Uses `noautocmd` to prevent BufEnter cascades. The `opening` flag guards against re-entrant calls. -- **`comments.lua`** — Facade module re-exporting `comments/data.lua`, `comments/sync.lua`, and `comments/pickers.lua`. Contains comment navigation (`next_comment`/`prev_comment`), creation (`create_comment`/`suggest_change`), viewing (`view_comments`), reply (`reply_to_comment`), editing (`edit_comment`), deletion (`delete_comment`), and thread resolution (`toggle_resolve`, local mode only). Dispatches mutations to `comments/sync.lua` (GitHub) or `comments/local_sync.lua` (local JSONL) by `state.review_mode`. Also provides ownership helpers: `is_own_comment`, `is_pending_comment`, `find_pending_key`. `require("fude.comments")` is the public interface. +- **`comments.lua`** — Facade module re-exporting `comments/data.lua`, `comments/sync.lua`, and `comments/pickers.lua`. Contains comment navigation (`next_comment`/`prev_comment`), creation (`create_comment`/`suggest_change`), viewing (`view_comments`), reply (`reply_to_comment`), editing (`edit_comment`), deletion (`delete_comment`), and thread resolution (`toggle_resolve`, local mode only). Dispatches mutations to `comments/sync.lua` (GitHub) or `comments/local_sync.lua` (local JSONL) by `state.review_mode`. Also provides ownership helpers: `is_own_comment`, `is_pending_comment`, `find_pending_key`, and `toggle_resolved_visibility()` (flips `config.toggle_show_resolved()` and refreshes extmarks — used by `:FudeReviewToggleResolved`). The toggle only affects inline comment boxes: `comment_map` always keeps resolved comments, so navigation, the comment viewer, the virtualText style, and the comment browser are unaffected. When boxes are hidden, `ui/extmarks.lua` renders the resolved comments with the virtualText EOL indicator instead. `require("fude.comments")` is the public interface. - **`comments/data.lua`** — Pure data functions with no state or side effects: `line_from_diff_hunk`, `build_comment_map`, `find_next_comment_line`, `find_prev_comment_line`, `find_comment_by_id`, `get_comment_thread`, `parse_draft_key`, `build_pending_comments_from_review`, `build_review_comment_object`, `merge_pending_into_comments`, `pending_comments_to_array`, `get_comment_line_range`, `get_reply_target_id`, `get_comments_at`, `get_comment_lines`, `build_comment_entries`, `build_comment_browser_entries`, `merge_draft_entries` (flags existing entries with `has_draft` for matching reply/edit/line/issue drafts and appends `type="draft"` rows for new line/suggest/issue drafts, then re-sorts by `last_ts`). - - **`comments/sync.lua`** — GitHub API sync/submit operations: `load_comments`, `sync_pending_review`, `submit_as_review`, `reply_to_comment`, `edit_comment`, `delete_comment`. `load_comments` is the main entry point: detects pending review via `GET /pulls/{pr}/reviews`, then fetches both submitted comments (`GET /pulls/{pr}/comments`) and pending review comments (`GET /reviews/{id}/comments`) in a single flow, converting `position` to `line` via `line_from_diff_hunk` and building both `comment_map` and `pending_comments`. Also fetches review threads via GraphQL (skipped when `outdated.show` and `resolved.show` are both false and no pending review exists) and applies `is_outdated` (root comments only, gated by `outdated.show`) and `is_resolved` (per-thread state, propagated to replies via `in_reply_to_id`, gated by `resolved.show`) to comment objects. Internal `fetch_comments` is used by other functions for refreshing after mutations. Uses lazy `require("fude.ui")` to avoid circular dependencies. + - **`comments/sync.lua`** — GitHub API sync/submit operations: `load_comments`, `sync_pending_review`, `submit_as_review`, `reply_to_comment`, `edit_comment`, `delete_comment`. `load_comments` is the main entry point: detects pending review via `GET /pulls/{pr}/reviews`, then fetches both submitted comments (`GET /pulls/{pr}/comments`) and pending review comments (`GET /reviews/{id}/comments`) in a single flow, converting `position` to `line` via `line_from_diff_hunk` and building both `comment_map` and `pending_comments`. Also fetches review threads via GraphQL (skipped when `outdated.show` and `resolved.show` are both false and no pending review exists) and applies `is_outdated` (root comments only, gated by `outdated.show`) and `is_resolved` (per-thread state, propagated to replies via `in_reply_to_id`, gated by `resolved.show`) to comment objects. `comment_map` always includes resolved comments (the resolved-visibility toggle acts only on inline-box rendering in `ui/extmarks.lua`). Internal `fetch_comments` is used by other functions for refreshing after mutations. Uses lazy `require("fude.ui")` to avoid circular dependencies. - **`comments/pickers.lua`** — Entry point for `list_comments`. Delegates to `ui/comment_browser.open()`. - **`ui.lua`** — Facade module re-exporting `ui/format.lua` and `ui/extmarks.lua`. Contains floating window UI: comment input editor, comment viewer, PR overview window, reply window, edit window, and review event selector. `require("fude.ui")` is the public interface. Also exports `should_confirm_discard(current_lines, original_lines)` (pure) and `confirm_close_with_draft(buf, original_lines, opts)` (side-effect) used to guard close/cancel actions on comment input buffers: when the buffer is dirty it shows a 3-way prompt (save local draft / discard / keep editing) via `opts.on_save_draft`/`opts.on_discard`/`opts.on_close`, falling back to a plain Yes/No discard confirmation when `opts.allow_draft` is false. - **`ui/format.lua`** — Pure format/calculation functions with no state or vim API side effects: `calculate_float_dimensions`, `format_comments_for_display`, `build_comments_float_title`, `normalize_check`, `format_check_status`, `deduplicate_checks`, `sort_checks`, `build_checks_summary`, `format_review_status`, `build_reviewers_list`, `build_re_request_candidates`, `build_reviewers_summary`, `calculate_overview_layout`, `calculate_comments_height`, `calculate_reply_window_dimensions`, `format_reply_comments_for_display`, `build_overview_left_lines`, `build_overview_right_lines`, `calculate_comment_browser_layout`, `format_comment_browser_list`, `format_comment_browser_thread`, `parse_markdown_line`, `build_highlighted_chunks`, `apply_markdown_highlight_to_line`, `normalize_newlines`. Purity is enforced by `scripts/check_purity.lua` (`make check-purity`). @@ -48,7 +48,7 @@ All plugin code lives under `lua/fude/`. The plugin entry point is `plugin/fude. - **`ui/comment_browser.lua`** — 3-pane floating comment browser for `FudeReviewListComments`. Left pane: comment list (review + PR-level, time-descending), including local drafts via `merge_draft_entries` (existing entries get a `✎draft` marker; new line/suggest/issue drafts appear as `[draft]` rows). Selecting a `type="draft"` row: `line`/`suggest` jumps to the file (``), `issue` focuses the lower pane (prefilled). Right upper: thread display (draft body preview for draft rows). Right lower: reply/edit/new comment input. Supports reply, edit, delete, new PR comment, jump to file, and refresh. Does not depend on Telescope. - **`ui/sidepanel.lua`** — Toggleable sidebar showing Review Scope and Changed Files. Files default to a flat list (`sidepanel.file_tree = "flat"`); `"tree"` groups files by directory via `ui/sidepanel/tree.lua`. Pure functions: `format_scope_section`, `format_files_section(file_entries, width, format_path_fn, viewed_count, current_path)`, `format_files_section_tree(tree_entries, total_file_count, width, viewed_count, current_path)`, `build_sidepanel_content`, `resolve_entry_at_cursor`. The Files section header displays viewed/total count (e.g., "Files (Reviewed: 3/10)"). The currently open file is marked with `▶` (`DiagnosticInfo` highlight), matching the Review Scope marker pattern; cursor follows the active file via `follow_current_file`. Side-effect functions: `open`, `close`, `toggle`, `refresh`, `follow_current_file`, `toggle_file_tree_mode`, `open_file`. Uses `nvim_open_win` with `split` for sidebar creation. Uses dedicated `fude_sidepanel` namespace for highlights (avoids `refresh_extmarks` clearing them on BufEnter). Auto-refreshes on scope change, reload, and BufEnter (to update the current-file marker). Panel-local mappings are configurable with `sidepanel.keymaps`; defaults are `` select/open, `` toggle reviewed/viewed, `t` toggle flat/tree, `R` reload, and `q` close. - **`ui/sidepanel/tree.lua`** — Pure tree helpers for the sidepanel Files section: `build_tree`, `collapse_singleton_chains`, `compute_aggregate`, and `flatten_tree`. - - **`ui/extmarks.lua`** — Extmark management: `flash_line`, `highlight_comment_lines`, `clear_comment_line_highlight`, `refresh_extmarks`, `clear_extmarks`, `clear_all_extmarks`. Uses lazy `require("fude.comments")` to avoid circular dependencies. `refresh_extmarks` also renders the `draft` indicator from `drafts.file_markers(rel_path)` (EOL virt_text, both virtualText and inline modes) and, in virtualText mode, a resolved indicator (`resolved.label`, EOL virt_text, priority 43) on lines where every comment is resolved (`util.all_comments_resolved`). + - **`ui/extmarks.lua`** — Extmark management: `flash_line`, `highlight_comment_lines`, `clear_comment_line_highlight`, `refresh_extmarks`, `clear_extmarks`, `clear_all_extmarks`. Uses lazy `require("fude.comments")` to avoid circular dependencies. `refresh_extmarks` also renders the `draft` indicator from `drafts.file_markers(rel_path)` (EOL virt_text, both virtualText and inline modes). A module-local `render_virt_text_indicators(buf, line, comments)` builds the EOL indicators (comment count, pending, resolved `resolved.label` at priority 43 when `util.all_comments_resolved`); it is used both for the virtualText style and, in the inline style, as the fallback for resolved comments whose box is hidden by `:FudeReviewToggleResolved` (so both paths show an identical indicator). - **`files.lua`** — Changed files display via Telescope picker, snacks.picker, or quickfix list. All pickers show diff preview and viewed state toggle via ``. Shows GitHub viewed status for each file. Exports `apply_viewed_toggle(path, on_done)` as a picker-agnostic state mutator that invokes gh GraphQL mark/unmark, updates `state.viewed_files`, and calls `on_done` with the updated display fields; the Telescope adapter `toggle_viewed_in_telescope` and the snacks adapter `toggle_viewed_in_snacks(picker, item)` both delegate to it. `show()` routes to `show_telescope` / `show_snacks` / `show_quickfix` based on `config.opts.file_list_mode`; snacks falls back to quickfix when snacks.nvim is missing. Also exports `next_file()` / `prev_file()` for jumping between changed files (used by `:FudeReviewNextFile` / `:FudeReviewPrevFile`); both wrap around at the edges and fall back to the first/last entry when the current buffer is not part of the PR. The pure helpers `find_adjacent_file_index(changed_files, current_path, direction)` computes the target index; `count_viewed(viewed_files, changed_files)` counts files with VIEWED state (used by sidepanel header). - **`scope.lua`** — Review scope selection and navigation. Provides a Telescope picker (or `vim.ui.select` fallback) for choosing between full PR scope and individual commit scope, with commit index display (`[1/10]`) and current scope marker (`▶`). Supports next/prev scope navigation (`next_scope`/`prev_scope`), marking commits as reviewed via `` in the Telescope picker (tracked locally in `state.reviewed_commits`), and statusline integration (`statusline()`). On commit scope: checks out the commit, fetches commit-specific changed files, updates gitsigns base to `sha^` (global), and refreshes the diff preview. On full PR scope: restores the original HEAD, re-fetches PR-wide changed files, and computes merge-base (per-buffer gitsigns base is applied via `GitSignsUpdate` autocmd in init.lua). Exports `apply_reviewed_toggle(sha)` as a picker-agnostic state mutator that toggles `state.reviewed_commits[sha]` and returns the updated display fields `{ is_reviewed, reviewed_icon, reviewed_hl }`; both the Telescope adapter `toggle_reviewed_in_telescope` and the snacks adapter `toggle_reviewed_in_snacks(picker, item)` delegate to it. `select_scope()` routes to `show_telescope` / `show_snacks` / `show_vim_select` based on `config.opts.file_list_mode`; snacks falls back to `vim.ui.select` when snacks.nvim is missing. - **`overview.lua`** — PR overview display: fetches extended PR info and issue-level comments, renders in a centered float with keymaps for commenting, refreshing, and re-requesting a review (`r` selects a reviewer who already reviewed via `vim.ui.select` and posts to the `requested_reviewers` endpoint through `gh.re_request_review`). @@ -102,6 +102,7 @@ All plugin code lives under `lua/fude/`. The plugin entry point is `plugin/fude. | `comment_browser` | ui/comment_browser | ui/comment_browser | | `github_user` | init, local/session | comments, comments/sync, comments/local_sync, ui/comment_browser | | `current_comment_style` | config | config | +| `show_resolved` | config | config | | `reload_timer` | init, local/session | init, config, local/session | | `reloading` | init, local/session | init, local/session | | `gitsigns_reset` | init, scope | init | diff --git a/README.md b/README.md index 8fdbfd9..cc81ddf 100644 --- a/README.md +++ b/README.md @@ -124,6 +124,7 @@ PR code review inside Neovim. Review GitHub pull requests without leaving your e | `:FudeCopyPRURL` | Copy PR URL to clipboard | | `:FudeReviewReload` | Reload review data from GitHub | | `:FudeReviewToggleCommentStyle` | Toggle comment display style (virtualText/inline) | +| `:FudeReviewToggleResolved` | Toggle visibility of resolved comments in the editor | | `:FudeReviewToggleGitsigns` | Toggle gitsigns between PR base and HEAD | | `:FudeReviewPanel` | Toggle review side panel | | `:FudeReviewToggleFileTree` | Toggle side panel files between flat list and tree | @@ -217,6 +218,8 @@ require("fude").setup({ -- comment browser, comment viewer, and virtual text with `label`. Inline -- comment boxes instead show a fixed "[resolved thread]" on the thread's -- head (oldest) comment only. + -- :FudeReviewToggleResolved hides resolved comments' inline boxes at runtime; + -- hidden ones fall back to the virtual text indicator (e.g. "[resolved] #1"). -- Set show = false to hide all resolved labels. (The review-threads fetch -- is shared with outdated detection; it is skipped only when outdated.show -- is also false and no pending review exists.) diff --git a/doc/fude.txt b/doc/fude.txt index 03cb667..f1122c1 100644 --- a/doc/fude.txt +++ b/doc/fude.txt @@ -388,6 +388,29 @@ Using lazy.nvim: >lua names. The hint disappears when the cursor moves to a non-comment line. Requires an active review session. +:FudeReviewToggleResolved *:FudeReviewToggleResolved* + Toggle whether resolved comments show their full inline comment box + (only relevant with the "inline" comment style). When hidden, a + resolved comment falls back to the same end-of-line indicator the + "virtualText" style uses (e.g. `[resolved] #1`), so the line stays + marked. Navigation (`]c` / `[c`), the comment viewer, the + "virtualText" style, and the comment browser + (|:FudeReviewListComments|) always show resolved threads regardless + of this toggle. + + Covers threads resolved on GitHub ("Resolve conversation") and, in + local review mode, threads resolved with |:FudeReviewResolve|. + This command only changes visibility; use |:FudeReviewResolve| to + change a thread's resolved state (local review mode). + + The visibility is stored in the session state, survives + |:FudeReviewReload| and auto-reloads, and resets to visible when + review mode is stopped. + + When `resolved.show = false`, resolved state is not applied at all + and this command only shows a warning. + Requires an active review session. + :FudeReviewPanel *:FudeReviewPanel* Toggle the review side panel. The panel is a sidebar split window that shows two sections: @@ -778,6 +801,8 @@ Options: review-threads fetch is shared with outdated comment detection, so it is skipped only when `outdated.show` is also false and no pending review exists. + Visibility of resolved comments' inline boxes can be + toggled at runtime with |:FudeReviewToggleResolved|. `resolved.label` Label text shown for resolved threads (default: "[resolved]"). Appears in the comment browser diff --git a/lua/fude/comments.lua b/lua/fude/comments.lua index 52e74d1..dfce63c 100644 --- a/lua/fude/comments.lua +++ b/lua/fude/comments.lua @@ -55,6 +55,19 @@ local function has_review_target(state) return state.active and (state.pr_number ~= nil or state.local_session ~= nil) end +--- Toggle editor visibility of resolved comments. +--- Only affects inline comment boxes (the virt_lines rendered in "inline" +--- comment style): refresh_extmarks skips resolved comments when hidden. +--- comment_map is left intact, so navigation, the comment viewer, virtualText +--- indicators, and the comment browser (FudeReviewListComments) always show +--- resolved threads regardless of this toggle. +--- @return boolean the new visibility +function M.toggle_resolved_visibility() + local visible = config.toggle_show_resolved() + ui.refresh_extmarks() + return visible +end + --- Get comments at a specific file and line. --- @param rel_path string repo-relative file path --- @param line number line number diff --git a/lua/fude/comments/sync.lua b/lua/fude/comments/sync.lua index 1112b5f..4b965b2 100644 --- a/lua/fude/comments/sync.lua +++ b/lua/fude/comments/sync.lua @@ -14,7 +14,7 @@ local is_null = require("fude.util").is_null --- also propagated to replies via in_reply_to_id. --- @param comments table[] array of comment objects --- @param thread_info_map table { [root_comment_id] = { is_outdated, is_resolved, original_line } } ---- @param opts table { apply_outdated = boolean, apply_resolved = boolean } +--- @param opts table { apply_outdated = boolean, apply_resolved = boolean, pending_review_id = number|nil } local function apply_thread_info(comments, thread_info_map, opts) -- Fallback index for threads whose root comment was deleted on GitHub: the -- GraphQL "root" is then the earliest surviving reply, whose in_reply_to_id @@ -31,7 +31,11 @@ local function apply_thread_info(comments, thread_info_map, opts) if opts.apply_outdated and info and info.is_outdated then c.is_outdated = true end - if opts.apply_resolved then + -- Never mark unsubmitted pending comments as resolved: the user's own + -- pending reply on a resolved thread still needs attention, so the line + -- must not render as fully resolved (nor be hidden by the visibility toggle). + local is_pending = opts.pending_review_id ~= nil and c.pull_request_review_id == opts.pending_review_id + if opts.apply_resolved and not is_pending then local thread_info = info if not thread_info and not is_null(c.in_reply_to_id) then thread_info = thread_info_map[c.in_reply_to_id] or info_by_parent[c.in_reply_to_id] @@ -107,6 +111,7 @@ local function fetch_comments(callback, opts) apply_thread_info(comments, thread_info_map, { apply_outdated = need_outdated, apply_resolved = need_resolved, + pending_review_id = state.pending_review_id, }) end apply(comments) diff --git a/lua/fude/config.lua b/lua/fude/config.lua index 572db57..1083ddc 100644 --- a/lua/fude/config.lua +++ b/lua/fude/config.lua @@ -153,6 +153,7 @@ M.state = { github_user = nil, -- Authenticated GitHub username (for ownership check) comment_browser = nil, -- 3-pane comment browser window state current_comment_style = nil, -- Runtime override for comment_style (nil = use opts.comment_style) + show_resolved = nil, -- Runtime override for resolved comment visibility in the editor (nil = visible) reload_timer = nil, -- vim.uv.new_timer() handle for auto-reload reloading = false, -- Guard flag to prevent concurrent reloads gitsigns_reset = false, -- true: HEAD表示(一時的に元のワークツリー状態)、false: PRベース表示 @@ -209,6 +210,7 @@ function M.reset_state() github_user = nil, comment_browser = nil, current_comment_style = nil, + show_resolved = nil, reload_timer = nil, reloading = false, gitsigns_reset = false, @@ -273,4 +275,22 @@ function M.toggle_comment_style() return new_style end +--- Get whether resolved comments are currently visible in the editor. +--- Returns the runtime override if set, otherwise defaults to visible. +--- Editor-only: the comment browser always shows resolved threads. +--- @return boolean +function M.get_show_resolved() + if M.state.show_resolved ~= nil then + return M.state.show_resolved + end + return true +end + +--- Toggle editor visibility of resolved comments. +--- @return boolean the new visibility +function M.toggle_show_resolved() + M.state.show_resolved = not M.get_show_resolved() + return M.state.show_resolved +end + return M diff --git a/lua/fude/ui/extmarks.lua b/lua/fude/ui/extmarks.lua index 07d6263..4fc0772 100644 --- a/lua/fude/ui/extmarks.lua +++ b/lua/fude/ui/extmarks.lua @@ -77,6 +77,58 @@ function M.clear_comment_line_highlight() comment_line_highlight.extmark_ids = {} end +--- Render end-of-line virtualText indicators (comment count, pending, resolved) +--- for a set of comments on a line. Shared by the virtualText comment style and +--- by the inline style's fallback for resolved comments whose box is hidden by +--- FudeReviewToggleResolved, so both paths produce an identical indicator. +--- @param buf number buffer handle +--- @param line number 1-indexed line number +--- @param comments table[] comments on the line +local function render_virt_text_indicators(buf, line, comments) + local state = config.state + local submitted_count = 0 + local has_pending = false + local all_resolved = util.all_comments_resolved(comments) + for _, c in ipairs(comments) do + if state.pending_review_id and c.pull_request_review_id == state.pending_review_id then + has_pending = true + else + submitted_count = submitted_count + 1 + end + end + + if submitted_count > 0 then + pcall(vim.api.nvim_buf_set_extmark, buf, state.ns_id, line - 1, 0, { + virt_text = { + { string.format(" %s%d", config.opts.signs.comment, submitted_count), config.opts.signs.comment_hl }, + }, + virt_text_pos = "eol", + priority = 50, + }) + end + if has_pending then + pcall(vim.api.nvim_buf_set_extmark, buf, state.ns_id, line - 1, 0, { + virt_text = { + { " " .. config.opts.signs.pending, config.opts.signs.pending_hl }, + }, + virt_text_pos = "eol", + priority = 45, + }) + end + -- Resolved indicator: only when every thread on the line is resolved + -- (a partially resolved line still needs attention). + if all_resolved then + local resolved_opts = config.opts.resolved or {} + pcall(vim.api.nvim_buf_set_extmark, buf, state.ns_id, line - 1, 0, { + virt_text = { + { " " .. (resolved_opts.label or "[resolved]"), resolved_opts.hl_group or "DiagnosticOk" }, + }, + virt_text_pos = "eol", + priority = 43, + }) + end +end + --- Refresh extmarks (virtual text) for the current buffer. function M.refresh_extmarks() local state = config.state @@ -98,78 +150,59 @@ function M.refresh_extmarks() local comment_lines = comments_mod.get_comment_lines(rel_path) local style = config.get_comment_style() - local inline_opts = config.opts.inline or {} + local inline_opts + if style == "inline" then + inline_opts = config.opts.inline or {} + end for _, line in ipairs(comment_lines) do local comments = comments_mod.get_comments_at(rel_path, line) if style == "inline" then - -- Inline mode: display full comment content below the line - -- Build arrays only when needed for inline display - local all_comments_for_display = {} + -- Inline mode: display full comment content below the line. + -- When FudeReviewToggleResolved is off, resolved comments do not get an + -- inline box. If that leaves the line with nothing to show, it falls back + -- to the same EOL virtualText indicator the virtualText style would show + -- (e.g. `[resolved] 🗒️1`) so the line stays marked. Pending comments are + -- never resolved, so they always keep their inline box. + local show_resolved = config.get_show_resolved() + local box_comments = {} + local hidden_resolved = {} for _, c in ipairs(comments) do if state.pending_review_id and c.pull_request_review_id == state.pending_review_id then local pc = vim.tbl_extend("force", {}, c) pc.is_pending = true - table.insert(all_comments_for_display, pc) + table.insert(box_comments, pc) + elseif show_resolved or not c.is_resolved then + -- Read `is_resolved` alone (local mode normalizes `resolved` onto it, + -- gated by resolved.show), matching util.all_comments_resolved so the + -- box/fallback split and the `[resolved]` label never disagree. + table.insert(box_comments, c) else - table.insert(all_comments_for_display, c) + table.insert(hidden_resolved, c) end end - if #all_comments_for_display > 0 then + if #box_comments > 0 then local inline = require("fude.ui.inline") - local result = inline.format_comments_for_inline(all_comments_for_display, config.format_date, inline_opts) + local result = inline.format_comments_for_inline(box_comments, config.format_date, inline_opts) pcall(vim.api.nvim_buf_set_extmark, buf, state.ns_id, line - 1, 0, { virt_lines = result.virt_lines, virt_lines_above = false, priority = 50, }) + elseif #hidden_resolved > 0 then + -- Only reached when the line has no box to show, i.e. every comment on + -- it is a hidden resolved one. hidden_resolved therefore equals the + -- whole line, so render_virt_text_indicators judges the count and the + -- `[resolved]` label over all of the line's comments (matching the + -- virtualText style). On a mixed line the box above already marks it, + -- so no fallback is rendered. + render_virt_text_indicators(buf, line, hidden_resolved) end else -- virtualText mode: display indicators at end of line (original behavior) - -- Only compute counts, avoid building arrays - local submitted_count = 0 - local has_pending = false - local all_resolved = util.all_comments_resolved(comments) - for _, c in ipairs(comments) do - if state.pending_review_id and c.pull_request_review_id == state.pending_review_id then - has_pending = true - else - submitted_count = submitted_count + 1 - end - end - - if submitted_count > 0 then - pcall(vim.api.nvim_buf_set_extmark, buf, state.ns_id, line - 1, 0, { - virt_text = { - { string.format(" %s%d", config.opts.signs.comment, submitted_count), config.opts.signs.comment_hl }, - }, - virt_text_pos = "eol", - priority = 50, - }) - end - if has_pending then - pcall(vim.api.nvim_buf_set_extmark, buf, state.ns_id, line - 1, 0, { - virt_text = { - { " " .. config.opts.signs.pending, config.opts.signs.pending_hl }, - }, - virt_text_pos = "eol", - priority = 45, - }) - end - -- Resolved indicator: only when every thread on the line is resolved - -- (a partially resolved line still needs attention). - if all_resolved then - local resolved_opts = config.opts.resolved or {} - pcall(vim.api.nvim_buf_set_extmark, buf, state.ns_id, line - 1, 0, { - virt_text = { - { " " .. (resolved_opts.label or "[resolved]"), resolved_opts.hl_group or "DiagnosticOk" }, - }, - virt_text_pos = "eol", - priority = 43, - }) - end + render_virt_text_indicators(buf, line, comments) end end diff --git a/plugin/fude.lua b/plugin/fude.lua index 4dc37f5..c3475b9 100644 --- a/plugin/fude.lua +++ b/plugin/fude.lua @@ -196,6 +196,20 @@ vim.api.nvim_create_user_command("FudeReviewToggleCommentStyle", function() require("fude.ui").refresh_extmarks() end, { desc = "Toggle comment display style (virtualText/inline)" }) +vim.api.nvim_create_user_command("FudeReviewToggleResolved", function() + local config = require("fude.config") + if not config.state.active then + vim.notify("fude.nvim: Not active", vim.log.levels.WARN) + return + end + if config.opts.resolved and config.opts.resolved.show == false then + vim.notify("fude.nvim: Resolved display is disabled (resolved.show = false)", vim.log.levels.WARN) + return + end + local visible = require("fude.comments").toggle_resolved_visibility() + vim.notify("fude.nvim: Resolved comments: " .. (visible and "shown" or "hidden"), vim.log.levels.INFO) +end, { desc = "Toggle visibility of resolved comments in the editor" }) + vim.api.nvim_create_user_command("FudeReviewToggleGitsigns", function() require("fude").toggle_gitsigns() end, { desc = "Toggle gitsigns between PR base and HEAD" }) diff --git a/tests/fude/comments_spec.lua b/tests/fude/comments_spec.lua index 9fce728..c3dc2d4 100644 --- a/tests/fude/comments_spec.lua +++ b/tests/fude/comments_spec.lua @@ -129,6 +129,58 @@ describe("build_comment_map", function() assert.are.equal(1, #map["a.lua"][20]) assert.are.equal(1, #map["b.lua"][5]) end) + + it("keeps resolved comments by default", function() + local input = { + { path = "a.lua", line = 10, body = "resolved", is_resolved = true }, + } + local map = comments.build_comment_map(input) + assert.are.equal(1, #map["a.lua"][10]) + end) + + it("keeps resolved comments in the map (visibility is handled at render time)", function() + local input = { + { path = "a.lua", line = 10, body = "resolved", is_resolved = true }, + { path = "a.lua", line = 10, body = "resolved reply", is_resolved = true }, + { path = "a.lua", line = 20, body = "local resolved", resolved = true }, + } + local map = comments.build_comment_map(input) + assert.are.equal(2, #map["a.lua"][10]) + assert.are.equal(1, #map["a.lua"][20]) + end) +end) + +describe("toggle_resolved_visibility", function() + before_each(function() + config.setup({}) + config.state.show_resolved = nil + config.state.comments = { + { id = 1, path = "a.lua", line = 10, body = "resolved", is_resolved = true }, + { id = 2, path = "a.lua", line = 20, body = "open" }, + } + config.state.comment_map = data.build_comment_map(config.state.comments) + end) + + after_each(function() + config.state.show_resolved = nil + config.state.comments = {} + config.state.comment_map = {} + end) + + it("flips visibility without removing resolved comments from comment_map", function() + -- The toggle only affects inline comment boxes (rendered in refresh_extmarks). + -- comment_map keeps resolved comments so navigation, the viewer, and the + -- comment browser stay unaffected. + local visible = comments.toggle_resolved_visibility() + assert.is_false(visible) + assert.are.equal(1, #config.state.comment_map["a.lua"][10]) + assert.are.equal(1, #config.state.comment_map["a.lua"][20]) + + visible = comments.toggle_resolved_visibility() + assert.is_true(visible) + assert.are.equal(1, #config.state.comment_map["a.lua"][10]) + assert.are.equal(1, #config.state.comment_map["a.lua"][20]) + end) end) describe("find_next_comment_line", function() @@ -1470,6 +1522,21 @@ describe("merge_pending_into_comments", function() assert.is_not_nil(map["b.lua"][2]) end) + it("keeps resolved comments in the merged comment_map", function() + -- Resolved comments are never dropped from the map; the resolved-visibility + -- toggle only hides their inline boxes at render time. + local existing = { + { id = 1, path = "foo.lua", line = 10, body = "resolved", is_resolved = true }, + } + local pending = { + ["bar.lua:5:5"] = { path = "bar.lua", body = "pending", line = 5, side = "RIGHT" }, + } + local merged, map = data.merge_pending_into_comments(existing, pending, 100, "user1") + assert.are.equal(2, #merged) + assert.is_not_nil(map["foo.lua"][10]) + assert.is_not_nil(map["bar.lua"][5]) + end) + it("deduplicates by removing existing comments with same pending_review_id", function() local existing = { { id = 1, path = "foo.lua", line = 10, body = "submitted", pull_request_review_id = 50 }, diff --git a/tests/fude/config_spec.lua b/tests/fude/config_spec.lua index d69294b..b31ba86 100644 --- a/tests/fude/config_spec.lua +++ b/tests/fude/config_spec.lua @@ -129,6 +129,40 @@ describe("config", function() end) end) + describe("get_show_resolved", function() + it("defaults to visible", function() + config.setup({}) + config.state.show_resolved = nil + assert.is_true(config.get_show_resolved()) + end) + + it("returns state override when set", function() + config.setup({}) + config.state.show_resolved = false + assert.is_false(config.get_show_resolved()) + end) + end) + + describe("toggle_show_resolved", function() + it("toggles from visible to hidden and back", function() + config.setup({}) + config.state.show_resolved = nil + assert.is_false(config.toggle_show_resolved()) + assert.is_false(config.get_show_resolved()) + assert.is_true(config.toggle_show_resolved()) + assert.is_true(config.get_show_resolved()) + end) + end) + + describe("reset_state clears show_resolved", function() + it("clears show_resolved on reset", function() + config.setup({}) + config.state.show_resolved = false + config.reset_state() + assert.is_nil(config.state.show_resolved) + end) + end) + describe("format_path", function() it("returns path as-is when format_path option is nil", function() config.setup({}) diff --git a/tests/fude/extmarks_integration_spec.lua b/tests/fude/extmarks_integration_spec.lua index ac42931..5cdc3cb 100644 --- a/tests/fude/extmarks_integration_spec.lua +++ b/tests/fude/extmarks_integration_spec.lua @@ -261,6 +261,88 @@ describe("extmarks integration", function() assert.is_true(find_virt_text(buf, 0, "%[DONE%]"), "Should use custom resolved label") end) + local function has_virt_lines(buf, line0) + local marks = vim.api.nvim_buf_get_extmarks(buf, config.state.ns_id, 0, -1, { details = true }) + for _, mark in ipairs(marks) do + if mark[2] == line0 and mark[4].virt_lines and #mark[4].virt_lines > 0 then + return true + end + end + return false + end + + it("inline mode: hides resolved box and shows virtualText fallback when show_resolved is off", function() + local buf = helpers.create_buf({ "line1", "line2", "line3" }, "test.lua") + vim.api.nvim_set_current_buf(buf) + + config.state.active = true + config.state.current_comment_style = "inline" + config.state.comment_map = { + ["test.lua"] = { + [2] = { + { + id = 1, + body = "resolved", + is_resolved = true, + user = { login = "t" }, + created_at = "2024-01-01T00:00:00Z", + }, + }, + [3] = { + { id = 2, body = "open", user = { login = "t" }, created_at = "2024-01-01T00:00:00Z" }, + }, + }, + } + config.state.pending_comments = {} + + -- show_resolved on: the resolved comment renders as an inline box + config.state.show_resolved = nil + extmarks.refresh_extmarks() + assert.is_true(has_virt_lines(buf, 1), "resolved comment should have an inline box when show_resolved is on") + + -- show_resolved off: box hidden, EOL virtualText fallback shown instead + config.state.show_resolved = false + extmarks.refresh_extmarks() + assert.is_false(has_virt_lines(buf, 1), "resolved inline box should be hidden when show_resolved is off") + assert.is_true(find_virt_text(buf, 1, "%[resolved%]"), "resolved line should show the [resolved] fallback") + assert.is_true(find_virt_text(buf, 1, "#1"), "fallback should include the comment count sign") + -- the unresolved comment keeps its inline box regardless + assert.is_true(has_virt_lines(buf, 2), "unresolved comment should keep its inline box") + end) + + it("inline mode: no resolved fallback on a mixed line (unresolved + resolved)", function() + -- Two threads anchored to the same line: one resolved, one not. With + -- show_resolved off the unresolved one keeps its box, and the line must + -- NOT get a [resolved] EOL fallback (that would contradict virtualText + -- mode, which labels a line resolved only when every comment is). + local buf = helpers.create_buf({ "line1", "line2" }, "test.lua") + vim.api.nvim_set_current_buf(buf) + + config.state.active = true + config.state.current_comment_style = "inline" + config.state.comment_map = { + ["test.lua"] = { + [2] = { + { + id = 1, + body = "resolved", + is_resolved = true, + user = { login = "t" }, + created_at = "2024-01-01T00:00:00Z", + }, + { id = 2, body = "open", user = { login = "t" }, created_at = "2024-01-01T00:00:00Z" }, + }, + }, + } + config.state.pending_comments = {} + config.state.show_resolved = false + + extmarks.refresh_extmarks() + + assert.is_true(has_virt_lines(buf, 1), "unresolved comment should still render an inline box") + assert.is_false(find_virt_text(buf, 1, "%[resolved%]"), "mixed line must not show the [resolved] fallback") + end) + it("marks pending comments with is_pending flag in inline mode", function() local buf = helpers.create_buf({ "line1", "line2", "line3" }, "test.lua") vim.api.nvim_set_current_buf(buf) diff --git a/tests/fude/sync_integration_spec.lua b/tests/fude/sync_integration_spec.lua index 399b9b7..5deeb6d 100644 --- a/tests/fude/sync_integration_spec.lua +++ b/tests/fude/sync_integration_spec.lua @@ -308,6 +308,91 @@ describe("sync integration", function() assert.is_true(by_id[3].is_resolved, "sibling should inherit is_resolved via the shared deleted-root id") end) + it("does not mark unsubmitted pending replies on a resolved thread as resolved", function() + local gh = require("fude.gh") + helpers.mock(gh, "get_review_threads", function(_, callback) + vim.schedule(function() + callback(nil, { + [1] = { is_outdated = false, is_resolved = true, original_line = 10 }, + }, {}) + end) + end) + helpers.mock_gh({ + ["api:repos/{owner}/{repo}/pulls/42/reviews"] = { + { id = 99, state = "PENDING" }, + }, + ["api:repos/{owner}/{repo}/pulls/42/comments"] = { + { id = 1, path = "foo.lua", line = 10, body = "resolved root", in_reply_to_id = vim.NIL }, + }, + ["api:repos/{owner}/{repo}/pulls/42/reviews/99/comments"] = { + { + id = 2, + path = "foo.lua", + line = 10, + body = "unsubmitted pending reply", + side = "RIGHT", + in_reply_to_id = 1, + pull_request_review_id = 99, + }, + }, + }) + + config.state.pr_number = 42 + config.state.active = true + + sync.load_comments() + + local ok = helpers.wait_for(function() + return #config.state.comments > 0 + end) + assert.is_true(ok, "Should have fetched comments") + + local by_id = {} + for _, c in ipairs(config.state.comments) do + by_id[c.id] = c + end + assert.is_true(by_id[1].is_resolved) + assert.is_nil(by_id[2].is_resolved, "unsubmitted pending reply must not be marked resolved") + end) + + it("keeps resolved comments in comment_map across reloads even when show_resolved is off", function() + -- The resolved-visibility toggle only hides inline comment boxes at + -- render time; comment_map always keeps resolved comments so navigation, + -- the viewer, and the comment browser stay unaffected. + local gh = require("fude.gh") + helpers.mock(gh, "get_review_threads", function(_, callback) + vim.schedule(function() + callback(nil, { + [1] = { is_outdated = false, is_resolved = true, original_line = 10 }, + }, {}) + end) + end) + helpers.mock_gh({ + ["api:repos/{owner}/{repo}/pulls/42/reviews"] = {}, + ["api:repos/{owner}/{repo}/pulls/42/comments"] = { + { id = 1, path = "foo.lua", line = 10, body = "resolved", in_reply_to_id = vim.NIL }, + { id = 2, path = "foo.lua", line = 20, body = "open", in_reply_to_id = vim.NIL }, + }, + }) + + config.state.pr_number = 42 + config.state.active = true + config.state.show_resolved = false -- user toggled resolved comments off + + local cb_called = false + sync.load_comments(function() + cb_called = true + end) + + local ok = helpers.wait_for(function() + return cb_called + end) + assert.is_true(ok) + assert.are.equal(2, #config.state.comments, "resolved comments stay in state.comments") + assert.is_not_nil(config.state.comment_map["foo.lua"][10], "resolved comment stays in comment_map") + assert.is_not_nil(config.state.comment_map["foo.lua"][20]) + end) + it("applies only is_resolved when outdated disabled but resolved enabled", function() config.setup({ outdated = { show = false } })