From b558f82094d8908c70e0b95b98b0c74ca4dd9a01 Mon Sep 17 00:00:00 2001 From: Valdemar Lorenzen Date: Fri, 16 Oct 2026 15:15:19 +0200 Subject: [PATCH] feat: persist local reviews automatically in SQLite --- TODO.md | 8 +- docs/architecture.md | 4 +- docs/configuration.md | 5 +- docs/development.md | 2 + docs/editing.md | 13 ++- src/editor.rs | 253 ++++++++++++++++++++++++++++++++++++++++++ src/editor_review.rs | 226 +++++++++++++++++++++++++++++++++---- src/main.rs | 5 +- src/review.rs | 2 + src/ruby.rs | 115 ++++++++++++++++++- src/ruby_config.rb | 11 ++ src/session.rs | 5 + tests/diff.rs | 25 ++++- tests/lsp.rs | 4 +- tests/navigation.rs | 4 +- 15 files changed, 634 insertions(+), 48 deletions(-) diff --git a/TODO.md b/TODO.md index a971f2d..4d88737 100644 --- a/TODO.md +++ b/TODO.md @@ -13,7 +13,7 @@ - preserve immutable snapshot identity and existing stale detection; never silently reanchor comments after source changes. - persist precise ranges in version 2 JSON import/export. SQLite storage follows in the next item. - future GitHub publishing may map precise local anchors to supported line ranges without reducing precision in the local review. -- [ ] persist local reviews automatically in a local SQLite database using rusqlite; no server, hosted storage, or synchronization. +- [x] persist local reviews automatically in a local SQLite database using rusqlite; no server, hosted storage, or synchronization. - keep one database under $XDG_DATA_HOME/vamp/reviews/, falling back to ~/.local/share/vamp/reviews/; allow a configurable review_directory. - key reviews by repository identity and persistent review ID, preserving reviews and stale comments across revision changes. - autosave committed comment edits and summaries when dismissing or switching review editors; save verdicts and deletions immediately, using transactions rather than per-keystroke writes. @@ -22,13 +22,15 @@ - version the database schema; use review-version checks to reject conflicting edits from multiple editor instances. - surface write failures and conflicts without discarding drafts or marking unsuccessful saves as persisted. - persist the precise start/end grapheme anchors without reducing them to line ranges. -- [ ] add an option for three pane lef/base/right view for conflict resolution. - - alternative pane layout as left/right with base underneath (configurable) +- [ ] diff view: consider an MCP or a something for a local review loop when using coding agents. - [ ] 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. - keep authentication, network requests, publishing, and remote thread synchronization out of the local UX work; treat timed-out writes as unknown outcomes rather than automatically retrying. - define follow-on scope for reading/replying to remote threads, editing/deleting published comments, and resolving threads. +- [ ] add an option for three pane lef/base/right view for conflict resolution. + - alternative pane layout as left/right with base underneath (configurable) +- [ ] take a look at the configuration options and their names, and consider whether renaming some of them is warranted, as well as maybe nesting some under some common namespace (like some already are). - [ ] remove the diff palette polling fallback once a released Herdr fix delivers palette-ready DEC 2031 notifications, including palette-only changes with an unchanged light/dark preference. Related upstream issue: [herdrdev/herdr#4183](https://github.com/herdrdev/herdr/issues/4183). - verify that notification-triggered queries return fresh colors in Ghostty through Herdr before removal; retain startup, focus-regain, and notification-driven palette refresh. - [ ] pull a pr description and display it in a buffer: diff --git a/docs/architecture.md b/docs/architecture.md index cb98f4a..7f84761 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -16,8 +16,8 @@ One editor crate; the workspace also includes its patched Crossterm dependency. Virtual diff leaves retain their original catalog input index for caller-owned annotations. Normal scanned leaves accept optional repository status and old-path metadata without filtering unchanged files; missing tracked deletions add virtual leaves and required directories. Metadata-only updates preserve query, selection, expansion, and view without rebuilding the catalog. - `diff`: owns searchable Before/After selection, category, focus and explorer visibility, request freshness, read-only comparison DTOs, aligned rows with cached grapheme-level change ranges, and shared vertical/independent horizontal offsets. Changed paths form a virtual `files::FilePicker` catalog, reusing the normal tree projection, filtering, and navigation without scanning disk. It has no subprocess or terminal dependencies. - `repository`: discovers jj/Git repositories and local/advertised remote revisions, builds disk snapshots and changed-file hunks, and owns isolated temporary comparison repositories with bounded, cancellable subprocesses. It never saves editor buffers or modifies active refs/checkouts. -- `review`: owns versioned local JSON review documents, immutable source anchors, validation, bounded reads, and atomic writes with external-change protection. It has no terminal or network operations. Comparison state owns the active review; `editor_review` keeps native comment/summary buffers and the read-only overview attached to their owning tab, synchronizes drafts on review commands, and guards unsaved teardown. The floating review editor has a tab-owned native pane outside the workspace `PaneTree`; rendering overlays it on the diff owner's rectangle without changing the source layout. Dismissal retains drafts and restores source focus. Working snapshot IDs hash the private captured paths, modes, contents, and notices; anchors also retain exact file text to account for live edits without reanchoring. - Version 2 anchors use the existing `plugin::TextPosition` type for zero-based grapheme start/end positions with an exclusive end. JSON loading accepts only the current schema and retains original disk bytes for external-write checks. Selected-comment painting reuses source-side grapheme clipping and wrapping; native selections retain higher priority. +- `review`: owns the validated local review model, exact immutable anchors, SQLite storage through bundled rusqlite, and atomic JSON import/export with external-change protection. `Store` uses schema-versioned transactions and compare-and-swap review revisions; repository roots and persistent review IDs are independent of comparison revisions. One bounded JSON document is stored per row, retaining exact grapheme endpoints and stale snapshots. Incompatible schemas and concurrent edits fail without replacing data or advancing in-memory saved state. + `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. - `clipboard`: owns native plain-text provider selection and bounded subprocess I/O; it never computes selections or editing offsets. - `syntax`: owns Tree-sitter grammar detection, per-buffer incremental trees, bundled highlight/local/injection queries, and syntax byte spans; it has no terminal color/geometry or editing dependency. - `markdown`: owns native Markdown link/reference resolution, local destination decoding, and heading fragment lookup through the installed block/inline grammars. It has no editor lifecycle, terminal, LSP, or external-process dependency. diff --git a/docs/configuration.md b/docs/configuration.md index d64125b..dea8b29 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -30,6 +30,7 @@ Vamp.configure do |config| config.diff_view_mode = "split" # or "unified" config.diff_split_orientation = "vertical" # or "horizontal" config.diff_follow_working_copy = true + config.review_directory = nil # XDG data directory; override with an absolute or config-relative path config.bind "ctrl+h", config.core.left, mode: config.modes.tree config.bind "ctrl+l", config.core.right, mode: config.modes.tree config.plugin "plugins/greeting.rb" @@ -229,7 +230,7 @@ Normal and Visual Ctrl+u/Ctrl+d invoke `config.core.scroll_step_up` / `config.co | 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.next`, `diff.review.previous`, `diff.review.overview`, `diff.review.summary`, `diff.review.verdict`, `diff.review.save`, `diff.review.open` | +| Normal | `diff.review.edit`, `diff.review.delete`, `diff.review.next`, `diff.review.previous`, `diff.review.overview`, `diff.review.summary`, `diff.review.verdict`, `diff.review.save`, `diff.review.open`, `diff.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` | @@ -324,7 +325,7 @@ config.bind "leader r o", config.core.diff.review.overview config.bind "leader r n", config.core.diff.review.next ``` -Comment and summary composition uses native editing in a floating window over the diff workspace, without adding workspace splits. Escape in Normal mode or `:pane.close` dismisses it while retaining drafts. `:w` there writes the whole local JSON review, including hidden draft text; choose its destination with `:diff.review.save path`. These actions do not publish reviews or contact a forge. See [local review behavior](editing.md#repository-diff). +Review editing uses a floating window without adding workspace splits. Dismissing or switching editors commits text to local SQLite; verdicts and deletions 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). `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 0efb698..43ed58f 100644 --- a/docs/development.md +++ b/docs/development.md @@ -61,6 +61,8 @@ Diff checks cover staged category/filter navigation, stale request cancellation, 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. +SQLite review checks cover exact anchors across restarts and revision changes, repository isolation, independent connections and optimistic conflicts, locked/unwritable stores, incompatible schema preservation, bounded/corrupt documents, private file permissions, and XDG/config path resolution. Editor checks commit text on dismissal and switching, 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 restarts three editor processes against one temporary XDG database and verifies stable IDs, Unicode anchors, automatic summary saves, JSON export isolation, and selector/verdict persistence. + Conflicted-revision regressions preserve unchanged symlinks, defer oversized unselected blobs until file selection, compare selected conflict text with jj's projection, and exclude unchanged conflicts when comparing later revisions. Live terminal smoke covers local bookmark selection and opening an actual conflicted file without changing the checkout. diff --git a/docs/editing.md b/docs/editing.md index a90a522..b422b39 100644 --- a/docs/editing.md +++ b/docs/editing.md @@ -466,12 +466,17 @@ Comments, summaries, and the review overview open in a focused floating editor o | `: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]` | Save the entire review as local JSON; the first save requires a path | -| `:diff.review.open path` | Reopen a saved review for the current repository | +| `: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 | -In a review text pane, **`:w` saves the entire review**, not a source file. Choose a destination with `:diff.review.save path` first, or use `:w path`. Saving includes edits in hidden comment/summary buffers. Normal close, quit, restart, comparison replacement, and source-buffer deletion protect unsaved reviews; forced close/quit retains its explicit discard semantics. An externally changed review file is never silently overwritten, including with `:w!`; use a new path or reopen it after preserving your edits. +Reviews save automatically to a local SQLite database when you dismiss or switch comment/summary editors. Verdicts and deletions commit immediately; typing alone does not write per keystroke. **`:w`** explicitly commits the entire review, including hidden drafts. `:diff.review.save path` or `:w path` additionally exports JSON; later autosaves still go to SQLite and leave that export unchanged. Imports create a new stored review identity rather than overwriting another review. -Version 2 JSON is the canonical, agent-readable artifact: repository root, summary, optional verdict, and comments with stable IDs, bodies, and anchors. Each anchor retains its immutable comparison revision, side, relative path (old path for renamed Before files), and exact whole-file text including the final newline. `start` and `end` contain zero-based source `row` and extended-grapheme `column` coordinates; `end` is exclusive. Columns count complete Unicode graphemes, not bytes, code points, UTF-16 units, or screen cells. Overview/window labels display one-based rows and columns. Only the current format is supported; old review files are not migrated. Working comparisons use content-addressed snapshot IDs; captured text additionally freezes live unsaved edits. The file limit remains 64 MiB. Prefer saving outside the checkout so the review itself does not become another changed file. +Storage defaults to `$XDG_DATA_HOME/vamp/reviews/reviews.sqlite3`, or `~/.local/share/vamp/reviews/reviews.sqlite3` when XDG_DATA_HOME is unset or relative. Configure `review_directory` to override its directory; relative configuration paths resolve against the config file. Repository identity is the canonical checkout root. Each review has a persistent database ID independent of comparison revisions, so reopening changed revisions retains stale comments. Opening a comparison automatically restores the repository's sole review. Multiple reviews open a floating selector: move to an entry and press Enter, or use `:diff.review.open id`. Use `:diff.review.new` for another independent review. + +Write errors, locked databases, incompatible schemas, and conflicting edits from another editor instance retain dirty drafts and report the failure. Normal destructive closes remain guarded until persistence succeeds. `:w!` does not bypass database revision conflicts; export your draft before reopening the current stored review. JSON exports retain external-file-change protection. Only the current database schema is accepted; existing incompatible databases are preserved rather than reset or migrated. + +Version 2 JSON is the agent-readable sharing format: repository root, summary, optional verdict, and comments with stable IDs, bodies, and anchors. Database review IDs and optimistic storage revisions stay internal. Each anchor retains its immutable comparison revision, side, relative path (old path for renamed Before files), and exact whole-file text including the final newline. `start` and `end` contain zero-based source `row` and extended-grapheme `column` coordinates; `end` is exclusive. Columns count complete Unicode graphemes, not bytes, code points, UTF-16 units, or screen cells. Overview/window labels display one-based rows and columns. Only the current format is supported. Working comparisons use content-addressed snapshot IDs; captured text additionally freezes live unsaved edits. Each stored or exported review is limited to 64 MiB. Reopening requires the same repository root. Comments whose revision, path, or exact file text no longer matches are **stale**: their original anchors remain unchanged and their gutter markers disappear. Unloaded files are shown as **not checked** until visited; refresh the overview to update its statuses. There is no automatic reanchoring, server, authentication, or review-publishing operation. diff --git a/src/editor.rs b/src/editor.rs index 82c7b00..3804c2d 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -990,6 +990,12 @@ core_actions![ true, true ), + ( + "core.diff.review.new", + Action::ReviewNew, + &[Mode::Normal], + true + ), ( "core.word_wrap.toggle", Action::WordWrapToggle, @@ -2072,6 +2078,7 @@ pub struct Editor { render_pane_focused: bool, diff_owner: Option, parked_diff: Option, + review_directory: Option, diff_pane_bounds: [(Focus, PaneRect); 3], // Closed destinations stay until bounded eviction so they cannot shift jump_index. jumps: VecDeque, @@ -2178,6 +2185,7 @@ impl Editor { render_pane_focused: true, diff_owner: None, parked_diff: None, + review_directory: None, diff_pane_bounds: [( Focus::Files, PaneRect { @@ -4502,6 +4510,9 @@ impl Editor { } } self.finish_review_navigation(); + if let Err(error) = self.initialize_local_review() { + self.set_message(format!("Local review: {error}")); + } true } @@ -5250,6 +5261,7 @@ impl Editor { | Action::ReviewVerdict | Action::ReviewSave | Action::ReviewOpen + | Action::ReviewNew | Action::WordWrapToggle | Action::GitBlameToggle | Action::Quit @@ -8448,6 +8460,7 @@ impl Editor { | Action::ReviewVerdict | Action::ReviewSave | Action::ReviewOpen + | Action::ReviewNew | Action::WordWrapToggle | Action::GitBlameToggle | Action::LspDefinition @@ -9361,6 +9374,7 @@ impl Editor { | Action::ReviewVerdict | Action::ReviewSave | Action::ReviewOpen + | Action::ReviewNew ) { self.finish_prompt(); if let Err(error) = self.local_review_command(entry.action, args) { @@ -9368,6 +9382,21 @@ impl Editor { } return None; } + if self.review_directory.is_some() + && matches!( + entry.action, + Action::TabPrevious + | Action::TabNext + | Action::TabNew + | Action::TabClose + | Action::Quit + | Action::Restart + ) + && let Err(error) = self.autosave_local_review() + { + self.set_message(format!("Review not saved: {error}")); + return None; + } if self.review_panel().is_some() && matches!( entry.action, @@ -15911,11 +15940,235 @@ mod tests { }], })), ); + if editor.review_panel().is_some() { + return; + } editor.handle(Input::Char('\t')); assert_eq!(editor.diff().unwrap().focus, Focus::Before); assert!(editor.diff_interacting()); } + #[test] + fn sqlite_reviews_autosave_restore_select_and_keep_exports_independent() { + use crate::review::{Review, Store, Verdict}; + let directory = Directory::new(); + let root = directory.0.canonicalize().unwrap(); + let storage = root.join("data"); + let mut first = editor(); + first.set_review_directory(storage.clone()); + historical_diff( + &mut first, + &root, + vec!["p界e\u{301}z".into()], + vec!["changed".into()], + ); + type_text(&mut first, "lvl:diff.comment"); + first.handle(Input::Enter); + type_text(&mut first, "iFirst feedback"); + first.handle(Input::Escape); + assert!( + Store::open(&storage) + .unwrap() + .list(&root) + .unwrap() + .is_empty() + ); + first.handle(Input::Escape); + let id = first + .workspace_diff() + .unwrap() + .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"); + assert_eq!( + persisted.comments[0].anchor.start, + TextPosition { row: 0, column: 1 } + ); + assert_eq!( + persisted.comments[0].anchor.end, + TextPosition { row: 0, column: 3 } + ); + first.execute("diff.review.summary"); + type_text(&mut first, "iSummary"); + first.handle(Input::Escape); + first.execute("diff.review.verdict changes_requested"); + let persisted = store.load(&root, id).unwrap(); + assert_eq!(persisted.summary, "Summary"); + assert_eq!(persisted.verdict, Some(Verdict::ChangesRequested)); + let exported = root.join("shared.json"); + first.execute(&format!("diff.review.save {}", exported.display())); + first.execute("diff.review.edit 1"); + type_text(&mut first, "A revised"); + first.handle(Input::Escape); + first.handle(Input::Escape); + assert_eq!( + store.load(&root, id).unwrap().comments[0].body, + "First feedback revised" + ); + assert_eq!( + Review::load(&exported, &root).unwrap().comments[0].body, + "First feedback" + ); + let mut reopened = editor(); + reopened.set_review_directory(storage.clone()); + historical_diff( + &mut reopened, + &root, + vec!["different revision".into()], + vec!["changed".into()], + ); + let restored = &reopened.workspace_diff().unwrap().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_current(&restored.comments[0].anchor) + ); + reopened.execute("diff.review.new"); + let second_id = reopened + .workspace_diff() + .unwrap() + .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()]); + assert_eq!( + choosing + .workspace_diff() + .unwrap() + .local_review + .choices + .iter() + .map(|(id, _)| *id) + .collect::>(), + std::collections::BTreeSet::from([id, second_id]) + ); + choosing.handle(Input::Enter); + assert_eq!( + choosing + .workspace_diff() + .unwrap() + .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) + ); + 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(); + assert_ne!(imported_id, id); + assert_eq!( + store.load(&root, imported_id).unwrap().comments[0].body, + "First feedback" + ); + } + + #[test] + fn sqlite_review_conflicts_and_write_failures_keep_dirty_drafts() { + use crate::review::Store; + let directory = Directory::new(); + let root = directory.0.canonicalize().unwrap(); + let storage = root.join("data"); + let mut first = editor(); + first.set_review_directory(storage.clone()); + historical_diff(&mut first, &root, vec!["old".into()], vec!["new".into()]); + first.execute("diff.comment"); + type_text(&mut first, "iStored feedback"); + first.handle(Input::Escape); + first.handle(Input::Escape); + let id = first + .workspace_diff() + .unwrap() + .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; + 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!( + Store::open(&storage) + .unwrap() + .load(&root, id) + .unwrap() + .verdict, + Some(crate::review::Verdict::LooksGood) + ); + let blocked = root.join("not-a-directory"); + fs::write(&blocked, "preserve").unwrap(); + first.set_review_directory(blocked.clone()); + first.execute("diff.review.edit 1"); + type_text(&mut first, "A unsaved"); + first.handle(Input::Escape); + first.handle(Input::Escape); + assert!(first.current_review_dirty()); + assert_eq!( + first + .workspace_diff() + .unwrap() + .local_review + .document + .comments[0] + .body, + "Stored feedback unsaved" + ); + assert_eq!(fs::read_to_string(blocked).unwrap(), "preserve"); + assert_eq!( + Store::open(&storage) + .unwrap() + .load(&root, id) + .unwrap() + .comments[0] + .body, + "Stored feedback" + ); + } + #[test] fn local_review_native_composition_roundtrip_and_draft_lifecycle() { use crate::review::{Review, Side, Verdict}; diff --git a/src/editor_review.rs b/src/editor_review.rs index 51e3796..da744ab 100644 --- a/src/editor_review.rs +++ b/src/editor_review.rs @@ -6,6 +6,7 @@ enum ReviewText { Comment(u64), Summary, Overview, + Reviews, } pub(super) struct ReviewBuffer { @@ -15,6 +16,109 @@ pub(super) struct ReviewBuffer { } impl Editor { + pub fn set_review_directory(&mut self, directory: PathBuf) { + self.review_directory = Some(directory); + } + + 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 { + 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 choices = store.list(&root)?; + if choices.len() == 1 { + self.review_diff_mut().unwrap().local_review.document = + store.load(&root, choices[0].0)?; + } else if choices.len() > 1 { + self.review_diff_mut().unwrap().local_review.choices = choices; + self.open_review_selector()?; + } + Ok(()) + } + + fn open_review_selector(&mut self) -> Result<(), String> { + let diff = self.workspace_diff().ok_or("No comparison is open")?; + 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 choices = crate::review::Store::open(directory)?.list(root)?; + let mut text = String::from( + "# Local reviews\n\nEnter opens the review under the caret.\n:diff.review.new starts a separate review.\n", + ); + for (id, label) in &choices { + use std::fmt::Write as _; + writeln!(text, "\n## Review #{id} · {label}").unwrap(); + } + self.review_diff_mut().unwrap().local_review.choices = choices; + let row = text + .lines() + .position(|line| line.starts_with("## Review #")) + .unwrap_or(0); + self.open_review_buffer( + ReviewText::Reviews, + &text, + "Local reviews · Enter opens · :diff.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; + for buffer in std::iter::once(&mut self.active).chain(self.parked.values_mut()) { + if buffer + .review_buffer + .as_ref() + .is_some_and(|binding| binding.owner == self.tab_id) + { + buffer.saved_lines.clone_from(&buffer.lines); + } + } + } + + pub(super) fn autosave_local_review(&mut self) -> Result<(), String> { + self.sync_review_text(); + let Some(directory) = self.review_directory.as_deref() else { + return Ok(()); + }; + if !self.current_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.mark_review_saved(); + Ok(()) + } pub fn review_panel_title(&self) -> Option<&str> { self.active .review_buffer @@ -35,6 +139,9 @@ impl Editor { .origin .expect("review panel has a source pane"); self.finish_pane_interaction(); + if let Err(error) = self.autosave_local_review() { + self.set_message(format!("Review not saved: {error}")); + } self.select_pane(source); self.bind_diff_buffer(); true @@ -71,6 +178,12 @@ impl Editor { } pub(super) fn local_review_close_blocked(&mut self) -> bool { + if self.review_directory.is_some() + && let Err(error) = self.autosave_local_review() + { + self.set_message(format!("Review not saved: {error}")); + return true; + } if self.current_review_dirty() { self.set_message("Unsaved local review; use :diff.review.save before closing"); true @@ -97,8 +210,9 @@ impl Editor { .chain(self.parked.values()) .filter_map(|buffer| { let binding = buffer.review_buffer.as_ref()?; - (binding.owner == owner && binding.kind != ReviewText::Overview) - .then(|| (binding.kind, buffer.lines.join("\n"))) + (binding.owner == owner + && matches!(binding.kind, ReviewText::Comment(_) | ReviewText::Summary)) + .then(|| (binding.kind, buffer.lines.join("\n"))) }) .collect(); let Some(diff) = self.review_diff_mut() else { @@ -115,6 +229,7 @@ impl Editor { .map(|comment| &mut comment.body), ReviewText::Summary => Some(&mut diff.local_review.document.summary), ReviewText::Overview => None, + ReviewText::Reviews => None, }; if let Some(destination) = destination && *destination != text @@ -127,6 +242,23 @@ impl Editor { pub(super) fn save_local_review(&mut self, path: Option<&Path>) -> Result<(), String> { self.sync_review_text(); + 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.autosave_local_review()?; + self.set_message(if path.is_some() { + "Review exported as JSON; autosave remains in SQLite" + } else { + "Local review saved to SQLite" + }); + return Ok(()); + } let state = &mut self .review_diff_mut() .ok_or("No comparison is open")? @@ -159,6 +291,7 @@ impl Editor { if self.resolver().is_some() { return Err("Local reviews require an ordinary diff comparison".into()); } + self.initialize_local_review()?; let diff = self .workspace_diff() .filter(|diff| diff.phase == Phase::View) @@ -191,26 +324,49 @@ impl Editor { { self.review_diff_mut().unwrap().local_review.document = Review::new(root.clone()); } + if !matches!(action, Action::ReviewSave) { + self.autosave_local_review()?; + } if action == Action::ReviewOpen { - if args.is_empty() { - return Err("Usage: diff.review.open ".into()); - } if self.current_review_dirty() { return Err("Save the current local review before opening another".into()); } - let document = Review::load(Path::new(args), &root)?; + if args.is_empty() { + return self.open_review_selector(); + } + let document = if let (Some(directory), Ok(id)) = + (self.review_directory.as_deref(), args.parse::()) + { + crate::review::Store::open(directory)?.load(&root, id)? + } else { + Review::load(Path::new(args), &root)? + }; + let imported = document.storage_id.is_none(); + let dirty = imported && self.review_directory.is_some(); self.detach_review_buffers(); let state = &mut self.review_diff_mut().unwrap().local_review; state.document = document; - state.path = Some(PathBuf::from(args)); + state.path = imported.then(|| PathBuf::from(args)); state.selected = None; state.pending_navigation = None; - self.open_review_overview()?; - return Ok(()); + state.dirty = dirty; + self.autosave_local_review()?; + return self.open_review_overview(); } self.sync_review_text(); match action { Action::DiffComment => self.add_review_comment()?, + Action::ReviewNew => { + self.detach_review_buffers(); + let state = &mut self.review_diff_mut().unwrap().local_review; + state.document = Review::new(root); + state.selected = None; + state.pending_navigation = None; + state.path = None; + state.dirty = true; + self.autosave_local_review()?; + self.open_review_overview()?; + } Action::ReviewEdit => { let id = self.review_comment_id(args)?; self.show_review_comment(id)?; @@ -231,6 +387,7 @@ impl Editor { buffer.saved_lines.clone_from(&buffer.lines); } } + self.autosave_local_review()?; self.open_review_overview()?; } Action::ReviewNext | Action::ReviewPrevious => { @@ -281,6 +438,7 @@ impl Editor { state.document.verdict = verdict; state.dirty = true; } + self.autosave_local_review()?; self.open_review_overview()?; } Action::ReviewSave => { @@ -517,12 +675,24 @@ impl Editor { Some(Verdict::ChangesRequested) => "Changes requested", None => "Not set", }; - let mut text = format!( - "# Local review\n\nVerdict: {verdict}\nFile: {}\n\n## Summary\n{}\n\nEnter edits the comment under the caret.\n:diff.review.summary · :diff.review.verdict · :diff.review.save\n", + let storage = if self.review_directory.is_some() { + state.document.storage_id.map_or_else( + || "SQLite · uncommitted draft".into(), + |id| { + format!( + "SQLite · review #{id} · revision {}", + state.document.storage_revision + ) + }, + ) + } else { state .path .as_ref() - .map_or_else(|| "not saved".into(), |path| path.display().to_string()), + .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", state.document.summary ); for comment in &state.document.comments { @@ -604,7 +774,7 @@ impl Editor { self.select_pane(panel); if let Some(id) = existing { self.switch_buffer(id); - if kind == ReviewText::Overview { + if matches!(kind, ReviewText::Overview | ReviewText::Reviews) { self.active.lines = split_lines(text); self.active.saved_lines.clone_from(&self.active.lines); self.active.text_revision = self.active.text_revision.wrapping_add(1); @@ -618,10 +788,14 @@ impl Editor { kind, title, }); - buffer.resolver_readonly = kind == ReviewText::Overview; + buffer.resolver_readonly = matches!(kind, ReviewText::Overview | ReviewText::Reviews); self.add_buffer(buffer); } - self.set_message("Local review · :w saves JSON; first choose :diff.review.save "); + self.set_message(if self.review_directory.is_some() { + "Local review · dismiss to commit · :w saves · :diff.review.save path exports JSON" + } else { + "Local review · :w saves JSON; first choose :diff.review.save " + }); Ok(()) } @@ -638,13 +812,23 @@ impl Editor { if *input == Input::Enter && self.active.view.mode == Mode::Normal && self.palette.is_none() - && self - .active - .review_buffer - .as_ref() - .is_some_and(|binding| binding.kind == ReviewText::Overview) + && self.active.review_buffer.as_ref().is_some_and(|binding| { + matches!(binding.kind, ReviewText::Overview | ReviewText::Reviews) + }) { - if let Err(error) = self.local_review_command(Action::ReviewEdit, "") { + let choice = if self.active.review_buffer.as_ref().unwrap().kind == ReviewText::Reviews + { + self.active.lines[..=self.cursor().row] + .iter() + .rev() + .find_map(|line| line.strip_prefix("## Review #")?.split_whitespace().next()) + .map(str::to_owned) + .ok_or_else(|| "Place the caret on a review".to_string()) + .and_then(|id| self.local_review_command(Action::ReviewOpen, &id)) + } else { + self.local_review_command(Action::ReviewEdit, "") + }; + if let Err(error) = choice { self.set_message(error); } true diff --git a/src/main.rs b/src/main.rs index adf926d..90aab5f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -87,8 +87,9 @@ Local reviews: :diff.comment anchors exact Visual text, Visual Line rows, or the 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.save PATH / :diff.review.open PATH; :w in floating review editors saves local JSON - Normal Esc or :pane.close dismisses the floating editor and retains drafts + Local SQLite autosave on dismissal/switch; verdicts/deletions commit immediately; :w commits explicitly + :diff.review.save PATH exports JSON; :diff.review.open [ID|PATH] selects/imports; :diff.review.new + Normal Esc or :pane.close commits and dismisses the floating editor; failures retain drafts Snapshot anchors stay immutable; stale comments lose gutter markers; no publishing Pull requests: :pull_requests [REMOTE]; GitHub/GitLab/Gitea Open/Closed/All tabs, type to search Enter opens the immutable PR head against its merge base; API auth uses provider tokens or gh/glab diff --git a/src/review.rs b/src/review.rs index 92f5227..b4df995 100644 --- a/src/review.rs +++ b/src/review.rs @@ -114,6 +114,8 @@ impl Default for Review { #[derive(Clone, Debug, Default)] pub struct ReviewState { pub document: Review, + pub initialized: bool, + pub choices: Vec<(i64, String)>, pub path: Option, pub selected: Option, pub pending_navigation: Option, diff --git a/src/ruby.rs b/src/ruby.rs index 85d84b7..701245c 100644 --- a/src/ruby.rs +++ b/src/ruby.rs @@ -33,6 +33,7 @@ const INVOKE_TIMEOUT: Duration = Duration::from_millis(500); pub struct Config { pub plugins: Vec, pub lsp: Vec, + pub review_directory: Option, pub bindings: Vec, pub leader: String, pub file_explorer_side: String, @@ -85,6 +86,7 @@ impl Default for Config { Self { plugins: Vec::new(), lsp: Vec::new(), + review_directory: None, bindings: Vec::new(), leader: "space".into(), file_explorer_side: "right".into(), @@ -113,7 +115,7 @@ impl Default for Config { } } -/// Relative plugin paths are resolved against the configuration file's directory. +/// Relative plugin and review paths are resolved against the configuration file's directory. pub fn load_config(path: &Path) -> Result { let deadline = Instant::now() + STARTUP_TIMEOUT; let path = absolute_file(path)?; @@ -124,6 +126,7 @@ pub fn load_config(path: &Path) -> Result { let ConfigFrame { plugins, lsp, + review_directory, bindings, leader, file_explorer_side, @@ -152,6 +155,7 @@ pub fn load_config(path: &Path) -> Result { } = *response; let mut config = Config::default(); let directory = path.parent().expect("absolute file has a parent"); + config.review_directory = review_directory.map(|path| directory.join(path)); for plugin in plugins { let plugin_path = plugin.path.to_string_lossy(); if plugin_path.is_empty() || plugin_path.contains('\0') { @@ -490,6 +494,21 @@ fn scroll_distance<'de, D: serde::Deserializer<'de>>( }) } +fn review_directory<'de, D: serde::Deserializer<'de>>( + deserializer: D, +) -> Result, D::Error> { + let directory = Option::::deserialize(deserializer)?; + if directory + .as_ref() + .is_some_and(|path| path.is_empty() || path.contains('\0')) + { + return Err(serde::de::Error::custom( + "review_directory must not be empty or contain NUL", + )); + } + Ok(directory.map(PathBuf::from)) +} + // Startup-only configuration does not inflate every queued plugin response. #[derive(Deserialize)] #[serde(deny_unknown_fields)] @@ -499,6 +518,8 @@ struct ConfigFrame { // Keep this wire field required; resolve its configuration after decoding. #[serde(deserialize_with = "serde_json::Value::deserialize")] lsp: serde_json::Value, + #[serde(deserialize_with = "review_directory")] + review_directory: Option, bindings: Vec, leader: String, file_explorer_side: String, @@ -1003,6 +1024,87 @@ mod tests { } } + #[test] + fn review_directory_resolves_config_paths_and_keeps_session_startup_lazy() { + let fixture = Fixture::new(); + let absolute = fixture.0.join("absolute-reviews"); + for (value, expected) in [ + ( + r#""relative-reviews""#.to_owned(), + Some(fixture.0.join("relative-reviews")), + ), + (serde_json::to_string(&absolute).unwrap(), Some(absolute)), + ("nil".to_owned(), None), + ] { + let path = fixture.file( + "config.rb", + &format!("Vamp.configure {{ |config| config.review_directory = {value} }}"), + ); + assert_eq!(load_config(&path).unwrap().review_directory, expected); + let session = EditorSession::open(None, vec![], Some(&path)).unwrap(); + assert!(session.editor().startup_diagnostic().is_none()); + if let Some(directory) = expected { + assert!( + !directory.exists(), + "startup created {}", + directory.display() + ); + } + } + let path = fixture.file("config.rb", "Vamp.configure { |_| }"); + assert_eq!(load_config(&path).unwrap().review_directory, None); + for value in [ + "7", + "false", + "[]", + "{}", + ":reviews", + r#""""#, + r#""bad\0path""#, + ] { + let path = fixture.file( + "config.rb", + &format!("Vamp.configure {{ |config| config.review_directory = {value} }}"), + ); + let error = load_config(&path).unwrap_err(); + assert!(error.contains("review_directory"), "{value}: {error}"); + } + } + + #[test] + fn review_directory_wire_field_is_required_nullable_and_validated() { + let mut frame = serde_json::json!({ + "version": 1, "type": "config", "plugins": [], "lsp": {}, "bindings": [], + "review_directory": null, "leader": "space", + "file_explorer_side": "right", "file_picker_view": "tree", "file_explorer_view": "tree", + "clipboard_mode": "system", "git_blame": true, "git_gutter": "bar", + "top_bar": {}, "bottom_bar": {}, "popup": {"command": {}, "search": {}}, "labels": {}, + "scroll_distance": 0, "scroll_step_lines": null, "highlight_trailing_whitespace": false, + "whitespace_characters": {}, "whitespace_scope": "all", + "tab_spaces": null, "language_tab_spaces": {}, + "diff_context_lines": 10, "diff_expand_lines": 15, + "diff_view_mode": "split", "diff_split_orientation": "vertical", + "diff_follow_working_copy": true + }); + for value in [serde_json::Value::Null, serde_json::json!("reviews")] { + frame["review_directory"] = value; + assert!(serde_json::from_value::(frame.clone()).is_ok()); + } + for value in [ + serde_json::json!(""), + serde_json::json!("bad\0path"), + serde_json::json!(7), + serde_json::json!(false), + serde_json::json!([]), + serde_json::json!({}), + ] { + frame["review_directory"] = value; + assert!(serde_json::from_value::(frame.clone()).is_err()); + } + frame.as_object_mut().unwrap().remove("review_directory"); + assert!(serde_json::from_value::(frame).is_err()); + } + #[test] fn tab_spaces_follow_active_language_and_preserve_native_edit_semantics() { let fixture = Fixture::new(); @@ -1410,9 +1512,9 @@ end assert!(load_config(&path).unwrap_err().contains(error), "{value}"); } for frame in [ - r#"{"type":"config","version":1,"plugins":[],"lsp":{},"bindings":[],"file_explorer_side":"right","file_picker_view":"list","file_explorer_view":"tree","clipboard_mode":"system","git_blame":true,"git_gutter":"bar","scroll_distance":0,"highlight_trailing_whitespace":false,"whitespace_characters":{},"whitespace_scope":"all","tab_spaces":null,"language_tab_spaces":{},"top_bar":{},"bottom_bar":{},"popup":{"command":{},"search":{}},"labels":{},"diff_context_lines":10,"diff_expand_lines":15,"diff_view_mode":"split","diff_split_orientation":"vertical","diff_follow_working_copy":true}"#, - r#"{"type":"config","version":1,"plugins":[],"lsp":{},"bindings":[],"leader":null,"file_explorer_side":"right","file_picker_view":"list","file_explorer_view":"tree","clipboard_mode":"system","git_blame":true,"git_gutter":"bar","scroll_distance":0,"highlight_trailing_whitespace":false,"whitespace_characters":{},"whitespace_scope":"all","tab_spaces":null,"language_tab_spaces":{},"top_bar":{},"bottom_bar":{},"popup":{"command":{},"search":{}},"labels":{},"diff_context_lines":10,"diff_expand_lines":15,"diff_view_mode":"split","diff_split_orientation":"vertical","diff_follow_working_copy":true}"#, - r#"{"type":"config","version":1,"plugins":[],"lsp":{},"bindings":[],"leader":7,"file_explorer_side":"right","file_picker_view":"list","file_explorer_view":"tree","clipboard_mode":"system","git_blame":true,"git_gutter":"bar","scroll_distance":0,"highlight_trailing_whitespace":false,"whitespace_characters":{},"whitespace_scope":"all","tab_spaces":null,"language_tab_spaces":{},"top_bar":{},"bottom_bar":{},"popup":{"command":{},"search":{}},"labels":{},"diff_context_lines":10,"diff_expand_lines":15,"diff_view_mode":"split","diff_split_orientation":"vertical","diff_follow_working_copy":true}"#, + r#"{"type":"config","version":1,"plugins":[],"lsp":{},"review_directory":null,"bindings":[],"file_explorer_side":"right","file_picker_view":"list","file_explorer_view":"tree","clipboard_mode":"system","git_blame":true,"git_gutter":"bar","scroll_distance":0,"highlight_trailing_whitespace":false,"whitespace_characters":{},"whitespace_scope":"all","tab_spaces":null,"language_tab_spaces":{},"top_bar":{},"bottom_bar":{},"popup":{"command":{},"search":{}},"labels":{},"diff_context_lines":10,"diff_expand_lines":15,"diff_view_mode":"split","diff_split_orientation":"vertical","diff_follow_working_copy":true}"#, + r#"{"type":"config","version":1,"plugins":[],"lsp":{},"review_directory":null,"bindings":[],"leader":null,"file_explorer_side":"right","file_picker_view":"list","file_explorer_view":"tree","clipboard_mode":"system","git_blame":true,"git_gutter":"bar","scroll_distance":0,"highlight_trailing_whitespace":false,"whitespace_characters":{},"whitespace_scope":"all","tab_spaces":null,"language_tab_spaces":{},"top_bar":{},"bottom_bar":{},"popup":{"command":{},"search":{}},"labels":{},"diff_context_lines":10,"diff_expand_lines":15,"diff_view_mode":"split","diff_split_orientation":"vertical","diff_follow_working_copy":true}"#, + r#"{"type":"config","version":1,"plugins":[],"lsp":{},"review_directory":null,"bindings":[],"leader":7,"file_explorer_side":"right","file_picker_view":"list","file_explorer_view":"tree","clipboard_mode":"system","git_blame":true,"git_gutter":"bar","scroll_distance":0,"highlight_trailing_whitespace":false,"whitespace_characters":{},"whitespace_scope":"all","tab_spaces":null,"language_tab_spaces":{},"top_bar":{},"bottom_bar":{},"popup":{"command":{},"search":{}},"labels":{},"diff_context_lines":10,"diff_expand_lines":15,"diff_view_mode":"split","diff_split_orientation":"vertical","diff_follow_working_copy":true}"#, ] { assert!(serde_json::from_str::(frame).is_err(), "{frame}"); } @@ -1485,6 +1587,7 @@ end } let mut frame = serde_json::json!({ "type": "config", "version": 1, "plugins": [], "lsp": {}, "bindings": [], + "review_directory": null, "leader": "space", "file_explorer_side": "right", "file_picker_view": "tree", "file_explorer_view": "tree", "clipboard_mode": "system", "git_blame": true, "git_gutter": "bar", @@ -1627,6 +1730,7 @@ end"#, } let mut frame = serde_json::json!({ "type": "config", "version": 1, "plugins": [], "lsp": {}, "bindings": [], + "review_directory": null, "leader": "space", "file_explorer_side": "right", "file_picker_view": "tree", "file_explorer_view": "tree", "clipboard_mode": "system", "git_blame": true, "git_gutter": "bar", "scroll_distance": 0, "highlight_trailing_whitespace": false, @@ -1886,6 +1990,7 @@ end } let frame = serde_json::json!({ "version": 1, "type": "config", "plugins": [], "lsp": {}, + "review_directory": null, "bindings": [], "leader": "space", "file_explorer_side": "right", "file_picker_view": "tree", "file_explorer_view": "tree", "top_bar": {}, "bottom_bar": {}, "labels": {}, @@ -1997,6 +2102,7 @@ end" let fixture = Fixture::new(); let frame = serde_json::json!({ "version": 1, "type": "config", "plugins": [], "lsp": {}, + "review_directory": null, "bindings": [], "leader": "space", "file_explorer_side": "right", "file_picker_view": "tree", "file_explorer_view": "tree", "clipboard_mode": "system", "git_blame": true, "git_gutter": "bar", @@ -2427,6 +2533,7 @@ end"#, fn lsp_wire_configuration_is_resolved_after_decoding() { let mut frame = serde_json::json!({ "version": 1, "type": "config", "plugins": [], "lsp": {}, + "review_directory": null, "bindings": [], "leader": "space", "file_explorer_side": "right", "file_picker_view": "tree", "file_explorer_view": "tree", "git_blame": true, diff --git a/src/ruby_config.rb b/src/ruby_config.rb index 03b5dff..892a694 100644 --- a/src/ruby_config.rb +++ b/src/ruby_config.rb @@ -133,6 +133,7 @@ module Vamp def initialize @plugins = [] @lsp = {} + @review_directory = nil @bindings = [] @leader = "space" @file_explorer_side = "right" @@ -170,6 +171,15 @@ module Vamp nil end + # @param path [String, nil] relative to the configuration file; nil uses the XDG data directory + # @return [String, nil] + def review_directory=(path) + path = nonempty_string!(path, "review_directory") unless path.nil? + raise ArgumentError, "review_directory must not contain NUL" if path&.include?("\0") + + @review_directory = path + end + # @param key [String] # @return [String] def leader=(key) @@ -345,6 +355,7 @@ module Vamp # @return [Hash{String => Object}] validated startup configuration frame def response { "version" => 1, "type" => "config", "plugins" => @plugins, "lsp" => lsp_options, "bindings" => @bindings, + "review_directory" => @review_directory, "leader" => @leader, "file_explorer_side" => @file_explorer_side, "file_picker_view" => @file_picker_view, "file_explorer_view" => @file_explorer_view, "clipboard_mode" => @clipboard_mode, "git_blame" => @git_blame, diff --git a/src/session.rs b/src/session.rs index 4abb80f..03f6b81 100644 --- a/src/session.rs +++ b/src/session.rs @@ -482,6 +482,10 @@ impl EditorSession { }), None => Config::default(), }; + editor.set_review_directory(match config.review_directory { + Some(directory) => directory, + None => crate::review::default_directory().map_err(io::Error::other)?, + }); if let Err(error) = editor.set_leader(&config.leader) { notices.push(format!("leader {:?}: {error}", config.leader)); } @@ -2664,6 +2668,7 @@ mod tests { Some(&config), ) .unwrap(); + session.editor.set_review_directory(self.0.join("reviews")); // Legacy editing scenarios exercise the internal register, never the user's desktop. session.editor.set_clipboard_mode("separate").unwrap(); session diff --git a/tests/diff.rs b/tests/diff.rs index bc89b8e..702b731 100644 --- a/tests/diff.rs +++ b/tests/diff.rs @@ -109,7 +109,7 @@ fn typed_diff_edits_unsaved_working_after_and_preserves_undo_on_explicit_close() let config = fixture.0.join("config.rb"); fs::write( &config, - "Vamp.configure do |config|\n config.clipboard_mode = 'separate'\n config.bind 'leader d', config.core.diff\nend\n", + "Vamp.configure do |config|\n config.review_directory = 'reviews'\n config.clipboard_mode = 'separate'\n config.bind 'leader d', config.core.diff\nend\n", ) .unwrap(); let mut session = @@ -228,7 +228,7 @@ fn tabs_keep_repository_workers_and_hidden_comparisons_independent() { let config = first.0.join("config.rb"); fs::write( &config, - "Vamp.configure do |config|\n config.bind 'ctrl+option+n', config.core.tab.new\n config.bind 'ctrl+option+w', config.core.tab.close\nend\n", + "Vamp.configure do |config|\n config.review_directory = 'reviews'\n config.bind 'ctrl+option+n', config.core.tab.new\n config.bind 'ctrl+option+w', config.core.tab.close\nend\n", ) .unwrap(); let mut session = @@ -330,7 +330,7 @@ fn editable_diff_navigation_skips_omitted_lines_without_expanding_excerpts() { let text = format!("{}\n", lines.join("\n")); fs::write(&source, &text).unwrap(); let config = fixture.0.join("config.rb"); - fs::write(&config, "Vamp.configure do |config|\n config.clipboard_mode = 'separate'\n config.diff_context_lines = 1\n config.diff_expand_lines = 2\n config.bind 'ctrl+f', config.core.down\n config.bind 'ctrl+b', config.core.up\nend\n").unwrap(); + fs::write(&config, "Vamp.configure do |config|\n config.review_directory = 'reviews'\n config.clipboard_mode = 'separate'\n config.diff_context_lines = 1\n config.diff_expand_lines = 2\n config.bind 'ctrl+f', config.core.down\n config.bind 'ctrl+b', config.core.up\nend\n").unwrap(); let mut session = EditorSession::open(Some(source.clone()), vec!["text".into()], Some(&config)).unwrap(); session.execute("diff"); @@ -467,7 +467,14 @@ fn readonly_jump_history_restores_the_comparison_file_and_source_cursor() { let text = fs::read_to_string(&path).unwrap(); fs::write(path, text.replace("_before", "_after")).unwrap(); } - let mut session = EditorSession::open(Some(root.join("first.rs")), vec![], None).unwrap(); + let config = fixture.0.join("review-config.rb"); + fs::write( + &config, + "Vamp.configure { |config| config.review_directory = 'reviews' }", + ) + .unwrap(); + let mut session = + EditorSession::open(Some(root.join("first.rs")), vec![], Some(&config)).unwrap(); session.execute("diff"); wait_until(&mut session, |session| { !session.editor().diff().unwrap().loading @@ -530,7 +537,7 @@ fn branch_changes_default_and_mode_switches_compare_the_selected_divergent_tips( fixture.git(&["commit", "-qm", "feature only"]); let after = fixture.git(&["rev-parse", "HEAD"]); let config = fixture.0.join("config.rb"); - fs::write(&config, "Vamp.configure do |config|\n config.clipboard_mode = 'separate'\n config.bind 'ctrl+b', config.core.diff.mode.toggle\nend\n").unwrap(); + fs::write(&config, "Vamp.configure do |config|\n config.review_directory = 'reviews'\n config.clipboard_mode = 'separate'\n config.bind 'ctrl+b', config.core.diff.mode.toggle\nend\n").unwrap(); let mut session = EditorSession::open(Some(root.join("common")), vec![], Some(&config)).unwrap(); session.execute("diff"); @@ -643,7 +650,13 @@ fn jj_bookmark_picker_keeps_symbolic_targets_searchable_by_resolved_commit_id() fixture.jj(&["bookmark", "create", "topic"]); let after = fixture.jj(&["log", "--no-graph", "-r", "@", "-T", "commit_id"]); fixture.jj(&["new"]); - let mut session = EditorSession::open(Some(source), vec![], None).unwrap(); + let config = fixture.0.join("review-config.rb"); + fs::write( + &config, + "Vamp.configure { |config| config.review_directory = 'reviews' }", + ) + .unwrap(); + let mut session = EditorSession::open(Some(source), vec![], Some(&config)).unwrap(); session.execute("diff"); wait_until(&mut session, |session| { !session.editor().diff().unwrap().loading diff --git a/tests/lsp.rs b/tests/lsp.rs index c62c518..c441dc2 100644 --- a/tests/lsp.rs +++ b/tests/lsp.rs @@ -374,7 +374,7 @@ impl Fixture { "settings": {"rust-analyzer": {"cargo": {"allFeatures": true}}}, }); let config = format!( - "Vamp.configure do |config|\n config.clipboard_mode = 'separate'\n config.diff_context_lines = 1\n config.diff_expand_lines = 2\n config.lsp['fixture-language'] = JSON.parse({})\n config.bind 'leader d', config.core.lsp.definition\n config.bind 'leader D', config.core.lsp.diagnostics\n config.bind 'K', config.core.lsp.hover\nend\n", + "Vamp.configure do |config|\n config.review_directory = 'reviews'\n config.clipboard_mode = 'separate'\n config.diff_context_lines = 1\n config.diff_expand_lines = 2\n config.lsp['fixture-language'] = JSON.parse({})\n config.bind 'leader d', config.core.lsp.definition\n config.bind 'leader D', config.core.lsp.diagnostics\n config.bind 'K', config.core.lsp.hover\nend\n", serde_json::to_string(&options.to_string()).unwrap(), ); let config_path = self.root.join("config.rb"); @@ -457,7 +457,7 @@ impl Fixture { json!({"extensions": ["rs"], "command": command(2, "controlled")}); } let config = format!( - "Vamp.configure do |config|\n config.clipboard_mode = 'separate'\n JSON.parse({}).each {{ |language, options| config.lsp[language] = options }}\n config.bind 'K', config.core.lsp.hover\nend\n", + "Vamp.configure do |config|\n config.review_directory = 'reviews'\n config.clipboard_mode = 'separate'\n JSON.parse({}).each {{ |language, options| config.lsp[language] = options }}\n config.bind 'K', config.core.lsp.hover\nend\n", serde_json::to_string(&languages.to_string()).unwrap(), ); let config_path = self.root.join("config.rb"); diff --git a/tests/navigation.rs b/tests/navigation.rs index 3c3bbf4..9511ba1 100644 --- a/tests/navigation.rs +++ b/tests/navigation.rs @@ -46,7 +46,7 @@ impl Fixture { let config = self.file( "config.rb", &format!( - "Vamp.configure do |config|\n config.clipboard_mode = 'separate'\n{bindings}\nend\n" + "Vamp.configure do |config|\n config.review_directory = 'reviews'\n config.clipboard_mode = 'separate'\n{bindings}\nend\n" ), ); let session = @@ -488,7 +488,7 @@ fn readonly_before_and_historical_after_seeks_preserve_snapshots_and_folded_exce fs::write(&source, &working_text).unwrap(); let config = fixture.file( "config.rb", - "Vamp.configure do |config|\n config.clipboard_mode = 'separate'\n config.diff_context_lines = 1\n config.diff_expand_lines = 2\nend\n", + "Vamp.configure do |config|\n config.review_directory = 'reviews'\n config.clipboard_mode = 'separate'\n config.diff_context_lines = 1\n config.diff_expand_lines = 2\nend\n", ); let mut session = EditorSession::open(Some(source.clone()), vec!["text".into()], Some(&config)).unwrap();