hl-valdemar/vamp
1
0
Fork 0
Code Issues Pull Requests Actions Packages Releases Wiki Activity

feat: resolve local review comments manually

Browse Source
This commit is contained in:
Valdemar Lorenzen
2026-10-16 15:42:33 +02:00
parent b558f82094
commit 6ae8bf06a8
6 changed files with 267 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/editor.rs
+126
View File
@@ -945,6 +945,20 @@ core_actions![
true,
true
),
(
"core.diff.review.resolve",
Action::ReviewResolve,
&[Mode::Normal],
true,
true
),
(
"core.diff.review.unresolve",
Action::ReviewUnresolve,
&[Mode::Normal],
true,
true
),
(
"core.diff.review.next",
Action::ReviewNext,
@@ -5254,6 +5268,8 @@ impl Editor {
| Action::DiffComment
| Action::ReviewEdit
| Action::ReviewDelete
| Action::ReviewResolve
| Action::ReviewUnresolve
| Action::ReviewNext
| Action::ReviewPrevious
| Action::ReviewOverview
@@ -8453,6 +8469,8 @@ impl Editor {
| Action::DiffComment
| Action::ReviewEdit
| Action::ReviewDelete
| Action::ReviewResolve
| Action::ReviewUnresolve
| Action::ReviewNext
| Action::ReviewPrevious
| Action::ReviewOverview
@@ -9367,6 +9385,8 @@ impl Editor {
Action::DiffComment
| Action::ReviewEdit
| Action::ReviewDelete
| Action::ReviewResolve
| Action::ReviewUnresolve
| Action::ReviewNext
| Action::ReviewPrevious
| Action::ReviewOverview
@@ -16090,6 +16110,112 @@ mod tests {
);
}
#[test]
fn review_resolution_persists_without_hiding_comments_or_changing_stale_anchors() {
use crate::review::{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, "iFeedback");
first.handle(Input::Escape);
assert!(
first
.review_panel_title()
.unwrap()
.contains(" · unresolved · ")
);
first.execute("diff.review.resolve");
assert!(
first
.context()
.text
.contains("Comment #1 · resolved · current")
);
let store = Store::open(&storage).unwrap();
let document = &first.workspace_diff().unwrap().local_review.document;
let id = document.storage_id.unwrap();
let anchor = document.comments[0].anchor.clone();
assert!(store.load(&root, id).unwrap().comments[0].resolved);
first.execute("diff.review.next");
assert_eq!(first.context().text, "Feedback");
assert!(
first
.review_panel_title()
.unwrap()
.contains(" · resolved · ")
);
let export = root.join("resolved.json");
first.execute(&format!("diff.review.save {}", export.display()));
assert!(Review::load(&export, &root).unwrap().comments[0].resolved);
let mut stale = editor();
stale.set_review_directory(storage.clone());
historical_diff(
&mut stale,
&root,
vec!["changed".into()],
vec!["new".into()],
);
stale.execute("diff.review.overview");
assert!(
stale
.context()
.text
.contains("Comment #1 · resolved · stale")
);
first.execute("diff.review.unresolve 1");
assert!(!store.load(&root, id).unwrap().comments[0].resolved);
stale.execute("diff.review.unresolve 1");
assert!(stale.current_review_dirty());
assert!(
!stale
.workspace_diff()
.unwrap()
.local_review
.document
.comments[0]
.resolved
);
assert_eq!(
stale
.workspace_diff()
.unwrap()
.local_review
.document
.comments[0]
.anchor,
anchor
);
assert!(stale.message().contains("changed in another session"));
first.execute("diff.review.resolve");
assert!(
first
.context()
.text
.contains("Comment #1 · resolved · current")
);
first.execute("diff.review.unresolve");
assert!(
first
.context()
.text
.contains("Comment #1 · unresolved · current")
);
first.execute(&format!("diff.review.open {}", export.display()));
let imported = &first.workspace_diff().unwrap().local_review.document;
assert_ne!(imported.storage_id, Some(id));
assert!(
store
.load(&root, imported.storage_id.unwrap())
.unwrap()
.comments[0]
.resolved
);
}
#[test]
fn sqlite_review_conflicts_and_write_failures_keep_dirty_drafts() {
use crate::review::Store;
src/editor_review.rs
+50 -2
View File
@@ -306,6 +306,8 @@ impl Editor {
action,
Action::ReviewEdit
| Action::ReviewDelete
| Action::ReviewResolve
| Action::ReviewUnresolve
| Action::ReviewVerdict
| Action::ReviewSave
| Action::ReviewOpen
@@ -371,6 +373,40 @@ impl Editor {
let id = self.review_comment_id(args)?;
self.show_review_comment(id)?;
}
Action::ReviewResolve | Action::ReviewUnresolve => {
let id = self.review_comment_id(args)?;
let resolved = action == Action::ReviewResolve;
let state = &mut self.review_diff_mut().unwrap().local_review;
let comment = state
.document
.comments
.iter_mut()
.find(|comment| comment.id == id)
.unwrap();
if comment.resolved != resolved {
comment.resolved = resolved;
state.dirty = true;
}
let status = if resolved { "resolved" } else { "unresolved" };
for buffer in std::iter::once(&mut self.active).chain(self.parked.values_mut()) {
if let Some(binding) = buffer.review_buffer.as_mut()
&& binding.owner == self.tab_id
&& binding.kind == ReviewText::Comment(id)
{
binding.title = binding.title.replacen(
if resolved {
" · unresolved · "
} else {
" · resolved · "
},
&format!(" · {status} · "),
1,
);
}
}
self.autosave_local_review()?;
self.open_review_overview()?;
}
Action::ReviewDelete => {
let id = self.review_comment_id(args)?;
let state = &mut self.review_diff_mut().unwrap().local_review;
@@ -562,6 +598,7 @@ impl Editor {
id,
anchor,
body: String::new(),
resolved: false,
});
state.dirty = true;
self.show_review_comment(id)
@@ -578,8 +615,13 @@ impl Editor {
.ok_or("Comment no longer exists")?;
let text = comment.body.clone();
let anchor = &comment.anchor;
let status = if comment.resolved {
"resolved"
} else {
"unresolved"
};
let title = format!(
"Comment #{id} · {}:{}:{}–{}:{} · {:?} · :w saves review",
"Comment #{id} · {status} · {}:{}:{}–{}:{} · {:?} · :w saves review",
anchor.path.display(),
anchor.start.row + 1,
anchor.start.column + 1,
@@ -714,10 +756,15 @@ impl Editor {
} else {
"stale"
};
let resolution = if comment.resolved {
"resolved"
} else {
"unresolved"
};
use std::fmt::Write as _;
write!(
text,
"\n## Comment #{} · {status}\n{}:{}:{}–{}:{} · {:?}\nRevision: {}\n{}\n",
"\n## Comment #{} · {resolution} · {status}\n{}:{}:{}–{}:{} · {:?}\nRevision: {}\n{}\n",
comment.id,
anchor.path.display(),
anchor.start.row + 1,
@@ -774,6 +821,7 @@ impl Editor {
self.select_pane(panel);
if let Some(id) = existing {
self.switch_buffer(id);
self.active.review_buffer.as_mut().unwrap().title = title;
if matches!(kind, ReviewText::Overview | ReviewText::Reviews) {
self.active.lines = split_lines(text);
self.active.saved_lines.clone_from(&self.active.lines);
src/review.rs
+81 -8
View File
@@ -87,6 +87,7 @@ pub struct Comment {
pub id: u64,
pub anchor: Anchor,
pub body: String,
pub resolved: bool,
}
#[derive(Clone, Debug, Serialize, Deserialize)]
@@ -130,7 +131,7 @@ impl Review {
fs::canonicalize(&root).unwrap_or(root)
};
Self {
version: 2,
version: 3,
root,
summary: String::new(),
verdict: None,
@@ -142,7 +143,7 @@ impl Review {
}
fn validate(&self) -> Result<(), String> {
if self.version != 2 {
if self.version != 3 {
return Err(format!("Unsupported review version {}", self.version));
}
if !self.root.is_absolute() || !self.root.is_dir() {
@@ -188,10 +189,22 @@ impl Review {
Ok(())
}
fn from_json(bytes: &[u8]) -> Result<Self, String> {
#[derive(Deserialize)]
struct Header {
version: u32,
}
let header: Header = serde_json::from_slice(bytes)
.map_err(|error| format!("Invalid review JSON: {error}"))?;
if header.version != 3 {
return Err(format!("Unsupported review version {}", header.version));
}
serde_json::from_slice(bytes).map_err(|error| format!("Invalid review JSON: {error}"))
}
pub fn load(path: &Path, root: &Path) -> Result<Self, String> {
let bytes = read_file(path)?.ok_or("Review file does not exist")?;
let mut review: Self = serde_json::from_slice(&bytes)
.map_err(|error| format!("Invalid review JSON: {error}"))?;
let mut review = Self::from_json(&bytes)?;
review.validate()?;
let expected = fs::canonicalize(root).map_err(|error| error.to_string())?;
let actual = fs::canonicalize(&review.root).map_err(|error| error.to_string())?;
@@ -483,8 +496,7 @@ fn stored_review(row: &rusqlite::Row<'_>, root: &Path) -> Result<Review, String>
if bytes.len() > LIMIT {
return Err("Review exceeds the 64 MiB limit".into());
}
let mut review: Review = serde_json::from_slice(bytes)
.map_err(|error| format!("Invalid stored review JSON: {error}"))?;
let mut review = Review::from_json(bytes)?;
review.validate()?;
if review.root != root {
return Err("Stored review belongs to a different repository".into());
@@ -578,6 +590,7 @@ mod tests {
content: "a👩‍🔬e\u{301}\r\n界x\n".into(),
},
body: "Please preserve 👩‍🔬".into(),
resolved: true,
});
{
let mut store = Store::open(&storage).unwrap();
@@ -606,7 +619,8 @@ mod tests {
loaded.save(&path).unwrap();
let exported = fs::read(&path).unwrap();
let json: serde_json::Value = serde_json::from_slice(&exported).unwrap();
assert_eq!(json["version"], 2);
assert_eq!(json["comments"][0]["resolved"], true);
assert_eq!(json["version"], 3);
assert!(json.get("storage_id").is_none());
assert!(json.get("storage_revision").is_none());
loaded.summary = "Database only".into();
@@ -857,6 +871,7 @@ mod tests {
content: "first\nlast\n".into(),
},
body: "Please adjust this".into(),
resolved: false,
});
review.save(&path).unwrap();
let mut loaded = Review::load(&path, &root).unwrap();
@@ -895,7 +910,7 @@ mod tests {
assert!(Review::new(root.clone()).save(&path).is_err());
let valid = fs::read(&path).unwrap();
for (field, value) in [
("version", serde_json::json!(3)),
("version", serde_json::json!(2)),
("root", serde_json::json!(root.join("other"))),
(
"comments",
@@ -948,6 +963,46 @@ mod tests {
}
}
#[test]
fn stored_version_two_is_rejected_without_rewriting_it() {
let directory = directory();
let root = directory.0.canonicalize().unwrap();
let mut store = Store::open(&root.join("storage")).unwrap();
let mut review = Review::new(root.clone());
store.save(&mut review).unwrap();
let mut old = serde_json::to_value(&review).unwrap();
old["version"] = 2.into();
let document = serde_json::to_string(&old).unwrap();
store
.connection
.execute(
"UPDATE reviews SET document = ?1 WHERE id = ?2",
params![document, review.storage_id.unwrap()],
)
.unwrap();
assert!(
store
.load(&root, review.storage_id.unwrap())
.unwrap_err()
.contains("Unsupported review version 2")
);
assert!(
store
.list(&root)
.unwrap_err()
.contains("Unsupported review version 2")
);
let preserved: String = store
.connection
.query_row(
"SELECT document FROM reviews WHERE id = ?1",
[review.storage_id.unwrap()],
|row| row.get(0),
)
.unwrap();
assert_eq!(preserved, document);
}
#[test]
fn exact_grapheme_ranges_roundtrip_and_reject_invalid_coordinates() {
use crate::plugin::TextPosition;
@@ -966,12 +1021,30 @@ mod tests {
content: "a👩‍🔬e\u{301}\n界x\n".into(),
},
body: "Exact text".into(),
resolved: false,
});
review.save(&path).unwrap();
let mut loaded = Review::load(&path, &root).unwrap();
assert_eq!(loaded.comments, review.comments);
assert_eq!(loaded.comments[0].anchor.rows(), 0..=1);
let valid = serde_json::to_value(&review).unwrap();
let mut missing_resolution = valid.clone();
missing_resolution["comments"][0]
.as_object_mut()
.unwrap()
.remove("resolved");
for version in [2, 3] {
missing_resolution["version"] = version.into();
let bytes = serde_json::to_vec(&missing_resolution).unwrap();
fs::write(&path, &bytes).unwrap();
let error = Review::load(&path, &root).unwrap_err();
assert!(error.contains(if version == 2 {
"Unsupported review version 2"
} else {
"missing field `resolved`"
}));
assert_eq!(fs::read(&path).unwrap(), bytes);
}
for (field, value) in [
("start", serde_json::json!({"row": 0, "column": 4})),
("end", serde_json::json!({"row": 1, "column": 3})),
src/tui.rs
+2
View File
@@ -9409,6 +9409,7 @@ mod tests {
content: before,
},
body: "Before range".into(),
resolved: false,
},
Comment {
id: 2,
@@ -9421,6 +9422,7 @@ mod tests {
content: after,
},
body: "After line".into(),
resolved: false,
},
];
diff.local_review.selected = Some(1);