feat: expose read-only local review CLI
This commit is contained in:
@@ -13,6 +13,21 @@ cargo run -- --config examples/config.rb notes.txt
|
|||||||
|
|
||||||
Without a file, vamp opens an unnamed buffer. A nonexistent file opens an empty buffer. `cargo run -- --help` lists the command-line options.
|
Without a file, vamp opens an unnamed buffer. A nonexistent file opens an empty buffer. `cargo run -- --help` lists the command-line options.
|
||||||
|
|
||||||
|
### Read local reviews without opening the editor
|
||||||
|
|
||||||
|
```sh
|
||||||
|
vamp --review list --repo .
|
||||||
|
vamp --review list --repo . --json
|
||||||
|
vamp --review show 1 --repo . --json
|
||||||
|
vamp --review show 1 --repo . --all
|
||||||
|
```
|
||||||
|
|
||||||
|
The review CLI discovers the Git/Jujutsu checkout from `--repo` (default: current directory). It uses the same default configuration, `--config PATH`, `--no-config`, and `review_directory` as the editor, but starts no editor workers or terminal UI. Missing storage lists no reviews without creating a database; incompatible storage is rejected and preserved.
|
||||||
|
|
||||||
|
`show` includes unresolved comments by default; `--all` includes resolved comments too. Human-readable output is the default. JSON includes the canonical repository root, storage review ID/revision, summary/verdict, total/resolved/unresolved counts, and comments with exact zero-based, end-exclusive grapheme anchors and selected source excerpts—not whole source files. `list --json` returns the repository and review metadata/counts without comments. Explicit editor JSON exports still contain complete anchor source text.
|
||||||
|
|
||||||
|
`vamp review` still opens a file named `review`; `vamp -- --review` opens a file named `--review`.
|
||||||
|
|
||||||
## Getting started
|
## Getting started
|
||||||
|
|
||||||
Vamp starts in Normal mode. Press `i`, type text, then Esc to return to Normal. Type `:save` (alias `:w`) and Enter to save, or `:save path` to name an unnamed buffer. Type `:quit` (alias `:q`) and Enter to quit; it refuses unsaved changes in any open buffer. `:force_quit` (alias `:q!`) explicitly discards changes and quits.
|
Vamp starts in Normal mode. Press `i`, type text, then Esc to return to Normal. Type `:save` (alias `:w`) and Enter to save, or `:save path` to name an unnamed buffer. Type `:quit` (alias `:q`) and Enter to quit; it refuses unsaved changes in any open buffer. `:force_quit` (alias `:q!`) explicitly discards changes and quits.
|
||||||
|
|||||||
@@ -22,14 +22,18 @@
|
|||||||
- version the database schema; use review-version checks to reject conflicting edits from multiple editor instances.
|
- 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.
|
- 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.
|
- persist the precise start/end grapheme anchors without reducing them to line ranges.
|
||||||
- [ ] diff view: consider an MCP or a something for a local review loop when using coding agents.
|
- [x] diff view: local coding-agent review loop through a read-only `vamp --review` CLI.
|
||||||
|
- list repository reviews and show committed feedback; return exact anchors and grapheme excerpts without repeating full source files.
|
||||||
|
- show unresolved comments by default; `--all` includes resolved comments.
|
||||||
|
- manually resolve/unresolve comments in Vamp, independently of staleness; persist status in SQLite and version 3 JSON.
|
||||||
|
- preserve filename arguments, configured storage, and incompatible data; no MCP, agent replies, or automatic dispatch.
|
||||||
|
- [ ] 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: GitHub code review integration after the local review UX is usable.
|
- [ ] 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.
|
- 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.
|
- 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.
|
- 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.
|
- 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).
|
- [ ] 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).
|
- [ ] 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.
|
- verify that notification-triggered queries return fresh colors in Ghostty through Herdr before removal; retain startup, focus-regain, and notification-driven palette refresh.
|
||||||
|
|||||||
@@ -63,6 +63,9 @@ Local review checks cover native comment/summary composition and undo, Visual ra
|
|||||||
|
|
||||||
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.
|
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.
|
||||||
|
|
||||||
|
Manual-resolution checks cover immediate persistence, reopen/export/import, independent stale status, conflict-retained drafts, and incompatible version preservation. CLI checks cover filename-safe dispatch, Unicode excerpts, unresolved filtering/counts, absent storage, read-only access and permissions, symlink rejection, and incompatible schema preservation.
|
||||||
|
Live PTY/CLI smoke composes and autosaves a comment, resolves it in the editor, observes an empty default CLI comment list with explicit counts, retrieves the resolved comment and exact excerpt with `--all`, and unresolves it. Each transition advances the same stored review revision; listing absent storage creates no data directory.
|
||||||
|
|
||||||
|
|
||||||
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.
|
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.
|
||||||
|
|
||||||
|
|||||||
+84
-9
@@ -2,12 +2,19 @@ use std::{env, io, path::PathBuf};
|
|||||||
|
|
||||||
use vamp::{session::EditorSession, tui};
|
use vamp::{session::EditorSession, tui};
|
||||||
|
|
||||||
|
mod review_cli;
|
||||||
|
|
||||||
// Only the first argument can be this process-to-process terminal handoff marker.
|
// Only the first argument can be this process-to-process terminal handoff marker.
|
||||||
const INHERITED_SCREEN: &str = "--vamp-inherit-alternate-screen";
|
const INHERITED_SCREEN: &str = "--vamp-inherit-alternate-screen";
|
||||||
|
|
||||||
const HELP: &str = "vamp — modal editor with isolated Ruby plugins
|
const HELP: &str = "vamp — modal editor with isolated Ruby plugins
|
||||||
|
|
||||||
Usage: vamp [--config PATH | --no-config] [FILE]
|
Usage: vamp [--config PATH | --no-config] [FILE]
|
||||||
|
vamp [--config PATH | --no-config] --review list [--repo PATH] [--json]
|
||||||
|
vamp [--config PATH | --no-config] --review show ID [--repo PATH] [--json] [--all]
|
||||||
|
|
||||||
|
Review CLI: read-only local storage; show defaults to unresolved comments; --all includes resolved.
|
||||||
|
Use vamp -- --review to edit a file named --review.
|
||||||
|
|
||||||
Default keys (configurable with config.bind):
|
Default keys (configurable with config.bind):
|
||||||
All modes: Ctrl+p command palette; type to filter, Ctrl+j/k or arrows select, Enter accept, Esc cancel
|
All modes: Ctrl+p command palette; type to filter, Ctrl+j/k or arrows select, Enter accept, Esc cancel
|
||||||
@@ -114,20 +121,33 @@ Hover binding: config.bind \"K\", config.core.lsp.hover (no default key)";
|
|||||||
struct Options {
|
struct Options {
|
||||||
file: Option<PathBuf>,
|
file: Option<PathBuf>,
|
||||||
config: Option<PathBuf>,
|
config: Option<PathBuf>,
|
||||||
|
review: Option<review_cli::Request>,
|
||||||
}
|
}
|
||||||
|
|
||||||
fn options(inherited_screen: bool) -> Result<Option<Options>, String> {
|
fn options(inherited_screen: bool) -> Result<Option<Options>, String> {
|
||||||
let mut args = env::args_os().skip(1 + usize::from(inherited_screen));
|
parse_options(env::args_os().skip(1 + usize::from(inherited_screen)))
|
||||||
|
}
|
||||||
|
|
||||||
|
fn parse_options(
|
||||||
|
args: impl IntoIterator<Item = std::ffi::OsString>,
|
||||||
|
) -> Result<Option<Options>, String> {
|
||||||
|
let mut args = args.into_iter();
|
||||||
let mut file = None;
|
let mut file = None;
|
||||||
let mut config = None;
|
let mut config = None;
|
||||||
let mut config_selected = false;
|
let mut config_selected = false;
|
||||||
let mut positional = false;
|
let mut positional = false;
|
||||||
|
let mut review = None;
|
||||||
while let Some(arg) = args.next() {
|
while let Some(arg) = args.next() {
|
||||||
if !positional && (arg == "--help" || arg == "-h") {
|
if !positional && (arg == "--help" || arg == "-h") {
|
||||||
println!("{HELP}");
|
println!("{HELP}");
|
||||||
return Ok(None);
|
return Ok(None);
|
||||||
} else if !positional && arg == "--" {
|
} else if !positional && arg == "--" {
|
||||||
|
if review.is_some() {
|
||||||
|
return Err("unexpected review argument: --".into());
|
||||||
|
}
|
||||||
positional = true;
|
positional = true;
|
||||||
|
} else if !positional && arg == "--review" && file.is_none() && review.is_none() {
|
||||||
|
review = Some(review_cli::Request::parse(&mut args)?);
|
||||||
} else if !positional && arg == "--config" {
|
} else if !positional && arg == "--config" {
|
||||||
if config_selected {
|
if config_selected {
|
||||||
return Err("use only one of --config and --no-config".into());
|
return Err("use only one of --config and --no-config".into());
|
||||||
@@ -141,6 +161,8 @@ fn options(inherited_screen: bool) -> Result<Option<Options>, String> {
|
|||||||
return Err("use only one of --config and --no-config".into());
|
return Err("use only one of --config and --no-config".into());
|
||||||
}
|
}
|
||||||
config_selected = true;
|
config_selected = true;
|
||||||
|
} else if !positional && review.is_some() {
|
||||||
|
review.as_mut().unwrap().option(&arg, &mut args)?;
|
||||||
} else if !positional && arg.to_string_lossy().starts_with('-') {
|
} else if !positional && arg.to_string_lossy().starts_with('-') {
|
||||||
return Err(format!("unknown option: {}", arg.to_string_lossy()));
|
return Err(format!("unknown option: {}", arg.to_string_lossy()));
|
||||||
} else if file.replace(PathBuf::from(arg)).is_some() {
|
} else if file.replace(PathBuf::from(arg)).is_some() {
|
||||||
@@ -155,24 +177,27 @@ fn options(inherited_screen: bool) -> Result<Option<Options>, String> {
|
|||||||
.map(|home| home.join("vamp/config.rb"))
|
.map(|home| home.join("vamp/config.rb"))
|
||||||
.filter(|path| path.exists());
|
.filter(|path| path.exists());
|
||||||
}
|
}
|
||||||
Ok(Some(Options { file, config }))
|
Ok(Some(Options {
|
||||||
|
file,
|
||||||
|
config,
|
||||||
|
review,
|
||||||
|
}))
|
||||||
}
|
}
|
||||||
|
|
||||||
fn run() -> Result<(), String> {
|
fn run() -> Result<(), String> {
|
||||||
let inherited_screen = env::args_os()
|
let inherited_screen = env::args_os()
|
||||||
.nth(1)
|
.nth(1)
|
||||||
.is_some_and(|arg| arg == INHERITED_SCREEN);
|
.is_some_and(|arg| arg == INHERITED_SCREEN);
|
||||||
if inherited_screen {
|
let options = options(inherited_screen)?;
|
||||||
return tui::TerminalSession::run(true, || {
|
if let Some(options) = &options {
|
||||||
run_editor(options(true).map_err(io::Error::other)?, true)
|
if let Some(request) = &options.review {
|
||||||
})
|
return review_cli::run(request, options.config.as_deref());
|
||||||
.map_err(|error| error.to_string());
|
}
|
||||||
}
|
}
|
||||||
let options = options(false)?;
|
|
||||||
if options.is_none() {
|
if options.is_none() {
|
||||||
return Ok(());
|
return Ok(());
|
||||||
}
|
}
|
||||||
tui::TerminalSession::run(false, || run_editor(options, false))
|
tui::TerminalSession::run(inherited_screen, || run_editor(options, inherited_screen))
|
||||||
.map_err(|error| error.to_string())
|
.map_err(|error| error.to_string())
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -226,3 +251,53 @@ fn main() {
|
|||||||
std::process::exit(1);
|
std::process::exit(1);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
|
||||||
|
fn parse(args: &[&str]) -> Result<Option<Options>, String> {
|
||||||
|
parse_options(args.iter().map(std::ffi::OsString::from))
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn review_commands_preserve_filenames_and_validate_arguments() {
|
||||||
|
assert_eq!(
|
||||||
|
parse(&["review"]).unwrap().unwrap().file,
|
||||||
|
Some("review".into())
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
parse(&["--", "--review"]).unwrap().unwrap().file,
|
||||||
|
Some("--review".into())
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
parse(&["--no-config", "--review", "list", "--repo", ".", "--json"])
|
||||||
|
.unwrap()
|
||||||
|
.unwrap()
|
||||||
|
.review
|
||||||
|
.is_some()
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
parse(&["--review", "show", "1", "--all", "--no-config"])
|
||||||
|
.unwrap()
|
||||||
|
.unwrap()
|
||||||
|
.review
|
||||||
|
.is_some()
|
||||||
|
);
|
||||||
|
for args in [
|
||||||
|
vec!["--review"],
|
||||||
|
vec!["--review", "show"],
|
||||||
|
vec!["--review", "show", "0"],
|
||||||
|
vec!["--review", "show", "-1"],
|
||||||
|
vec!["--review", "show", "abc"],
|
||||||
|
vec!["--review", "list", "--all"],
|
||||||
|
vec!["--review", "list", "--repo"],
|
||||||
|
vec!["--review", "list", "--repo", ".", "--repo", "."],
|
||||||
|
vec!["--review", "list", "extra"],
|
||||||
|
vec!["--review", "list", "--", "extra"],
|
||||||
|
vec!["--review", "list", "--config", "x", "--no-config"],
|
||||||
|
] {
|
||||||
|
assert!(parse(&args).is_err(), "{args:?}");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
+24
-11
@@ -1091,22 +1091,35 @@ pub struct Repository {
|
|||||||
comparison: Option<PreparedComparison>,
|
comparison: Option<PreparedComparison>,
|
||||||
cancelled: Arc<AtomicBool>,
|
cancelled: Arc<AtomicBool>,
|
||||||
}
|
}
|
||||||
|
/// Discover the canonical checkout root without opening a comparison repository.
|
||||||
|
pub fn discover_root(path: &Path) -> Result<PathBuf, String> {
|
||||||
|
discover_root_with_cancellation(path, &AtomicBool::new(false))
|
||||||
|
}
|
||||||
|
|
||||||
|
fn discover_root_with_cancellation(path: &Path, cancelled: &AtomicBool) -> Result<PathBuf, String> {
|
||||||
|
let path = fs::canonicalize(path).map_err(|error| error.to_string())?;
|
||||||
|
let root = path
|
||||||
|
.ancestors()
|
||||||
|
.find(|path| path.join(".jj").exists() || path.join(".git").exists())
|
||||||
|
.ok_or("No Jujutsu or Git working-tree repository found")?;
|
||||||
|
if root.join(".jj").exists() {
|
||||||
|
let mut command = jj_command(root);
|
||||||
|
command.arg("root");
|
||||||
|
let output = checked(run(command, vec![], cancelled)?)?;
|
||||||
|
let root = path_bytes(output.strip_suffix(b"\n").unwrap_or(&output))?;
|
||||||
|
fs::canonicalize(root).map_err(|error| error.to_string())
|
||||||
|
} else {
|
||||||
|
Ok(root.to_path_buf())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
impl Repository {
|
impl Repository {
|
||||||
pub fn open(root: &Path, cancelled: Arc<AtomicBool>) -> Result<Self, String> {
|
pub fn open(root: &Path, cancelled: Arc<AtomicBool>) -> Result<Self, String> {
|
||||||
let root = fs::canonicalize(root).map_err(|e| e.to_string())?;
|
let actual = discover_root_with_cancellation(root, &cancelled)?;
|
||||||
let discovered = root
|
let (root, kind, store) = if actual.join(".jj").exists() {
|
||||||
.ancestors()
|
|
||||||
.find(|p| p.join(".jj").exists() || p.join(".git").exists())
|
|
||||||
.ok_or("No Jujutsu or Git working-tree repository found")?;
|
|
||||||
let (root, kind, store) = if discovered.join(".jj").exists() {
|
|
||||||
let mut cmd = jj_command(discovered);
|
|
||||||
cmd.arg("root");
|
|
||||||
let output = checked(run(cmd, vec![], &cancelled)?)?;
|
|
||||||
let actual = path_bytes(output.strip_suffix(b"\n").unwrap_or(&output))?;
|
|
||||||
let store = store_for_jj(&actual)?;
|
let store = store_for_jj(&actual)?;
|
||||||
(actual, RepoKind::Jujutsu, store)
|
(actual, RepoKind::Jujutsu, store)
|
||||||
} else {
|
} else {
|
||||||
let actual = discovered.to_path_buf();
|
|
||||||
let mut cmd = Command::new("git");
|
let mut cmd = Command::new("git");
|
||||||
cmd.arg("-C")
|
cmd.arg("-C")
|
||||||
.arg(&actual)
|
.arg(&actual)
|
||||||
|
|||||||
+45
-17
@@ -299,6 +299,50 @@ pub struct Store {
|
|||||||
}
|
}
|
||||||
|
|
||||||
impl Store {
|
impl Store {
|
||||||
|
/// Open existing storage without creating files, changing permissions, or migrating schemas.
|
||||||
|
pub fn open_read_only(directory: &Path) -> Result<Option<Self>, String> {
|
||||||
|
let path = directory.join("reviews.sqlite3");
|
||||||
|
let metadata = match fs::symlink_metadata(&path) {
|
||||||
|
Ok(metadata) => metadata,
|
||||||
|
Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(None),
|
||||||
|
Err(error) => return Err(error.to_string()),
|
||||||
|
};
|
||||||
|
if !metadata.is_file() || metadata.file_type().is_symlink() {
|
||||||
|
return Err("Review database must be a regular file, not a symbolic link".into());
|
||||||
|
}
|
||||||
|
let path = fs::canonicalize(directory)
|
||||||
|
.map_err(|error| format!("Cannot resolve review directory: {error}"))?
|
||||||
|
.join("reviews.sqlite3");
|
||||||
|
let connection = Connection::open_with_flags(
|
||||||
|
&path,
|
||||||
|
OpenFlags::SQLITE_OPEN_READ_ONLY
|
||||||
|
| OpenFlags::SQLITE_OPEN_NO_MUTEX
|
||||||
|
| OpenFlags::SQLITE_OPEN_NOFOLLOW,
|
||||||
|
)
|
||||||
|
.map_err(database_error)?;
|
||||||
|
Self::check_schema(&connection)?;
|
||||||
|
Ok(Some(Self { connection }))
|
||||||
|
}
|
||||||
|
|
||||||
|
fn check_schema(connection: &Connection) -> Result<(), String> {
|
||||||
|
let version: i64 = connection
|
||||||
|
.pragma_query_value(None, "user_version", |row| row.get(0))
|
||||||
|
.map_err(database_error)?;
|
||||||
|
let schema: Vec<String> = connection
|
||||||
|
.prepare("SELECT sql FROM sqlite_schema WHERE name NOT GLOB 'sqlite_*' ORDER BY name")
|
||||||
|
.map_err(database_error)?
|
||||||
|
.query_map([], |row| row.get(0))
|
||||||
|
.map_err(database_error)?
|
||||||
|
.collect::<Result<_, _>>()
|
||||||
|
.map_err(database_error)?;
|
||||||
|
if version != 1 || schema != [SCHEMA] {
|
||||||
|
return Err(format!(
|
||||||
|
"Unsupported review database schema (version {version}); existing data was not changed"
|
||||||
|
));
|
||||||
|
}
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
pub fn open(directory: &Path) -> Result<Self, String> {
|
pub fn open(directory: &Path) -> Result<Self, String> {
|
||||||
let mut builder = fs::DirBuilder::new();
|
let mut builder = fs::DirBuilder::new();
|
||||||
builder.recursive(true);
|
builder.recursive(true);
|
||||||
@@ -352,23 +396,7 @@ impl Store {
|
|||||||
.map_err(database_error)?;
|
.map_err(database_error)?;
|
||||||
transaction.commit().map_err(database_error)?;
|
transaction.commit().map_err(database_error)?;
|
||||||
} else {
|
} else {
|
||||||
let version: i64 = connection
|
Self::check_schema(&connection)?;
|
||||||
.pragma_query_value(None, "user_version", |row| row.get(0))
|
|
||||||
.map_err(database_error)?;
|
|
||||||
let schema: Vec<String> = connection
|
|
||||||
.prepare(
|
|
||||||
"SELECT sql FROM sqlite_schema WHERE name NOT GLOB 'sqlite_*' ORDER BY name",
|
|
||||||
)
|
|
||||||
.map_err(database_error)?
|
|
||||||
.query_map([], |row| row.get(0))
|
|
||||||
.map_err(database_error)?
|
|
||||||
.collect::<Result<_, _>>()
|
|
||||||
.map_err(database_error)?;
|
|
||||||
if version != 1 || schema != [SCHEMA] {
|
|
||||||
return Err(format!(
|
|
||||||
"Unsupported review database schema (version {version}); existing data was not changed"
|
|
||||||
));
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
#[cfg(unix)]
|
#[cfg(unix)]
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -0,0 +1,306 @@
|
|||||||
|
use serde_json::{Value, json};
|
||||||
|
use std::{
|
||||||
|
ffi::{OsStr, OsString},
|
||||||
|
path::{Path, PathBuf},
|
||||||
|
};
|
||||||
|
use unicode_segmentation::UnicodeSegmentation;
|
||||||
|
use vamp::{
|
||||||
|
repository,
|
||||||
|
review::{Anchor, Review, Store},
|
||||||
|
ruby,
|
||||||
|
};
|
||||||
|
|
||||||
|
pub struct Request {
|
||||||
|
id: Option<i64>,
|
||||||
|
repo: Option<PathBuf>,
|
||||||
|
json: bool,
|
||||||
|
all: bool,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl Request {
|
||||||
|
pub fn parse(args: &mut impl Iterator<Item = OsString>) -> Result<Self, String> {
|
||||||
|
let command = args.next().ok_or("--review requires list or show ID")?;
|
||||||
|
let id = if command == "list" {
|
||||||
|
None
|
||||||
|
} else if command == "show" {
|
||||||
|
let id = args.next().ok_or("show requires a positive review ID")?;
|
||||||
|
Some(
|
||||||
|
id.to_str()
|
||||||
|
.and_then(|id| id.parse::<i64>().ok())
|
||||||
|
.filter(|id| *id > 0)
|
||||||
|
.ok_or("show requires a positive review ID")?,
|
||||||
|
)
|
||||||
|
} else {
|
||||||
|
return Err("--review requires list or show ID".into());
|
||||||
|
};
|
||||||
|
Ok(Self {
|
||||||
|
id,
|
||||||
|
repo: None,
|
||||||
|
json: false,
|
||||||
|
all: false,
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
pub fn option(
|
||||||
|
&mut self,
|
||||||
|
arg: &OsStr,
|
||||||
|
args: &mut impl Iterator<Item = OsString>,
|
||||||
|
) -> Result<(), String> {
|
||||||
|
if arg == "--repo" && self.repo.is_none() {
|
||||||
|
self.repo = Some(PathBuf::from(args.next().ok_or("--repo requires a path")?));
|
||||||
|
} else if arg == "--json" && !self.json {
|
||||||
|
self.json = true;
|
||||||
|
} else if arg == "--all" && self.id.is_some() && !self.all {
|
||||||
|
self.all = true;
|
||||||
|
} else {
|
||||||
|
return Err(format!(
|
||||||
|
"unexpected review argument: {}",
|
||||||
|
arg.to_string_lossy()
|
||||||
|
));
|
||||||
|
}
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn excerpt(anchor: &Anchor) -> String {
|
||||||
|
// Positions use extended graphemes, not bytes or Unicode scalar values.
|
||||||
|
let offset = |position: vamp::plugin::TextPosition| {
|
||||||
|
let line = anchor.content.split('\n').nth(position.row).unwrap_or("");
|
||||||
|
anchor
|
||||||
|
.content
|
||||||
|
.split('\n')
|
||||||
|
.take(position.row)
|
||||||
|
.map(|line| line.len() + 1)
|
||||||
|
.sum::<usize>()
|
||||||
|
+ line
|
||||||
|
.grapheme_indices(true)
|
||||||
|
.nth(position.column)
|
||||||
|
.map_or(line.len(), |(index, _)| index)
|
||||||
|
};
|
||||||
|
anchor.content[offset(anchor.start)..offset(anchor.end)].to_owned()
|
||||||
|
}
|
||||||
|
|
||||||
|
fn envelope(review: &Review, all: bool, comments: bool) -> Value {
|
||||||
|
let resolved = review
|
||||||
|
.comments
|
||||||
|
.iter()
|
||||||
|
.filter(|comment| comment.resolved)
|
||||||
|
.count();
|
||||||
|
let mut value = json!({
|
||||||
|
"repository": review.root,
|
||||||
|
"id": review.storage_id,
|
||||||
|
"revision": review.storage_revision,
|
||||||
|
"summary": review.summary,
|
||||||
|
"verdict": review.verdict,
|
||||||
|
"counts": { "total": review.comments.len(), "resolved": resolved, "unresolved": review.comments.len() - resolved },
|
||||||
|
});
|
||||||
|
if comments {
|
||||||
|
value["comments"] = review.comments.iter().filter(|comment| all || !comment.resolved).map(|comment| {
|
||||||
|
json!({"id": comment.id, "body": comment.body, "resolved": comment.resolved,
|
||||||
|
"anchor": { "revision": comment.anchor.revision, "path": comment.anchor.path,
|
||||||
|
"side": comment.anchor.side, "start": comment.anchor.start, "end": comment.anchor.end },
|
||||||
|
"excerpt": excerpt(&comment.anchor)})
|
||||||
|
}).collect();
|
||||||
|
}
|
||||||
|
value
|
||||||
|
}
|
||||||
|
|
||||||
|
pub fn run(request: &Request, config: Option<&Path>) -> Result<(), String> {
|
||||||
|
let directory = config
|
||||||
|
.map(ruby::load_config)
|
||||||
|
.transpose()?
|
||||||
|
.and_then(|config| config.review_directory)
|
||||||
|
.map_or_else(vamp::review::default_directory, Ok)?;
|
||||||
|
let cwd = std::env::current_dir().map_err(|error| error.to_string())?;
|
||||||
|
let root = repository::discover_root(request.repo.as_deref().unwrap_or(&cwd))?;
|
||||||
|
let store = Store::open_read_only(&directory)?;
|
||||||
|
let output = if let Some(id) = request.id {
|
||||||
|
let review = store
|
||||||
|
.as_ref()
|
||||||
|
.ok_or("Review does not exist in this repository")?
|
||||||
|
.load(&root, id)?;
|
||||||
|
envelope(&review, request.all, true)
|
||||||
|
} else {
|
||||||
|
let reviews = match store {
|
||||||
|
Some(store) => store
|
||||||
|
.list(&root)?
|
||||||
|
.into_iter()
|
||||||
|
.map(|(id, _)| {
|
||||||
|
store
|
||||||
|
.load(&root, id)
|
||||||
|
.map(|review| envelope(&review, true, false))
|
||||||
|
})
|
||||||
|
.collect::<Result<Vec<_>, _>>()?,
|
||||||
|
None => Vec::new(),
|
||||||
|
};
|
||||||
|
json!({"repository": root, "reviews": reviews})
|
||||||
|
};
|
||||||
|
if request.json {
|
||||||
|
println!(
|
||||||
|
"{}",
|
||||||
|
serde_json::to_string_pretty(&output).map_err(|error| error.to_string())?
|
||||||
|
);
|
||||||
|
} else {
|
||||||
|
println!("Repository: {}", root.display());
|
||||||
|
if let Some(reviews) = output["reviews"].as_array() {
|
||||||
|
if reviews.is_empty() {
|
||||||
|
println!("No stored reviews.");
|
||||||
|
}
|
||||||
|
for review in reviews {
|
||||||
|
print_review(review);
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
print_review(&output);
|
||||||
|
for comment in output["comments"].as_array().unwrap() {
|
||||||
|
let anchor = &comment["anchor"];
|
||||||
|
println!(
|
||||||
|
"\nComment {} [{}] {} ({}) {}:{}–{}:{} @ {}",
|
||||||
|
comment["id"],
|
||||||
|
if comment["resolved"] == true {
|
||||||
|
"resolved"
|
||||||
|
} else {
|
||||||
|
"unresolved"
|
||||||
|
},
|
||||||
|
anchor["path"].as_str().unwrap(),
|
||||||
|
anchor["side"].as_str().unwrap(),
|
||||||
|
anchor["start"]["row"],
|
||||||
|
anchor["start"]["column"],
|
||||||
|
anchor["end"]["row"],
|
||||||
|
anchor["end"]["column"],
|
||||||
|
anchor["revision"].as_str().unwrap()
|
||||||
|
);
|
||||||
|
println!(
|
||||||
|
"{}\n{}",
|
||||||
|
comment["excerpt"].as_str().unwrap(),
|
||||||
|
comment["body"].as_str().unwrap()
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
fn print_review(review: &Value) {
|
||||||
|
println!(
|
||||||
|
"Review {} (revision {}): {}\n Verdict: {}; {} unresolved, {} resolved ({} total)",
|
||||||
|
review["id"],
|
||||||
|
review["revision"],
|
||||||
|
review["summary"].as_str().unwrap(),
|
||||||
|
review["verdict"].as_str().unwrap_or("none"),
|
||||||
|
review["counts"]["unresolved"],
|
||||||
|
review["counts"]["resolved"],
|
||||||
|
review["counts"]["total"]
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
use vamp::{
|
||||||
|
plugin::TextPosition,
|
||||||
|
review::{Comment, Side},
|
||||||
|
};
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn output_filters_resolution_and_extracts_exact_graphemes() {
|
||||||
|
let mut review = Review::new(PathBuf::new());
|
||||||
|
review.storage_id = Some(7);
|
||||||
|
review.storage_revision = 2;
|
||||||
|
review.comments = [false, true]
|
||||||
|
.into_iter()
|
||||||
|
.enumerate()
|
||||||
|
.map(|(id, resolved)| Comment {
|
||||||
|
id: id as u64 + 1,
|
||||||
|
body: "fix".into(),
|
||||||
|
resolved,
|
||||||
|
anchor: Anchor {
|
||||||
|
revision: "commit".into(),
|
||||||
|
path: "a.rs".into(),
|
||||||
|
side: Side::After,
|
||||||
|
start: TextPosition { row: 0, column: 1 },
|
||||||
|
end: TextPosition { row: 1, column: 1 },
|
||||||
|
content: "xe\u{301}👩💻\n界z\n".into(),
|
||||||
|
},
|
||||||
|
})
|
||||||
|
.collect();
|
||||||
|
let output = envelope(&review, false, true);
|
||||||
|
assert_eq!(output["comments"].as_array().unwrap().len(), 1);
|
||||||
|
assert_eq!(output["comments"][0]["excerpt"], "e\u{301}👩💻\n界");
|
||||||
|
assert!(output["comments"][0]["anchor"].get("content").is_none());
|
||||||
|
assert_eq!(output["counts"]["resolved"], 1);
|
||||||
|
assert_eq!(
|
||||||
|
envelope(&review, true, true)["comments"]
|
||||||
|
.as_array()
|
||||||
|
.unwrap()
|
||||||
|
.len(),
|
||||||
|
2
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn absent_storage_is_not_created_and_incompatible_storage_is_preserved() {
|
||||||
|
let directory =
|
||||||
|
std::env::temp_dir().join(format!("vamp-review-cli-{}", std::process::id()));
|
||||||
|
std::fs::create_dir(&directory).unwrap();
|
||||||
|
let absent = directory.join("absent");
|
||||||
|
assert!(Store::open_read_only(&absent).unwrap().is_none());
|
||||||
|
assert!(!absent.exists());
|
||||||
|
let path = directory.join("reviews.sqlite3");
|
||||||
|
std::fs::write(&path, b"incompatible").unwrap();
|
||||||
|
assert!(Store::open_read_only(&directory).is_err());
|
||||||
|
assert_eq!(std::fs::read(&path).unwrap(), b"incompatible");
|
||||||
|
std::fs::remove_file(&path).unwrap();
|
||||||
|
let connection = rusqlite::Connection::open(&path).unwrap();
|
||||||
|
connection
|
||||||
|
.execute_batch("CREATE TABLE incompatible (value TEXT); PRAGMA user_version = 42")
|
||||||
|
.unwrap();
|
||||||
|
drop(connection);
|
||||||
|
let before = std::fs::read(&path).unwrap();
|
||||||
|
assert!(
|
||||||
|
Store::open_read_only(&directory)
|
||||||
|
.err()
|
||||||
|
.unwrap()
|
||||||
|
.contains("schema")
|
||||||
|
);
|
||||||
|
assert_eq!(std::fs::read(&path).unwrap(), before);
|
||||||
|
std::fs::remove_dir_all(directory).unwrap();
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn existing_storage_is_read_only_and_unchanged() {
|
||||||
|
let directory =
|
||||||
|
std::env::temp_dir().join(format!("vamp-review-cli-existing-{}", std::process::id()));
|
||||||
|
let mut store = Store::open(&directory).unwrap();
|
||||||
|
let mut review = Review::new(directory.clone());
|
||||||
|
review.summary = "stored".into();
|
||||||
|
store.save(&mut review).unwrap();
|
||||||
|
drop(store);
|
||||||
|
let path = directory.join("reviews.sqlite3");
|
||||||
|
#[cfg(unix)]
|
||||||
|
{
|
||||||
|
use std::os::unix::fs::PermissionsExt;
|
||||||
|
std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o640)).unwrap();
|
||||||
|
}
|
||||||
|
let before = std::fs::read(&path).unwrap();
|
||||||
|
let mut store = Store::open_read_only(&directory).unwrap().unwrap();
|
||||||
|
let mut loaded = store.load(&directory, review.storage_id.unwrap()).unwrap();
|
||||||
|
assert_eq!(loaded.summary, "stored");
|
||||||
|
loaded.summary = "changed".into();
|
||||||
|
assert!(store.save(&mut loaded).is_err());
|
||||||
|
drop(store);
|
||||||
|
assert_eq!(std::fs::read(&path).unwrap(), before);
|
||||||
|
#[cfg(unix)]
|
||||||
|
{
|
||||||
|
use std::os::unix::fs::PermissionsExt;
|
||||||
|
assert_eq!(
|
||||||
|
std::fs::metadata(&path).unwrap().permissions().mode() & 0o777,
|
||||||
|
0o640
|
||||||
|
);
|
||||||
|
let links = directory.join("links");
|
||||||
|
std::fs::create_dir(&links).unwrap();
|
||||||
|
std::os::unix::fs::symlink(&path, links.join("reviews.sqlite3")).unwrap();
|
||||||
|
assert!(Store::open_read_only(&links).is_err());
|
||||||
|
}
|
||||||
|
std::fs::remove_dir_all(directory).unwrap();
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user