cli: Use pretty diffs for code review

We use the pretty diff renderer for code review.
This commit is contained in:
cloudhead 2024-07-12 16:13:20 +02:00
parent 823ece875e
commit 297646412a
No known key found for this signature in database
4 changed files with 486 additions and 197 deletions

View File

@ -33,3 +33,7 @@ resolver = "2"
inherits = "release" inherits = "release"
debug = true debug = true
incremental = false incremental = false
[workspace.lints]
clippy.type_complexity = "allow"
clippy.enum_variant_names = "allow"

View File

@ -71,3 +71,6 @@ path = "../radicle-term"
pretty_assertions = { version = "1.3.0" } pretty_assertions = { version = "1.3.0" }
radicle = { version = "0.11.0", path = "../radicle", features = ["test"] } radicle = { version = "0.11.0", path = "../radicle", features = ["test"] }
radicle-node = { path = "../radicle-node", features = ["test"] } radicle-node = { path = "../radicle-node", features = ["test"] }
[lints]
workspace = true

View File

@ -71,46 +71,47 @@ match `git diff master -W100% -U5 --patience`:
``` ```
$ rad patch review --patch -U5 7a2ac7e2841cc1e7394f99f107555a499b1d3f23 --no-announce $ rad patch review --patch -U5 7a2ac7e2841cc1e7394f99f107555a499b1d3f23 --no-announce
diff --git a/.gitignore b/.gitignore ╭──────────────────────╮
deleted file mode 100644 │ .gitignore deleted
index 7937fb3..0000000 ├──────────────────────┤
--- a/.gitignore │ @@ -1,1 +0,0 @@ │
+++ /dev/null │ 1 - *.draft │
@@ -1 +0,0 @@ ╰──────────────────────╯
-*.draft ╭──────────────────────────────────────────────────────────╮
diff --git a/DISCLAIMER.txt b/DISCLAIMER.txt │ DISCLAIMER.txt created
new file mode 100644 ├──────────────────────────────────────────────────────────┤
index 0000000..2b5bd86 │ @@ -0,0 +1,1 @@ │
--- /dev/null │ 1 + All food is served as-is, with no warranty! │
+++ b/DISCLAIMER.txt ╰──────────────────────────────────────────────────────────╯
@@ -0,0 +1 @@ ╭─────────────────────────────╮
+All food is served as-is, with no warranty! │ MENU.txt │
diff --git a/MENU.txt b/MENU.txt ├─────────────────────────────┤
index 867958c..3af9741 100644 │ @@ -1,7 +1,8 @@ │
--- a/MENU.txt │ 1 1 Classics │
+++ b/MENU.txt │ 2 2 -------- │
@@ -1,7 +1,8 @@ │ 3 + Baked Brie │
Classics │ 3 4 Salmon Tartare │
-------- │ 4 5 Mac & Cheese │
+Baked Brie │ 5 6 │
Salmon Tartare │ 6 7 Comfort Food │
Mac & Cheese │ 7 8 ------------ │
[..] ╰─────────────────────────────╯
Comfort Food ╭──────────────────────────────────╮
------------ │ MENU.txt │
@@ -9,6 +10,7 @@ Reuben Sandwich ├──────────────────────────────────┤
Club Sandwich │ @@ -9,6 +10,7 @@ Reuben Sandwich │
Fried Shrimp Basket │ 9 10 Club Sandwich │
[..] │ 10 11 Fried Shrimp Basket │
Sides │ 11 12 │
----- │ 12 13 Sides │
-French Fries │ 13 14 ----- │
+French Fries! │ 14 - French Fries │
+Garlic Green Beans │ 15 + French Fries! │
diff --git a/INSTRUCTIONS.txt b/notes/INSTRUCTIONS.txt │ 16 + Garlic Green Beans │
similarity index 100% ╰──────────────────────────────────╯
rename from INSTRUCTIONS.txt ╭────────────────────────────────────────────────────╮
rename to notes/INSTRUCTIONS.txt │ INSTRUCTIONS.txt -> notes/INSTRUCTIONS.txt moved
╰────────────────────────────────────────────────────╯
``` ```
Now let's accept these hunks one by one.. Now let's accept these hunks one by one..
@ -118,67 +119,64 @@ Now let's accept these hunks one by one..
``` ```
$ rad patch review --patch --accept --hunk 1 7a2ac7e2841cc1e7394f99f107555a499b1d3f23 --no-announce $ rad patch review --patch --accept --hunk 1 7a2ac7e2841cc1e7394f99f107555a499b1d3f23 --no-announce
✓ Loaded existing review ([..]) for patch 7a2ac7e2841cc1e7394f99f107555a499b1d3f23 ✓ Loaded existing review ([..]) for patch 7a2ac7e2841cc1e7394f99f107555a499b1d3f23
diff --git a/.gitignore b/.gitignore ╭──────────────────────╮
deleted file mode 100644 │ .gitignore deleted
index 7937fb3..0000000 ├──────────────────────┤
--- a/.gitignore │ @@ -1,1 +0,0 @@ │
+++ /dev/null │ 1 - *.draft │
@@ -1 +0,0 @@ ╰──────────────────────╯
-*.draft
✓ Updated review tree to a5fccf0e977225ff13c3f74c43faf4cb679bf835 ✓ Updated review tree to a5fccf0e977225ff13c3f74c43faf4cb679bf835
``` ```
``` ```
$ rad patch review --patch --accept --hunk 1 7a2ac7e2841cc1e7394f99f107555a499b1d3f23 --no-announce $ rad patch review --patch --accept --hunk 1 7a2ac7e2841cc1e7394f99f107555a499b1d3f23 --no-announce
✓ Loaded existing review ([..]) for patch 7a2ac7e2841cc1e7394f99f107555a499b1d3f23 ✓ Loaded existing review ([..]) for patch 7a2ac7e2841cc1e7394f99f107555a499b1d3f23
diff --git a/DISCLAIMER.txt b/DISCLAIMER.txt ╭──────────────────────────────────────────────────────────╮
new file mode 100644 │ DISCLAIMER.txt created
index 0000000..2b5bd86 ├──────────────────────────────────────────────────────────┤
--- /dev/null │ @@ -0,0 +1,1 @@ │
+++ b/DISCLAIMER.txt │ 1 + All food is served as-is, with no warranty! │
@@ -0,0 +1 @@ ╰──────────────────────────────────────────────────────────╯
+All food is served as-is, with no warranty!
✓ Updated review tree to 2cdb82ea726e64d3b52847c7699d0d4759198f5c ✓ Updated review tree to 2cdb82ea726e64d3b52847c7699d0d4759198f5c
``` ```
``` ```
$ rad patch review --patch --accept -U3 --hunk 1 7a2ac7e2841cc1e7394f99f107555a499b1d3f23 --no-announce $ rad patch review --patch --accept -U3 --hunk 1 7a2ac7e2841cc1e7394f99f107555a499b1d3f23 --no-announce
✓ Loaded existing review ([..]) for patch 7a2ac7e2841cc1e7394f99f107555a499b1d3f23 ✓ Loaded existing review ([..]) for patch 7a2ac7e2841cc1e7394f99f107555a499b1d3f23
diff --git a/MENU.txt b/MENU.txt ╭─────────────────────────────╮
index 867958c..3af9741 100644 │ MENU.txt │
--- a/MENU.txt ├─────────────────────────────┤
+++ b/MENU.txt │ @@ -1,5 +1,6 @@ │
@@ -1,5 +1,6 @@ │ 1 1 Classics │
Classics │ 2 2 -------- │
-------- │ 3 + Baked Brie │
+Baked Brie │ 3 4 Salmon Tartare │
Salmon Tartare │ 4 5 Mac & Cheese │
Mac & Cheese │ 5 6 │
[..] ╰─────────────────────────────╯
✓ Updated review tree to d4aecbb859a802a3215def0b538358bf63593953 ✓ Updated review tree to d4aecbb859a802a3215def0b538358bf63593953
``` ```
``` ```
$ rad patch review --patch --accept -U3 --hunk 1 7a2ac7e2841cc1e7394f99f107555a499b1d3f23 --no-announce $ rad patch review --patch --accept -U3 --hunk 1 7a2ac7e2841cc1e7394f99f107555a499b1d3f23 --no-announce
✓ Loaded existing review ([..]) for patch 7a2ac7e2841cc1e7394f99f107555a499b1d3f23 ✓ Loaded existing review ([..]) for patch 7a2ac7e2841cc1e7394f99f107555a499b1d3f23
diff --git a/MENU.txt b/MENU.txt ╭───────────────────────────────────────╮
index 4e2e828..3af9741 100644 │ MENU.txt │
--- a/MENU.txt ├───────────────────────────────────────┤
+++ b/MENU.txt │ @@ -12,4 +12,5 @@ Fried Shrimp Basket │
@@ -12,4 +12,5 @@ Fried Shrimp Basket │ 12 12 │
[..] │ 13 13 Sides │
Sides │ 14 14 ----- │
----- │ 15 - French Fries │
-French Fries │ 15 + French Fries! │
+French Fries! │ 16 + Garlic Green Beans │
+Garlic Green Beans ╰───────────────────────────────────────╯
✓ Updated review tree to 59cee720b0642b1491b241400912b35926a76c3f ✓ Updated review tree to 59cee720b0642b1491b241400912b35926a76c3f
``` ```
``` ```
$ rad patch review --patch --accept --hunk 1 7a2ac7e2841cc1e7394f99f107555a499b1d3f23 --no-announce $ rad patch review --patch --accept --hunk 1 7a2ac7e2841cc1e7394f99f107555a499b1d3f23 --no-announce
✓ Loaded existing review ([..]) for patch 7a2ac7e2841cc1e7394f99f107555a499b1d3f23 ✓ Loaded existing review ([..]) for patch 7a2ac7e2841cc1e7394f99f107555a499b1d3f23
diff --git a/INSTRUCTIONS.txt b/notes/INSTRUCTIONS.txt ╭────────────────────────────────────────────────────╮
similarity index 100% │ INSTRUCTIONS.txt -> notes/INSTRUCTIONS.txt moved
rename from INSTRUCTIONS.txt ╰────────────────────────────────────────────────────╯
rename to notes/INSTRUCTIONS.txt
✓ Updated review tree to 3effc8f6462fa2573697072245e57708c4dcbe62 ✓ Updated review tree to 3effc8f6462fa2573697072245e57708c4dcbe62
``` ```

View File

@ -13,9 +13,8 @@
//! //!
use std::collections::VecDeque; use std::collections::VecDeque;
use std::fmt::Write as _; use std::fmt::Write as _;
use std::io::IsTerminal as _; use std::ops::{Deref, Not, Range};
use std::ops::{Not, Range}; use std::path::{Path, PathBuf};
use std::path::PathBuf;
use std::str::FromStr; use std::str::FromStr;
use std::{fmt, io}; use std::{fmt, io};
@ -26,10 +25,14 @@ use radicle::prelude::*;
use radicle::storage::git::Repository; use radicle::storage::git::Repository;
use radicle_git_ext::Oid; use radicle_git_ext::Oid;
use radicle_surf::diff::*; use radicle_surf::diff::*;
use radicle_term::{Element, VStack};
use crate::git::unified_diff; use crate::git::pretty_diff::ToPretty;
use crate::git::pretty_diff::{Blob, Blobs, Repo};
use crate::git::unified_diff::{self, FileHeader};
use crate::git::unified_diff::{Encode, HunkHeader}; use crate::git::unified_diff::{Encode, HunkHeader};
use crate::terminal as term; use crate::terminal as term;
use crate::terminal::highlight::Highlighter;
/// Help message shown to user. /// Help message shown to user.
const HELP: &str = "\ const HELP: &str = "\
@ -42,6 +45,24 @@ s - split the current hunk into smaller hunks
q - quit; do not accept this hunk nor any of the remaining ones q - quit; do not accept this hunk nor any of the remaining ones
? - print help"; ? - print help";
/// A terminal or file where the review UI output can be written to.
trait PromptWriter: io::Write {
/// Is the writer a terminal?
fn is_terminal(&self) -> bool;
}
impl PromptWriter for Box<dyn PromptWriter> {
fn is_terminal(&self) -> bool {
self.deref().is_terminal()
}
}
impl<T: io::Write + io::IsTerminal> PromptWriter for T {
fn is_terminal(&self) -> bool {
<Self as io::IsTerminal>::is_terminal(self)
}
}
/// The actions that a user can carry out on a review item. /// The actions that a user can carry out on a review item.
#[derive(Debug, Copy, Clone, PartialEq, Eq)] #[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub enum ReviewAction { pub enum ReviewAction {
@ -113,58 +134,345 @@ impl FromStr for ReviewAction {
} }
/// A single review item. Can be a hunk or eg. a file move. /// A single review item. Can be a hunk or eg. a file move.
pub struct ReviewItem<'a> { /// Files are usually split into multiple review items.
file: &'a FileDiff, #[derive(Debug)]
hunk: Option<&'a Hunk<Modification>>, pub enum ReviewItem {
FileAdded {
path: PathBuf,
header: FileHeader,
new: DiffFile,
hunk: Option<Hunk<Modification>>,
},
FileDeleted {
path: PathBuf,
header: FileHeader,
old: DiffFile,
hunk: Option<Hunk<Modification>>,
},
FileModified {
path: PathBuf,
header: FileHeader,
old: DiffFile,
new: DiffFile,
hunk: Option<Hunk<Modification>>,
},
FileMoved {
moved: Moved,
},
FileCopied {
copied: Copied,
},
FileEofChanged {
path: PathBuf,
header: FileHeader,
old: DiffFile,
new: DiffFile,
eof: EofNewLine,
},
FileModeChanged {
path: PathBuf,
header: FileHeader,
old: DiffFile,
new: DiffFile,
},
}
impl ReviewItem {
fn hunk(&self) -> Option<&Hunk<Modification>> {
match self {
Self::FileAdded { hunk, .. } => hunk.as_ref(),
Self::FileDeleted { hunk, .. } => hunk.as_ref(),
Self::FileModified { hunk, .. } => hunk.as_ref(),
_ => None,
}
}
fn hunk_header(&self) -> Option<HunkHeader> {
self.hunk().and_then(|h| HunkHeader::try_from(h).ok())
}
fn paths(&self) -> (Option<(&Path, Oid)>, Option<(&Path, Oid)>) {
match self {
Self::FileAdded { path, new, .. } => (None, Some((&path, new.oid))),
Self::FileDeleted { path, old, .. } => (Some((&path, old.oid)), None),
Self::FileMoved { moved } => (
Some((&moved.old_path, moved.old.oid)),
Some((&moved.new_path, moved.new.oid)),
),
Self::FileCopied { copied } => (
Some((&copied.old_path, copied.old.oid)),
Some((&copied.new_path, copied.new.oid)),
),
Self::FileModified { path, old, new, .. } => {
(Some((path, old.oid)), Some((path, new.oid)))
}
Self::FileEofChanged { path, old, new, .. } => {
(Some((path, old.oid)), Some((path, new.oid)))
}
Self::FileModeChanged { path, old, new, .. } => {
(Some((path, old.oid)), Some((path, new.oid)))
}
}
}
fn file_header(&self) -> FileHeader {
match self {
Self::FileAdded { header, .. } => header.clone(),
Self::FileDeleted { header, .. } => header.clone(),
Self::FileMoved { moved } => FileHeader::Moved {
old_path: moved.old_path.clone(),
new_path: moved.new_path.clone(),
},
Self::FileCopied { copied } => FileHeader::Copied {
old_path: copied.old_path.clone(),
new_path: copied.new_path.clone(),
},
Self::FileModified { header, .. } => header.clone(),
Self::FileEofChanged { header, .. } => header.clone(),
Self::FileModeChanged { header, .. } => header.clone(),
}
}
fn blobs<R: Repo>(&self, repo: &R) -> Blobs<(PathBuf, Blob)> {
let (old, new) = self.paths();
Blobs::from_paths(old, new, repo)
}
fn pretty<R: Repo>(&self, repo: &R) -> Box<dyn Element> {
let mut hi = Highlighter::default();
let blobs = self.blobs(repo);
let highlighted = blobs.highlight(&mut hi);
let header = self.file_header();
match self {
Self::FileMoved { moved } => moved.pretty(&mut hi, &header, repo),
Self::FileCopied { copied } => copied.pretty(&mut hi, &header, repo),
Self::FileModified { hunk, .. }
| Self::FileAdded { hunk, .. }
| Self::FileDeleted { hunk, .. } => {
let header = header.pretty(&mut hi, &None, repo);
let vstack = term::VStack::default()
.border(Some(term::colors::FAINT))
.padding(1)
.child(header);
if let Some(hunk) = hunk {
let hunk = hunk.pretty(&mut hi, &highlighted, repo);
if !hunk.is_empty() {
return vstack.divider().merge(hunk).boxed();
}
}
vstack
}
Self::FileEofChanged { eof, .. } => match eof {
EofNewLine::NewMissing => {
VStack::default().child(term::Label::new("`\\n` missing at end-of-file"))
}
EofNewLine::OldMissing => {
VStack::default().child(term::Label::new("`\\n` added at end-of-file"))
}
_ => VStack::default(),
},
Self::FileModeChanged { .. } => VStack::default(),
}
.boxed()
}
} }
/// Queue of items (usually hunks) left to review. /// Queue of items (usually hunks) left to review.
#[derive(Default)] #[derive(Default)]
pub struct ReviewQueue<'a> { pub struct ReviewQueue {
queue: VecDeque<(usize, ReviewItem<'a>)>, /// Hunks left to review.
queue: VecDeque<(usize, ReviewItem)>,
} }
impl<'a> ReviewQueue<'a> { impl ReviewQueue {
/// Push an item to the queue. /// Add a file to the queue.
fn push(&mut self, file: &'a FileDiff, hunks: Option<&'a Hunks<Modification>>) { /// Mostly splits files into individual review items (eg. hunks) to review.
let mut queue_item = |hunk| { fn add_file(&mut self, file: FileDiff) {
self.queue let header = FileHeader::from(&file);
.push_back((self.queue.len(), ReviewItem { file, hunk }))
};
if let Some(hunks) = hunks { match file {
for hunk in hunks.iter() { FileDiff::Moved(moved) => {
queue_item(Some(hunk)); self.add_item(ReviewItem::FileMoved { moved });
}
FileDiff::Copied(copied) => {
self.add_item(ReviewItem::FileCopied { copied });
}
FileDiff::Added(a) => {
self.add_item(ReviewItem::FileAdded {
path: a.path,
header: header.clone(),
new: a.new,
hunk: if let DiffContent::Plain {
hunks: Hunks(mut hs),
..
} = a.diff
{
hs.pop()
} else {
None
},
});
}
FileDiff::Deleted(d) => {
self.add_item(ReviewItem::FileDeleted {
path: d.path,
header: header.clone(),
old: d.old,
hunk: if let DiffContent::Plain {
hunks: Hunks(mut hs),
..
} = d.diff
{
hs.pop()
} else {
None
},
});
}
FileDiff::Modified(m) => {
if m.old.mode != m.new.mode {
self.add_item(ReviewItem::FileModeChanged {
path: m.path.clone(),
header: header.clone(),
old: m.old.clone(),
new: m.new.clone(),
});
}
match m.diff {
DiffContent::Empty => {
// Likely a file mode change, which is handled above.
}
DiffContent::Binary => {
self.add_item(ReviewItem::FileModified {
path: m.path.clone(),
header: header.clone(),
old: m.old.clone(),
new: m.new.clone(),
hunk: None,
});
}
DiffContent::Plain {
hunks: Hunks(hunks),
eof,
..
} => {
for hunk in hunks {
self.add_item(ReviewItem::FileModified {
path: m.path.clone(),
header: header.clone(),
old: m.old.clone(),
new: m.new.clone(),
hunk: Some(hunk),
});
}
if let EofNewLine::OldMissing | EofNewLine::NewMissing = eof {
self.add_item(ReviewItem::FileEofChanged {
path: m.path.clone(),
header: header.clone(),
old: m.old.clone(),
new: m.new.clone(),
eof,
})
}
}
}
} }
} else {
queue_item(None);
} }
} }
fn add_item(&mut self, item: ReviewItem) {
self.queue.push_back((self.queue.len(), item));
}
} }
impl<'a> std::ops::Deref for ReviewQueue<'a> { impl std::ops::Deref for ReviewQueue {
type Target = VecDeque<(usize, ReviewItem<'a>)>; type Target = VecDeque<(usize, ReviewItem)>;
fn deref(&self) -> &Self::Target { fn deref(&self) -> &Self::Target {
&self.queue &self.queue
} }
} }
impl<'a> std::ops::DerefMut for ReviewQueue<'a> { impl std::ops::DerefMut for ReviewQueue {
fn deref_mut(&mut self) -> &mut Self::Target { fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.queue &mut self.queue
} }
} }
impl<'a> Iterator for ReviewQueue<'a> { impl Iterator for ReviewQueue {
type Item = (usize, ReviewItem<'a>); type Item = (usize, ReviewItem);
fn next(&mut self) -> Option<Self::Item> { fn next(&mut self) -> Option<Self::Item> {
self.queue.pop_front() self.queue.pop_front()
} }
} }
/// Builds a patch review interactively. /// Builds a review for a single file.
pub struct FileReviewBuilder {
header: FileHeader,
delta: i32,
}
impl FileReviewBuilder {
fn new(item: &ReviewItem) -> Self {
Self {
header: item.file_header(),
delta: 0,
}
}
fn set_item(&mut self, item: &ReviewItem) -> &mut Self {
let header = item.file_header();
if self.header != header {
self.header = header;
self.delta = 0;
}
self
}
fn ignore_item(&mut self, item: &ReviewItem) {
if let Some(h) = item.hunk_header() {
self.delta += h.new_size as i32 - h.old_size as i32;
}
}
fn apply_item<'a>(
&mut self,
item: ReviewItem,
brain: &mut git::raw::Tree<'a>,
repo: &'a git::raw::Repository,
) -> Result<(), Error> {
let mut buf = Vec::new();
let mut writer = unified_diff::Writer::new(&mut buf);
writer.encode(&self.header)?;
if let (Some(h), Some(mut header)) = (item.hunk(), item.hunk_header()) {
header.old_line_no -= self.delta as u32;
header.new_line_no -= self.delta as u32;
let h = Hunk {
header: header.to_unified_string()?.as_bytes().to_owned().into(),
lines: h.lines.clone(),
old: h.old.clone(),
new: h.new.clone(),
};
writer.encode(&h)?;
}
drop(writer);
let diff = git::raw::Diff::from_buffer(&buf)?;
let mut index = repo.apply_to_tree(brain, &diff, None)?;
let brain_oid = index.write_tree_to(repo)?;
*brain = repo.find_tree(brain_oid)?;
Ok(())
}
}
/// Builds a patch review interactively, across multiple files.
pub struct ReviewBuilder<'a> { pub struct ReviewBuilder<'a> {
/// Patch being reviewed. /// Patch being reviewed.
patch_id: PatchId, patch_id: PatchId,
@ -215,8 +523,13 @@ impl<'a> ReviewBuilder<'a> {
commit.tree()? commit.tree()?
}; };
let stdout = io::stdout().lock();
let mut stdin = io::stdin().lock(); let mut stdin = io::stdin().lock();
let mut stderr = io::stderr().lock(); let mut writer: Box<dyn PromptWriter> = if self.hunk.is_some() || !stdout.is_terminal() {
Box::new(stdout)
} else {
Box::new(io::stderr().lock())
};
let mut review = if let Ok(c) = self.current() { let mut review = if let Ok(c) = self.current() {
term::success!( term::success!(
"Loaded existing review {} for patch {}", "Loaded existing review {} for patch {}",
@ -237,51 +550,22 @@ impl<'a> ReviewBuilder<'a> {
repo.find_commit(oid)? repo.find_commit(oid)?
}; };
let mut brain = review.tree()?; let mut brain = review.tree()?;
let mut writer = unified_diff::Writer::new(io::stdout()).styled(true); let mut queue = ReviewQueue::default();
let mut queue = ReviewQueue::default(); // Queue of hunks to review. let diff = self.diff(&brain, &tree, repo, opts)?;
let mut current = None; // File of the current hunk.
let mut find_opts = git::raw::DiffFindOptions::new(); // Build the review queue.
find_opts.exact_match_only(true); for file in diff.into_files() {
find_opts.all(true); queue.add_file(file);
find_opts.copies(false); // We don't support finding copies at the moment. }
if queue.is_empty() {
let mut diff = repo.diff_tree_to_tree(Some(&brain), Some(&tree), Some(opts))?;
diff.find_similar(Some(&mut find_opts))?;
if diff.deltas().next().is_none() {
term::success!("All hunks have been reviewed"); term::success!("All hunks have been reviewed");
return Ok(()); return Ok(());
} }
let diff = Diff::try_from(diff)?;
for file in diff.files() { // File review for the current file. Starts out as `None` and is set on the first hunk.
match file { // Keeps track of deltas for hunk offsets.
FileDiff::Modified(f) => match &f.diff { let mut file: Option<FileReviewBuilder> = None;
DiffContent::Plain { hunks, .. } => queue.push(file, Some(hunks)),
DiffContent::Binary => queue.push(file, None),
DiffContent::Empty => {}
},
FileDiff::Added(f) => match &f.diff {
DiffContent::Plain { hunks, .. } => queue.push(file, Some(hunks)),
DiffContent::Binary => queue.push(file, None),
DiffContent::Empty => {}
},
FileDiff::Deleted(f) => match &f.diff {
DiffContent::Plain { hunks, .. } => queue.push(file, Some(hunks)),
DiffContent::Binary => queue.push(file, None),
DiffContent::Empty => {}
},
FileDiff::Moved(_) => queue.push(file, None),
FileDiff::Copied(_) => {
// Copies are not supported and should never be generated due to the diff
// options we pass.
panic!("ReviewBuilder::by_hunk: copy diffs are not supported");
}
}
}
let total = queue.len(); let total = queue.len();
let mut delta: i32 = 0;
while let Some((ix, item)) = queue.next() { while let Some((ix, item)) = queue.next() {
if let Some(hunk) = self.hunk { if let Some(hunk) = self.hunk {
@ -290,66 +574,42 @@ impl<'a> ReviewBuilder<'a> {
} }
} }
let progress = term::format::secondary(format!("({}/{total})", ix + 1)); let progress = term::format::secondary(format!("({}/{total})", ix + 1));
let ReviewItem { file, hunk } = item; let file = match file.as_mut() {
Some(fr) => fr.set_item(&item),
None => file.insert(FileReviewBuilder::new(&item)),
};
term::element::write_to(
&item.pretty(repo),
&mut writer,
term::Constraint::from_env().unwrap_or_default(),
)?;
if current.map_or(true, |c| c != file) { // Prompts the user for action on the above hunk.
writer.encode(&unified_diff::FileHeader::from(file))?; match self.prompt(&mut stdin, &mut writer, progress) {
current = Some(file); // When a hunk is accepted, we convert it to unified diff format,
delta = 0; // and apply it to the `brain`.
}
let header = hunk
.map(|h| {
let header = unified_diff::HunkHeader::try_from(h)?;
writer.encode(h)?;
Ok::<_, anyhow::Error>(header)
})
.transpose()?;
match self.prompt(&mut stdin, &mut stderr, progress) {
Some(ReviewAction::Accept) => { Some(ReviewAction::Accept) => {
let mut buf = Vec::new(); // Update brain with accepted hunk.
{ file.apply_item(item, &mut brain, repo)?;
let mut writer = unified_diff::Writer::new(&mut buf); // Update review with new brain.
writer.encode(&unified_diff::FileHeader::from(file))?; let review_oid =
if let (Some(h), Some(mut header)) = (hunk, header) {
header.old_line_no -= delta as u32;
header.new_line_no -= delta as u32;
let h = Hunk {
header: header.to_unified_string()?.as_bytes().to_owned().into(),
lines: h.lines.clone(),
old: h.old.clone(),
new: h.new.clone(),
};
writer.encode(&h)?;
}
}
let diff = git::raw::Diff::from_buffer(&buf)?;
let mut index = repo.apply_to_tree(&brain, &diff, None)?;
let brain_oid = index.write_tree_to(repo)?;
brain = repo.find_tree(brain_oid)?;
let oid =
review.amend(Some(&self.refname), None, None, None, None, Some(&brain))?; review.amend(Some(&self.refname), None, None, None, None, Some(&brain))?;
review = repo.find_commit(oid)?; review = repo.find_commit(review_oid)?;
if self.hunk.is_some() { if self.hunk.is_some() {
term::success!("Updated review tree to {brain_oid}"); term::success!("Updated review tree to {}", brain.id());
} }
} }
Some(ReviewAction::Ignore) => { Some(ReviewAction::Ignore) => {
// Do nothing. Hunk will be reviewable again next time. // Do nothing. Hunk will be reviewable again next time.
if let Some(h) = header { file.ignore_item(&item);
delta += h.new_size as i32 - h.old_size as i32;
}
} }
Some(ReviewAction::Comment) => { Some(ReviewAction::Comment) => {
if let Some(hunk) = hunk { let (old, new) = item.paths();
let mut builder = let path = old.or(new);
CommentBuilder::new(revision.head(), item.file.path().to_path_buf());
if let (Some(hunk), Some((path, _))) = (item.hunk(), path) {
let mut builder = CommentBuilder::new(revision.head(), path.to_path_buf());
builder.edit(hunk)?; builder.edit(hunk)?;
let _comments = builder.comments(); let _comments = builder.comments();
@ -406,10 +666,30 @@ impl<'a> ReviewBuilder<'a> {
Ok(()) Ok(())
} }
fn diff(
&self,
brain: &git::raw::Tree<'_>,
tree: &git::raw::Tree<'_>,
repo: &'a git::raw::Repository,
opts: &mut git::raw::DiffOptions,
) -> Result<Diff, Error> {
let mut find_opts = git::raw::DiffFindOptions::new();
find_opts.exact_match_only(true);
find_opts.all(true);
find_opts.copies(false); // We don't support finding copies at the moment.
let mut diff = repo.diff_tree_to_tree(Some(brain), Some(tree), Some(opts))?;
diff.find_similar(Some(&mut find_opts))?;
let diff = Diff::try_from(diff)?;
Ok(diff)
}
fn prompt( fn prompt(
&self, &self,
mut input: impl io::BufRead, mut input: impl io::BufRead,
mut output: &mut io::StderrLock, mut output: &mut impl PromptWriter,
progress: impl fmt::Display, progress: impl fmt::Display,
) -> Option<ReviewAction> { ) -> Option<ReviewAction> {
if let Some(v) = self.verdict { if let Some(v) = self.verdict {
@ -446,9 +726,13 @@ enum Error {
#[error(transparent)] #[error(transparent)]
Diff(#[from] unified_diff::Error), Diff(#[from] unified_diff::Error),
#[error(transparent)] #[error(transparent)]
Surf(#[from] radicle_surf::diff::git::error::Diff),
#[error(transparent)]
Io(#[from] io::Error), Io(#[from] io::Error),
#[error(transparent)] #[error(transparent)]
Format(#[from] std::fmt::Error), Format(#[from] std::fmt::Error),
#[error(transparent)]
Git(#[from] git::raw::Error),
} }
#[derive(Debug)] #[derive(Debug)]