From e6d94dcb41a570a77eb8dfad4cc872cff2a1d079 Mon Sep 17 00:00:00 2001 From: hl-valdemar Date: Thu, 22 Oct 2026 22:30:16 +0200 Subject: [PATCH] feat: support comments in ordinary source buffers --- TODO.md | 2 - docs/architecture.md | 2 +- docs/configuration.md | 16 +- docs/development.md | 2 + docs/editing.md | 34 +-- src/diff.rs | 66 +++--- src/editor.rs | 158 +++++++++----- src/editor_review.rs | 498 +++++++++++++++++++++++++----------------- src/main.rs | 12 +- src/repository.rs | 18 ++ src/review.rs | 2 +- src/tui.rs | 89 +++++++- tests/unit/diff.rs | 185 +++++++++------- tests/unit/editor.rs | 353 ++++++++++++++++-------------- tests/unit/ruby.rs | 27 +++ tests/unit/tui.rs | 141 +++++++++--- 16 files changed, 1005 insertions(+), 600 deletions(-) diff --git a/TODO.md b/TODO.md index 08f3d51..5f58303 100644 --- a/TODO.md +++ b/TODO.md @@ -1,5 +1,3 @@ -- [x] feature: syntax/lsp - add a default configuration for typst. - - for the lsp, i'm thinking tinymist is a good candidate. - [ ] feature: diff view - GitHub code review integration after the local review UX is usable. - publish local reviews explicitly: general PR comments, inline line/range comments, review summaries, approval, and requests for changes. - preserve explicit PR identity and reviewed commit IDs; validate anchors against GitHub's PR diff and recheck the PR head before submission. diff --git a/docs/architecture.md b/docs/architecture.md index a9b8271..b0ad6e5 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -20,7 +20,7 @@ One editor crate; the workspace also includes its patched Crossterm dependency. - `review`: owns the validated local review model, exact immutable anchors, persisted file checkmarks, SQLite storage through bundled rusqlite, and atomic JSON import/export with external-change protection. Schema 3 stores review identity, repository, revision, summary, and verdict in `reviews`; `comments` stores ordered comment IDs, bodies, resolution states, and explicit immutable anchor fields. `reviewed_files` stores revision pairs, paths, optional old paths, and optional exact live After text. Foreign keys, unique keys, and checks enforce relationships and scalar invariants. Full `u64` comment IDs use checked decimal text because SQLite integers are signed. No JSON documents are stored in SQLite. `Store` retains compare-and-swap review revisions and atomic whole-review saves: metadata and replacement comment/checkmark rows commit together. Read transactions reconstruct validated reviews from one consistent snapshot, including when listing in descending ID order. Stored text is bounded to 64 MiB per review, including a fixed per-comment/checkmark overhead. JSON version 4 is an independent import/export format. Incompatible databases, including schemas 1 and 2, are rejected and preserved without migration or reset. CLI metadata, sole-review restoration, and selector labels reuse loaded reviews; selector choices live only in the displayed buffer. The review CLI keeps `list`/`show` read-only. Comment deletion validates the explicit review/comment IDs through an existing read-only store before opening it for a compare-and-swap save; absent storage and invalid targets do not create reviews or a database. - `editor_review` commits native comment/summary buffers on dismissal or switching, and verdicts/deletions immediately. It restores a sole stored review or opens the floating selector for multiple reviews. Exports retain their own file baseline and do not become automatic persistence targets; imports get fresh database identities. Review editors remain tab-owned native panes outside the workspace `PaneTree`. Storage opens lazily when a comparison is ready; the session supplies the configured or XDG default directory. Direct `Editor` callers inject a directory with `set_review_directory` to enable persistence. + `editor_review` owns tab-local review state independently of comparisons. Ordinary repository source buffers and diff panes share the comment editor, immutable anchors, storage, and review commands. Comment/summary text saves explicitly; dismissing discards that editor's unsaved text, while switching may retain hidden drafts. Verdicts/deletions/resolution changes commit immediately without saving drafts. It restores a sole stored review or opens the floating selector for multiple reviews. Exports retain their own file baseline and do not become automatic persistence targets; imports get fresh database identities. Review editors remain tab-owned native panes outside the workspace `PaneTree`. The session supplies the configured or XDG default directory; direct `Editor` callers inject a directory with `set_review_directory` to enable persistence. Successful SQLite autosaves and JSON-only saves share one acknowledgement operation: clear review dirtiness and advance the owning tab's native draft saved baselines only after persistence succeeds. Failed writes retain unsaved state; JSON exports do not acknowledge a pending SQLite save. Review replacement and comment deletion are private editor-owned operations. Replacement loads and validates its Stored/JSON/New source before retiring old bindings, installing the document, and resetting selection/navigation. Deletion retires and acknowledges only the matching comment drafts, preventing stale text from overwriting reused IDs. Both persist before opening the overview; a post-mutation save failure retains the applied, unsaved identity change. Native text/undo and general transition guards remain unchanged. - `clipboard`: owns native plain-text provider selection and bounded subprocess I/O; it never computes selections or editing offsets. diff --git a/docs/configuration.md b/docs/configuration.md index 22fd2d9..8d90ebe 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -259,8 +259,8 @@ Normal and Visual Ctrl+u/Ctrl+d invoke `config.core.scroll_step_up` / `config.co | Normal | `edit`, `buffers`, `buffer`, `buffer_next`, `buffer_previous`, `buffer_delete`, `buffer_force_delete` | | Normal | `follow_link` | | Normal | `diff`, `diff.mode.toggle`, `diff.view.toggle`, `diff.orientation.toggle`, `diff.review.toggle` | -| Normal, Visual | `diff.comment` | -| Normal | `diff.review.edit`, `diff.review.delete`, `diff.review.resolve`, `diff.review.unresolve`, `diff.review.next`, `diff.review.previous`, `diff.review.overview`, `diff.review.copy`, `diff.review.summary`, `diff.review.verdict`, `diff.review.save`, `diff.review.open`, `diff.review.new` | +| Normal, Visual | `comment`, `diff.comment` | +| Normal | `review.edit`, `review.delete`, `review.resolve`, `review.unresolve`, `review.next`, `review.previous`, `review.overview`, `review.copy`, `review.summary`, `review.verdict`, `review.save`, `review.open`, `review.new` | | Normal, Insert, Visual, Command, Search | `diff.close` | | Normal | `resolve`, `resolve.next`, `resolve.previous`, `resolve.use_source`, `resolve.source`, `resolve.base`, `resolve.next_file`, `resolve.previous_file`, `resolve.files`, `resolve.draft`, `resolve.history`, `resolve.view.toggle`, `resolve.close` | | Normal, Insert, Visual, Command, Search | `word_wrap.toggle` | @@ -346,16 +346,16 @@ Ordinary `config.core.file_picker` and `config.core.file_explorer` bindings work `config.core.diff.review.toggle` is a non-mutating Normal-mode action, also available as `:diff.review.toggle` and in the palette. It marks/unmarks the highlighted changed-file leaf when the comparison's explorer is focused, or the current comparison file from a text pane. Read-only snapshots permit review marks. The default `c` shortcut applies only while browsing the diff's changed-file tree; explicit bindings and disabled keys override it. For example, `config.bind "ctrl+g", config.core.diff.review.toggle`; use the palette or a control-key binding while filtering, including List view, so printable input stays in the query. -Local comments use `config.core.diff.comment` in Normal or Visual mode; the remaining `config.core.diff.review` actions in the table above use Normal mode. They have no default bindings and are available through the palette and canonical Ex commands. `edit`, `delete`, `resolve`, `unresolve`, `verdict`, `save`, and `open` open argument prompts when selected as actions. Comment IDs are optional and default to the overview entry under the caret or selected comment. For example: +Local comments use `config.core.comment` in Normal or Visual mode in ordinary repository source buffers and diff text panes; the remaining `config.core.review` actions in the table above use Normal mode. Update custom Ruby bindings from `config.core.diff.comment` and `config.core.diff.review.*` to these names; only the old Ex command spellings (`:diff.comment` and `:diff.review.*`) remain aliases. File checkmarks still use `config.core.diff.review.toggle`. These actions have no default bindings and are available through the palette and Ex commands. `edit`, `delete`, `resolve`, `unresolve`, `verdict`, `save`, and `open` open argument prompts when selected as actions. Comment IDs are optional and default to the overview entry under the caret or selected comment. For example: ```ruby -config.bind "leader r c", config.core.diff.comment -config.bind "leader r c", config.core.diff.comment, mode: config.modes.visual -config.bind "leader r o", config.core.diff.review.overview -config.bind "leader r n", config.core.diff.review.next +config.bind "leader r c", config.core.comment +config.bind "leader r c", config.core.comment, mode: config.modes.visual +config.bind "leader r o", config.core.review.overview +config.bind "leader r n", config.core.review.next ``` -Review editing uses a floating window without adding workspace splits. Dismissing or switching editors commits text to local SQLite; verdicts, deletions, and resolution changes commit immediately. `:w` commits explicitly; `:diff.review.save path` exports JSON without redirecting autosaves. `config.review_directory` defaults to `nil`, selecting `$XDG_DATA_HOME/vamp/reviews/` (absolute XDG path only), otherwise `~/.local/share/vamp/reviews/`. String overrides are resolved relative to the configuration file; empty/NUL-containing strings and non-string/non-nil values are rejected. These actions do not contact a forge. See [local review behavior](editing.md#repository-diff). +Review editing uses a floating window without adding workspace splits. `:w` or `:review.save` commits comment/summary text, including hidden drafts; dismissing an editor discards its unsaved text, and switching editors does not save it. Verdicts, deletions, and resolution changes commit immediately without saving drafts. `:review.save path` additionally exports JSON without redirecting autosaves. `config.review_directory` defaults to `nil`, selecting `$XDG_DATA_HOME/vamp/reviews/` (absolute XDG path only), otherwise `~/.local/share/vamp/reviews/`. String overrides are resolved relative to the configuration file; empty/NUL-containing strings and non-string/non-nil values are rejected. These actions do not contact a forge. See [local review behavior](editing.md#repository-diff). `config.core.pane.close` and `config.core.pane.force_close` close a focused **docked** file list instead of its editor pane, without discarding buffers or exiting the comparison. A floating picker does not intercept these actions: normal close still protects underlying dirty buffers, while force-close retains its existing discard semantics. For example, bind `config.bind "ctrl+x", config.core.pane.close`; focused tree/selection overrides and printable filter input retain priority. Palette-originated close actions retain the originating dock target through force confirmation. diff --git a/docs/development.md b/docs/development.md index 7dd7d6a..6504e3c 100644 --- a/docs/development.md +++ b/docs/development.md @@ -98,6 +98,8 @@ Diff-stat checks cover unopened frozen files, additions/deletions/renames, binar Local review checks cover native comment/summary composition and undo, Visual ranges on either side, verdicts, JSON save/reopen, external-write refusal, hidden drafts and teardown guards, immutable Working identities in Git/jj, renamed paths, live EOF, and stale anchors. Render checks cover selected/range markers, split/unified context, wrapped continuations, and narrow gutters. Live PTY smoke creates and saves a comment/summary/verdict, closes and reopens the review, edits the reopened comment, changes the reviewed source, and observes stale status without altering the saved anchor. Fixtures are local repositories; no forge writes. +Ordinary-source review checks cover unsaved Unicode selections, line/empty-line anchors, no storage creation on initialization, explicit save/discard, optimistic conflicts, tab/pane guards, JSON round-trips, stale read-only snapshots, and rendered exact-range markers. Live PTY smoke selects a source line, creates a comment with `:comment`, saves with `:w`, verifies its exact excerpt through the review CLI, restarts with `:review.next`, and reopens after changing the selected source to observe the preserved original snapshot. Review state is tab-owned rather than comparison-owned; diff regression coverage remains required. + Review clipboard checks cover System/Separate routing with isolated providers, exact Unicode excerpts, resolved/stale comments across files, retained unsaved drafts, unchanged storage revisions, and empty/invalid/failed-copy preservation. Live PTY smoke imports a review, copies it, pastes into an empty buffer, and verifies the saved bytes exactly match CLI `show --all`; default CLI output still excludes resolved comments. SQLite review checks cover exact anchors across restarts and revision changes, repository isolation, independent connections and optimistic conflicts, locked/unwritable stores, incompatible schema preservation, relational constraints, full unsigned comment IDs, stable comment order, bounded/corrupt rows, rollback after partial comment insertion, private file permissions, and XDG/config path resolution. Schema 1 document storage is rejected without modification. Editor checks require explicit writes for comment/summary text, discard unsaved text on dismissal without retaining discarded undo history, prevent metadata autosaves from committing hidden drafts, persist verdicts/deletions, restore a sole review, choose among multiple reviews, and keep exports independent from autosaves. Test fixtures use isolated review directories. Live PTY smoke verifies that Escape leaves no new stored comment, `:w` persists exact text, and pane close discards a later edit; reopening and writing again preserves the saved body. diff --git a/docs/editing.md b/docs/editing.md index b281143..87504c5 100644 --- a/docs/editing.md +++ b/docs/editing.md @@ -482,27 +482,29 @@ Collapsed folders show a green **`✓`** when every changed file beneath them is Review marks are saved with the selected local review and restored by immutable per-file Before/After identities and file paths. Changes to unrelated files or commit metadata do not clear them; changes to either side's bytes, final newline, file mode, or rename paths require re-review. They survive filtering, file navigation, lazy loading, tabs, split/unified/orientation changes, and pinned checkout-only transitions. Editing a writable After file clears only that file's mark and saves the removal; unchanged refreshes and saves preserve it. Marks on live text also retain its exact contents, including the final newline, so different unsaved text cannot inherit a checkmark. Marks do not change document text, undo history, or repository state. Without configured SQLite storage, use `:diff.review.save path` to save them as JSON. Older revision-pair-scoped marks remain stored but must be marked again to acquire per-file identities; comments, summaries, and verdicts are unaffected. -**Local review comments** share local storage with the file-reviewed checkmarks above; neither feature publishes to GitHub. Focus either diff text pane and run **`:diff.comment`** to comment on the current line. A characterwise Visual selection (`v`) anchors its exact text, including partial lines and multiline spans; Visual Line (`V`) anchors whole selected lines. Type **`:diff.comment`** and press Enter, or use the palette/a configured binding. Multiple independent carets/ranges must be reviewed separately. +**Local review comments** work in ordinary repository source buffers and diff text panes; they share storage with file-reviewed checkmarks and never publish to GitHub. Run **`:comment`** to comment on the current line. A characterwise Visual selection (`v`) anchors its exact text, including partial lines and multiline spans; Visual Line (`V`) anchors whole selected lines. Multiple independent carets/ranges must be reviewed separately. `:diff.comment` remains available for the same operation. -Comments, summaries, and the review overview open in a focused floating editor over the whole diff text workspace, not a source-side split. The source layout, file explorer, and viewports stay unchanged. Normal editing, selections, and undo/redo work in editable review buffers. Escape in Normal mode or `:pane.close` dismisses the window and returns to the diff, discarding that editor's unsaved text. An unsaved new comment is not attached to the review; an existing comment or summary reopens with its last saved text and no discarded undo history. Insert/Visual/Command Escape first retains its usual modal behavior. Source rows are never replaced with comment text. Magenta **`○`** gutter markers identify saved comment ranges; **`●`** identifies the selected comment. Its exact current text range has a magenta background, including wrapped continuations and selected newline cells; native Visual selections take precedence. Stale comments have neither markers nor text highlighting. Markers preserve addition/deletion signs, appear only on the first wrapped segment, and also work in unified view. +Comments, summaries, and the review overview open in a focused floating editor over the source workspace, not a source-side split. The source layout, file explorer, and viewports stay unchanged. Normal editing, selections, and undo/redo work in editable review buffers. Escape in Normal mode or `:pane.close` dismisses the window and returns to the source, discarding that editor's unsaved text. An unsaved new comment is not attached to the review; an existing comment or summary reopens with its last saved text and no discarded undo history. Insert/Visual/Command Escape first retains its usual modal behavior. Source rows are never replaced with comment text. Magenta **`○`** gutter markers identify saved comment ranges; **`●`** identifies the selected comment. Its exact current text range has a magenta background, including wrapped continuations and selected newline cells; native Visual selections take precedence. Stale comments have neither markers nor text highlighting. Markers preserve addition/deletion signs, appear only on the first wrapped segment, and also work in unified view. Local review buffers (comments, summaries, overview, and stored-review selector) soft-wrap by default without inserting newlines into saved text. Use `:word_wrap.toggle` to disable wrapping for the focused review buffer; reopening it retains that choice for the session. Source diff wrapping remains independent. | Command | Effect | | --- | --- | -| `:diff.comment` | Add a comment on the current line or exact selected text range, on the focused side | -| `:diff.review.edit [id]` | Edit an existing comment; defaults to the overview entry under the caret or selected comment | -| `:diff.review.delete [id]` | Delete that draft comment | -| `:diff.review.resolve [id]` / `:diff.review.unresolve [id]` | Mark a comment resolved/unresolved; uses the same default selection as edit/delete | -| `:diff.review.next` / `:diff.review.previous` | Cycle comments, opening their text and revealing a matching source anchor | -| `:diff.review.overview` | Refresh/open a read-only summary and comment list; Enter edits the entry under the caret | -| `:diff.review.copy` | Copy all saved comments in the selected review to the configured clipboard, using the CLI text format | -| `:diff.review.summary` | Compose the review summary | -| `:diff.review.verdict looks_good` | Record **Looks good** | -| `:diff.review.verdict changes_requested` | Record **Changes requested**; `none` clears the verdict | -| `:diff.review.save [path]` | Commit to SQLite; with a path, also export JSON for sharing | -| `:diff.review.open [id or path]` | Show stored reviews, open a stored review ID, or import a JSON review as a new review | -| `:diff.review.new` | Create a separate stored review for the current repository | +| `:comment` | Add a comment on the current source line or exact selected text range | +| `:review.edit [id]` | Edit an existing comment; defaults to the overview entry under the caret or selected comment | +| `:review.delete [id]` | Delete that draft comment | +| `:review.resolve [id]` / `:review.unresolve [id]` | Mark a comment resolved/unresolved; uses the same default selection as edit/delete | +| `:review.next` / `:review.previous` | Cycle comments, opening their text and revealing a matching source anchor | +| `:review.overview` | Refresh/open a read-only summary and comment list; Enter edits the entry under the caret | +| `:review.copy` | Copy all saved comments in the selected review to the configured clipboard, using the CLI text format | +| `:review.summary` | Compose the review summary | +| `:review.verdict looks_good` | Record **Looks good** | +| `:review.verdict changes_requested` | Record **Changes requested**; `none` clears the verdict | +| `:review.save [path]` | Commit to SQLite; with a path, also export JSON for sharing | +| `:review.open [id or path]` | Show stored reviews, open a stored review ID, or import a JSON review as a new review | +| `:review.new` | Create a separate stored review for the current repository | + +The `:diff.review.*` spellings remain available for these same operations. File checkmarks remain diff-specific (`:diff.review.toggle`). Comment and summary text is committed only by **`:w`** or **`:diff.review.save`**, which saves the entire review, including hidden drafts. Dismissing an editor discards its unsaved text; switching between review editors can retain hidden drafts but does not save them. File checkmarks, verdicts, deletions, and manual resolution changes commit immediately to local SQLite without committing draft text. `:diff.review.save path` or `:w path` additionally exports JSON; later metadata autosaves still go to SQLite and leave that export unchanged. Imports create a new stored review identity rather than overwriting another review. @@ -520,6 +522,8 @@ Version 4 JSON is the agent-readable sharing format: repository root, summary, o Reopening requires the same repository root. Unchanged comment ranges follow edits elsewhere in the file, including inserted/deleted lines above and column shifts before a partial-line selection. Working-snapshot refreshes can retain these attachments; unrelated immutable revisions and paths do not match. Mapping requires an unchanged, unambiguous text region containing the selection. Modified/deleted selections or ambiguous repeated regions are **stale**: comments remain available through the overview and next/previous navigation, with original snapshot coordinates and no current-source marker or jump. Saved anchors and exported excerpts remain unchanged. Unloaded files are shown as **not checked** until visited; refresh the overview to update its statuses. There is no server, authentication, or review-publishing operation. +Ordinary source comments use an immutable content snapshot identity and capture the current buffer, including unsaved text; they do not require a commit or a diff. They retain the same SQLite/JSON formats. Opening a source comment navigates to its file and mapped selection; if that text is stale or the file is unavailable, it opens the preserved original text read-only instead of attaching feedback to different code. + Resolution is a manual human decision, independent of anchor freshness: a comment can be both resolved and stale. Overview entries and comment titles label resolved/unresolved status. Resolved comments remain in the overview and next/previous navigation, and can still be edited or reopened. Resolving never changes an anchor or deletes feedback. diff --git a/src/diff.rs b/src/diff.rs index 25de452..2662e51 100644 --- a/src/diff.rs +++ b/src/diff.rs @@ -641,7 +641,6 @@ pub struct DiffState { pub loading: bool, pub error: Option, pub comparison: Option, - pub local_review: crate::review::ReviewState, pub comparison_id: Option, pub reviewed_files: BTreeSet, pub(crate) snapshot_roots: [Option; 2], @@ -708,7 +707,6 @@ impl DiffState { loading: true, error: None, comparison: None, - local_review: Default::default(), comparison_id: None, reviewed_files: BTreeSet::new(), files: None, @@ -1988,7 +1986,10 @@ impl DiffState { }) } - pub(crate) fn toggle_reviewed(&mut self) -> Result { + pub(crate) fn toggle_reviewed( + &mut self, + review: &mut crate::review::ReviewState, + ) -> Result { let comparison = self .comparison .as_ref() @@ -2030,10 +2031,10 @@ impl DiffState { .iter() .all(|revision| !revision.is_empty()) { - if self.local_review.document.root.as_os_str().is_empty() { - self.local_review.document = crate::review::Review::new(comparison.root.clone()); + if review.document.root.as_os_str().is_empty() { + review.document = crate::review::Review::new(comparison.root.clone()); } - let records = &mut self.local_review.document.reviewed_files; + let records = &mut review.document.reviewed_files; records.retain(|record| { record.revisions != *file @@ -2057,21 +2058,20 @@ impl DiffState { .then(|| file.after.join("\n")), }); } - self.local_review.dirty = true; + review.dirty = true; } Ok(reviewed) } - pub(crate) fn restore_reviewed_files(&mut self) { + pub(crate) fn restore_reviewed_files(&mut self, review: &crate::review::ReviewState) { self.reviewed_files.clear(); let Some(comparison) = self.comparison.as_ref() else { return; }; - if self.local_review.document.root != comparison.root { + if review.document.root != comparison.root { return; } - let records: BTreeMap<_, _> = self - .local_review + let records: BTreeMap<_, _> = review .document .reviewed_files .iter() @@ -2369,12 +2369,13 @@ impl DiffState { pub fn review_anchor_range( &self, anchor: &crate::review::Anchor, + review: &crate::review::ReviewState, ) -> Option> { use crate::review::Side; let Some(comparison) = &self.comparison else { return None; }; - if self.local_review.document.root != comparison.root { + if review.document.root != comparison.root { return None; } comparison @@ -2481,7 +2482,12 @@ impl DiffState { } } - pub fn refresh_after(&mut self, lines: &[String], revision: u64) { + pub fn refresh_after( + &mut self, + lines: &[String], + revision: u64, + review: &mut crate::review::ReviewState, + ) { if !self.live_after_available() { return; } @@ -2510,29 +2516,21 @@ impl DiffState { self.modified_after.insert(self.file); } let comparison = self.comparison.as_ref().unwrap(); - let record = (self.local_review.document.root == comparison.root) + let record = (review.document.root == comparison.root) .then(|| { - self.local_review - .document - .reviewed_files - .iter() - .position(|record| { - record.revisions - == *file - .review_revisions - .as_ref() - .unwrap_or(&comparison.revisions) - && record.path == file.path - && record.old_path == file.old_path - }) + review.document.reviewed_files.iter().position(|record| { + record.revisions + == *file + .review_revisions + .as_ref() + .unwrap_or(&comparison.revisions) + && record.path == file.path + && record.old_path == file.old_path + }) }) .flatten(); let matches = record - .and_then(|position| { - self.local_review.document.reviewed_files[position] - .after - .as_ref() - }) + .and_then(|position| review.document.reviewed_files[position].after.as_ref()) .map(|text| text.split('\n').eq(lines.iter().map(String::as_str))); if matches == Some(true) { self.reviewed_files.insert(self.file); @@ -2540,8 +2538,8 @@ impl DiffState { if self.reviewed_files.contains(&self.file) && !matches.unwrap_or(unchanged) { self.reviewed_files.remove(&self.file); if let Some(position) = record { - self.local_review.document.reviewed_files.remove(position); - self.local_review.dirty = true; + review.document.reviewed_files.remove(position); + review.dirty = true; } } if was_live && changed_lines.0 == changed_lines.1 && changed_lines.1 == changed_lines.2 { diff --git a/src/editor.rs b/src/editor.rs index 40694f5..397f314 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -990,96 +990,91 @@ core_actions![ true, ), ( - "core.diff.comment", - Action::DiffComment, + "core.comment", + Action::Comment, &[Mode::Normal, Mode::Visual], true ), ( - "core.diff.review.edit", + "core.review.edit", Action::ReviewEdit, &[Mode::Normal], true, true ), ( - "core.diff.review.delete", + "core.review.delete", Action::ReviewDelete, &[Mode::Normal], true, true ), ( - "core.diff.review.resolve", + "core.review.resolve", Action::ReviewResolve, &[Mode::Normal], true, true ), ( - "core.diff.review.unresolve", + "core.review.unresolve", Action::ReviewUnresolve, &[Mode::Normal], true, true ), ( - "core.diff.review.next", + "core.review.next", Action::ReviewNext, &[Mode::Normal], true ), ( - "core.diff.review.previous", + "core.review.previous", Action::ReviewPrevious, &[Mode::Normal], true ), ( - "core.diff.review.overview", + "core.review.overview", Action::ReviewOverview, &[Mode::Normal], true ), ( - "core.diff.review.copy", + "core.review.copy", Action::ReviewCopy, &[Mode::Normal], true ), ( - "core.diff.review.summary", + "core.review.summary", Action::ReviewSummary, &[Mode::Normal], true ), ( - "core.diff.review.verdict", + "core.review.verdict", Action::ReviewVerdict, &[Mode::Normal], true, true ), ( - "core.diff.review.save", + "core.review.save", Action::ReviewSave, &[Mode::Normal], true, true ), ( - "core.diff.review.open", + "core.review.open", Action::ReviewOpen, &[Mode::Normal], true, true ), - ( - "core.diff.review.new", - Action::ReviewNew, - &[Mode::Normal], - true - ), + ("core.review.new", Action::ReviewNew, &[Mode::Normal], true), ( "core.word_wrap.toggle", Action::WordWrapToggle, @@ -1193,6 +1188,20 @@ const COMMAND_ALIASES: &[(&str, Action)] = &[ ("bprev", Action::BufferPrevious), ("bdelete", Action::BufferDelete), ("bdelete!", Action::BufferForceDelete), + ("diff.comment", Action::Comment), + ("diff.review.edit", Action::ReviewEdit), + ("diff.review.delete", Action::ReviewDelete), + ("diff.review.resolve", Action::ReviewResolve), + ("diff.review.unresolve", Action::ReviewUnresolve), + ("diff.review.next", Action::ReviewNext), + ("diff.review.previous", Action::ReviewPrevious), + ("diff.review.overview", Action::ReviewOverview), + ("diff.review.copy", Action::ReviewCopy), + ("diff.review.summary", Action::ReviewSummary), + ("diff.review.verdict", Action::ReviewVerdict), + ("diff.review.save", Action::ReviewSave), + ("diff.review.open", Action::ReviewOpen), + ("diff.review.new", Action::ReviewNew), ]; fn core_command(name: &str) -> Option<&'static CoreAction> { @@ -1893,6 +1902,7 @@ struct Tab { pane_id: usize, pane_layout: PaneTree, review_pane: Option, + local_review: crate::review::ReviewState, file_picker: Option, diff: Option, parked_diff: Option, @@ -2200,6 +2210,7 @@ pub struct Editor { panes: BTreeMap, pane_layout: PaneTree, review_pane: Option, + pub(crate) local_review: crate::review::ReviewState, tab_id: usize, next_tab_id: usize, tab_order: Vec, @@ -2313,6 +2324,7 @@ impl Editor { )]), pane_layout: PaneTree::new(1), review_pane: None, + local_review: Default::default(), tab_id: 1, next_tab_id: 2, tab_order: vec![1], @@ -3663,6 +3675,7 @@ impl Editor { pane_id: std::mem::replace(&mut self.pane_id, next.pane_id), pane_layout: std::mem::replace(&mut self.pane_layout, next.pane_layout), review_pane: std::mem::replace(&mut self.review_pane, next.review_pane), + local_review: std::mem::replace(&mut self.local_review, next.local_review), file_picker: std::mem::replace(&mut self.file_picker, next.file_picker), diff: std::mem::replace(&mut self.diff, next.diff), parked_diff: std::mem::replace(&mut self.parked_diff, next.parked_diff), @@ -3712,6 +3725,7 @@ impl Editor { pane_id, pane_layout: PaneTree::new(pane_id), review_pane: None, + local_review: Default::default(), file_picker: None, diff: None, parked_diff: None, @@ -3814,11 +3828,6 @@ impl Editor { // No input dispatch or document copies: source documents stay unique in active/parked. fn select_pane(&mut self, id: usize) { - let id = if self.review_pane == Some(id) && self.workspace_diff().is_none() { - self.panes[&id].origin.unwrap_or(id) - } else { - id - }; if id == self.pane_id { return; } @@ -4223,7 +4232,12 @@ impl Editor { return; } self.finish_pane_interaction(); - if self.diff_owner == Some(id) && !force && self.local_review_close_blocked() { + let review_source = + self.review_pane.and_then(|panel| self.panes[&panel].origin) == Some(id); + if (self.diff_owner == Some(id) || review_source) + && !force + && self.local_review_close_blocked() + { return; } if self.diff_owner == Some(id) { @@ -4236,6 +4250,9 @@ impl Editor { } let next = self.pane_layout.close(id).expect("another pane exists"); self.select_pane(next); + if review_source && let Some(panel) = self.review_pane { + self.panes.get_mut(&panel).unwrap().origin = Some(next); + } self.panes.remove(&id); for buffer in buffers { self.forget_buffer(buffer); @@ -4627,11 +4644,17 @@ impl Editor { view.clamp(&self.active.lines); self.active.view = view; } - if let Err(error) = self.initialize_local_review() { + if self.review_directory.is_some() + && self.resolver().is_none() + && self + .workspace_diff() + .is_some_and(|diff| diff.phase == Phase::View) + && let Err(error) = self.initialize_local_review() + { self.set_message(format!("Local review: {error}")); } if restore_reviewed && let Some(diff) = &mut self.diff { - diff.restore_reviewed_files(); + diff.restore_reviewed_files(&self.local_review); } let reviewed = self.diff.as_mut().and_then(|diff| { (refresh @@ -5190,16 +5213,19 @@ impl Editor { pub(crate) fn refresh_live_diff(&mut self) { self.refresh_resolver_comparison(); - for diff in self - .diff - .iter_mut() - .chain(self.parked_diff.iter_mut()) - .chain( - self.tabs - .values_mut() - .flat_map(|tab| tab.diff.iter_mut().chain(tab.parked_diff.iter_mut())), - ) - { + for (diff, parked, review) in std::iter::once(( + &mut self.diff, + &mut self.parked_diff, + &mut self.local_review, + )) + .chain( + self.tabs + .values_mut() + .map(|tab| (&mut tab.diff, &mut tab.parked_diff, &mut tab.local_review)), + ) { + let Some(diff) = diff.as_mut().or(parked.as_mut()) else { + continue; + }; let buffer = diff.live_buffer.and_then(|id| { if id == self.active.id { Some(&self.active) @@ -5211,7 +5237,7 @@ impl Editor { && diff.live_after_available() && diff.live_revision != Some(buffer.text_revision) { - diff.refresh_after(&buffer.lines, buffer.text_revision); + diff.refresh_after(&buffer.lines, buffer.text_revision, review); diff.follow_caret = true; } } @@ -5485,7 +5511,7 @@ impl Editor { | Action::DiffViewToggle | Action::DiffOrientationToggle | Action::DiffReviewToggle - | Action::DiffComment + | Action::Comment | Action::ReviewEdit | Action::ReviewDelete | Action::ReviewResolve @@ -8917,7 +8943,7 @@ impl Editor { | Action::DiffViewToggle | Action::DiffOrientationToggle | Action::DiffReviewToggle - | Action::DiffComment + | Action::Comment | Action::ReviewEdit | Action::ReviewDelete | Action::ReviewResolve @@ -9559,15 +9585,22 @@ impl Editor { .map(|buffer| buffer.id); if let Some(id) = loaded { self.switch_buffer(id); - return Ok(()); + } else { + let buffer = Buffer::open( + self.next_buffer_id, + Some(path), + Some(identity), + allow_missing, + )?; + self.add_buffer(buffer); + } + if self.review_directory.is_some() + && self.workspace_diff().is_none() + && self.resolver().is_none() + && let Err(error) = self.initialize_local_review() + { + self.set_message(format!("Local review: {error}")); } - let buffer = Buffer::open( - self.next_buffer_id, - Some(path), - Some(identity), - allow_missing, - )?; - self.add_buffer(buffer); Ok(()) } @@ -9745,7 +9778,7 @@ impl Editor { fn restart(&mut self, force: bool) { if !force && self.local_reviews_dirty() { - self.set_message("Unsaved local review; use :diff.review.save "); + self.set_message("Unsaved local review; use :review.save "); return; } if force || !self.any_dirty() { @@ -9876,7 +9909,7 @@ impl Editor { } if matches!( entry.action, - Action::DiffComment + Action::Comment | Action::ReviewEdit | Action::ReviewDelete | Action::ReviewResolve @@ -10102,15 +10135,15 @@ impl Editor { self.diff_return_buffer = Some(self.active.id); return None; } - let Some(diff) = self - .picker_diff_mut() - .filter(|diff| diff.phase == Phase::View) - else { - self.set_message("No comparison is open"); - return None; - }; if entry.action == Action::DiffReviewToggle { - match diff.toggle_reviewed() { + let mut review = std::mem::take(&mut self.local_review); + let result = self + .picker_diff_mut() + .filter(|diff| diff.phase == Phase::View) + .ok_or("No comparison is open") + .and_then(|diff| diff.toggle_reviewed(&mut review)); + self.local_review = review; + match result { Ok(reviewed) => { if self.resolver().is_none() && let Err(error) = self.autosave_local_review() @@ -10128,6 +10161,13 @@ impl Editor { } return None; } + let Some(diff) = self + .picker_diff_mut() + .filter(|diff| diff.phase == Phase::View) + else { + self.set_message("No comparison is open"); + return None; + }; if entry.action == Action::DiffViewToggle { diff.set_view_mode(match diff.view_mode { ViewMode::Split => ViewMode::Unified, diff --git a/src/editor_review.rs b/src/editor_review.rs index 1d6df42..14d8b5a 100644 --- a/src/editor_review.rs +++ b/src/editor_review.rs @@ -25,58 +25,81 @@ pub(super) struct ReviewBuffer { impl Editor { pub fn set_review_directory(&mut self, directory: PathBuf) { self.review_directory = Some(directory); + if self.review_root().is_ok() + && let Err(error) = self.initialize_local_review() + { + self.set_message(format!("Local review: {error}")); + } } pub(super) fn initialize_local_review(&mut self) -> Result<(), String> { - let Some(directory) = self.review_directory.as_ref() else { - return Ok(()); - }; - let Some(diff) = self - .workspace_diff() - .filter(|diff| diff.phase == Phase::View) - else { - return Ok(()); - }; - if diff.local_review.initialized { + let root = self.review_root()?; + if self.local_review.document.root == root { + self.local_review.initialized = true; return Ok(()); } - let root = diff - .comparison - .as_ref() - .ok_or("Comparison is loading")? - .root - .clone(); - let directory = directory.clone(); - self.review_diff_mut().unwrap().local_review.initialized = true; - self.review_diff_mut().unwrap().local_review.document = Review::new(root.clone()); - let store = crate::review::Store::open(&directory)?; - let mut reviews = store.list(&root)?; + if self.current_review_dirty() { + return Err("Save the current review before changing repositories".into()); + } + let reviews = match self.review_directory.as_deref() { + Some(directory) => crate::review::Store::open_read_only(directory)? + .map(|store| store.list(&root)) + .transpose()? + .unwrap_or_default(), + None => Vec::new(), + }; + self.detach_review_buffers(); + self.local_review = crate::review::ReviewState { + document: Review::new(root), + initialized: true, + ..Default::default() + }; + let mut reviews = reviews; if reviews.len() == 1 { - self.review_diff_mut().unwrap().local_review.document = reviews.pop().unwrap(); + self.local_review.document = reviews.pop().unwrap(); } else if reviews.len() > 1 { self.show_review_selector(reviews)?; } Ok(()) } + fn review_root(&self) -> Result { + if let Some(diff) = self.workspace_diff() { + return diff + .comparison + .as_ref() + .map(|comparison| { + fs::canonicalize(&comparison.root).unwrap_or_else(|_| comparison.root.clone()) + }) + .ok_or_else(|| "Comparison is still loading".into()); + } + if self.active.review_buffer.is_some() + || (self.active.path.is_none() && self.local_review.initialized) + { + return Ok(self.local_review.document.root.clone()); + } + let path = self + .active + .identity + .as_deref() + .or(self.active.path.as_deref()) + .ok_or("Save the source file before commenting")?; + crate::repository::discover_root(path.parent().ok_or("Source has no parent directory")?) + } + fn open_review_selector(&mut self) -> Result<(), String> { - let diff = self.workspace_diff().ok_or("No comparison is open")?; + let root = self.review_root()?; let directory = self .review_directory .as_deref() .ok_or("Review storage is not configured")?; - let root = &diff - .comparison - .as_ref() - .ok_or("Comparison is loading")? - .root; - let reviews = crate::review::Store::open(directory)?.list(root)?; + let reviews = crate::review::Store::open(directory)?.list(&root)?; self.show_review_selector(reviews) } fn show_review_selector(&mut self, reviews: Vec) -> Result<(), String> { let mut text = String::from( - "# Local reviews\n\nEnter opens the review under the caret.\n:diff.review.new starts a separate review.\n", + "# Local reviews\n\nEnter opens the review under the caret.\n:review.new starts a separate review.\n", ); for review in reviews { let id = review.storage_id.unwrap(); @@ -95,14 +118,14 @@ impl Editor { self.open_review_buffer( ReviewText::Reviews, &text, - "Local reviews · Enter opens · :diff.review.new".into(), + "Local reviews · Enter opens · :review.new".into(), )?; self.place_navigation(TextPosition { row, column: 0 }); Ok(()) } fn mark_review_saved(&mut self) { - self.review_diff_mut().unwrap().local_review.dirty = false; + self.local_review.dirty = false; for buffer in std::iter::once(&mut self.active).chain(self.parked.values_mut()) { if buffer .review_buffer @@ -120,21 +143,12 @@ impl Editor { let Some(directory) = self.review_directory.as_deref() else { return Ok(()); }; - if !self - .workspace_diff() - .is_some_and(|diff| diff.local_review.dirty) - { + if !self.local_review.dirty { return Ok(()); } let mut store = crate::review::Store::open(directory)?; - store.save( - &mut self - .review_diff_mut() - .ok_or("No comparison is open")? - .local_review - .document, - )?; - self.review_diff_mut().unwrap().local_review.dirty = false; + store.save(&mut self.local_review.document)?; + self.local_review.dirty = false; Ok(()) } pub fn review_panel_title(&self) -> Option<&str> { @@ -145,8 +159,7 @@ impl Editor { } pub fn review_panel(&self) -> Option { - self.review_pane - .filter(|pane| *pane == self.pane_id && self.workspace_diff().is_some()) + self.review_pane.filter(|pane| *pane == self.pane_id) } pub(super) fn dismiss_review_panel(&mut self) -> bool { @@ -165,7 +178,7 @@ impl Editor { buffer.view.word_wrap = wrap; buffer.resolver_readonly = self.active.resolver_readonly; if binding.pending_comment.take().is_some() { - self.review_diff_mut().unwrap().local_review.selected = None; + self.local_review.selected = None; } buffer.review_buffer = Some(binding); self.active = buffer; @@ -175,25 +188,16 @@ impl Editor { true } - fn review_diff_mut(&mut self) -> Option<&mut DiffState> { - self.diff.as_mut().or(self.parked_diff.as_mut()) - } - pub(super) fn local_reviews_dirty(&self) -> bool { - self.workspace_diff() - .is_some_and(|diff| diff.local_review.dirty) - || self - .tabs - .values() - .any(|tab| tab.diff().is_some_and(|diff| diff.local_review.dirty)) + self.local_review.dirty + || self.tabs.values().any(|tab| tab.local_review.dirty) || std::iter::once(&self.active) .chain(self.parked.values()) .any(|buffer| buffer.review_buffer.is_some() && buffer.dirty()) } pub(super) fn current_review_dirty(&self) -> bool { - self.workspace_diff() - .is_some_and(|diff| diff.local_review.dirty) + self.local_review.dirty || std::iter::once(&self.active) .chain(self.parked.values()) .any(|buffer| { @@ -213,7 +217,7 @@ impl Editor { return true; } if self.current_review_dirty() { - self.set_message("Unsaved local review; use :diff.review.save before closing"); + self.set_message("Unsaved local review; use :review.save before closing"); true } else { false @@ -258,13 +262,15 @@ impl Editor { }; // Retire old drafts before comment IDs can be reused. self.detach_review_buffers(); - let state = &mut self.review_diff_mut().unwrap().local_review; + let state = &mut self.local_review; state.document = document; state.path = path; state.selected = None; state.pending_navigation = None; state.dirty = dirty; - self.review_diff_mut().unwrap().restore_reviewed_files(); + if let Some(diff) = self.diff.as_mut().or(self.parked_diff.as_mut()) { + diff.restore_reviewed_files(&self.local_review); + } // Persistence failure leaves the identity change applied and unsaved. self.autosave_local_review()?; self.open_review_overview() @@ -272,7 +278,7 @@ impl Editor { fn delete_local_review_comment(&mut self, args: &str) -> Result<(), String> { let id = self.review_comment_id(args)?; - let state = &mut self.review_diff_mut().unwrap().local_review; + let state = &mut self.local_review; state.document.comments.retain(|comment| comment.id != id); state.selected = None; state.pending_navigation = None; @@ -308,14 +314,11 @@ impl Editor { }) }) .collect(); - let Some(diff) = self.review_diff_mut() else { - return Vec::new(); - }; + let state = &mut self.local_review; let mut previous = Vec::new(); for (kind, text, pending) in updates { if let Some(mut comment) = pending - && !diff - .local_review + && !state .document .comments .iter() @@ -323,19 +326,18 @@ impl Editor { { previous.push((kind, None)); comment.body = text; - diff.local_review.document.comments.push(comment); - diff.local_review.dirty = true; + state.document.comments.push(comment); + state.dirty = true; continue; } let destination = match kind { - ReviewText::Comment(id) => diff - .local_review + ReviewText::Comment(id) => state .document .comments .iter_mut() .find(|comment| comment.id == id) .map(|comment| &mut comment.body), - ReviewText::Summary => Some(&mut diff.local_review.document.summary), + ReviewText::Summary => Some(&mut state.document.summary), ReviewText::Overview => None, ReviewText::Reviews => None, }; @@ -344,22 +346,18 @@ impl Editor { { let old = std::mem::replace(destination, text); previous.push((kind, Some(old))); - diff.local_review.dirty = true; + state.dirty = true; } } previous } pub(super) fn save_local_review(&mut self, path: Option<&Path>) -> Result<(), String> { - let dirty = self - .review_diff_mut() - .ok_or("No comparison is open")? - .local_review - .dirty; + let dirty = self.local_review.dirty; let previous = self.sync_review_text(); let result = self.write_local_review(path); if result.is_err() { - let state = &mut self.review_diff_mut().unwrap().local_review; + let state = &mut self.local_review; for (kind, text) in previous { match (kind, text) { (ReviewText::Comment(id), None) => { @@ -386,12 +384,8 @@ impl Editor { fn write_local_review(&mut self, path: Option<&Path>) -> Result<(), String> { if self.review_directory.is_some() { if let Some(path) = path { - self.review_diff_mut() - .ok_or("No comparison is open")? - .local_review - .document - .save(path)?; - self.review_diff_mut().unwrap().local_review.path = Some(path.to_path_buf()); + self.local_review.document.save(path)?; + self.local_review.path = Some(path.to_path_buf()); } self.autosave_local_review()?; self.mark_review_saved(); @@ -402,14 +396,11 @@ impl Editor { }); return Ok(()); } - let state = &mut self - .review_diff_mut() - .ok_or("No comparison is open")? - .local_review; + let state = &mut self.local_review; let path = path .map(Path::to_path_buf) .or_else(|| state.path.clone()) - .ok_or("Use :diff.review.save to choose a local JSON review file")?; + .ok_or("Use :review.save to choose a local JSON review file")?; state.document.save(&path)?; state.path = Some(path); self.mark_review_saved(); @@ -423,19 +414,10 @@ impl Editor { args: &str, ) -> Result<(), String> { if self.resolver().is_some() { - return Err("Local reviews require an ordinary diff comparison".into()); + return Err("Local reviews are unavailable in conflict resolution workspaces".into()); } self.initialize_local_review()?; - let diff = self - .workspace_diff() - .filter(|diff| diff.phase == Phase::View) - .ok_or("No comparison is open")?; - let root = diff - .comparison - .as_ref() - .ok_or("Comparison is still loading")? - .root - .clone(); + let root = self.review_root()?; if !matches!( action, Action::ReviewEdit @@ -449,16 +431,8 @@ impl Editor { { return Err("This review command takes no arguments".into()); } - if self - .workspace_diff() - .unwrap() - .local_review - .document - .root - .as_os_str() - .is_empty() - { - self.review_diff_mut().unwrap().local_review.document = Review::new(root.clone()); + if self.local_review.document.root.as_os_str().is_empty() { + self.local_review.document = Review::new(root.clone()); } if !matches!(action, Action::ReviewSave | Action::ReviewCopy) { self.autosave_local_review()?; @@ -480,7 +454,7 @@ impl Editor { return self.replace_local_review(root, source); } match action { - Action::DiffComment => self.add_review_comment()?, + Action::Comment => self.add_review_comment()?, Action::ReviewNew => self.replace_local_review(root, ReviewSource::New)?, Action::ReviewEdit => { let id = self.review_comment_id(args)?; @@ -489,7 +463,7 @@ impl Editor { Action::ReviewResolve | Action::ReviewUnresolve => { let id = self.review_comment_id(args)?; let resolved = action == Action::ReviewResolve; - let state = &mut self.review_diff_mut().unwrap().local_review; + let state = &mut self.local_review; let comment = state .document .comments @@ -522,7 +496,7 @@ impl Editor { } Action::ReviewDelete => self.delete_local_review_comment(args)?, Action::ReviewNext | Action::ReviewPrevious => { - let state = &self.workspace_diff().unwrap().local_review; + let state = &self.local_review; let comments = &state.document.comments; if comments.is_empty() { return Err("No local review comments".into()); @@ -540,7 +514,7 @@ impl Editor { } Action::ReviewOverview => self.open_review_overview()?, Action::ReviewCopy => { - let review = &self.workspace_diff().unwrap().local_review.document; + let review = &self.local_review.document; let count = review.comments.len(); if count == 0 { return Err("No local review comments".into()); @@ -554,13 +528,7 @@ impl Editor { self.set_message(format!("Copied {count} review comments")); } Action::ReviewSummary => { - let text = self - .workspace_diff() - .unwrap() - .local_review - .document - .summary - .clone(); + let text = self.local_review.document.summary.clone(); self.open_review_buffer( ReviewText::Summary, &text, @@ -574,11 +542,11 @@ impl Editor { "none" => None, _ => { return Err( - "Usage: diff.review.verdict ".into(), + "Usage: review.verdict ".into() ); } }; - let state = &mut self.review_diff_mut().unwrap().local_review; + let state = &mut self.local_review; if state.document.verdict != verdict { state.document.verdict = verdict; state.dirty = true; @@ -595,10 +563,7 @@ impl Editor { } fn review_comment_id(&self, args: &str) -> Result { - let state = &self - .workspace_diff() - .ok_or("No comparison is open")? - .local_review; + let state = &self.local_review; let under_cursor = self .active .review_buffer @@ -633,16 +598,65 @@ impl Editor { .ok_or_else(|| format!("No local comment #{id}")) } - fn add_review_comment(&mut self) -> Result<(), String> { - let owner = self.diff_owner.ok_or("No comparison is open")?; - if self.pane_id != owner { - self.select_pane(owner); + pub(crate) fn source_review_anchor_range( + &self, + anchor: &Anchor, + ) -> Option> { + if self.active.review_buffer.is_some() || !anchor.revision.starts_with("working:") { + return None; + } + let path = self + .active + .identity + .as_ref() + .or(self.active.path.as_ref())?; + let relative = path.strip_prefix(&self.local_review.document.root).ok()?; + anchor.mapped_range( + &anchor.revision, + relative, + Side::After, + &self.active.lines, + false, + ) + } + + fn review_anchor_range(&self, anchor: &Anchor) -> Option> { + if let Some(diff) = self.workspace_diff() { + return diff.review_anchor_range(anchor, &self.local_review); + } + let path = self.local_review.document.root.join(&anchor.path); + let buffer = std::iter::once(&self.active) + .chain(self.parked.values()) + .find(|buffer| { + buffer.identity.as_ref() == Some(&path) && buffer.review_buffer.is_none() + })?; + if !anchor.revision.starts_with("working:") { + return None; + } + anchor.mapped_range( + &anchor.revision, + &anchor.path, + Side::After, + &buffer.lines, + false, + ) + } + fn add_review_comment(&mut self) -> Result<(), String> { + if let Some(owner) = self.diff_owner { + if self.pane_id != owner { + self.select_pane(owner); + } + self.bind_diff_buffer(); + if self + .diff_buffer_focus() + .filter(|_| self.diff_interacting()) + .is_none() + { + return Err("Focus a diff text pane to add a comment".into()); + } + } else if self.active.review_buffer.is_some() || self.active.resolver_readonly { + return Err("Focus a source text pane to add a comment".into()); } - self.bind_diff_buffer(); - let focus = self - .diff_buffer_focus() - .filter(|_| self.diff_interacting()) - .ok_or("Focus a diff text pane to add a comment")?; if self.selection_count() != 1 { return Err("Use one caret or one contiguous selection for a review comment".into()); } @@ -663,22 +677,45 @@ impl Editor { }, } }); - let diff = self.diff.as_ref().unwrap(); - let comparison = diff.comparison.as_ref().unwrap(); - let file = &comparison.files[diff.file]; - let side = if focus == Focus::Before { - Side::Before + let content = self.active.lines.join("\n"); + let (revision, path, side) = if let Some(diff) = self.diff.as_ref() { + let comparison = diff.comparison.as_ref().ok_or("Comparison is loading")?; + let file = &comparison.files[diff.file]; + let side = if self.diff_buffer_focus() == Some(Focus::Before) { + Side::Before + } else { + Side::After + }; + ( + comparison.revisions[side.index()].clone(), + if side == Side::Before { + file.old_path.as_ref().unwrap_or(&file.path) + } else { + &file.path + } + .clone(), + side, + ) } else { - Side::After + let root = &self.local_review.document.root; + let path = self + .active + .identity + .as_ref() + .or(self.active.path.as_ref()) + .ok_or("Save the source file before commenting")? + .strip_prefix(root) + .map_err(|_| "Source is outside the review repository")? + .to_path_buf(); + ( + crate::repository::source_snapshot_revision(root, &content)?, + path, + Side::After, + ) }; let anchor = Anchor { - revision: comparison.revisions[side.index()].clone(), - path: if side == Side::Before { - file.old_path.as_ref().unwrap_or(&file.path) - } else { - &file.path - } - .clone(), + revision, + path, side, start: TextPosition { row: range.start.row, @@ -688,9 +725,9 @@ impl Editor { row: range.end.row, column: range.end.column, }, - content: self.active.lines.join("\n"), + content, }; - let id = diff + let id = self .local_review .document .comments @@ -727,10 +764,10 @@ impl Editor { } fn open_review_comment(&mut self, id: u64, draft: Option<&Comment>) -> Result<(), String> { - let diff = self.workspace_diff().unwrap(); + let state = &self.local_review; let comment = draft .or_else(|| { - diff.local_review + state .document .comments .iter() @@ -744,7 +781,7 @@ impl Editor { } else { "unresolved" }; - let mapped = diff.review_anchor_range(anchor); + let mapped = self.review_anchor_range(anchor); let current = mapped.is_some(); let location = if current { "current" @@ -761,31 +798,80 @@ impl Editor { range.end.column + 1, anchor.side ); - let target = diff.comparison.as_ref().and_then(|comparison| { - comparison.files.iter().position(|file| { - let path = if anchor.side == Side::Before { - file.old_path.as_ref().unwrap_or(&file.path) + let anchor = anchor.clone(); + let mut stale = !current; + self.local_review.selected = Some(id); + if let Some(owner) = self.diff_owner { + let target = self + .workspace_diff() + .and_then(|diff| diff.comparison.as_ref()) + .and_then(|comparison| { + comparison.files.iter().position(|file| { + if anchor.side == Side::Before { + file.old_path.as_ref().unwrap_or(&file.path) == &anchor.path + } else { + file.path == anchor.path + } + }) + }); + stale = !current + && target.is_none_or(|index| { + self.workspace_diff() + .unwrap() + .comparison + .as_ref() + .unwrap() + .files[index] + .loaded + }); + self.select_pane(owner); + if let Some(index) = target { + self.diff.as_mut().unwrap().select_file_index(index); + self.local_review.pending_navigation = Some(id); + } + self.finish_review_navigation(); + if draft.is_some() { + self.place_navigation(anchor.start); + } + } else { + if let Some(panel) = self.review_panel() { + self.select_pane(self.panes[&panel].origin.unwrap()); + } + let path = self.local_review.document.root.join(&anchor.path); + if draft.is_none() { + let opened = self.edit_buffer(path.clone(), false).is_ok(); + let mapped = opened.then(|| self.review_anchor_range(&anchor)).flatten(); + stale = mapped.is_none(); + if let Some(range) = mapped { + self.place_navigation(range.start); } else { - &file.path - }; - path == &anchor.path - }) - }); - let stale = !current - && target.is_none_or(|index| diff.comparison.as_ref().unwrap().files[index].loaded); - let start = anchor.start; - let owner = self.diff_owner.unwrap(); - self.select_pane(owner); - let diff = self.diff.as_mut().unwrap(); - diff.local_review.selected = Some(id); - if let Some(index) = target { - diff.select_file_index(index); - diff.local_review.pending_navigation = Some(id); - } - self.finish_review_navigation(); - if draft.is_some() { - self.place_navigation(start); + let mut snapshot = Buffer::from_text( + self.next_buffer_id, + Some(path), + None, + &anchor.content, + None, + ) + .map_err(|error| error.to_string())?; + snapshot.resolver_readonly = true; + self.add_buffer(snapshot); + self.place_navigation(anchor.start); + } + } } + let title = title.replacen( + if stale { + " · current · " + } else { + " · original snapshot · " + }, + if stale { + " · original snapshot · " + } else { + " · current · " + }, + 1, + ); self.open_review_buffer(ReviewText::Comment(id), &text, title)?; if stale { self.set_message("Comment is stale; original snapshot anchor preserved"); @@ -794,18 +880,18 @@ impl Editor { } pub(super) fn finish_review_navigation(&mut self) { + let state = &mut self.local_review; let Some(diff) = self.diff.as_mut() else { return; }; - let Some(id) = diff.local_review.pending_navigation else { + let Some(id) = state.pending_navigation else { return; }; if !diff.selected_file_loaded() { return; } - diff.local_review.pending_navigation = None; - let Some(comment) = diff - .local_review + state.pending_navigation = None; + let Some(comment) = state .document .comments .iter() @@ -827,7 +913,7 @@ impl Editor { if selected_path != Some(&comment.anchor.path) { return; } - let Some(range) = diff.review_anchor_range(&comment.anchor) else { + let Some(range) = diff.review_anchor_range(&comment.anchor, state) else { self.set_message("Comment is stale; original snapshot anchor preserved"); return; }; @@ -844,8 +930,7 @@ impl Editor { } fn open_review_overview(&mut self) -> Result<(), String> { - let diff = self.workspace_diff().unwrap(); - let state = &diff.local_review; + let state = &self.local_review; let verdict = match state.document.verdict { Some(Verdict::LooksGood) => "Looks good", Some(Verdict::ChangesRequested) => "Changes requested", @@ -868,25 +953,29 @@ impl Editor { .map_or_else(|| "not saved".into(), |path| path.display().to_string()) }; let mut text = format!( - "# Local review\n\nVerdict: {verdict}\nStorage: {storage}\n\n## Summary\n{}\n\nEnter edits the comment under the caret.\n:diff.review.summary · :diff.review.verdict · :diff.review.save\n", + "# Local review\n\nVerdict: {verdict}\nStorage: {storage}\n\n## Summary\n{}\n\nEnter edits the comment under the caret.\n:review.summary · :review.verdict · :review.save\n", state.document.summary ); for comment in &state.document.comments { let anchor = &comment.anchor; - let mapped = diff.review_anchor_range(anchor); + let mapped = self.review_anchor_range(anchor); let range = mapped.clone().unwrap_or(anchor.start..anchor.end); let status = if mapped.is_some() { "current" - } else if diff.comparison.as_ref().is_some_and(|comparison| { - comparison.files.iter().any(|file| { - !file.loaded - && if anchor.side == Side::Before { - file.old_path.as_ref().unwrap_or(&file.path) == &anchor.path - } else { - file.path == anchor.path - } + } else if self + .workspace_diff() + .and_then(|diff| diff.comparison.as_ref()) + .is_some_and(|comparison| { + comparison.files.iter().any(|file| { + !file.loaded + && if anchor.side == Side::Before { + file.old_path.as_ref().unwrap_or(&file.path) == &anchor.path + } else { + file.path == anchor.path + } + }) }) - }) { + { "not checked (file not loaded)" } else { "stale" @@ -915,7 +1004,7 @@ impl Editor { self.open_review_buffer( ReviewText::Overview, &text, - "Local review overview · Enter edits · :diff.review.next".into(), + "Local review overview · Enter edits · :review.next".into(), ) } @@ -927,6 +1016,11 @@ impl Editor { ) -> Result<(), String> { self.prepare_command(); self.finish_command_mode(); + let source = if self.review_panel().is_some() { + self.panes[&self.pane_id].origin.unwrap() + } else { + self.pane_id + }; let existing = std::iter::once(&self.active) .chain(self.parked.values()) .find(|buffer| { @@ -945,14 +1039,14 @@ impl Editor { id, Pane { buffer_id: self.active.id, - origin: self.diff_owner, + origin: Some(source), views: BTreeMap::from([(self.active.id, self.active.view.inherit())]), }, ); self.review_pane = Some(id); id }; - self.panes.get_mut(&panel).unwrap().origin = self.diff_owner; + self.panes.get_mut(&panel).unwrap().origin = Some(source); self.select_pane(panel); if let Some(id) = existing { self.switch_buffer(id); @@ -978,9 +1072,9 @@ impl Editor { self.add_buffer(buffer); } self.set_message(if self.review_directory.is_some() { - "Local review · :w saves · Escape discards unsaved text · :diff.review.save path exports JSON" + "Local review · :w saves · Escape discards unsaved text · :review.save path exports JSON" } else { - "Local review · :w saves JSON; first choose :diff.review.save " + "Local review · :w saves JSON; first choose :review.save " }); Ok(()) } diff --git a/src/main.rs b/src/main.rs index 15600ed..20d8766 100644 --- a/src/main.rs +++ b/src/main.rs @@ -96,13 +96,15 @@ Diff: :diff (Normal); choose Before then After; Working checkout After edits liv Esc closes Before/After selectors; :diff.close exits an opened comparison explicitly Remote refs/history load into temporary comparison repos without changing active refs/checkout Changed files: normal picker/explorer bindings, Ctrl+w close; A/M/D/R status and rename paths -Local reviews: :diff.comment anchors exact Visual text, Visual Line rows, or the current line +Local reviews: :comment anchors exact Visual text, Visual Line rows, or the current line + Works in ordinary repository source buffers and diff text panes Selected comments highlight their precise text - :diff.review.edit/delete [ID], :diff.review.next/previous, :diff.review.overview - :diff.review.summary; :diff.review.verdict looks_good|changes_requested|none - :diff.review.copy copies all saved comments in CLI format using the configured clipboard + :review.edit/delete [ID], :review.next/previous, :review.overview + :review.summary; :review.verdict looks_good|changes_requested|none + :review.copy copies all saved comments in CLI format using the configured clipboard :w saves comment/summary text, including hidden drafts; verdicts/deletions commit immediately - :diff.review.save PATH exports JSON; :diff.review.open [ID|PATH] selects/imports; :diff.review.new + :review.save PATH exports JSON; :review.open [ID|PATH] selects/imports; :review.new + Existing :diff.comment and :diff.review.* spellings remain available Normal Esc or :pane.close discards unsaved text and dismisses the floating editor Snapshot anchors stay immutable; stale comments lose gutter markers; no publishing Pull requests: :pull_requests [REMOTE]; GitHub/GitLab/Gitea Open/Closed/Drafts/All tabs, type to search diff --git a/src/repository.rs b/src/repository.rs index b44ee3d..d103ca0 100644 --- a/src/repository.rs +++ b/src/repository.rs @@ -1093,6 +1093,24 @@ pub fn discover_root(path: &Path) -> Result { discover_root_with_cancellation(path, &AtomicBool::new(false)) } +/// Identify the exact source bytes without writing objects or snapshotting the checkout. +pub(crate) fn source_snapshot_revision(root: &Path, content: &str) -> Result { + let mut command = if root.join(".jj").exists() { + git_command(&store_for_jj(root)?) + } else { + let mut command = Command::new("git"); + command.arg("-C").arg(root); + command + }; + command.args(["hash-object", "--no-filters", "--stdin"]); + let id = text(checked(run( + command, + content.as_bytes().to_vec(), + &AtomicBool::new(false), + )?)?)?; + Ok(format!("working:{}", id.trim())) +} + fn discover_root_with_cancellation(path: &Path, cancelled: &AtomicBool) -> Result { let path = fs::canonicalize(path).map_err(|error| error.to_string())?; let root = path diff --git a/src/review.rs b/src/review.rs index 46fea99..02b2d40 100644 --- a/src/review.rs +++ b/src/review.rs @@ -43,7 +43,7 @@ pub enum Verdict { #[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] #[serde(deny_unknown_fields)] pub struct Anchor { - /// Frozen comparison context; `content` additionally captures any live buffer edits. + /// Frozen repository context; source comments use the exact buffer's Git blob identity. pub revision: String, pub path: PathBuf, pub side: Side, diff --git a/src/tui.rs b/src/tui.rs index a30e912..14d5f55 100644 --- a/src/tui.rs +++ b/src/tui.rs @@ -807,6 +807,7 @@ impl BufferViewports { editor.file_navigation_hint(), &mut self.panels, Some(editor), + Some(&editor.local_review), ); } else { draw_overlays( @@ -1009,6 +1010,7 @@ impl BufferViewports { view.whitespace_scope(), view.resolver(), true, + None, ); if view.render_pane_focused() && !view.file_picker().is_some_and(|picker| picker.focused) @@ -1823,7 +1825,14 @@ fn chrome_items<'a>( tab: None, accent: !matches!( name.as_str(), - "path" | "language" | "diagnostics" | "selections" | "count" | "pending" | "buffer" | "diff" + "path" + | "language" + | "diagnostics" + | "selections" + | "count" + | "pending" + | "buffer" + | "diff" ), bold: false, first: true, @@ -2236,6 +2245,7 @@ fn draw_document( editor.file_navigation_hint(), panels, Some(editor), + Some(&editor.local_review), ); return; } @@ -2275,6 +2285,17 @@ fn draw_document( let mut document_caret = None; let carets = editor.carets(); let selections = editor.selections(); + let review_comments: Vec<_> = editor + .local_review + .document + .comments + .iter() + .filter_map(|comment| { + editor + .source_review_anchor_range(&comment.anchor) + .map(|range| (comment, range)) + }) + .collect(); let wrapped = editor.word_wrap(); let presentation = (editor.buffer_id(), wrapped, width, height); let identity = (editor.buffer_id(), editor.text_revision(), cursor); @@ -2576,6 +2597,25 @@ fn draw_document( }) .collect(); frame.render_widget(Paragraph::new(numbers), gutter); + for (screen, &(row, continuation, _)) in document.visible.iter().enumerate() { + if continuation != 0 { + continue; + } + if let Some((comment, _)) = review_comments + .iter() + .filter(|(_, range)| { + let end = range.end.row + - usize::from(range.end.column == 0 && range.end.row > range.start.row); + (range.start.row..=end).contains(&row) + }) + .max_by_key(|(comment, _)| editor.local_review.selected == Some(comment.id)) + { + let selected = editor.local_review.selected == Some(comment.id); + frame.buffer_mut()[(gutter.right() - 1, gutter.y + screen as u16)] + .set_symbol(if selected { "●" } else { "○" }) + .set_fg(Color::Magenta); + } + } } let lines: Vec = document .visible @@ -2627,6 +2667,33 @@ fn draw_document( if editor.highlight_trailing_whitespace() { paint_trailing_whitespace(frame, line, area, segment.start); } + if let Some((_, range)) = review_comments + .iter() + .find(|(comment, _)| editor.local_review.selected == Some(comment.id)) + { + let end_row = range.end.row + - usize::from(range.end.column == 0 && range.end.row > range.start.row); + if (range.start.row..=end_row).contains(&row) { + let start = if row == range.start.row { + range.start.column + } else { + 0 + }; + let end = if row == range.end.row { + range.end.column + } else { + usize::MAX + }; + paint_grapheme_range( + frame, + area, + segment.start, + line, + start..end, + Style::default().bg(Color::Magenta), + ); + } + } paint_editor_line( frame, editor, @@ -3337,6 +3404,7 @@ fn draw_diff_popups( } } +#[allow(clippy::too_many_arguments)] fn draw_diff( frame: &mut Frame, diff: &DiffState, @@ -3345,6 +3413,7 @@ fn draw_diff( file_navigation_hint: &str, panels: &mut WheelPanels, editor: Option<&Editor>, + review: Option<&crate::review::ReviewState>, ) { if let Some(editor) = editor { panels.picker_scroll.distance = editor.scroll_distance(); @@ -3814,7 +3883,7 @@ fn draw_diff( comparison.map(|comparison| (comparison.files.as_slice(), &diff.reviewed_files)), ); } - let document_caret = panels.draw_documents(frame, diff, panes, navigator, editor); + let document_caret = panels.draw_documents(frame, diff, panes, navigator, editor, review); if let Some(editor) = editor { draw_diff_popups(frame, editor, area, document_caret, panels); } @@ -3839,6 +3908,7 @@ impl WheelPanels { panes: Rect, navigator: Option, editor: Option<&Editor>, + review: Option<&crate::review::ReviewState>, ) -> Option<(Rect, Position)> { self.cached_diff_error(Some(diff)); let selected = diff @@ -4087,6 +4157,7 @@ impl WheelPanels { editor.map_or(WhitespaceScope::All, Editor::whitespace_scope), resolver, false, + review, ); text_widths[0] = text_area.width; if let Some(editor) = @@ -4127,6 +4198,7 @@ impl WheelPanels { editor.map_or(WhitespaceScope::All, Editor::whitespace_scope), resolver, false, + review, ); text_widths[1] = text_area.width; if let Some(editor) = @@ -4464,6 +4536,7 @@ fn draw_diff_pane( whitespace_scope: WhitespaceScope, resolver: Option<&ResolverWorkspace>, content_only: bool, + review: Option<&crate::review::ReviewState>, ) -> Rect { if area.width == 0 || area.height == 0 { return area; @@ -4487,11 +4560,9 @@ fn draw_diff_pane( *left = diff.bound_horizontal(focus, *left, width); } let left = *left; - let review_comments = diff - .local_review - .document - .comments - .iter() + let review_comments = review + .into_iter() + .flat_map(|review| review.document.comments.iter()) .filter_map(|comment| { let before = comment.anchor.side == crate::review::Side::Before; let path = if before { @@ -4502,7 +4573,7 @@ fn draw_diff_pane( if !(unified || before == (focus == Focus::Before)) || comment.anchor.path != *path { return None; } - diff.review_anchor_range(&comment.anchor) + diff.review_anchor_range(&comment.anchor, review.unwrap()) .map(|range| (comment, range)) }); paint_diff_content( @@ -4518,7 +4589,7 @@ fn draw_diff_pane( whitespace_characters, whitespace_scope, review_comments, - diff.local_review.selected, + review.and_then(|review| review.selected), ) } diff --git a/tests/unit/diff.rs b/tests/unit/diff.rs index 09d2113..f10142a 100644 --- a/tests/unit/diff.rs +++ b/tests/unit/diff.rs @@ -2460,6 +2460,7 @@ fn column_view_suppresses_whole_file_emphasis_without_losing_change_markers() { #[test] fn working_refresh_preserves_noops_errors_and_view_state_but_invalidates_old_identity() { + let mut review = crate::review::ReviewState::default(); let mut state = live_state(vec!["old".into()], vec!["new".into()], false); state.loading = false; state.pending = None; @@ -2554,7 +2555,7 @@ fn working_refresh_preserves_noops_errors_and_view_state_but_invalidates_old_ide .unwrap() .editable_paths .insert("file".into()); - state.refresh_after(&["dirty".into()], 1); + state.refresh_after(&["dirty".into()], 1, &mut review); assert_eq!(state.comparison.as_ref().unwrap().files[1].after, ["dirty"]); state.queue_checkout_refresh(false); state.take_request(); @@ -2575,6 +2576,7 @@ fn working_refresh_preserves_noops_errors_and_view_state_but_invalidates_old_ide #[test] fn reviewed_files_use_highlighted_catalog_indices_not_sorted_or_filtered_rows() { + let mut review = crate::review::ReviewState::default(); let mut state = DiffState::new(); let mut changes = comparison(); changes.files[0].path = "nested/z.txt".into(); @@ -2591,9 +2593,9 @@ fn reviewed_files_use_highlighted_catalog_indices_not_sorted_or_filtered_rows() } files.reveal_path(Path::new("nested/a.txt")); assert_eq!(state.file, 0); - assert_eq!(state.toggle_reviewed(), Ok(true)); + assert_eq!(state.toggle_reviewed(&mut review), Ok(true)); assert_eq!(state.reviewed_files, BTreeSet::from([1])); - assert_eq!(state.toggle_reviewed(), Ok(false)); + assert_eq!(state.toggle_reviewed(&mut review), Ok(false)); assert!(state.reviewed_files.is_empty()); } let files = state.files.as_mut().unwrap(); @@ -2602,7 +2604,7 @@ fn reviewed_files_use_highlighted_catalog_indices_not_sorted_or_filtered_rows() let selected = files.entries().position(|entry| entry.is_dir).unwrap(); files.selected = selected; assert_eq!( - state.toggle_reviewed(), + state.toggle_reviewed(&mut review), Err("Select a changed file, not a directory") ); assert!(state.reviewed_files.is_empty()); @@ -2611,32 +2613,42 @@ fn reviewed_files_use_highlighted_catalog_indices_not_sorted_or_filtered_rows() for ch in "missing".chars() { files.handle_filter_input(&Input::Char(ch)); } - assert_eq!(state.toggle_reviewed(), Err("No changed file is selected")); + assert_eq!( + state.toggle_reviewed(&mut review), + Err("No changed file is selected") + ); state.files = Some(FilePicker::from_paths( PathBuf::new(), [(0, PathBuf::from("ordinary.txt"))], )); - assert_eq!(state.toggle_reviewed(), Err("No changed file is selected")); + assert_eq!( + state.toggle_reviewed(&mut review), + Err("No changed file is selected") + ); state.focus = Focus::Before; - assert_eq!(state.toggle_reviewed(), Ok(true)); + assert_eq!(state.toggle_reviewed(&mut review), Ok(true)); assert_eq!(state.reviewed_files, BTreeSet::from([0])); assert!(state.select_file_index(2)); state.focus = Focus::Before; - assert_eq!(state.toggle_reviewed(), Ok(true)); + assert_eq!(state.toggle_reviewed(&mut review), Ok(true)); assert_eq!(state.reviewed_files, BTreeSet::from([0, 2])); } #[test] fn reviewed_files_survive_lazy_details_and_views_but_reset_on_comparison() { + let mut review = crate::review::ReviewState::default(); let mut state = DiffState::new(); - assert_eq!(state.toggle_reviewed(), Err("No comparison is open")); + assert_eq!( + state.toggle_reviewed(&mut review), + Err("No comparison is open") + ); let mut changes = comparison(); changes.files[0].loaded = false; changes.files[0].before.clear(); changes.files[0].after.clear(); state.request_kind = Some(RequestKind::Comparison); state.apply_result(1, Ok(Outcome::Comparison(changes))); - assert_eq!(state.toggle_reviewed(), Ok(true)); + assert_eq!(state.toggle_reviewed(&mut review), Ok(true)); assert_eq!(state.reviewed_files, BTreeSet::from([0])); let file = comparison().files.remove(0); state.apply_result( @@ -2667,10 +2679,13 @@ fn reviewed_files_survive_lazy_details_and_views_but_reset_on_comparison() { )); state.toggle_comparison_mode().unwrap(); assert!(state.reviewed_files.is_empty()); - assert_eq!(state.toggle_reviewed(), Err("No comparison is open")); + assert_eq!( + state.toggle_reviewed(&mut review), + Err("No comparison is open") + ); state.apply_result(3, Ok(Outcome::Comparison(comparison()))); assert!(state.reviewed_files.is_empty()); - assert_eq!(state.toggle_reviewed(), Ok(true)); + assert_eq!(state.toggle_reviewed(&mut review), Ok(true)); state.request_kind = Some(RequestKind::Comparison); state.apply_result(4, Ok(Outcome::Comparison(comparison()))); assert!(state.reviewed_files.is_empty()); @@ -2678,17 +2693,18 @@ fn reviewed_files_survive_lazy_details_and_views_but_reset_on_comparison() { #[test] fn restored_review_marks_use_file_identities_and_rename_paths_not_catalog_order() { + let mut review = crate::review::ReviewState::default(); let mut state = DiffState::new(); state.request_kind = Some(RequestKind::Comparison); state.apply_result(1, Ok(Outcome::Comparison(comparison()))); state.comparison.as_mut().unwrap().files[0].review_revisions = Some(["file:before".into(), "file:after".into()]); - assert_eq!(state.toggle_reviewed(), Ok(true)); + assert_eq!(state.toggle_reviewed(&mut review), Ok(true)); state.comparison.as_mut().unwrap().files.swap(0, 1); - state.restore_reviewed_files(); + state.restore_reviewed_files(&review); assert_eq!(state.reviewed_files, BTreeSet::from([1])); state.comparison.as_mut().unwrap().files[1].old_path = Some("different.txt".into()); - state.restore_reviewed_files(); + state.restore_reviewed_files(&review); assert!(state.reviewed_files.is_empty()); state.comparison.as_mut().unwrap().files[1].old_path = Some("old.txt".into()); for side in [0, 1] { @@ -2698,7 +2714,7 @@ fn restored_review_marks_use_file_identities_and_rename_paths_not_catalog_order( .unwrap(); let original = identities[side].clone(); identities[side].push_str("-different"); - state.restore_reviewed_files(); + state.restore_reviewed_files(&review); assert!(state.reviewed_files.is_empty()); state.comparison.as_mut().unwrap().files[1] .review_revisions @@ -2706,12 +2722,13 @@ fn restored_review_marks_use_file_identities_and_rename_paths_not_catalog_order( .unwrap()[side] = original; } state.comparison.as_mut().unwrap().files[1].loaded = false; - state.restore_reviewed_files(); + state.restore_reviewed_files(&review); assert_eq!(state.reviewed_files, BTreeSet::from([1])); } #[test] fn unchanged_file_review_survives_other_files_changing() { + let mut review = crate::review::ReviewState::default(); use crate::repository::Repository; use std::sync::{Arc, atomic::AtomicBool}; let root = std::env::temp_dir().join(format!("vamp-review-files-{}", std::process::id())); @@ -2759,13 +2776,13 @@ fn unchanged_file_review_survives_other_files_changing() { let original_revisions = changes.revisions.clone(); state.request_kind = Some(RequestKind::Comparison); state.apply_result(1, Ok(Outcome::Comparison(changes))); - state.toggle_reviewed().unwrap(); + state.toggle_reviewed(&mut review).unwrap(); state.file = 1; state.focus = Focus::Before; - state.toggle_reviewed().unwrap(); + state.toggle_reviewed(&mut review).unwrap(); let mut store = crate::review::Store::open(&root.join("reviews")).unwrap(); - store.save(&mut state.local_review.document).unwrap(); - let id = state.local_review.document.storage_id.unwrap(); + store.save(&mut review.document).unwrap(); + let id = review.document.storage_id.unwrap(); std::fs::write(root.join("b.txt"), "edited again\n").unwrap(); let changes = repository .compare(2, &before, &after, ComparisonMode::Snapshots, &[]) @@ -2773,8 +2790,8 @@ fn unchanged_file_review_survives_other_files_changing() { assert_ne!(changes.revisions, original_revisions); state.request_kind = Some(RequestKind::Comparison); state.apply_result(2, Ok(Outcome::Comparison(changes))); - state.local_review.document = store.load(&root, id).unwrap(); - state.restore_reviewed_files(); + review.document = store.load(&root, id).unwrap(); + state.restore_reviewed_files(&review); assert_eq!(state.reviewed_files, BTreeSet::from([0])); let detail = repository.load_file(2, 0).unwrap(); state.apply_result( @@ -2785,33 +2802,36 @@ fn unchanged_file_review_survives_other_files_changing() { file: detail, }), ); - state.restore_reviewed_files(); + state.restore_reviewed_files(&review); assert_eq!(state.reviewed_files, BTreeSet::from([0])); std::fs::remove_dir_all(root).unwrap(); } #[test] fn restored_live_review_marks_require_exact_text_and_final_newline() { + let mut review = crate::review::ReviewState::default(); + let mut reopened_review = crate::review::ReviewState::default(); let mut state = live_state(vec!["base".into()], vec!["changed".into()], false); - state.refresh_after(&["unsaved".into(), String::new()], 1); - assert_eq!(state.toggle_reviewed(), Ok(true)); - let document = state.local_review.document.clone(); + state.refresh_after(&["unsaved".into(), String::new()], 1, &mut review); + assert_eq!(state.toggle_reviewed(&mut review), Ok(true)); + let document = review.document.clone(); let mut reopened = live_state(vec!["base".into()], vec!["changed".into()], false); - reopened.local_review.document = document.clone(); - reopened.restore_reviewed_files(); + reopened_review.document = document.clone(); + reopened.restore_reviewed_files(&reopened_review); assert!(reopened.reviewed_files.is_empty()); - reopened.refresh_after(&["unsaved".into(), String::new()], 1); + reopened.refresh_after(&["unsaved".into(), String::new()], 1, &mut reopened_review); assert_eq!(reopened.reviewed_files, BTreeSet::from([0])); - reopened.refresh_after(&["unsaved".into()], 2); + reopened.refresh_after(&["unsaved".into()], 2, &mut reopened_review); assert!(reopened.reviewed_files.is_empty()); - assert!(reopened.local_review.document.reviewed_files.is_empty()); - reopened.local_review.document = document; - reopened.restore_reviewed_files(); + assert!(reopened_review.document.reviewed_files.is_empty()); + reopened_review.document = document; + reopened.restore_reviewed_files(&reopened_review); assert!(reopened.reviewed_files.is_empty()); } #[test] fn working_review_marks_clear_only_for_changed_bytes_or_eof() { + let mut review = crate::review::ReviewState::default(); for (after, eof, lines) in [ (vec!["changed".into()], false, vec!["changed".into()]), ( @@ -2822,24 +2842,24 @@ fn working_review_marks_clear_only_for_changed_bytes_or_eof() { (Vec::new(), false, vec![String::new()]), ] { let mut state = live_state(vec!["base".into()], after, eof); - assert_eq!(state.toggle_reviewed(), Ok(true)); - state.refresh_after(&lines, 1); + assert_eq!(state.toggle_reviewed(&mut review), Ok(true)); + state.refresh_after(&lines, 1, &mut review); assert_eq!(state.reviewed_files, BTreeSet::from([0])); - state.refresh_after(&lines, 2); + state.refresh_after(&lines, 2, &mut review); assert_eq!(state.reviewed_files, BTreeSet::from([0])); - state.refresh_after(&["edited".into()], 3); + state.refresh_after(&["edited".into()], 3, &mut review); assert!(state.reviewed_files.is_empty()); - assert_eq!(state.toggle_reviewed(), Ok(true)); - state.refresh_after(&["edited".into(), String::new()], 4); + assert_eq!(state.toggle_reviewed(&mut review), Ok(true)); + state.refresh_after(&["edited".into(), String::new()], 4, &mut review); assert!(state.reviewed_files.is_empty()); - assert_eq!(state.toggle_reviewed(), Ok(true)); - state.refresh_after(&["edited".into()], 5); + assert_eq!(state.toggle_reviewed(&mut review), Ok(true)); + state.refresh_after(&["edited".into()], 5, &mut review); assert!(state.reviewed_files.is_empty()); } let mut state = live_state(vec!["base".into()], vec!["changed".into()], false); state.comparison.as_mut().unwrap().files[0].loaded = false; - assert_eq!(state.toggle_reviewed(), Ok(true)); - state.refresh_after(&["edited".into()], 1); + assert_eq!(state.toggle_reviewed(&mut review), Ok(true)); + state.refresh_after(&["edited".into()], 1, &mut review); assert_eq!(state.reviewed_files, BTreeSet::from([0])); } @@ -2901,19 +2921,20 @@ fn unified_groups_changed_runs_and_roundtrips_side_specific_source_rows() { #[test] fn unified_live_reflow_preserves_removed_top_and_expanded_source_excerpts() { + let mut review = crate::review::ReviewState::default(); let base: Vec<_> = (0..40).map(|row| format!("line {row}")).collect(); let mut lines = base.clone(); lines[20] = "new line".into(); let mut state = live_state(base, lines.clone(), false); state.set_context_options(1, 3); - state.refresh_after(&lines, 1); + state.refresh_after(&lines, 1, &mut review); state.set_view_mode(ViewMode::Unified); state.top = state.source_display_row(Focus::Before, 20).unwrap(); state.expand_source_excerpt(Focus::After, false, 20); assert_eq!(state.display_source_row(Focus::Before, state.top), Some(20)); assert!(state.source_display_row(Focus::After, 16).is_some()); lines.insert(0, "inserted".into()); - state.refresh_after(&lines, 2); + state.refresh_after(&lines, 2, &mut review); assert_eq!(state.display_focus(state.top), Some(Focus::Before)); assert_eq!(state.display_source_row(Focus::Before, state.top), Some(20)); assert!(state.source_display_row(Focus::After, 17).is_some()); @@ -2932,6 +2953,7 @@ fn unified_live_reflow_preserves_removed_top_and_expanded_source_excerpts() { #[test] fn character_ranges_follow_insertions_deletions_unicode_and_live_edits() { + let mut review = crate::review::ReviewState::default(); for (before, after, expected) in [ ("ab", "a界b", vec![[1..1, 1..2]]), ("a界b", "ab", vec![[1..2, 1..1]]), @@ -2941,17 +2963,18 @@ fn character_ranges_follow_insertions_deletions_unicode_and_live_edits() { assert_eq!(state.rows[0].changes, expected); } let mut state = live_state(vec!["abc".into()], vec!["aXbc".into()], false); - state.refresh_after(&["aXYbc".into()], 1); + state.refresh_after(&["aXYbc".into()], 1, &mut review); assert_eq!(state.rows[0].changes, vec![[1..1, 1..3]]); - state.refresh_after(&["abYZc".into()], 2); + state.refresh_after(&["abYZc".into()], 2, &mut review); assert_eq!(state.rows[0].changes, vec![[2..2, 2..4]]); - state.refresh_after(&["abc".into()], 3); + state.refresh_after(&["abc".into()], 3, &mut review); assert_eq!(state.rows[0].kind, RowKind::Equal); assert!(state.rows[0].changes.is_empty()); } #[test] fn character_highlights_merge_incidental_matches_but_preserve_meaningful_gaps() { + let mut review = crate::review::ReviewState::default(); let mut cases = vec![ ("a..c".into(), "b..d".into(), vec![[0..4, 0..4]]), ("base".into(), "edited".into(), vec![[0..4, 0..6]]), @@ -2995,14 +3018,15 @@ fn character_highlights_merge_incidental_matches_but_preserve_meaningful_gaps() for (before, after, expected) in cases { let mut state = live_state(vec![before], vec![after.clone()], false); assert_eq!(state.rows[0].changes, expected, "{after:?}"); - state.refresh_after(&[format!("{after}!")], 1); - state.refresh_after(std::slice::from_ref(&after), 2); + state.refresh_after(&[format!("{after}!")], 1, &mut review); + state.refresh_after(std::slice::from_ref(&after), 2, &mut review); assert_eq!(state.rows[0].changes, expected, "live edit: {after:?}"); } } #[test] fn reverting_loaded_file_keeps_unloaded_changes_selectable() { + let mut review = crate::review::ReviewState::default(); let mut state = live_state(vec!["base".into()], vec!["changed".into()], false); state.comparison_id = Some(17); state.pending = None; @@ -3020,7 +3044,7 @@ fn reverting_loaded_file_keeps_unloaded_changes_selectable() { hunks: vec![], notice: None, }); - state.refresh_after(&["base".into()], 1); + state.refresh_after(&["base".into()], 1, &mut review); assert_eq!( state .files @@ -3043,36 +3067,37 @@ fn reverting_loaded_file_keeps_unloaded_changes_selectable() { #[test] fn live_width_cache_scans_only_changed_lines_and_shrinks_after_deletion() { + let mut review = crate::review::ReviewState::default(); let base = vec!["original".into(), "medium".into(), "short".into()]; let mut lines = vec!["界\tz".into(), "medium".into(), "short".into()]; let mut state = live_state(base, lines.clone(), false); - state.refresh_after(&lines, 1); + state.refresh_after(&lines, 1, &mut review); assert_eq!(state.after_widths, [5, 6, 5]); lines[0] = "界界\t🙂".into(); WIDTH_SCANS.set(0); - state.refresh_after(&lines, 2); + state.refresh_after(&lines, 2, &mut review); assert_eq!(WIDTH_SCANS.get(), 1); assert_eq!(state.max_after, 10); state.left_after = 9; lines[0] = "界".into(); WIDTH_SCANS.set(0); - state.refresh_after(&lines, 3); + state.refresh_after(&lines, 3, &mut review); assert_eq!(WIDTH_SCANS.get(), 1); assert_eq!(state.max_after, 6); assert_eq!(state.left_after, 6); lines.remove(1); WIDTH_SCANS.set(0); - state.refresh_after(&lines, 4); + state.refresh_after(&lines, 4, &mut review); assert_eq!(WIDTH_SCANS.get(), 0); assert_eq!(state.after_widths, [2, 5]); assert_eq!(state.max_after, 5); lines.insert(1, "巨大\t🙂".into()); WIDTH_SCANS.set(0); - state.refresh_after(&lines, 5); + state.refresh_after(&lines, 5, &mut review); assert_eq!(WIDTH_SCANS.get(), 1); assert_eq!(state.after_widths, [2, 10, 5]); lines = vec!["".into()]; - state.refresh_after(&lines, 6); + state.refresh_after(&lines, 6, &mut review); assert_eq!(state.after_widths, [0]); assert_eq!(state.max_after, 0); assert_eq!(state.left_after, 0); @@ -3080,6 +3105,7 @@ fn live_width_cache_scans_only_changed_lines_and_shrinks_after_deletion() { #[test] fn stable_line_edits_keep_fresh_hunks_expansion_and_scroll_anchor_until_revert() { + let mut review = crate::review::ReviewState::default(); let base = (0..100) .map(|row| format!("line {row}")) .collect::>(); @@ -3088,7 +3114,7 @@ fn stable_line_edits_keep_fresh_hunks_expansion_and_scroll_anchor_until_revert() let mut state = live_state(base.clone(), lines.clone(), true); state.set_context_options(2, 5); lines.push(String::new()); - state.refresh_after(&lines, 1); + state.refresh_after(&lines, 1, &mut review); state.top = state.after_display_row(30).unwrap(); state.handle(Input::Char('[')); let display = state.display_rows.clone(); @@ -3102,7 +3128,7 @@ fn stable_line_edits_keep_fresh_hunks_expansion_and_scroll_anchor_until_revert() changed_line_range(&file.after, &lines) )); WIDTH_SCANS.set(0); - state.refresh_after(&lines, 2); + state.refresh_after(&lines, 2, &mut review); assert_eq!(WIDTH_SCANS.get(), 1); assert_eq!(state.display_rows, display); assert_eq!(state.excerpt_ranges, excerpts); @@ -3131,14 +3157,14 @@ fn stable_line_edits_keep_fresh_hunks_expansion_and_scroll_anchor_until_revert() &lines, changed_line_range(&file.after, &lines) )); - state.refresh_after(&lines, 3); + state.refresh_after(&lines, 3, &mut review); assert!(state.comparison.as_ref().unwrap().files[0].hunks.is_empty()); assert!(state.rows.iter().all(|row| row.kind == RowKind::Equal)); assert!(state.after_display_row(23).is_some()); assert_eq!(state.top, state.after_display_row(30).unwrap()); assert_eq!(state.files.as_ref().unwrap().entries().len(), 0); lines[50] = "changed again".into(); - state.refresh_after(&lines, 4); + state.refresh_after(&lines, 4, &mut review); assert_eq!(state.files.as_ref().unwrap().entries().len(), 1); assert!(state.after_display_row(23).is_some()); assert!(state.after_display_row(50).is_some()); @@ -3146,6 +3172,7 @@ fn stable_line_edits_keep_fresh_hunks_expansion_and_scroll_anchor_until_revert() #[test] fn live_reflow_keeps_separate_hunks_and_expanded_context_after_revert() { + let mut review = crate::review::ReviewState::default(); let base = (0..100) .map(|row| format!("line {row}")) .collect::>(); @@ -3155,7 +3182,7 @@ fn live_reflow_keeps_separate_hunks_and_expanded_context_after_revert() { let mut state = live_state(base.clone(), changed.clone(), true); state.set_context_options(2, 5); changed.push(String::new()); - state.refresh_after(&changed, 1); + state.refresh_after(&changed, 1, &mut review); assert_eq!(state.comparison.as_ref().unwrap().files[0].hunks.len(), 2); assert!( state @@ -3167,7 +3194,7 @@ fn live_reflow_keeps_separate_hunks_and_expanded_context_after_revert() { state.handle(Input::Char('[')); assert!(state.after_display_row(23).is_some()); changed.insert(10, "inserted outside excerpts".into()); - state.refresh_after(&changed, 2); + state.refresh_after(&changed, 2, &mut review); assert!(state.after_display_row(24).is_some()); assert_eq!(state.comparison.as_ref().unwrap().files[0].hunks.len(), 3); assert!( @@ -3178,7 +3205,7 @@ fn live_reflow_keeps_separate_hunks_and_expanded_context_after_revert() { ); let mut reverted = base; reverted.push(String::new()); - state.refresh_after(&reverted, 3); + state.refresh_after(&reverted, 3, &mut review); assert!(state.comparison.as_ref().unwrap().files[0].hunks.is_empty()); assert!(state.rows.iter().all(|row| row.kind == RowKind::Equal)); assert!(state.after_display_row(23).is_some()); @@ -3186,21 +3213,22 @@ fn live_reflow_keeps_separate_hunks_and_expanded_context_after_revert() { assert_eq!(state.files.as_ref().unwrap().entries().len(), 0); assert_eq!(state.file, 0); reverted[50] = "changed again".into(); - state.refresh_after(&reverted, 4); + state.refresh_after(&reverted, 4, &mut review); assert_eq!(state.files.as_ref().unwrap().entries().len(), 1); assert!(state.after_display_row(50).is_some()); } #[test] fn live_empty_and_eof_rows_match_buffer_without_false_eof_highlights() { + let mut review = crate::review::ReviewState::default(); let mut state = live_state(vec!["same".into()], vec!["changed".into()], true); - state.refresh_after(&["same".into(), "".into()], 1); + state.refresh_after(&["same".into(), "".into()], 1, &mut review); assert!(state.comparison.as_ref().unwrap().files[0].hunks.is_empty()); assert_eq!(state.rows.len(), 2); assert!(state.rows.iter().all(|row| row.kind == RowKind::Equal)); assert!(state.after_display_row(0).is_some()); assert!(state.after_display_row(1).is_none()); - state.refresh_after(&["same".into()], 2); + state.refresh_after(&["same".into()], 2, &mut review); assert_eq!(state.rows[0].kind, RowKind::Changed); assert!( state.comparison.as_ref().unwrap().files[0] @@ -3209,18 +3237,18 @@ fn live_empty_and_eof_rows_match_buffer_without_false_eof_highlights() { .unwrap() .contains("After missing") ); - state.refresh_after(&["".into()], 3); + state.refresh_after(&["".into()], 3, &mut review); assert!(state.after_display_row(0).is_some()); assert_eq!(state.comparison.as_ref().unwrap().files[0].after, [""]); let mut empty = live_state(vec![], vec!["text".into()], false); - empty.refresh_after(&["".into()], 1); + empty.refresh_after(&["".into()], 1, &mut review); assert!(empty.comparison.as_ref().unwrap().files[0].hunks.is_empty()); assert_eq!(empty.rows.len(), 1); assert_eq!(empty.rows[0].kind, RowKind::Equal); assert_eq!(empty.after_display_row(0), Some(0)); empty.after.as_mut().unwrap().target = RevisionTarget::Commit("historic".into()); empty.comparison.as_mut().unwrap().editable_paths.clear(); - empty.refresh_after(&["must not apply".into()], 2); + empty.refresh_after(&["must not apply".into()], 2, &mut review); assert_eq!(empty.comparison.as_ref().unwrap().files[0].after, [""]); empty.handle(Input::Escape); assert_eq!(empty.phase, Phase::View); @@ -3372,6 +3400,7 @@ fn mode_action_keeps_filter_input_and_rejects_superseded_comparisons() { #[test] fn review_anchors_match_frozen_context_and_exact_live_eof_without_allocating() { + let review = crate::review::ReviewState::default(); use crate::review::{Anchor, Side}; let mut state = live_state(vec!["old".into()], vec!["new".into()], true); let anchor = Anchor { @@ -3382,15 +3411,15 @@ fn review_anchors_match_frozen_context_and_exact_live_eof_without_allocating() { end: crate::plugin::TextPosition { row: 1, column: 0 }, content: "new\n".into(), }; - assert!(state.review_anchor_range(&anchor).is_some()); + assert!(state.review_anchor_range(&anchor, &review).is_some()); state.live_files.insert(0); - assert!(state.review_anchor_range(&anchor).is_none()); + assert!(state.review_anchor_range(&anchor, &review).is_none()); state.comparison.as_mut().unwrap().files[0] .after .push(String::new()); - assert!(state.review_anchor_range(&anchor).is_some()); + assert!(state.review_anchor_range(&anchor, &review).is_some()); state.comparison.as_mut().unwrap().files[0].after[0] = "changed".into(); - assert!(state.review_anchor_range(&anchor).is_none()); + assert!(state.review_anchor_range(&anchor, &review).is_none()); state.comparison.as_mut().unwrap().files[0].old_path = Some("original".into()); let before = Anchor { revision: "base".into(), @@ -3399,12 +3428,12 @@ fn review_anchors_match_frozen_context_and_exact_live_eof_without_allocating() { content: "old\n".into(), ..anchor }; - assert!(state.review_anchor_range(&before).is_some()); + assert!(state.review_anchor_range(&before, &review).is_some()); state.comparison.as_mut().unwrap().files[0].loaded = false; - assert!(state.review_anchor_range(&before).is_none()); + assert!(state.review_anchor_range(&before, &review).is_none()); state.comparison.as_mut().unwrap().files[0].loaded = true; state.comparison.as_mut().unwrap().revisions[0] = "other".into(); - assert!(state.review_anchor_range(&before).is_none()); + assert!(state.review_anchor_range(&before, &review).is_none()); } #[test] diff --git a/tests/unit/editor.rs b/tests/unit/editor.rs index 44ac50d..402b415 100644 --- a/tests/unit/editor.rs +++ b/tests/unit/editor.rs @@ -3139,6 +3139,160 @@ fn historical_diff(editor: &mut Editor, root: &Path, before: Vec, after: assert!(editor.diff_interacting()); } +fn source_review_editor(directory: &Directory, text: &str) -> (Editor, PathBuf, PathBuf) { + let root = directory.0.canonicalize().unwrap(); + assert!( + std::process::Command::new("git") + .args(["init", "-q", "--template="]) + .current_dir(&root) + .status() + .unwrap() + .success() + ); + let path = root.join("source.txt"); + fs::write(&path, text).unwrap(); + let mut editor = Editor::open(Some(path.clone()), vec!["text".into()]).unwrap(); + editor.set_clipboard_mode("separate").unwrap(); + editor.set_review_directory(root.join("reviews")); + (editor, root, path) +} + +#[test] +fn source_review_drafts_guard_source_pane_close_and_saved_panels_retarget() { + let directory = Directory::new(); + let (mut editor, _, _) = source_review_editor(&directory, "source\n"); + let source = editor.pane_id; + editor.split_pane(SplitAxis::Vertical); + let other = editor.pane_id; + editor.select_pane(source); + editor.execute("comment"); + type_text(&mut editor, "iDraft"); + editor.handle(Input::Escape); + editor.select_pane(source); + editor.close_pane(false); + assert!(editor.panes.contains_key(&source)); + editor.execute("review.save"); + editor.close_pane(false); + assert!(!editor.panes.contains_key(&source)); + assert_eq!(editor.pane_id, other); + editor.execute("review.edit 1"); + assert_eq!(editor.context().text, "Draft"); + editor.handle(Input::Escape); + assert_eq!(editor.pane_id, other); + assert_eq!(editor.context().text, "source\n"); +} + +#[test] +fn source_comments_capture_unsaved_exact_unicode_and_reopen_without_a_diff() { + let directory = Directory::new(); + let (mut editor, root, path) = source_review_editor(&directory, "disk\n"); + assert!(!root.join("reviews").exists()); + let source = "first\nprefix e\u{301}界 suffix\nlast\n"; + editor.apply(vec![Effect::Replace(source.into())]).unwrap(); + set_carets(&mut editor, &[(1, 7)], 0); + type_text(&mut editor, "vl"); + editor.execute("comment"); + assert!(editor.review_panel().is_some()); + assert!(editor.workspace_diff().is_none()); + type_text(&mut editor, "iExact feedback"); + editor.handle(Input::Escape); + editor.execute("w"); + let review = editor.local_review.document.clone(); + let anchor = review.comments[0].anchor.clone(); + assert_eq!(anchor.excerpt(), "e\u{301}界"); + assert_eq!(anchor.content, source); + assert!(anchor.revision.starts_with("working:")); + assert_eq!(fs::read_to_string(&path).unwrap(), "disk\n"); + editor.handle(Input::Escape); + assert_eq!(editor.context().text, source); + editor.execute("w"); + drop(editor); + let mut reopened = Editor::open(Some(path), vec!["text".into()]).unwrap(); + reopened.set_review_directory(root.join("reviews")); + assert!(reopened.workspace_diff().is_none()); + assert_eq!(reopened.local_review.document.storage_id, review.storage_id); + reopened.execute("review.next"); + assert_eq!(reopened.context().text, "Exact feedback"); + assert!(reopened.review_panel_title().unwrap().contains("current")); + reopened.handle(Input::Escape); + assert_eq!(reopened.cursor(), Cursor { row: 1, column: 7 }); + assert_eq!(reopened.local_review.document.comments[0].anchor, anchor); +} + +#[test] +fn source_comment_dismissal_conflicts_tabs_and_stale_snapshots_preserve_text() { + let directory = Directory::new(); + let (mut editor, root, path) = source_review_editor(&directory, "original line\n"); + editor.execute("comment"); + type_text(&mut editor, "iDiscarded"); + editor.handle(Input::Escape); + editor.handle(Input::Escape); + assert!(editor.local_review.document.comments.is_empty()); + editor.execute("comment"); + type_text(&mut editor, "iSaved feedback"); + editor.handle(Input::Escape); + editor.execute("w"); + editor.handle(Input::Escape); + let anchor = editor.local_review.document.comments[0].anchor.clone(); + assert_eq!(anchor.excerpt(), "original line\n"); + editor.execute("tab.new"); + editor.execute("review.edit 1"); + assert_eq!(editor.context().text, "Saved feedback"); + editor.handle(Input::Escape); + editor.execute("tab.previous"); + editor + .apply(vec![Effect::Replace("entirely changed\n".into())]) + .unwrap(); + editor.execute("review.edit 1"); + assert!(editor.message().contains("stale")); + assert!( + editor + .review_panel_title() + .unwrap() + .contains("original snapshot") + ); + editor.handle(Input::Escape); + assert_eq!(editor.context().text, "original line\n"); + assert!(editor.active.resolver_readonly); + assert_eq!(editor.local_review.document.comments[0].anchor, anchor); + assert_eq!(fs::read_to_string(&path).unwrap(), "original line\n"); + let mut concurrent = Editor::open(Some(path), vec!["text".into()]).unwrap(); + concurrent.set_review_directory(root.join("reviews")); + concurrent.execute("review.verdict looks_good"); + editor.execute("review.edit 1"); + type_text(&mut editor, "A unsaved"); + editor.handle(Input::Escape); + editor.execute("w"); + assert!(editor.message().contains("changed in another session")); + assert_eq!(editor.context().text, "Saved feedback unsaved"); + assert!(editor.current_review_dirty()); +} + +#[test] +fn source_comments_reject_multiple_carets_and_roundtrip_json_with_empty_lines() { + let directory = Directory::new(); + let (mut editor, root, _) = source_review_editor(&directory, "one\n\n"); + set_carets(&mut editor, &[(0, 0), (1, 0)], 0); + editor.execute("comment"); + assert!(editor.message().contains("one caret")); + assert!(editor.review_panel().is_none()); + set_carets(&mut editor, &[(1, 0)], 0); + editor.execute("comment"); + type_text(&mut editor, "iEmpty line"); + editor.handle(Input::Escape); + let export = root.join("review.json"); + editor.execute(&format!("review.save {}", export.display())); + let exported = crate::review::Review::load(&export, &root).unwrap(); + assert_eq!(exported.comments[0].anchor.excerpt(), "\n"); + assert_eq!(exported.comments[0].body, "Empty line"); + editor.handle(Input::Escape); + editor.execute(&format!("review.open {}", export.display())); + assert_eq!( + editor.local_review.document.comments[0].anchor, + exported.comments[0].anchor + ); +} + #[test] fn unsaved_review_comments_are_discarded_on_escape() { let directory = Directory::new(); @@ -3151,15 +3305,7 @@ fn unsaved_review_comments_are_discarded_on_escape() { type_text(&mut editor, "iDiscard me"); editor.handle(Input::Escape); editor.handle(Input::Escape); - assert!( - editor - .workspace_diff() - .unwrap() - .local_review - .document - .comments - .is_empty() - ); + assert!(editor.local_review.document.comments.is_empty()); assert!( crate::review::Store::open(&storage) .unwrap() @@ -3299,7 +3445,7 @@ fn review_navigation_maps_working_ranges_and_preserves_stale_snapshots() { .as_mut() .unwrap() .revisions[1] = "working:old".into(); - editor.diff.as_mut().unwrap().local_review.document = crate::review::Review::new(root); + editor.local_review.document = crate::review::Review::new(root); let anchor = Anchor { revision: "working:old".into(), path: "file.txt".into(), @@ -3308,19 +3454,12 @@ fn review_navigation_maps_working_ranges_and_preserves_stale_snapshots() { end: TextPosition { row: 1, column: 13 }, content: editor.lines().join("\n"), }; - editor - .diff - .as_mut() - .unwrap() - .local_review - .document - .comments - .push(Comment { - id: 1, - anchor: anchor.clone(), - body: "Feedback".into(), - resolved: false, - }); + editor.local_review.document.comments.push(Comment { + id: 1, + anchor: anchor.clone(), + body: "Feedback".into(), + resolved: false, + }); editor .apply(vec![Effect::Replace( "inserted\nheader\nprefix unique target\ntail\n".into(), @@ -3339,10 +3478,7 @@ fn review_navigation_maps_working_ranges_and_preserves_stale_snapshots() { let owner = editor.diff_owner.unwrap(); editor.select_pane(owner); assert_eq!(editor.cursor(), Cursor { row: 2, column: 14 }); - assert_eq!( - editor.diff.as_ref().unwrap().local_review.document.comments[0].anchor, - anchor - ); + assert_eq!(editor.local_review.document.comments[0].anchor, anchor); editor .apply(vec![Effect::Replace( @@ -3355,10 +3491,7 @@ fn review_navigation_maps_working_ranges_and_preserves_stale_snapshots() { assert!(editor.message().contains("stale")); editor.select_pane(owner); assert_eq!(editor.cursor(), Cursor::default()); - assert_eq!( - editor.diff.as_ref().unwrap().local_review.document.comments[0].anchor, - anchor - ); + assert_eq!(editor.local_review.document.comments[0].anchor, anchor); } #[test] @@ -3388,13 +3521,7 @@ fn sqlite_reviews_autosave_restore_select_and_keep_exports_independent() { ); first.execute("w"); first.handle(Input::Escape); - let id = first - .workspace_diff() - .unwrap() - .local_review - .document - .storage_id - .unwrap(); + let id = first.local_review.document.storage_id.unwrap(); let store = Store::open(&storage).unwrap(); let persisted = store.load(&root, id).unwrap(); assert_eq!(persisted.comments[0].body, "First feedback"); @@ -3437,58 +3564,30 @@ fn sqlite_reviews_autosave_restore_select_and_keep_exports_independent() { vec!["different revision".into()], vec!["changed".into()], ); - let restored = &reopened.workspace_diff().unwrap().local_review.document; + let restored = &reopened.local_review.document; assert_eq!(restored.storage_id, Some(id)); assert_eq!(restored.comments[0].body, "First feedback revised"); assert!( reopened .workspace_diff() .unwrap() - .review_anchor_range(&restored.comments[0].anchor) + .review_anchor_range(&restored.comments[0].anchor, &reopened.local_review) .is_none() ); reopened.execute("diff.review.new"); - let second_id = reopened - .workspace_diff() - .unwrap() - .local_review - .document - .storage_id - .unwrap(); + let second_id = reopened.local_review.document.storage_id.unwrap(); assert_ne!(id, second_id); let mut choosing = editor(); choosing.set_review_directory(storage); historical_diff(&mut choosing, &root, vec!["old".into()], vec!["new".into()]); choosing.handle(Input::Enter); - assert_eq!( - choosing - .workspace_diff() - .unwrap() - .local_review - .document - .storage_id, - Some(second_id) - ); + assert_eq!(choosing.local_review.document.storage_id, Some(second_id)); choosing.execute(&format!("diff.review.open {id}")); - assert_eq!( - choosing - .workspace_diff() - .unwrap() - .local_review - .document - .storage_id, - Some(id) - ); + assert_eq!(choosing.local_review.document.storage_id, Some(id)); choosing.execute("diff.review.delete 1"); assert!(store.load(&root, id).unwrap().comments.is_empty()); choosing.execute(&format!("diff.review.open {}", exported.display())); - let imported_id = choosing - .workspace_diff() - .unwrap() - .local_review - .document - .storage_id - .unwrap(); + let imported_id = choosing.local_review.document.storage_id.unwrap(); assert_ne!(imported_id, id); assert_eq!( store.load(&root, imported_id).unwrap().comments[0].body, @@ -3523,7 +3622,7 @@ fn review_resolution_persists_without_hiding_comments_or_changing_stale_anchors( .contains("Comment #1 · resolved · current") ); let store = Store::open(&storage).unwrap(); - let document = &first.workspace_diff().unwrap().local_review.document; + let document = &first.local_review.document; let id = document.storage_id.unwrap(); let anchor = document.comments[0].anchor.clone(); assert!(store.load(&root, id).unwrap().comments[0].resolved); @@ -3557,25 +3656,8 @@ fn review_resolution_persists_without_hiding_comments_or_changing_stale_anchors( assert!(!store.load(&root, id).unwrap().comments[0].resolved); stale.execute("diff.review.unresolve 1"); assert!(stale.current_review_dirty()); - assert!( - !stale - .workspace_diff() - .unwrap() - .local_review - .document - .comments[0] - .resolved - ); - assert_eq!( - stale - .workspace_diff() - .unwrap() - .local_review - .document - .comments[0] - .anchor, - anchor - ); + assert!(!stale.local_review.document.comments[0].resolved); + assert_eq!(stale.local_review.document.comments[0].anchor, anchor); assert!(stale.message().contains("changed in another session")); first.execute("diff.review.resolve"); assert!( @@ -3592,7 +3674,7 @@ fn review_resolution_persists_without_hiding_comments_or_changing_stale_anchors( .contains("Comment #1 · unresolved · current") ); first.execute(&format!("diff.review.open {}", export.display())); - let imported = &first.workspace_diff().unwrap().local_review.document; + let imported = &first.local_review.document; assert_ne!(imported.storage_id, Some(id)); assert!( store @@ -3617,34 +3699,15 @@ fn sqlite_review_conflicts_and_write_failures_keep_dirty_drafts() { first.handle(Input::Escape); first.execute("w"); first.handle(Input::Escape); - let id = first - .workspace_diff() - .unwrap() - .local_review - .document - .storage_id - .unwrap(); + let id = first.local_review.document.storage_id.unwrap(); let mut stale = editor(); stale.set_review_directory(storage.clone()); historical_diff(&mut stale, &root, vec!["old".into()], vec!["new".into()]); - let version = stale - .workspace_diff() - .unwrap() - .local_review - .document - .storage_revision; + let version = stale.local_review.document.storage_revision; first.execute("diff.review.verdict looks_good"); stale.execute("diff.review.verdict changes_requested"); assert!(stale.current_review_dirty()); - assert_eq!( - stale - .workspace_diff() - .unwrap() - .local_review - .document - .storage_revision, - version - ); + assert_eq!(stale.local_review.document.storage_revision, version); assert_eq!( Store::open(&storage) .unwrap() @@ -3665,13 +3728,7 @@ fn sqlite_review_conflicts_and_write_failures_keep_dirty_drafts() { first.handle(Input::Escape); assert!(!first.current_review_dirty()); assert_eq!( - first - .workspace_diff() - .unwrap() - .local_review - .document - .comments[0] - .body, + first.local_review.document.comments[0].body, "Stored feedback" ); assert_eq!(fs::read_to_string(blocked).unwrap(), "preserve"); @@ -3843,7 +3900,7 @@ fn precise_review_ranges_capture_graphemes_and_navigate_to_their_start() { root.join("range.json").display() )); let diff = editor.workspace_diff().unwrap(); - let anchor = &diff.local_review.document.comments[0].anchor; + let anchor = &editor.local_review.document.comments[0].anchor; let (start, end) = if keys == "llvj" { ( TextPosition { row: 0, column: 2 }, @@ -3857,7 +3914,10 @@ fn precise_review_ranges_capture_graphemes_and_navigate_to_their_start() { }; assert_eq!(anchor.start, start); assert_eq!(anchor.end, end); - assert!(diff.review_anchor_range(anchor).is_some()); + assert!( + diff.review_anchor_range(anchor, &editor.local_review) + .is_some() + ); editor.handle(Input::Escape); assert_eq!(editor.pane_id(), owner); assert_eq!( @@ -3881,13 +3941,7 @@ fn precise_review_ranges_capture_graphemes_and_navigate_to_their_start() { "diff.review.save {}", root.join("range.json").display() )); - let anchor = &editor - .workspace_diff() - .unwrap() - .local_review - .document - .comments[0] - .anchor; + let anchor = &editor.local_review.document.comments[0].anchor; assert_eq!(anchor.start, TextPosition { row: 0, column: 0 }); assert_eq!( anchor.end, @@ -3975,7 +4029,7 @@ fn local_review_visual_commands_and_palette_preserve_both_side_ranges() { root.join("range.json").display() )); let diff = editor.workspace_diff().unwrap(); - let anchor = diff.local_review.document.comments[0].anchor.clone(); + let anchor = editor.local_review.document.comments[0].anchor.clone(); assert_eq!(anchor.start, TextPosition { row: 0, column: 0 }); assert_eq!(anchor.end, TextPosition { row: 2, column: 0 }); assert_eq!( @@ -3986,7 +4040,10 @@ fn local_review_visual_commands_and_palette_preserve_both_side_ranges() { Side::After } ); - assert!(diff.review_anchor_range(&anchor).is_some()); + assert!( + diff.review_anchor_range(&anchor, &editor.local_review) + .is_some() + ); let source_pane = editor.diff_owner.unwrap(); editor.select_pane(source_pane); editor @@ -4000,16 +4057,7 @@ fn local_review_visual_commands_and_palette_preserve_both_side_ranges() { editor.execute("diff.review.overview"); assert!(editor.context().text.contains("stale")); editor.execute("diff.review.edit 1"); - assert_eq!( - editor - .workspace_diff() - .unwrap() - .local_review - .document - .comments[0] - .anchor, - anchor - ); + assert_eq!(editor.local_review.document.comments[0].anchor, anchor); } } @@ -4045,15 +4093,7 @@ fn local_review_guards_metadata_and_hidden_native_drafts_against_loss() { let invalid = root.join("missing/review.json"); editor.execute(&format!("diff.review.save {}", invalid.display())); assert!(editor.local_reviews_dirty()); - assert!( - editor - .workspace_diff() - .unwrap() - .local_review - .document - .comments - .is_empty() - ); + assert!(editor.local_review.document.comments.is_empty()); assert!( editor .parked @@ -7633,14 +7673,7 @@ mod system_clipboard { } editor.execute("diff.review.copy unexpected"); editor.with_pane(editor.diff_owner.unwrap(), |editor| { - editor - .diff - .as_mut() - .unwrap() - .local_review - .document - .comments - .clear(); + editor.local_review.document.comments.clear(); }); editor.execute("diff.review.copy"); if mode == "system" { diff --git a/tests/unit/ruby.rs b/tests/unit/ruby.rs index 7ba1685..d045c89 100644 --- a/tests/unit/ruby.rs +++ b/tests/unit/ruby.rs @@ -626,6 +626,33 @@ fn config_leader_defaults_custom_keys_validation_and_required_wire_field() { } } +#[test] +fn source_review_actions_and_diff_command_aliases_share_the_core_catalog() { + let fixture = Fixture::new(); + let path = fixture.file( + "config.rb", + r#" +Vamp.configure do |config| + config.bind "C", config.core.comment + config.bind "N", config.core.review.next + config.bind "S", config.core.review.save + raise "missing diff comment alias" unless Vamp.builtin_command?("diff.comment") + raise "missing diff review alias" unless Vamp.builtin_command?("diff.review.edit") + raise "mutable review action" unless config.core.review.frozen? && config.core.comment.frozen? +end +"#, + ); + let loaded = load_config(&path).unwrap(); + let actions: Vec<_> = loaded + .bindings + .iter() + .filter_map(|binding| binding.command.as_deref()) + .collect(); + assert!(actions.contains(&"core.comment")); + assert!(actions.contains(&"core.review.next")); + assert!(actions.contains(&"core.review.save")); +} + #[test] fn git_gutter_config_modes_default_and_rejection() { use crate::gutter::GutterStyle; diff --git a/tests/unit/tui.rs b/tests/unit/tui.rs index f6b6a74..64d0e74 100644 --- a/tests/unit/tui.rs +++ b/tests/unit/tui.rs @@ -2951,6 +2951,62 @@ fn diff_status_layouts_hide_absent_panes_and_leave_unchanged_renames_blank() { assert!(text.contains("After · M")); } +#[test] +fn source_review_markers_highlight_exact_text_and_reject_stale_ranges() { + use crate::editor::Effect; + use crate::plugin::TextPosition; + use crate::review::{Anchor, Comment, Review, Side}; + for wrapped in [false, true] { + let directory = Directory::new(); + let text = format!("aé界{}\nsecond\n", " trailing".repeat(10)); + let path = directory.file("source.txt", &text); + let mut editor = Editor::open(Some(path), vec!["text".into()]).unwrap(); + editor.local_review.document = Review::new(directory.0.canonicalize().unwrap()); + editor.local_review.document.comments.push(Comment { + id: 1, + anchor: Anchor { + revision: "working:source-snapshot".into(), + path: "source.txt".into(), + side: Side::After, + start: TextPosition { row: 0, column: 1 }, + end: TextPosition { row: 0, column: 3 }, + content: text, + }, + body: "Feedback".into(), + resolved: false, + }); + editor.local_review.selected = Some(1); + if wrapped { + editor.execute("word_wrap.toggle"); + } + let mut terminal = Terminal::new(ratatui::backend::TestBackend::new(40, 16)).unwrap(); + let mut views = BufferViewports::default(); + terminal + .draw(|frame| views.draw(frame, &mut editor)) + .unwrap(); + assert!(screen_text(&terminal).contains('●')); + assert!( + terminal + .backend() + .buffer() + .content + .iter() + .filter(|cell| cell.bg == Color::Magenta) + .count() + >= 2 + ); + let anchor = editor.local_review.document.comments[0].anchor.clone(); + editor + .apply(vec![Effect::Replace("changed entirely\n".into())]) + .unwrap(); + terminal + .draw(|frame| views.draw(frame, &mut editor)) + .unwrap(); + assert!(!screen_text(&terminal).contains('●')); + assert_eq!(editor.local_review.document.comments[0].anchor, anchor); + } +} + fn render_diff( terminal: &mut Terminal, diff: &DiffState, @@ -2959,12 +3015,44 @@ fn render_diff( terminal .draw(|frame| { panels.begin_frame(); - draw_diff(frame, diff, frame.area(), "↑/↓", "h/j/k/l", panels, None); + draw_diff( + frame, + diff, + frame.area(), + "↑/↓", + "h/j/k/l", + panels, + None, + None, + ); }) .unwrap(); } -fn commented_diff() -> DiffState { +fn render_review_diff( + terminal: &mut Terminal, + diff: &DiffState, + review: &crate::review::ReviewState, + panels: &mut WheelPanels, +) { + terminal + .draw(|frame| { + panels.begin_frame(); + draw_diff( + frame, + diff, + frame.area(), + "↑/↓", + "h/j/k/l", + panels, + None, + Some(review), + ); + }) + .unwrap(); +} + +fn commented_diff() -> (DiffState, crate::review::ReviewState) { use crate::review::{Anchor, Comment, Side}; let before = format!( "{}\nold e\u{301}界 second\nkeep", @@ -2980,8 +3068,9 @@ fn commented_diff() -> DiffState { file.old_path = Some("old.txt".into()); let mut diff = diff_comparison(vec![file]); diff.files_visible = false; - diff.local_review.document.root = diff.comparison.as_ref().unwrap().root.clone(); - diff.local_review.document.comments = vec![ + let mut review = crate::review::ReviewState::default(); + review.document.root = diff.comparison.as_ref().unwrap().root.clone(); + review.document.comments = vec![ Comment { id: 1, anchor: Anchor { @@ -3009,8 +3098,8 @@ fn commented_diff() -> DiffState { resolved: false, }, ]; - diff.local_review.selected = Some(1); - diff + review.selected = Some(1); + (diff, review) } #[test] @@ -3022,14 +3111,13 @@ fn local_review_highlights_exact_graphemes_newlines_clips_and_wrap_continuations for side in [Side::Before, Side::After] { for (wrapping, horizontal) in [(false, 0), (false, 6), (true, 0)] { for (start, end) in [((0, 4), (0, 6)), ((0, 5), (1, 5)), ((0, 84), (1, 0))] { - let mut diff = commented_diff(); + let (mut diff, mut review) = commented_diff(); diff.set_view_mode(mode); diff.top = 0; diff.word_wrap = wrapping; diff.left_before = horizontal; diff.left_after = horizontal; - let comment = diff - .local_review + let comment = review .document .comments .iter_mut() @@ -3044,11 +3132,11 @@ fn local_review_highlights_exact_graphemes_newlines_clips_and_wrap_continuations column: end.1, }; let anchor = comment.anchor.clone(); - diff.local_review.selected = Some(comment.id); + review.selected = Some(comment.id); let mut terminal = Terminal::new(ratatui::backend::TestBackend::new(90, 40)).unwrap(); let mut panels = WheelPanels::default(); - render_diff(&mut terminal, &diff, &mut panels); + render_review_diff(&mut terminal, &diff, &review, &mut panels); let file = &diff.comparison.as_ref().unwrap().files[0]; let wrapped = panels .diff_wrap @@ -3164,13 +3252,13 @@ fn local_review_highlights_exact_graphemes_newlines_clips_and_wrap_continuations fn local_review_markers_preserve_diff_signs_ranges_and_wrapped_geometry() { for mode in [ViewMode::Split, ViewMode::Unified] { for wrapping in [false, true] { - let mut diff = commented_diff(); + let (mut diff, review) = commented_diff(); diff.set_view_mode(mode); diff.top = 0; diff.word_wrap = wrapping; let mut terminal = Terminal::new(ratatui::backend::TestBackend::new(90, 40)).unwrap(); let mut panels = WheelPanels::default(); - render_diff(&mut terminal, &diff, &mut panels); + render_review_diff(&mut terminal, &diff, &review, &mut panels); let buffer = terminal.backend().buffer(); let markers: Vec<_> = buffer .content @@ -3219,20 +3307,20 @@ fn local_review_markers_preserve_diff_signs_ranges_and_wrapped_geometry() { fn local_review_markers_reject_stale_identity_text_and_rename_paths() { for mode in [ViewMode::Split, ViewMode::Unified] { for mismatch in ["revision", "content", "path", "side", "root"] { - let mut diff = commented_diff(); + let (mut diff, mut review) = commented_diff(); diff.set_view_mode(mode); - diff.local_review.document.comments.truncate(1); - let anchor = &mut diff.local_review.document.comments[0].anchor; + review.document.comments.truncate(1); + let anchor = &mut review.document.comments[0].anchor; match mismatch { "revision" => anchor.revision.push_str("-newer"), "content" => anchor.content.replace_range(..1, "X"), "path" => anchor.path = "new.txt".into(), "side" => anchor.side = crate::review::Side::After, - "root" => diff.local_review.document.root = "another-repository".into(), + "root" => review.document.root = "another-repository".into(), _ => unreachable!(), } let mut terminal = Terminal::new(ratatui::backend::TestBackend::new(90, 20)).unwrap(); - render_diff(&mut terminal, &diff, &mut WheelPanels::default()); + render_review_diff(&mut terminal, &diff, &review, &mut WheelPanels::default()); assert!( terminal .backend() @@ -3258,18 +3346,18 @@ fn local_review_markers_reject_stale_identity_text_and_rename_paths() { #[test] fn unified_context_shows_comments_from_either_snapshot_side() { for side in [crate::review::Side::Before, crate::review::Side::After] { - let mut diff = commented_diff(); + let (mut diff, mut review) = commented_diff(); diff.set_view_mode(ViewMode::Unified); - diff.local_review + review .document .comments .retain(|comment| comment.anchor.side == side); - let comment = &mut diff.local_review.document.comments[0]; + let comment = &mut review.document.comments[0]; comment.anchor.start = crate::plugin::TextPosition { row: 2, column: 1 }; comment.anchor.end = crate::plugin::TextPosition { row: 2, column: 3 }; - diff.local_review.selected = Some(comment.id); + review.selected = Some(comment.id); let mut terminal = Terminal::new(ratatui::backend::TestBackend::new(90, 20)).unwrap(); - render_diff(&mut terminal, &diff, &mut WheelPanels::default()); + render_review_diff(&mut terminal, &diff, &review, &mut WheelPanels::default()); assert!( screen_text(&terminal).contains("●keep"), "{side:?}: {}", @@ -3296,13 +3384,13 @@ fn unified_context_shows_comments_from_either_snapshot_side() { fn local_review_markers_yield_to_text_on_narrow_surfaces() { for mode in [ViewMode::Split, ViewMode::Unified] { for (width, height) in [(1, 1), (2, 2), (3, 3), (8, 4), (16, 8), (30, 12)] { - let mut diff = commented_diff(); + let (mut diff, review) = commented_diff(); diff.set_view_mode(mode); diff.word_wrap = true; let mut terminal = Terminal::new(ratatui::backend::TestBackend::new(width, height)).unwrap(); let mut panels = WheelPanels::default(); - render_diff(&mut terminal, &diff, &mut panels); + render_review_diff(&mut terminal, &diff, &review, &mut panels); for (area, focus) in panels .diff .as_ref() @@ -3702,6 +3790,7 @@ fn pull_request_review_symbols_use_terminal_cells_and_keep_the_selection_arrow_i "h/j/k/l", &mut panels, with_editor.then_some(&editor), + None, ); }) .unwrap(); @@ -4834,7 +4923,7 @@ fn unavailable_file_documents_show_the_actual_error_in_managed_workspaces() { }; terminal .draw(|frame| { - panels.draw_documents(frame, &diff, frame.area(), None, None); + panels.draw_documents(frame, &diff, frame.area(), None, None, None); }) .unwrap(); let text = screen_text(&terminal);