rad:z3gqcJUoA1n9HaHKufZs5FCSGazv5 heartwood297646412ad38be27ab515245066b3783d83736f
{
"request": "trigger",
"version": 1,
"event_type": "patch",
"repository": {
"id": "rad:z3gqcJUoA1n9HaHKufZs5FCSGazv5",
"name": "heartwood",
"description": "Radicle Heartwood Protocol & Stack",
"private": false,
"default_branch": "master",
"delegates": [
"did:key:z6MksFqXN3Yhqk8pTJdUGLwATkRfQvwZXPqR2qMEhbS9wzpT",
"did:key:z6MkireRatUThvd3qzfKht1S44wpm4FEWSSa4PRMTSQZ3voM",
"did:key:z6MkkPvBfjP4bQmco5Dm7UGsX2ruDBieEHi8n9DVJWX5sTEz"
]
},
"action": "Created",
"patch": {
"id": "1ab0f223667f0dd819260f3ce367195313703a32",
"author": {
"id": "did:key:z6MksFqXN3Yhqk8pTJdUGLwATkRfQvwZXPqR2qMEhbS9wzpT",
"alias": "cloudhead"
},
"title": "cli: Use pretty diffs for code review",
"state": {
"status": "merged",
"conflicts": []
},
"before": "54551b117435a006acddefbf3850b02d6ee84ffa",
"after": "297646412ad38be27ab515245066b3783d83736f",
"commits": [
"297646412ad38be27ab515245066b3783d83736f",
"823ece875e48abc83c302ca2bee53dad16a8b2ef",
"795ee35f37c1823621525baa43f07380a6a911c5"
],
"target": "7e1cb406bb935c3de008d2171e8961f57b4336ea",
"labels": [],
"assignees": [],
"revisions": [
{
"id": "1ab0f223667f0dd819260f3ce367195313703a32",
"author": {
"id": "did:key:z6MksFqXN3Yhqk8pTJdUGLwATkRfQvwZXPqR2qMEhbS9wzpT",
"alias": "cloudhead"
},
"description": "We use the pretty diff renderer for code review.",
"base": "54551b117435a006acddefbf3850b02d6ee84ffa",
"oid": "297646412ad38be27ab515245066b3783d83736f",
"timestamp": 1720795194
}
]
}
}
{
"response": "triggered",
"run_id": {
"id": "82048b33-97bd-4f86-ba06-3f76569a9d95"
},
"info_url": "https://cci.rad.levitte.org//82048b33-97bd-4f86-ba06-3f76569a9d95.html"
}
Started at: 2026-09-09 01:09:49.248411+02:00
Commands:
$ rad clone rad:z3gqcJUoA1n9HaHKufZs5FCSGazv5 .
✓ Creating checkout in ./...
✓ Remote cloudhead@z6MksFqXN3Yhqk8pTJdUGLwATkRfQvwZXPqR2qMEhbS9wzpT added
✓ Remote-tracking branch cloudhead@z6MksFqXN3Yhqk8pTJdUGLwATkRfQvwZXPqR2qMEhbS9wzpT/master created for z6MksFqXN3Yhqk8pTJdUGLwATkRfQvwZXPqR2qMEhbS9wzpT
✓ Remote fintohaps@z6MkireRatUThvd3qzfKht1S44wpm4FEWSSa4PRMTSQZ3voM added
✓ Remote-tracking branch fintohaps@z6MkireRatUThvd3qzfKht1S44wpm4FEWSSa4PRMTSQZ3voM/master created for z6MkireRatUThvd3qzfKht1S44wpm4FEWSSa4PRMTSQZ3voM
✓ Remote lorenz@z6MkkPvBfjP4bQmco5Dm7UGsX2ruDBieEHi8n9DVJWX5sTEz added
✓ Remote-tracking branch lorenz@z6MkkPvBfjP4bQmco5Dm7UGsX2ruDBieEHi8n9DVJWX5sTEz/master created for z6MkkPvBfjP4bQmco5Dm7UGsX2ruDBieEHi8n9DVJWX5sTEz
✓ Repository successfully cloned under /opt/radcis/ci.rad.levitte.org/cci/state/82048b33-97bd-4f86-ba06-3f76569a9d95/w/
╭────────────────────────────────────╮
│ heartwood │
│ Radicle Heartwood Protocol & Stack │
│ 189 issues · 39 patches │
╰────────────────────────────────────╯
Run `cd ./.` to go to the repository directory.
Exit code: 0
$ rad patch checkout 1ab0f223667f0dd819260f3ce367195313703a32
✓ Switched to branch patch/1ab0f22 at revision 1ab0f22
✓ Branch patch/1ab0f22 setup to track rad/patches/1ab0f223667f0dd819260f3ce367195313703a32
Exit code: 0
$ git config advice.detachedHead false
Exit code: 0
$ git checkout 297646412ad38be27ab515245066b3783d83736f
HEAD is now at 29764641 cli: Use pretty diffs for code review
Exit code: 0
$ rad patch show 1ab0f223667f0dd819260f3ce367195313703a32 -p
╭────────────────────────────────────────────────────────────────────────────────╮
│ Title cli: Use pretty diffs for code review │
│ Patch 1ab0f223667f0dd819260f3ce367195313703a32 │
│ Author cloudhead z6MksFq…bS9wzpT │
│ Head 297646412ad38be27ab515245066b3783d83736f │
│ Base 54551b117435a006acddefbf3850b02d6ee84ffa │
│ Target master │
│ Branches patch/1ab0f22 │
│ Commits ahead 0, behind 1255 │
│ Status merged │
│ │
│ We use the pretty diff renderer for code review. │
├────────────────────────────────────────────────────────────────────────────────┤
│ 2976464 cli: Use pretty diffs for code review │
│ 823ece8 cli: Improve pretty diff printing │
│ 795ee35 cli: Output tree hash after review update │
├────────────────────────────────────────────────────────────────────────────────┤
│ ● Revision 1ab0f22 @ 54551b1..2976464 by cloudhead z6MksFq…bS9wzpT 2 years ago │
│ └─ ✓ merged by cloudhead z6MktaN…hzPZRZW │
╰────────────────────────────────────────────────────────────────────────────────╯
commit 297646412ad38be27ab515245066b3783d83736f
Author: cloudhead <cloudhead@radicle.xyz>
Date: Fri Jul 12 16:13:20 2024 +0200
cli: Use pretty diffs for code review
We use the pretty diff renderer for code review.
diff --git a/Cargo.toml b/Cargo.toml
index 42cef9bae1..ed3644b987 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -33,3 +33,7 @@ resolver = "2"
inherits = "release"
debug = true
incremental = false
+
+[workspace.lints]
+clippy.type_complexity = "allow"
+clippy.enum_variant_names = "allow"
diff --git a/radicle-cli/Cargo.toml b/radicle-cli/Cargo.toml
index 9de7d03da6..56771a20e3 100644
--- a/radicle-cli/Cargo.toml
+++ b/radicle-cli/Cargo.toml
@@ -71,3 +71,6 @@ path = "../radicle-term"
pretty_assertions = { version = "1.3.0" }
radicle = { version = "0.11.0", path = "../radicle", features = ["test"] }
radicle-node = { path = "../radicle-node", features = ["test"] }
+
+[lints]
+workspace = true
diff --git a/radicle-cli/examples/rad-review-by-hunk.md b/radicle-cli/examples/rad-review-by-hunk.md
index 4a13ee2798..2abaaba6d2 100644
--- a/radicle-cli/examples/rad-review-by-hunk.md
+++ b/radicle-cli/examples/rad-review-by-hunk.md
@@ -71,46 +71,47 @@ match `git diff master -W100% -U5 --patience`:
```
$ rad patch review --patch -U5 7a2ac7e2841cc1e7394f99f107555a499b1d3f23 --no-announce
-diff --git a/.gitignore b/.gitignore
-deleted file mode 100644
-index 7937fb3..0000000
---- a/.gitignore
-+++ /dev/null
-@@ -1 +0,0 @@
--*.draft
-diff --git a/DISCLAIMER.txt b/DISCLAIMER.txt
-new file mode 100644
-index 0000000..2b5bd86
---- /dev/null
-+++ b/DISCLAIMER.txt
-@@ -0,0 +1 @@
-+All food is served as-is, with no warranty!
-diff --git a/MENU.txt b/MENU.txt
-index 867958c..3af9741 100644
---- a/MENU.txt
-+++ b/MENU.txt
-@@ -1,7 +1,8 @@
- Classics
- --------
-+Baked Brie
- Salmon Tartare
- Mac & Cheese
-[..]
- Comfort Food
- ------------
-@@ -9,6 +10,7 @@ Reuben Sandwich
- Club Sandwich
- Fried Shrimp Basket
-[..]
- Sides
- -----
--French Fries
-+French Fries!
-+Garlic Green Beans
-diff --git a/INSTRUCTIONS.txt b/notes/INSTRUCTIONS.txt
-similarity index 100%
-rename from INSTRUCTIONS.txt
-rename to notes/INSTRUCTIONS.txt
+╭──────────────────────╮
+│ .gitignore ❲deleted❳ │
+├──────────────────────┤
+│ @@ -1,1 +0,0 @@ │
+│ 1 - *.draft │
+╰──────────────────────╯
+╭──────────────────────────────────────────────────────────╮
+│ DISCLAIMER.txt ❲created❳ │
+├──────────────────────────────────────────────────────────┤
+│ @@ -0,0 +1,1 @@ │
+│ 1 + All food is served as-is, with no warranty! │
+╰──────────────────────────────────────────────────────────╯
+╭─────────────────────────────╮
+│ MENU.txt │
+├─────────────────────────────┤
+│ @@ -1,7 +1,8 @@ │
+│ 1 1 Classics │
+│ 2 2 -------- │
+│ 3 + Baked Brie │
+│ 3 4 Salmon Tartare │
+│ 4 5 Mac & Cheese │
+│ 5 6 │
+│ 6 7 Comfort Food │
+│ 7 8 ------------ │
+╰─────────────────────────────╯
+╭──────────────────────────────────╮
+│ MENU.txt │
+├──────────────────────────────────┤
+│ @@ -9,6 +10,7 @@ Reuben Sandwich │
+│ 9 10 Club Sandwich │
+│ 10 11 Fried Shrimp Basket │
+│ 11 12 │
+│ 12 13 Sides │
+│ 13 14 ----- │
+│ 14 - French Fries │
+│ 15 + French Fries! │
+│ 16 + Garlic Green Beans │
+╰──────────────────────────────────╯
+╭────────────────────────────────────────────────────╮
+│ INSTRUCTIONS.txt -> notes/INSTRUCTIONS.txt ❲moved❳ │
+╰────────────────────────────────────────────────────╯
```
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
✓ Loaded existing review ([..]) for patch 7a2ac7e2841cc1e7394f99f107555a499b1d3f23
-diff --git a/.gitignore b/.gitignore
-deleted file mode 100644
-index 7937fb3..0000000
---- a/.gitignore
-+++ /dev/null
-@@ -1 +0,0 @@
--*.draft
+╭──────────────────────╮
+│ .gitignore ❲deleted❳ │
+├──────────────────────┤
+│ @@ -1,1 +0,0 @@ │
+│ 1 - *.draft │
+╰──────────────────────╯
✓ Updated review tree to a5fccf0e977225ff13c3f74c43faf4cb679bf835
```
```
$ rad patch review --patch --accept --hunk 1 7a2ac7e2841cc1e7394f99f107555a499b1d3f23 --no-announce
✓ Loaded existing review ([..]) for patch 7a2ac7e2841cc1e7394f99f107555a499b1d3f23
-diff --git a/DISCLAIMER.txt b/DISCLAIMER.txt
-new file mode 100644
-index 0000000..2b5bd86
---- /dev/null
-+++ b/DISCLAIMER.txt
-@@ -0,0 +1 @@
-+All food is served as-is, with no warranty!
+╭──────────────────────────────────────────────────────────╮
+│ DISCLAIMER.txt ❲created❳ │
+├──────────────────────────────────────────────────────────┤
+│ @@ -0,0 +1,1 @@ │
+│ 1 + All food is served as-is, with no warranty! │
+╰──────────────────────────────────────────────────────────╯
✓ Updated review tree to 2cdb82ea726e64d3b52847c7699d0d4759198f5c
```
```
$ rad patch review --patch --accept -U3 --hunk 1 7a2ac7e2841cc1e7394f99f107555a499b1d3f23 --no-announce
✓ Loaded existing review ([..]) for patch 7a2ac7e2841cc1e7394f99f107555a499b1d3f23
-diff --git a/MENU.txt b/MENU.txt
-index 867958c..3af9741 100644
---- a/MENU.txt
-+++ b/MENU.txt
-@@ -1,5 +1,6 @@
- Classics
- --------
-+Baked Brie
- Salmon Tartare
- Mac & Cheese
-[..]
+╭─────────────────────────────╮
+│ MENU.txt │
+├─────────────────────────────┤
+│ @@ -1,5 +1,6 @@ │
+│ 1 1 Classics │
+│ 2 2 -------- │
+│ 3 + Baked Brie │
+│ 3 4 Salmon Tartare │
+│ 4 5 Mac & Cheese │
+│ 5 6 │
+╰─────────────────────────────╯
✓ Updated review tree to d4aecbb859a802a3215def0b538358bf63593953
```
```
$ rad patch review --patch --accept -U3 --hunk 1 7a2ac7e2841cc1e7394f99f107555a499b1d3f23 --no-announce
✓ Loaded existing review ([..]) for patch 7a2ac7e2841cc1e7394f99f107555a499b1d3f23
-diff --git a/MENU.txt b/MENU.txt
-index 4e2e828..3af9741 100644
---- a/MENU.txt
-+++ b/MENU.txt
-@@ -12,4 +12,5 @@ Fried Shrimp Basket
-[..]
- Sides
- -----
--French Fries
-+French Fries!
-+Garlic Green Beans
+╭───────────────────────────────────────╮
+│ MENU.txt │
+├───────────────────────────────────────┤
+│ @@ -12,4 +12,5 @@ Fried Shrimp Basket │
+│ 12 12 │
+│ 13 13 Sides │
+│ 14 14 ----- │
+│ 15 - French Fries │
+│ 15 + French Fries! │
+│ 16 + Garlic Green Beans │
+╰───────────────────────────────────────╯
✓ Updated review tree to 59cee720b0642b1491b241400912b35926a76c3f
```
```
$ rad patch review --patch --accept --hunk 1 7a2ac7e2841cc1e7394f99f107555a499b1d3f23 --no-announce
✓ Loaded existing review ([..]) for patch 7a2ac7e2841cc1e7394f99f107555a499b1d3f23
-diff --git a/INSTRUCTIONS.txt b/notes/INSTRUCTIONS.txt
-similarity index 100%
-rename from INSTRUCTIONS.txt
-rename to notes/INSTRUCTIONS.txt
+╭────────────────────────────────────────────────────╮
+│ INSTRUCTIONS.txt -> notes/INSTRUCTIONS.txt ❲moved❳ │
+╰────────────────────────────────────────────────────╯
✓ Updated review tree to 3effc8f6462fa2573697072245e57708c4dcbe62
```
diff --git a/radicle-cli/src/commands/patch/review/builder.rs b/radicle-cli/src/commands/patch/review/builder.rs
index 9694841e5f..6fb6a1c4d4 100644
--- a/radicle-cli/src/commands/patch/review/builder.rs
+++ b/radicle-cli/src/commands/patch/review/builder.rs
@@ -13,9 +13,8 @@
//!
use std::collections::VecDeque;
use std::fmt::Write as _;
-use std::io::IsTerminal as _;
-use std::ops::{Not, Range};
-use std::path::PathBuf;
+use std::ops::{Deref, Not, Range};
+use std::path::{Path, PathBuf};
use std::str::FromStr;
use std::{fmt, io};
@@ -26,10 +25,14 @@ use radicle::prelude::*;
use radicle::storage::git::Repository;
use radicle_git_ext::Oid;
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::terminal as term;
+use crate::terminal::highlight::Highlighter;
/// Help message shown to user.
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
? - 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.
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub enum ReviewAction {
@@ -113,58 +134,345 @@ impl FromStr for ReviewAction {
}
/// A single review item. Can be a hunk or eg. a file move.
-pub struct ReviewItem<'a> {
- file: &'a FileDiff,
- hunk: Option<&'a Hunk<Modification>>,
+/// Files are usually split into multiple review items.
+#[derive(Debug)]
+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.
#[derive(Default)]
-pub struct ReviewQueue<'a> {
- queue: VecDeque<(usize, ReviewItem<'a>)>,
+pub struct ReviewQueue {
+ /// Hunks left to review.
+ queue: VecDeque<(usize, ReviewItem)>,
}
-impl<'a> ReviewQueue<'a> {
- /// Push an item to the queue.
- fn push(&mut self, file: &'a FileDiff, hunks: Option<&'a Hunks<Modification>>) {
- let mut queue_item = |hunk| {
- self.queue
- .push_back((self.queue.len(), ReviewItem { file, hunk }))
- };
+impl ReviewQueue {
+ /// Add a file to the queue.
+ /// Mostly splits files into individual review items (eg. hunks) to review.
+ fn add_file(&mut self, file: FileDiff) {
+ let header = FileHeader::from(&file);
- if let Some(hunks) = hunks {
- for hunk in hunks.iter() {
- queue_item(Some(hunk));
+ match file {
+ FileDiff::Moved(moved) => {
+ 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> {
- type Target = VecDeque<(usize, ReviewItem<'a>)>;
+impl std::ops::Deref for ReviewQueue {
+ type Target = VecDeque<(usize, ReviewItem)>;
fn deref(&self) -> &Self::Target {
&self.queue
}
}
-impl<'a> std::ops::DerefMut for ReviewQueue<'a> {
+impl std::ops::DerefMut for ReviewQueue {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.queue
}
}
-impl<'a> Iterator for ReviewQueue<'a> {
- type Item = (usize, ReviewItem<'a>);
+impl Iterator for ReviewQueue {
+ type Item = (usize, ReviewItem);
fn next(&mut self) -> Option<Self::Item> {
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> {
/// Patch being reviewed.
patch_id: PatchId,
@@ -215,8 +523,13 @@ impl<'a> ReviewBuilder<'a> {
commit.tree()?
};
+ let stdout = io::stdout().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() {
term::success!(
"Loaded existing review {} for patch {}",
@@ -237,51 +550,22 @@ impl<'a> ReviewBuilder<'a> {
repo.find_commit(oid)?
};
let mut brain = review.tree()?;
- let mut writer = unified_diff::Writer::new(io::stdout()).styled(true);
- let mut queue = ReviewQueue::default(); // Queue of hunks to review.
- let mut current = None; // File of the current hunk.
+ let mut queue = ReviewQueue::default();
+ let diff = self.diff(&brain, &tree, repo, opts)?;
- 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))?;
-
- if diff.deltas().next().is_none() {
+ // Build the review queue.
+ for file in diff.into_files() {
+ queue.add_file(file);
+ }
+ if queue.is_empty() {
term::success!("All hunks have been reviewed");
return Ok(());
}
- let diff = Diff::try_from(diff)?;
- for file in diff.files() {
- match file {
- FileDiff::Modified(f) => match &f.diff {
- 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");
- }
- }
- }
+ // File review for the current file. Starts out as `None` and is set on the first hunk.
+ // Keeps track of deltas for hunk offsets.
+ let mut file: Option<FileReviewBuilder> = None;
let total = queue.len();
- let mut delta: i32 = 0;
while let Some((ix, item)) = queue.next() {
if let Some(hunk) = self.hunk {
@@ -290,66 +574,42 @@ impl<'a> ReviewBuilder<'a> {
}
}
let progress = term::format::secondary(format!("({}/{total})", ix + 1));
- let ReviewItem { file, hunk } = item;
-
- if current.map_or(true, |c| c != file) {
- writer.encode(&unified_diff::FileHeader::from(file))?;
- current = Some(file);
- delta = 0;
- }
-
- let header = hunk
- .map(|h| {
- let header = unified_diff::HunkHeader::try_from(h)?;
- writer.encode(h)?;
- Ok::<_, anyhow::Error>(header)
- })
- .transpose()?;
+ 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(),
+ )?;
- match self.prompt(&mut stdin, &mut stderr, progress) {
+ // Prompts the user for action on the above hunk.
+ match self.prompt(&mut stdin, &mut writer, progress) {
+ // When a hunk is accepted, we convert it to unified diff format,
+ // and apply it to the `brain`.
Some(ReviewAction::Accept) => {
- let mut buf = Vec::new();
- {
- let mut writer = unified_diff::Writer::new(&mut buf);
- writer.encode(&unified_diff::FileHeader::from(file))?;
-
- 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 =
+ // Update brain with accepted hunk.
+ file.apply_item(item, &mut brain, repo)?;
+ // Update review with new brain.
+ let review_oid =
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() {
- term::success!("Updated review tree to {brain_oid}");
+ term::success!("Updated review tree to {}", brain.id());
}
}
Some(ReviewAction::Ignore) => {
// Do nothing. Hunk will be reviewable again next time.
- if let Some(h) = header {
- delta += h.new_size as i32 - h.old_size as i32;
- }
+ file.ignore_item(&item);
}
Some(ReviewAction::Comment) => {
- if let Some(hunk) = hunk {
- let mut builder =
- CommentBuilder::new(revision.head(), item.file.path().to_path_buf());
+ let (old, new) = item.paths();
+ let path = old.or(new);
+
+ if let (Some(hunk), Some((path, _))) = (item.hunk(), path) {
+ let mut builder = CommentBuilder::new(revision.head(), path.to_path_buf());
builder.edit(hunk)?;
let _comments = builder.comments();
@@ -406,10 +666,30 @@ impl<'a> ReviewBuilder<'a> {
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(
&self,
mut input: impl io::BufRead,
- mut output: &mut io::StderrLock,
+ mut output: &mut impl PromptWriter,
progress: impl fmt::Display,
) -> Option<ReviewAction> {
if let Some(v) = self.verdict {
@@ -446,9 +726,13 @@ enum Error {
#[error(transparent)]
Diff(#[from] unified_diff::Error),
#[error(transparent)]
+ Surf(#[from] radicle_surf::diff::git::error::Diff),
+ #[error(transparent)]
Io(#[from] io::Error),
#[error(transparent)]
Format(#[from] std::fmt::Error),
+ #[error(transparent)]
+ Git(#[from] git::raw::Error),
}
#[derive(Debug)]
commit 823ece875e48abc83c302ca2bee53dad16a8b2ef
Author: cloudhead <cloudhead@radicle.xyz>
Date: Fri Jul 12 14:20:13 2024 +0200
cli: Improve pretty diff printing
Refactor, cleanup, improve tests etc.
diff --git a/radicle-cli/examples/rad-diff.md b/radicle-cli/examples/rad-diff.md
index 3c07e63cd8..64ce618a09 100644
--- a/radicle-cli/examples/rad-diff.md
+++ b/radicle-cli/examples/rad-diff.md
@@ -81,3 +81,79 @@ $ rad diff --staged
╰──────────────────────────────────────────────╯
```
+
+```
+$ git rm -f -q main.c
+$ rad diff --staged
+╭────────────────────────────────────────────╮
+│ main.c -6 ❲deleted❳ │
+├────────────────────────────────────────────┤
+│ @@ -1,6 +0,0 @@ │
+│ 1 - #include <stdio.h> │
+│ 2 - │
+│ 3 - int main(void) { │
+│ 4 - printf("Hello World!/n"); │
+│ 5 - return 0; │
+│ 6 - } │
+╰────────────────────────────────────────────╯
+
+```
+
+For now, copies are not detected.
+
+```
+$ git reset --hard master -q
+$ mkdir docs
+$ cp README.md docs/README.md
+$ git add docs
+$ rad diff --staged
+╭─────────────────────────────╮
+│ docs/README.md +1 ❲created❳ │
+├─────────────────────────────┤
+│ @@ -0,0 +1,1 @@ │
+│ 1 + Hello World! │
+╰─────────────────────────────╯
+
+$ git reset
+$ git checkout .
+```
+
+Empty file.
+
+```
+$ touch EMPTY
+$ git add EMPTY
+$ rad diff --staged
+╭─────────────────╮
+│ EMPTY ❲created❳ │
+╰─────────────────╯
+
+$ git reset
+$ git checkout .
+```
+
+File mode change.
+
+```
+$ chmod +x README.md
+$ rad diff
+╭───────────────────────────────────────────╮
+│ README.md 100644 -> 100755 ❲mode changed❳ │
+╰───────────────────────────────────────────╯
+
+$ git reset -q
+$ git checkout .
+```
+
+Binary file.
+
+```
+$ touch file.bin
+$ truncate -s 8 file.bin
+$ git add file.bin
+$ rad diff --staged
+╭─────────────────────────────╮
+│ file.bin ❲binary❳ ❲created❳ │
+╰─────────────────────────────╯
+
+```
diff --git a/radicle-cli/src/git/ddiff.rs b/radicle-cli/src/git/ddiff.rs
index 0cc6b28793..29761578ca 100644
--- a/radicle-cli/src/git/ddiff.rs
+++ b/radicle-cli/src/git/ddiff.rs
@@ -245,6 +245,7 @@ impl From<&FileDDiff> for unified_diff::FileHeader {
path: value.path.clone(),
old: value.old.clone(),
new: value.new.clone(),
+ binary: false,
}
}
}
diff --git a/radicle-cli/src/git/pretty_diff.rs b/radicle-cli/src/git/pretty_diff.rs
index 5098d4574d..22187398e6 100644
--- a/radicle-cli/src/git/pretty_diff.rs
+++ b/radicle-cli/src/git/pretty_diff.rs
@@ -1,11 +1,14 @@
use std::fs;
-use std::path::Path;
+use std::path::{Path, PathBuf};
use radicle::git;
+use radicle_git_ext::Oid;
use radicle_surf::diff;
+use radicle_surf::diff::{Added, Copied, Deleted, FileStats, Hunks, Modified, Moved};
use radicle_surf::diff::{Diff, DiffContent, FileDiff, Hunk, Modification};
use radicle_term as term;
use term::cell::Cell;
+use term::VStack;
use crate::git::unified_diff::FileHeader;
use crate::terminal::highlight::{Highlighter, Theme};
@@ -62,10 +65,59 @@ impl Repo for git::raw::Repository {
}
/// Blobs passed down to the hunk renderer.
-#[derive(Debug, Default)]
-pub struct Blobs {
- old: Option<Vec<term::Line>>,
- new: Option<Vec<term::Line>>,
+#[derive(Debug)]
+pub struct Blobs<T> {
+ pub old: Option<T>,
+ pub new: Option<T>,
+}
+
+impl<T> Blobs<T> {
+ pub fn new(old: Option<T>, new: Option<T>) -> Self {
+ Self { old, new }
+ }
+}
+
+impl Blobs<(PathBuf, Blob)> {
+ pub fn highlight(&self, hi: &mut Highlighter) -> Blobs<Vec<term::Line>> {
+ let mut blobs = Blobs::default();
+ if let Some((path, Blob::Plain(content))) = &self.old {
+ blobs.old = hi.highlight(path, content).ok();
+ }
+ if let Some((path, Blob::Plain(content))) = &self.new {
+ blobs.new = hi.highlight(path, content).ok();
+ }
+ blobs
+ }
+
+ pub fn from_paths<R: Repo>(
+ old: Option<(&Path, Oid)>,
+ new: Option<(&Path, Oid)>,
+ repo: &R,
+ ) -> Blobs<(PathBuf, Blob)> {
+ Blobs::new(
+ old.and_then(|(path, oid)| {
+ repo.blob(oid)
+ .ok()
+ .or_else(|| repo.file(path))
+ .map(|blob| (path.to_path_buf(), blob))
+ }),
+ new.and_then(|(path, oid)| {
+ repo.blob(oid)
+ .ok()
+ .or_else(|| repo.file(path))
+ .map(|blob| (path.to_path_buf(), blob))
+ }),
+ )
+ }
+}
+
+impl<T> Default for Blobs<T> {
+ fn default() -> Self {
+ Self {
+ old: None,
+ new: None,
+ }
+ }
}
/// Types that can be rendered as pretty diffs.
@@ -96,39 +148,108 @@ impl ToPretty for Diff {
) -> Self::Output {
term::VStack::default()
.padding(0)
- .children(self.files().map(|f| f.pretty(hi, context, repo).boxed()))
+ .children(self.files().flat_map(|f| {
+ [
+ f.pretty(hi, context, repo).boxed(),
+ term::Line::blank().boxed(), // Blank line between files.
+ ]
+ }))
}
}
impl ToPretty for FileHeader {
type Output = term::Line;
- type Context = ();
+ type Context = Option<FileStats>;
fn pretty<R: Repo>(
&self,
_hi: &mut Highlighter,
- _context: &Self::Context,
+ stats: &Self::Context,
_repo: &R,
) -> Self::Output {
- match self {
- FileHeader::Added { path, .. } => term::Line::new(path.display().to_string()),
+ let theme = Theme::default();
+ let (mut header, badge, binary) = match self {
+ FileHeader::Added { path, binary, .. } => (
+ term::Line::new(path.display().to_string()),
+ Some(term::format::badge_positive("created")),
+ *binary,
+ ),
FileHeader::Moved {
old_path, new_path, ..
- } => term::Line::spaced([
- term::label(old_path.display().to_string()),
- term::label("->".to_string()),
- term::label(new_path.display().to_string()),
- ]),
- FileHeader::Deleted { path, .. } => term::Line::new(path.display().to_string()),
- FileHeader::Modified { path, .. } => term::Line::new(path.display().to_string()),
+ } => (
+ term::Line::spaced([
+ term::label(old_path.display().to_string()),
+ term::label("->".to_string()),
+ term::label(new_path.display().to_string()),
+ ]),
+ Some(term::format::badge_secondary("moved")),
+ false,
+ ),
+ FileHeader::Deleted { path, binary, .. } => (
+ term::Line::new(path.display().to_string()),
+ Some(term::format::badge_negative("deleted")),
+ *binary,
+ ),
+ FileHeader::Modified {
+ path,
+ old,
+ new,
+ binary,
+ ..
+ } => {
+ if old.mode != new.mode {
+ (
+ term::Line::spaced([
+ term::label(path.display().to_string()),
+ term::label(format!("{:o}", u32::from(old.mode.clone())))
+ .fg(term::Color::Blue),
+ term::label("->".to_string()),
+ term::label(format!("{:o}", u32::from(new.mode.clone())))
+ .fg(term::Color::Blue),
+ ]),
+ Some(term::format::badge_secondary("mode changed")),
+ *binary,
+ )
+ } else {
+ (term::Line::new(path.display().to_string()), None, *binary)
+ }
+ }
FileHeader::Copied {
old_path, new_path, ..
- } => term::Line::spaced([
- term::label(old_path.display().to_string()),
- term::label("->".to_string()),
- term::label(new_path.display().to_string()),
- ]),
+ } => (
+ term::Line::spaced([
+ term::label(old_path.display().to_string()),
+ term::label("->".to_string()),
+ term::label(new_path.display().to_string()),
+ ]),
+ Some(term::format::badge_secondary("copied")),
+ false,
+ ),
+ };
+
+ if binary {
+ header.push(term::Label::space());
+ header.push(term::label(term::format::badge_yellow("binary")));
+ }
+
+ let (additions, deletions) = if let Some(stats) = stats {
+ (stats.additions, stats.deletions)
+ } else {
+ (0, 0)
+ };
+ if deletions > 0 {
+ header.push(term::Label::space());
+ header.push(term::label(format!("-{deletions}")).fg(theme.color("negative.light")));
+ }
+ if additions > 0 {
+ header.push(term::Label::space());
+ header.push(term::label(format!("+{additions}")).fg(theme.color("positive.light")));
}
+ if let Some(badge) = badge {
+ header.push(term::Label::space());
+ header.push(badge);
+ }
+ header
}
}
@@ -142,128 +263,141 @@ impl ToPretty for FileDiff {
_context: &Self::Context,
repo: &R,
) -> Self::Output {
- let content = match self {
- FileDiff::Added(f) => f.diff.pretty(hi, self, repo),
- FileDiff::Moved(f) => f.diff.pretty(hi, self, repo),
- FileDiff::Deleted(f) => f.diff.pretty(hi, self, repo),
- FileDiff::Modified(f) => f.diff.pretty(hi, self, repo),
- FileDiff::Copied(f) => f.diff.pretty(hi, self, repo),
- };
- term::VStack::default()
- .padding(0)
- .child(content)
- .child(term::Line::blank())
+ let header = FileHeader::from(self);
+
+ match self {
+ FileDiff::Added(f) => f.pretty(hi, &header, repo),
+ FileDiff::Deleted(f) => f.pretty(hi, &header, repo),
+ FileDiff::Modified(f) => f.pretty(hi, &header, repo),
+ FileDiff::Moved(f) => f.pretty(hi, &header, repo),
+ FileDiff::Copied(f) => f.pretty(hi, &header, repo),
+ }
}
}
impl ToPretty for DiffContent {
type Output = term::VStack<'static>;
- type Context = FileDiff;
+ type Context = Blobs<(PathBuf, Blob)>;
fn pretty<R: Repo>(
&self,
hi: &mut Highlighter,
- context: &Self::Context,
+ blobs: &Self::Context,
repo: &R,
) -> Self::Output {
- let header = FileHeader::from(context);
- let theme = Theme::default();
-
- let (old, new, badge) = match context {
- FileDiff::Added(f) => (
- None,
- Some((f.new.oid, f.path.clone())),
- Some(term::format::badge_positive("created")),
- ),
- FileDiff::Moved(f) => (
- Some((f.old.oid, f.old_path.clone())),
- Some((f.new.oid, f.new_path.clone())),
- Some(term::format::badge_secondary("moved")),
- ),
- FileDiff::Deleted(f) => (
- Some((f.old.oid, f.path.clone())),
- None,
- Some(term::format::badge_negative("deleted")),
- ),
- FileDiff::Modified(f) => (
- Some((f.old.oid, f.path.clone())),
- Some((f.new.oid, f.path.clone())),
- None,
- ),
- FileDiff::Copied(f) => (
- Some((f.old.oid, f.old_path.clone())),
- Some((f.old.oid, f.new_path.clone())),
- Some(term::format::badge_secondary("copied")),
- ),
- };
- let mut header = header.pretty(hi, &(), repo);
+ let mut vstack = term::VStack::default().padding(0);
- let (additions, deletions) = if let Some(stats) = self.stats() {
- (stats.additions, stats.deletions)
- } else {
- (0, 0)
- };
+ match self {
+ DiffContent::Plain {
+ hunks: Hunks(hunks),
+ ..
+ } => {
+ let blobs = blobs.highlight(hi);
- if deletions > 0 {
- header.push(term::Label::space());
- header.push(term::label(format!("-{deletions}")).fg(theme.color("negative.light")));
- }
- if additions > 0 {
- header.push(term::Label::space());
- header.push(term::label(format!("+{additions}")).fg(theme.color("positive.light")));
- }
- if let Some(badge) = badge {
- header.push(term::Label::space());
- header.push(badge);
+ for (i, h) in hunks.iter().enumerate() {
+ vstack.push(h.pretty(hi, &blobs, repo));
+ if i != hunks.len() - 1 {
+ vstack = vstack.divider();
+ }
+ }
+ }
+ DiffContent::Empty => {}
+ DiffContent::Binary => {}
}
+ vstack
+ }
+}
- let old = old.and_then(|(oid, path)| repo.blob(oid).ok().or_else(|| repo.file(&path)));
- let new = new.and_then(|(oid, path)| repo.blob(oid).ok().or_else(|| repo.file(&path)));
- let mut blobs = Blobs::default();
+impl ToPretty for Moved {
+ type Output = term::VStack<'static>;
+ type Context = FileHeader;
- if let Some(Blob::Plain(content)) = old {
- blobs.old = hi.highlight(context.path(), &content).ok();
- }
- if let Some(Blob::Plain(content)) = new {
- blobs.new = hi.highlight(context.path(), &content).ok();
- }
- let mut vstack = term::VStack::default()
+ fn pretty<R: Repo>(
+ &self,
+ hi: &mut Highlighter,
+ header: &Self::Context,
+ repo: &R,
+ ) -> Self::Output {
+ let header = header.pretty(hi, &self.diff.stats().copied(), repo);
+
+ term::VStack::default()
.border(Some(term::colors::FAINT))
.padding(1)
- .child(term::Line::default().extend(header));
+ .child(term::Line::default().extend(header))
+ }
+}
- match context {
- FileDiff::Moved(_) | FileDiff::Copied(_) => {}
- FileDiff::Added(_) if blobs.new.is_none() => {
- vstack = vstack.divider();
- vstack.push(term::Line::new(term::format::italic("Empty file")));
- }
- FileDiff::Deleted(_) if blobs.old.is_none() => {
- vstack = vstack.divider();
- vstack.push(term::Line::new(term::format::italic("Empty file")));
- }
- FileDiff::Added(_) | FileDiff::Deleted(_) | FileDiff::Modified(_) => {
- vstack = vstack.divider();
-
- match self {
- DiffContent::Plain { hunks, .. } => {
- for (i, h) in hunks.iter().enumerate() {
- vstack.push(h.pretty(hi, &blobs, repo));
- if i != hunks.0.len() - 1 {
- vstack = vstack.divider();
- }
- }
- }
- DiffContent::Empty => {
- vstack.push(term::Line::new(term::format::italic("Empty file")));
- }
- DiffContent::Binary => {
- vstack.push(term::Line::new(term::format::italic("Binary file")));
- }
- }
- }
+impl ToPretty for Added {
+ type Output = term::VStack<'static>;
+ type Context = FileHeader;
+
+ fn pretty<R: Repo>(
+ &self,
+ hi: &mut Highlighter,
+ header: &Self::Context,
+ repo: &R,
+ ) -> Self::Output {
+ let old = None;
+ let new = Some((self.path.as_path(), self.new.oid));
+
+ pretty_modification(header, &self.diff, old, new, repo, hi)
+ }
+}
+
+impl ToPretty for Deleted {
+ type Output = term::VStack<'static>;
+ type Context = FileHeader;
+
+ fn pretty<R: Repo>(
+ &self,
+ hi: &mut Highlighter,
+ header: &Self::Context,
+ repo: &R,
+ ) -> Self::Output {
+ let old = Some((self.path.as_path(), self.old.oid));
+ let new = None;
+
+ pretty_modification(header, &self.diff, old, new, repo, hi)
+ }
+}
+
+impl ToPretty for Modified {
+ type Output = term::VStack<'static>;
+ type Context = FileHeader;
+
+ fn pretty<R: Repo>(
+ &self,
+ hi: &mut Highlighter,
+ header: &Self::Context,
+ repo: &R,
+ ) -> Self::Output {
+ let old = Some((self.path.as_path(), self.old.oid));
+ let new = Some((self.path.as_path(), self.new.oid));
+
+ pretty_modification(header, &self.diff, old, new, repo, hi)
+ }
+}
+
+impl ToPretty for Copied {
+ type Output = term::VStack<'static>;
+ type Context = FileHeader;
+
+ fn pretty<R: Repo>(
+ &self,
+ hi: &mut Highlighter,
+ _context: &Self::Context,
+ repo: &R,
+ ) -> Self::Output {
+ let header = FileHeader::Copied {
+ old_path: self.old_path.clone(),
+ new_path: self.old_path.clone(),
}
- vstack
+ .pretty(hi, &self.diff.stats().copied(), repo);
+
+ term::VStack::default()
+ .border(Some(term::colors::FAINT))
+ .padding(1)
+ .child(header)
}
}
@@ -291,9 +425,14 @@ impl ToPretty for HunkHeader {
impl ToPretty for Hunk<Modification> {
type Output = term::VStack<'static>;
- type Context = Blobs;
+ type Context = Blobs<Vec<term::Line>>;
- fn pretty<R: Repo>(&self, hi: &mut Highlighter, blobs: &Blobs, repo: &R) -> Self::Output {
+ fn pretty<R: Repo>(
+ &self,
+ hi: &mut Highlighter,
+ blobs: &Self::Context,
+ repo: &R,
+ ) -> Self::Output {
let mut vstack = term::VStack::default().padding(0);
let mut table = term::Table::<5, term::Filled<term::Line>>::new(term::TableOptions {
overflow: false,
@@ -379,9 +518,14 @@ impl ToPretty for Hunk<Modification> {
impl ToPretty for Modification {
type Output = term::Line;
- type Context = Blobs;
+ type Context = Blobs<Vec<term::Line>>;
- fn pretty<R: Repo>(&self, _hi: &mut Highlighter, blobs: &Blobs, _repo: &R) -> Self::Output {
+ fn pretty<R: Repo>(
+ &self,
+ _hi: &mut Highlighter,
+ blobs: &Blobs<Vec<term::Line>>,
+ _repo: &R,
+ ) -> Self::Output {
match self {
Modification::Deletion(diff::Deletion { line, line_no }) => {
if let Some(lines) = &blobs.old.as_ref() {
@@ -411,6 +555,30 @@ impl ToPretty for Modification {
}
}
+/// Render a file added, deleted or modified.
+fn pretty_modification<R: Repo>(
+ header: &FileHeader,
+ diff: &DiffContent,
+ old: Option<(&Path, Oid)>,
+ new: Option<(&Path, Oid)>,
+ repo: &R,
+ hi: &mut Highlighter,
+) -> VStack<'static> {
+ let blobs = Blobs::from_paths(old, new, repo);
+ let header = header.pretty(hi, &diff.stats().copied(), repo);
+ let vstack = term::VStack::default()
+ .border(Some(term::colors::FAINT))
+ .padding(1)
+ .child(header);
+
+ let body = diff.pretty(hi, &blobs, repo);
+ if body.is_empty() {
+ vstack
+ } else {
+ vstack.divider().merge(body)
+ }
+}
+
#[cfg(test)]
mod test {
use std::ffi::OsStr;
diff --git a/radicle-cli/src/git/unified_diff.rs b/radicle-cli/src/git/unified_diff.rs
index e1d5a0231e..d7b698d5fa 100644
--- a/radicle-cli/src/git/unified_diff.rs
+++ b/radicle-cli/src/git/unified_diff.rs
@@ -50,6 +50,7 @@ pub enum FileHeader {
Added {
path: PathBuf,
new: DiffFile,
+ binary: bool,
},
Copied {
old_path: PathBuf,
@@ -58,11 +59,13 @@ pub enum FileHeader {
Deleted {
path: PathBuf,
old: DiffFile,
+ binary: bool,
},
Modified {
path: PathBuf,
old: DiffFile,
new: DiffFile,
+ binary: bool,
},
Moved {
old_path: PathBuf,
@@ -78,15 +81,21 @@ impl std::convert::From<&FileDiff> for FileHeader {
path: v.path.clone(),
old: v.old.clone(),
new: v.new.clone(),
+ binary: matches!(v.diff, DiffContent::Binary),
},
FileDiff::Added(v) => FileHeader::Added {
path: v.path.clone(),
new: v.new.clone(),
+ binary: matches!(v.diff, DiffContent::Binary),
+ },
+ FileDiff::Copied(c) => FileHeader::Copied {
+ old_path: c.old_path.clone(),
+ new_path: c.new_path.clone(),
},
- FileDiff::Copied(_) => todo!(),
FileDiff::Deleted(v) => FileHeader::Deleted {
path: v.path.clone(),
old: v.old.clone(),
+ binary: matches!(v.diff, DiffContent::Binary),
},
FileDiff::Moved(v) => FileHeader::Moved {
old_path: v.old_path.clone(),
@@ -288,7 +297,7 @@ impl Encode for FileDiff {
impl Encode for FileHeader {
fn encode(&self, w: &mut Writer) -> Result<(), Error> {
match self {
- FileHeader::Modified { path, old, new } => {
+ FileHeader::Modified { path, old, new, .. } => {
w.meta(format!(
"diff --git a/{} b/{}",
path.display(),
@@ -315,7 +324,7 @@ impl Encode for FileHeader {
w.meta(format!("--- a/{}", path.display()))?;
w.meta(format!("+++ b/{}", path.display()))?;
}
- FileHeader::Added { path, new } => {
+ FileHeader::Added { path, new, .. } => {
w.meta(format!(
"diff --git a/{} b/{}",
path.display(),
@@ -333,7 +342,7 @@ impl Encode for FileHeader {
w.meta(format!("+++ b/{}", path.display()))?;
}
FileHeader::Copied { .. } => todo!(),
- FileHeader::Deleted { path, old } => {
+ FileHeader::Deleted { path, old, .. } => {
w.meta(format!(
"diff --git a/{} b/{}",
path.display(),
diff --git a/radicle-term/src/format.rs b/radicle-term/src/format.rs
index e91f8d690a..50f3366531 100644
--- a/radicle-term/src/format.rs
+++ b/radicle-term/src/format.rs
@@ -52,6 +52,14 @@ pub fn badge_primary<D: std::fmt::Display>(input: D) -> Paint<String> {
}
}
+pub fn badge_yellow<D: std::fmt::Display>(input: D) -> Paint<String> {
+ if Paint::is_enabled() {
+ Paint::yellow(format!(" {input} ")).invert()
+ } else {
+ Paint::new(format!("❲{input}❳"))
+ }
+}
+
pub fn badge_positive<D: std::fmt::Display>(input: D) -> Paint<String> {
if Paint::is_enabled() {
Paint::green(format!(" {input} ")).invert()
diff --git a/radicle-term/src/vstack.rs b/radicle-term/src/vstack.rs
index 86332dd7de..844ca4028c 100644
--- a/radicle-term/src/vstack.rs
+++ b/radicle-term/src/vstack.rs
@@ -66,6 +66,11 @@ impl<'a> VStack<'a> {
self
}
+ /// Check if this stack is empty.
+ pub fn is_empty(&self) -> bool {
+ self.rows.is_empty()
+ }
+
/// Add multiple elements to the stack.
pub fn children<I>(self, children: I) -> Self
where
@@ -79,6 +84,14 @@ impl<'a> VStack<'a> {
vstack
}
+ /// Merge with another `VStack`.
+ pub fn merge(mut self, other: Self) -> Self {
+ for row in other.rows {
+ self.rows.push(row);
+ }
+ self
+ }
+
/// Set or unset the outer border.
pub fn border(mut self, color: Option<Color>) -> Self {
self.opts.border = color;
commit 795ee35f37c1823621525baa43f07380a6a911c5
Author: Alexis Sellier <self@cloudhead.io>
Date: Thu Jul 11 14:17:17 2024 +0200
cli: Output tree hash after review update
diff --git a/radicle-cli/examples/rad-review-by-hunk.md b/radicle-cli/examples/rad-review-by-hunk.md
index d6445b16b7..4a13ee2798 100644
--- a/radicle-cli/examples/rad-review-by-hunk.md
+++ b/radicle-cli/examples/rad-review-by-hunk.md
@@ -125,6 +125,7 @@ index 7937fb3..0000000
+++ /dev/null
@@ -1 +0,0 @@
-*.draft
+✓ Updated review tree to a5fccf0e977225ff13c3f74c43faf4cb679bf835
```
```
$ rad patch review --patch --accept --hunk 1 7a2ac7e2841cc1e7394f99f107555a499b1d3f23 --no-announce
@@ -136,6 +137,7 @@ index 0000000..2b5bd86
+++ b/DISCLAIMER.txt
@@ -0,0 +1 @@
+All food is served as-is, with no warranty!
+✓ Updated review tree to 2cdb82ea726e64d3b52847c7699d0d4759198f5c
```
```
$ rad patch review --patch --accept -U3 --hunk 1 7a2ac7e2841cc1e7394f99f107555a499b1d3f23 --no-announce
@@ -151,6 +153,7 @@ index 867958c..3af9741 100644
Salmon Tartare
Mac & Cheese
[..]
+✓ Updated review tree to d4aecbb859a802a3215def0b538358bf63593953
```
```
$ rad patch review --patch --accept -U3 --hunk 1 7a2ac7e2841cc1e7394f99f107555a499b1d3f23 --no-announce
@@ -166,6 +169,7 @@ index 4e2e828..3af9741 100644
-French Fries
+French Fries!
+Garlic Green Beans
+✓ Updated review tree to 59cee720b0642b1491b241400912b35926a76c3f
```
```
@@ -175,6 +179,7 @@ diff --git a/INSTRUCTIONS.txt b/notes/INSTRUCTIONS.txt
similarity index 100%
rename from INSTRUCTIONS.txt
rename to notes/INSTRUCTIONS.txt
+✓ Updated review tree to 3effc8f6462fa2573697072245e57708c4dcbe62
```
```
diff --git a/radicle-cli/src/commands/patch/review/builder.rs b/radicle-cli/src/commands/patch/review/builder.rs
index 63b25754e2..9694841e5f 100644
--- a/radicle-cli/src/commands/patch/review/builder.rs
+++ b/radicle-cli/src/commands/patch/review/builder.rs
@@ -335,6 +335,10 @@ impl<'a> ReviewBuilder<'a> {
let oid =
review.amend(Some(&self.refname), None, None, None, None, Some(&brain))?;
review = repo.find_commit(oid)?;
+
+ if self.hunk.is_some() {
+ term::success!("Updated review tree to {brain_oid}");
+ }
}
Some(ReviewAction::Ignore) => {
// Do nothing. Hunk will be reviewable again next time.
Exit code: 0
shell: 'cargo --version rustc --version cargo fmt --check cargo clippy --all-targets --workspace -- --deny clippy::all cargo build --all-targets --workspace cargo doc --workspace cargo test --workspace --no-fail-fast '
Commands:
$ podman run --name 82048b33-97bd-4f86-ba06-3f76569a9d95 -v /opt/radcis/ci.rad.levitte.org/cci/state/82048b33-97bd-4f86-ba06-3f76569a9d95/s:/82048b33-97bd-4f86-ba06-3f76569a9d95/s:ro -v /opt/radcis/ci.rad.levitte.org/cci/state/82048b33-97bd-4f86-ba06-3f76569a9d95/w:/82048b33-97bd-4f86-ba06-3f76569a9d95/w -w /82048b33-97bd-4f86-ba06-3f76569a9d95/w -v /opt/radcis/ci.rad.levitte.org/.radicle:/${id}/.radicle:ro -e RAD_HOME=/${id}/.radicle rust:trixie bash /82048b33-97bd-4f86-ba06-3f76569a9d95/s/script.sh
+ cargo --version
info: syncing channel updates for '1.77-x86_64-unknown-linux-gnu'
info: latest update on 2024-04-09, rust version 1.77.2 (25ef9e3d8 2024-04-09)
info: downloading component 'cargo'
info: downloading component 'rust-std'
info: downloading component 'rustc'
info: installing component 'cargo'
info: installing component 'rust-std'
info: installing component 'rustc'
cargo 1.77.2 (e52e36006 2024-03-26)
+ rustc --version
rustc 1.77.2 (25ef9e3d8 2024-04-09)
+ cargo fmt --check
error: 'cargo-fmt' is not installed for the toolchain '1.77-x86_64-unknown-linux-gnu'.
To install, run `rustup component add --toolchain 1.77-x86_64-unknown-linux-gnu rustfmt`
Exit code: 1
{
"response": "finished",
"result": "failure"
}