bin(patch): Improve hunk state handling and testing — radicle-tui

radicle-tui

rad:z39mP9rQAaGmERfUMPULfPUi473tY

Radicle terminal user interface

bin(patch): Improve hunk state handling and testing

Merged did:key:z6MkgFq6...nBGz opened 1 year ago

Activity Changes +712 -230

9 files changed +712 -230 bb30b21c → 634abc63

modified Cargo.lock

@@ -1873,6 +1873,7 @@ dependencies = [

  "ansi-to-tui",
  "anyhow",
  "fuzzy-matcher",
  "git2",
  "homedir",
  "inquire",
  "lazy_static",

@@ -1883,6 +1884,7 @@ dependencies = [

  "pretty_assertions",
  "radicle",
  "radicle-cli",
  "radicle-git-ext",
  "radicle-signals",
  "radicle-surf",
  "radicle-term",

modified Cargo.toml

@@ -54,4 +54,11 @@ tui-tree-widget = { version = "0.21.0" }

 [dev-dependencies]
 pretty_assertions = "^1.4.1"
 radicle = { version = "0.14.0", features = ["test"]}
 \ No newline at end of file
 radicle = { version = "0.14.0", features = ["test"]}
 radicle-git-ext = { version = "0.8.0", features = ["serde"] }
 [dev-dependencies.git2]
 version = "0.19.0"
 default-features = false
 features = ["vendored-libgit2"]
 \ No newline at end of file

modified bin/commands/patch.rs

@@ -256,7 +256,7 @@ mod interface {

     use radicle::patch::PatchId;
     use radicle::patch::Verdict;
     use radicle::storage::git::cob::DraftStore;
     use radicle::storage::{ReadStorage, WriteRepository};
     use radicle::storage::ReadStorage;
     use radicle::Profile;
     use radicle_cli::terminal;

@@ -269,7 +269,7 @@ mod interface {

     use crate::tui_patch::select;
     use super::review;
     use super::review::builder::{Brain, ReviewBuilder};
     use super::review::builder::ReviewBuilder;
     use super::{ReviewOptions, SelectOptions};
     pub async fn select(

@@ -304,9 +304,7 @@ mod interface {

             .ok_or_else(|| anyhow!("Patch `{patch_id}` not found"))?;
         let (_, revision) = opts.revision_or_latest(&patch, &repo)?;
         let brain = Brain::load_or_new(patch_id, revision, repo.raw(), &signer)?;
         let hunks = ReviewBuilder::new(&repo).hunks(&brain, revision)?;
         let hunks = ReviewBuilder::new(&repo).hunks(revision)?;
         let drafts = DraftStore::new(&repo, *signer.public_key());
         let mut patches = cob::patch::Cache::no_cache(&drafts)?;

@@ -368,7 +366,7 @@ mod interface {

                         let hunk = selection
                             .hunk
                             .ok_or_else(|| anyhow!("expected a selected hunk"))?;
                         let (_, item, _) = hunks
                         let item = hunks
                             .get(hunk)
                             .ok_or_else(|| anyhow!("expected a hunk to comment on"))?;

modified bin/commands/patch/review.rs

@@ -2,50 +2,44 @@

 pub mod builder;
 use std::fmt::Debug;
 use std::path::PathBuf;
 use std::sync::Arc;
 use std::sync::Mutex;
 use anyhow::Result;
 use radicle::storage::git::Repository;
 use termion::event::Key;
 use ratatui::layout::Constraint;
 use ratatui::layout::Position;
 use ratatui::layout::{Constraint, Position};
 use ratatui::style::Stylize;
 use ratatui::text::Text;
 use ratatui::{Frame, Viewport};
 use radicle::crypto::Signer;
 use radicle::identity::RepoId;
 use radicle::patch::PatchId;
 use radicle::patch::Review;
 use radicle::patch::Revision;
 use radicle::storage::ReadStorage;
 use radicle::storage::WriteRepository;
 use radicle::patch::{PatchId, Review, Revision};
 use radicle::storage::git::Repository;
 use radicle::storage::{ReadStorage, WriteRepository};
 use radicle::Storage;
 use radicle_tui as tui;
 use tui::store;
 use tui::ui::im::widget::PanesState;
 use tui::ui::im::widget::{TableState, TextViewState, Window};
 use tui::ui::im::Ui;
 use tui::ui::im::{Borders, Context, Show};
 use tui::ui::im::widget::{PanesState, TableState, TextViewState, Window};
 use tui::ui::im::{Borders, Context, Show, Ui};
 use tui::ui::span;
 use tui::ui::Column;
 use tui::{Channel, Exit};
 use crate::git::HunkState;
 use crate::git::StatefulHunkDiff;
 use crate::tui_patch::review::builder::DiffUtil;
 use crate::git::HunkDiff;
 use crate::git::{HunkState, StatefulHunkDiff};
 use crate::ui::format;
 use crate::ui::items::HunkItem;
 use crate::ui::layout;
 use self::builder::Brain;
 use self::builder::FileReviewBuilder;
 use self::builder::ReviewQueue;
 use super::review::builder::DiffUtil;
 use self::builder::{Brain, FileReviewBuilder, Hunks};
 /// The actions that a user can carry out on a review item.
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]

@@ -71,7 +65,7 @@ pub struct Tui {

     pub title: String,
     pub revision: Revision,
     pub review: Review,
     pub hunks: ReviewQueue,
     pub hunks: Hunks,
 }
 impl Tui {

@@ -84,7 +78,7 @@ impl Tui {

         title: String,
         revision: Revision,
         review: Review,
         hunks: ReviewQueue,
         hunks: Hunks,
     ) -> Self {
         Self {
             storage,

@@ -122,7 +116,7 @@ pub enum Message<'a> {

     ShowMain,
     PanesChanged { state: PanesState },
     HunkChanged { state: TableState },
     HunkViewChanged { state: HunkItemState },
     HunkViewChanged { state: DiffViewState },
     ShowHelp,
     HelpChanged { state: TextViewState<'a> },
     Comment,

@@ -138,12 +132,51 @@ pub enum AppPage {

 }
 #[derive(Clone, Debug)]
 pub struct HunkItemState {
 pub struct DiffViewState {
     cursor: Position,
 }
 pub type HunkItems<'a> = Vec<HunkItem<'a>>;
 pub type HunkItemStates = Vec<HunkItemState>;
 pub struct HunkList<'a> {
     items: Vec<HunkItem<'a>>,
     views: Vec<DiffViewState>,
     table: TableState,
 }
 impl<'a> HunkList<'a> {
     pub fn new(
         items: impl IntoIterator<Item = HunkItem<'a>>,
         views: impl IntoIterator<Item = DiffViewState>,
         table: TableState,
     ) -> Self {
         Self {
             items: items.into_iter().collect(),
             views: views.into_iter().collect(),
             table,
         }
     }
     pub fn item(&self, index: usize) -> Option<&HunkItem> {
         self.items.get(index)
     }
     pub fn view_state(&self, index: usize) -> Option<&DiffViewState> {
         self.views.get(index)
     }
     pub fn update_view_state(&mut self, index: usize, state: DiffViewState) {
         if let Some(view) = self.views.get_mut(index) {
             *view = state;
         }
     }
     pub fn update_table(&mut self, table: TableState) {
         self.table = table;
     }
     pub fn selected(&self) -> Option<usize> {
         self.table.selected()
     }
 }
 #[derive(Clone)]
 pub struct App<'a> {

@@ -159,8 +192,9 @@ pub struct App<'a> {

     title: String,
     /// Revision this review belongs to.
     revision: Revision,
     /// All hunks, their states and the lists' table widget state.
     hunks: Arc<Mutex<(HunkItems<'a>, HunkItemStates, TableState)>>,
     /// All hunks, their view states (cursor position is stored per hunk)
     /// and the lists' table widget state.
     hunks: Arc<Mutex<HunkList<'a>>>,
     /// Current app page.
     page: AppPage,
     /// State of panes widget on the main page.

@@ -196,20 +230,18 @@ impl<'a> App<'a> {

         title: String,
         revision: Revision,
         review: Review,
         hunks: ReviewQueue,
         hunks: Hunks,
     ) -> Result<Self, anyhow::Error> {
         let repo = storage.repository(rid)?;
         let states = hunks
             .iter()
             .map(|_| HunkItemState {
             .map(|_| DiffViewState {
                 cursor: Position::new(0, 0),
             })
             .collect::<Vec<_>>();
         let hunks = hunks
             .iter()
             .map(|(_, item, state)| {
                 HunkItem::from((&repo, &review, StatefulHunkDiff::from((item, state))))
             })
             .map(|item| HunkItem::from((&repo, &review, StatefulHunkDiff::from(item))))
             .collect::<Vec<_>>();
         let mut app = App {

@@ -219,7 +251,11 @@ impl<'a> App<'a> {

             patch,
             title,
             revision,
             hunks: Arc::new(Mutex::new((hunks, states, TableState::new(Some(0))))),
             hunks: Arc::new(Mutex::new(HunkList::new(
                 hunks,
                 states,
                 TableState::new(Some(0)),
             ))),
             page: AppPage::Main,
             group: PanesState::new(2, Some(0)),
             help: TextViewState::new(help_text(), Position::default()),

@@ -236,18 +272,40 @@ impl<'a> App<'a> {

         let signer: &Box<dyn Signer> = &self.signer.lock().unwrap();
         if let Some(selected) = self.selected_hunk_idx() {
             let items = &self.hunks.lock().unwrap().items;
             let mut brain = Brain::load_or_new(self.patch, &self.revision, repo.raw(), signer)?;
             let hunks = self.hunks.lock().unwrap().0.clone();
             if let Some(hunk) = hunks.get(selected) {
                 let mut file: Option<FileReviewBuilder> = None;
                 let file = match file.as_mut() {
                     Some(fr) => fr.set_item(hunk.inner.hunk()),
                     None => file.insert(FileReviewBuilder::new(hunk.inner.hunk())),
             let mut last_path: Option<&PathBuf> = None;
             let mut file: Option<FileReviewBuilder> = None;
             for (idx, item) in items.iter().enumerate() {
                 // Get file path.
                 let path = match item.inner.hunk() {
                     HunkDiff::Added { path, .. } => path,
                     HunkDiff::Deleted { path, .. } => path,
                     HunkDiff::Modified { path, .. } => path,
                     HunkDiff::Copied { copied } => &copied.new_path,
                     HunkDiff::Moved { moved } => &moved.new_path,
                     HunkDiff::EofChanged { path, .. } => path,
                     HunkDiff::ModeChanged { path, .. } => path,
                 };
                 let diff = file.item_diff(hunk.inner.hunk())?;
                 brain.accept(diff, repo.raw())?;
                 // Set new review builder if hunk belongs to new file.
                 if last_path.is_none() || last_path.unwrap() != path {
                     last_path = Some(path);
                     file = Some(FileReviewBuilder::new(item.inner.hunk()));
                 }
                 if let Some(file) = file.as_mut() {
                     file.set_item(item.inner.hunk());
                     if idx == selected {
                         let diff = file.item_diff(item.inner.hunk())?;
                         brain.accept(diff, repo.raw())?;
                     } else {
                         file.ignore_item(item.inner.hunk())
                     }
                 }
             }
         }

@@ -269,38 +327,32 @@ impl<'a> App<'a> {

     pub fn reload_states(&mut self) -> anyhow::Result<()> {
         let repo = self.repo()?;
         let signer: &Box<dyn Signer> = &self.signer.lock().unwrap();
         let items = &mut self.hunks.lock().unwrap().items;
         let brain = Brain::load_or_new(self.patch, &self.revision, repo.raw(), signer)?;
         let (base_diff, queue_diff) =
             DiffUtil::new(&repo).base_queue(brain.clone(), &self.revision)?;
         // Compute states
         let base_files = base_diff.into_files();
         let queue_files = queue_diff.into_files();
         let states = base_files
             .iter()
             .map(|file| {
                 if !queue_files.contains(file) {
                     HunkState::Accepted
                 } else {
                     HunkState::Rejected
                 }
             })
             .collect::<Vec<_>>();
         let mut queue = self.hunks.lock().unwrap();
         for (idx, new_state) in states.iter().enumerate() {
             if let Some(hunk) = queue.0.get_mut(idx) {
                 *hunk.inner.state_mut() = new_state.clone();
             }
         let rejected_hunks =
             Hunks::new(DiffUtil::new(&repo).rejected_diffs(&brain, &self.revision)?);
         log::debug!("Reloaded hunk states..");
         log::debug!("Rejected hunks: {:?}", rejected_hunks);
         log::debug!("Requested to reload hunks: {:?}", items);
         for item in &mut *items {
             let state = if rejected_hunks.contains(item.inner.hunk()) {
                 HunkState::Rejected
             } else {
                 HunkState::Accepted
             };
             *item.inner.state_mut() = state;
         }
         log::debug!("Reloaded hunks: {:?}", items);
         Ok(())
     }
     pub fn selected_hunk_idx(&self) -> Option<usize> {
         self.hunks.lock().unwrap().2.selected()
         self.hunks.lock().unwrap().selected()
     }
     pub fn repo(&self) -> Result<Repository> {

@@ -319,9 +371,9 @@ impl<'a> App<'a> {

         .to_vec();
         let hunks = self.hunks.lock().unwrap();
         let mut selected = hunks.2.selected();
         let mut selected = hunks.selected();
         let table = ui.headered_table(frame, &mut selected, &hunks.0, header, columns);
         let table = ui.headered_table(frame, &mut selected, &hunks.items, header, columns);
         if table.changed {
             ui.send_message(Message::HunkChanged {
                 state: TableState::new(selected),

@@ -332,8 +384,8 @@ impl<'a> App<'a> {

     fn show_hunk(&self, ui: &mut Ui<Message<'a>>, frame: &mut Frame) {
         let hunks = self.hunks.lock().unwrap();
         let selected = hunks.2.selected();
         let hunk = selected.and_then(|selected| hunks.0.get(selected));
         let selected = hunks.selected();
         let hunk = selected.and_then(|selected| hunks.item(selected));
         if let Some(hunk) = hunk {
             let empty_text = hunk

@@ -341,7 +393,7 @@ impl<'a> App<'a> {

                 .unwrap_or(Text::raw("Nothing to show.").dark_gray());
             let mut cursor = selected
                 .and_then(|selected| hunks.1.get(selected))
                 .and_then(|selected| hunks.view_state(selected))
                 .map(|state| state.cursor)
                 .unwrap_or_default();

@@ -352,7 +404,7 @@ impl<'a> App<'a> {

                     let diff = ui.text_view(frame, text, &mut cursor, Some(Borders::BottomSides));
                     if diff.changed {
                         ui.send_message(Message::HunkViewChanged {
                             state: HunkItemState { cursor },
                             state: DiffViewState { cursor },
                         })
                     }
                 } else {

@@ -363,7 +415,7 @@ impl<'a> App<'a> {

     }
     fn show_context_bar(&self, ui: &mut Ui<Message<'a>>, frame: &mut Frame) {
         let hunks = &self.hunks.lock().unwrap().0;
         let hunks = &self.hunks.lock().unwrap().items;
         let id = format!(" {} ", format::cob(&self.patch));
         let title = &self.title;

@@ -524,15 +576,13 @@ impl<'a> store::Update<Message<'a>> for App<'a> {

             }
             Message::HunkChanged { state } => {
                 let mut hunks = self.hunks.lock().unwrap();
                 hunks.2 = state;
                 hunks.update_table(state);
                 None
             }
             Message::HunkViewChanged { state } => {
                 let mut hunks = self.hunks.lock().unwrap();
                 if let Some(selected) = hunks.2.selected() {
                     if let Some(item_state) = hunks.1.get_mut(selected) {
                         *item_state = state;
                     }
                 let hunks = &mut self.hunks.lock().unwrap();
                 if let Some(selected) = hunks.selected() {
                     hunks.update_view_state(selected, state);
                 }
                 None
             }

@@ -545,14 +595,14 @@ impl<'a> store::Update<Message<'a>> for App<'a> {

                 Some(Exit {
                     value: Some(Selection {
                         action: ReviewAction::Comment,
                         hunk: hunks.2.selected(),
                         hunk: hunks.selected(),
                         args: None,
                     }),
                 })
             }
             Message::Accept => {
                 match self.accept_current_hunk() {
                     Ok(()) => log::info!("Accepted hunk."),
                     Ok(()) => log::info!("Hunk accepted."),
                     Err(err) => log::info!("An error occured while accepting hunk: {}", err),
                 }
                 let _ = self.reload_states();

@@ -617,7 +667,7 @@ mod test {

     impl<'a> App<'a> {
         pub fn hunks(&self) -> Vec<HunkItem> {
             self.hunks.lock().unwrap().0.clone()
             self.hunks.lock().unwrap().items.clone()
         }
     }

@@ -629,12 +679,11 @@ mod test {

         use radicle::prelude::Signer;
         use radicle::storage::git::cob::DraftStore;
         use radicle::storage::git::Repository;
         use radicle::storage::WriteRepository;
         use radicle::test::setup::NodeWithRepo;
         use crate::cob::patch;
         use crate::test::setup::NodeWithRepo;
         use super::builder::{Brain, ReviewBuilder};
         use super::builder::ReviewBuilder;
         use super::App;
         pub fn app<'a>(

@@ -648,8 +697,7 @@ mod test {

             let (_, revision) = patch.latest();
             let (_, review) = draft_review(&node, &mut draft, revision)?;
             let brain = Brain::load_or_new(*patch.id(), revision, node.repo.raw(), &node.signer)?;
             let hunks = ReviewBuilder::new(&node.repo).hunks(&brain, revision)?;
             let hunks = ReviewBuilder::new(&node.repo).hunks(revision)?;
             App::new(
                 node.storage.clone(),

@@ -684,9 +732,24 @@ mod test {

     }
     #[test]
     fn app_can_be_constructed() -> Result<()> {
     fn app_with_single_hunk_can_be_constructed() -> Result<()> {
         let alice = test::fixtures::node_with_repo();
         let branch = test::fixtures::branch_with_main_emptied(&alice);
         let mut patches = Cache::no_cache(&alice.repo.repo).unwrap();
         let patch = test::fixtures::patch(&alice, &branch, &mut patches)?;
         let app = fixtures::app(&alice, patch)?;
         assert_eq!(app.hunks().len(), 1);
         Ok(())
     }
     #[test]
     fn app_with_single_file_multiple_hunks_can_be_constructed() -> Result<()> {
         let alice = test::fixtures::node_with_repo();
         let branch = test::fixtures::branch(&alice);
         let branch = test::fixtures::branch_with_eof_removed(&alice);
         let mut patches = Cache::no_cache(&alice.repo.repo).unwrap();
         let patch = test::fixtures::patch(&alice, &branch, &mut patches)?;

@@ -701,7 +764,7 @@ mod test {

     #[test]
     fn first_hunk_is_selected_by_default() -> Result<()> {
         let alice = test::fixtures::node_with_repo();
         let branch = test::fixtures::branch(&alice);
         let branch = test::fixtures::branch_with_main_emptied(&alice);
         let mut patches = Cache::no_cache(&alice.repo.repo).unwrap();
         let patch = test::fixtures::patch(&alice, &branch, &mut patches)?;

@@ -714,9 +777,30 @@ mod test {

     }
     #[test]
     fn hunks_are_rejected_by_default() -> Result<()> {
         let alice = test::fixtures::node_with_repo();
         let branch = test::fixtures::branch_with_main_deleted_and_file_added(&alice);
         let mut patches = Cache::no_cache(&alice.repo.repo).unwrap();
         let patch = test::fixtures::patch(&alice, &branch, &mut patches)?;
         let app = fixtures::app(&alice, patch)?;
         let hunks = app.hunks();
         let states = hunks
             .iter()
             .map(|item| item.inner.state())
             .collect::<Vec<_>>();
         assert_eq!(states, [&HunkState::Rejected, &HunkState::Rejected,]);
         Ok(())
     }
     #[test]
     fn hunk_can_be_selected() -> Result<()> {
         let alice = test::fixtures::node_with_repo();
         let branch = test::fixtures::branch(&alice);
         let branch = test::fixtures::branch_with_eof_removed(&alice);
         let mut patches = Cache::no_cache(&alice.repo.repo).unwrap();
         let patch = test::fixtures::patch(&alice, &branch, &mut patches)?;

@@ -730,4 +814,99 @@ mod test {

         Ok(())
     }
     #[test]
     fn single_file_single_hunk_can_be_accepted() -> Result<()> {
         let alice = test::fixtures::node_with_repo();
         let branch = test::fixtures::branch_with_main_emptied(&alice);
         let mut patches = Cache::no_cache(&alice.repo.repo).unwrap();
         let patch = test::fixtures::patch(&alice, &branch, &mut patches)?;
         let mut app = fixtures::app(&alice, patch)?;
         app.update(Message::Accept);
         let hunks = app.hunks();
         let state = &hunks.get(0).unwrap().inner.state();
         assert_eq!(**state, HunkState::Accepted);
         Ok(())
     }
     #[test]
     #[ignore]
     fn single_file_multiple_hunks_only_first_can_be_accepted() -> Result<()> {
         let alice = test::fixtures::node_with_repo();
         let branch = test::fixtures::branch_with_main_changed(&alice);
         let mut patches = Cache::no_cache(&alice.repo.repo).unwrap();
         let patch = test::fixtures::patch(&alice, &branch, &mut patches)?;
         let mut app = fixtures::app(&alice, patch)?;
         app.update(Message::Accept);
         let hunks = app.hunks();
         let states = hunks
             .iter()
             .map(|item| item.inner.state())
             .collect::<Vec<_>>();
         assert_eq!(states, [&HunkState::Accepted, &HunkState::Rejected]);
         Ok(())
     }
     #[test]
     fn single_file_multiple_hunks_only_last_can_be_accepted() -> Result<()> {
         let alice = test::fixtures::node_with_repo();
         let branch = test::fixtures::branch_with_main_changed(&alice);
         let mut patches = Cache::no_cache(&alice.repo.repo).unwrap();
         let patch = test::fixtures::patch(&alice, &branch, &mut patches)?;
         let mut app = fixtures::app(&alice, patch)?;
         app.update(Message::HunkChanged {
             state: TableState::new(Some(1)),
         });
         app.update(Message::Accept);
         let hunks = app.hunks();
         let states = hunks
             .iter()
             .map(|item| item.inner.state())
             .collect::<Vec<_>>();
         assert_eq!(states, [&HunkState::Rejected, &HunkState::Accepted]);
         Ok(())
     }
     #[test]
     fn multiple_files_single_hunk_can_be_accepted() -> Result<()> {
         let alice = test::fixtures::node_with_repo();
         let branch = test::fixtures::branch_with_main_deleted_and_file_added(&alice);
         let mut patches = Cache::no_cache(&alice.repo.repo).unwrap();
         let patch = test::fixtures::patch(&alice, &branch, &mut patches)?;
         let mut app = fixtures::app(&alice, patch)?;
         app.update(Message::Accept);
         app.update(Message::HunkChanged {
             state: TableState::new(Some(1)),
         });
         app.update(Message::Accept);
         let hunks = app.hunks();
         let states = hunks
             .iter()
             .map(|item| item.inner.state())
             .collect::<Vec<_>>();
         assert_eq!(states, [&HunkState::Accepted, &HunkState::Accepted]);
         Ok(())
     }
 }

modified bin/commands/patch/review/builder.rs

@@ -10,7 +10,6 @@

 //! matches the tree of the patch being reviewed (by accepting hunks), we can say that the patch has
 //! been fully reviewed.
 //!
 use std::collections::VecDeque;
 use std::fmt::Write as _;
 use std::io;
 use std::ops::{Not, Range};

@@ -28,142 +27,121 @@ use radicle_cli::git::unified_diff::{self, FileHeader};

 use radicle_cli::git::unified_diff::{Encode, HunkHeader};
 use radicle_cli::terminal as term;
 use crate::git::{HunkDiff, HunkState};
 use crate::git::HunkDiff;
 /// Queue of items (usually hunks) left to review.
 #[derive(Clone, Default)]
 pub struct ReviewQueue {
     /// Hunks left to review.
     queue: VecDeque<(usize, HunkDiff, HunkState)>,
 #[derive(Clone, Default, Debug)]
 pub struct Hunks {
     hunks: Vec<HunkDiff>,
 }
 impl ReviewQueue {
     pub fn new(base: Diff, queue: Diff) -> Self {
 impl Hunks {
     pub fn new(base: Diff) -> Self {
         let base_files = base.into_files();
         let queue_files = queue.into_files();
         let mut queue = Self::default();
         for file in base_files {
             let state = if !queue_files.contains(&file) {
                 HunkState::Accepted
             } else {
                 HunkState::Rejected
             };
             queue.add_file(file, state);
             queue.add_file(file);
         }
         queue
     }
     /// Add a file to the queue.
     /// Mostly splits files into individual review items (eg. hunks) to review.
     fn add_file(&mut self, file: FileDiff, state: HunkState) {
     fn add_file(&mut self, file: FileDiff) {
         let header = FileHeader::from(&file);
         match file {
             FileDiff::Moved(moved) => {
                 self.add_item(HunkDiff::Moved { moved }, state);
                 self.add_item(HunkDiff::Moved { moved });
             }
             FileDiff::Copied(copied) => {
                 self.add_item(HunkDiff::Copied { copied }, state);
                 self.add_item(HunkDiff::Copied {
                     copied: copied.clone(),
                 });
             }
             FileDiff::Added(a) => {
                 self.add_item(
                     HunkDiff::Added {
                         path: a.path,
                         header: header.clone(),
                         new: a.new,
                         hunk: if let DiffContent::Plain {
                             hunks: Hunks(mut hs),
                             ..
                         } = a.diff.clone()
                         {
                             hs.pop()
                         } else {
                             None
                         },
                         _stats: a.diff.stats().cloned(),
                 self.add_item(HunkDiff::Added {
                     path: a.path.clone(),
                     header: header.clone(),
                     new: a.new.clone(),
                     hunk: if let DiffContent::Plain {
                         hunks: Hunks(mut hs),
                         ..
                     } = a.diff.clone()
                     {
                         hs.pop()
                     } else {
                         None
                     },
                     state,
                 );
                     _stats: a.diff.stats().cloned(),
                 });
             }
             FileDiff::Deleted(d) => {
                 self.add_item(
                     HunkDiff::Deleted {
                         path: d.path,
                         header: header.clone(),
                         old: d.old,
                         hunk: if let DiffContent::Plain {
                             hunks: Hunks(mut hs),
                             ..
                         } = d.diff.clone()
                         {
                             hs.pop()
                         } else {
                             None
                         },
                         _stats: d.diff.stats().cloned(),
                 self.add_item(HunkDiff::Deleted {
                     path: d.path.clone(),
                     header: header.clone(),
                     old: d.old.clone(),
                     hunk: if let DiffContent::Plain {
                         hunks: Hunks(mut hs),
                         ..
                     } = d.diff.clone()
                     {
                         hs.pop()
                     } else {
                         None
                     },
                     state,
                 );
                     _stats: d.diff.stats().cloned(),
                 });
             }
             FileDiff::Modified(m) => {
                 if m.old.mode != m.new.mode {
                     self.add_item(
                         HunkDiff::ModeChanged {
                             path: m.path.clone(),
                             header: header.clone(),
                             old: m.old.clone(),
                             new: m.new.clone(),
                         },
                         state.clone(),
                     );
                     self.add_item(HunkDiff::ModeChanged {
                         path: m.path.clone(),
                         header: header.clone(),
                         old: m.old.clone(),
                         new: m.new.clone(),
                     });
                 }
                 match m.diff {
                 match m.diff.clone() {
                     DiffContent::Empty => {
                         // Likely a file mode change, which is handled above.
                     }
                     DiffContent::Binary => {
                         self.add_item(
                             HunkDiff::Modified {
                                 path: m.path.clone(),
                                 header: header.clone(),
                                 old: m.old.clone(),
                                 new: m.new.clone(),
                                 hunk: None,
                                 _stats: m.diff.stats().cloned(),
                             },
                             state,
                         );
                         self.add_item(HunkDiff::Modified {
                             path: m.path.clone(),
                             header: header.clone(),
                             old: m.old.clone(),
                             new: m.new.clone(),
                             hunk: None,
                             _stats: m.diff.stats().cloned(),
                         });
                     }
                     DiffContent::Plain {
                         hunks: Hunks(hunks),
                         eof,
                         stats,
                     } => {
                         for hunk in hunks {
                             self.add_item(
                                 HunkDiff::Modified {
                                     path: m.path.clone(),
                                     header: header.clone(),
                                     old: m.old.clone(),
                                     new: m.new.clone(),
                                     hunk: Some(hunk),
                                     _stats: Some(stats),
                                 },
                                 state.clone(),
                             );
                         let base_hunks = hunks.clone();
                         for hunk in base_hunks {
                             self.add_item(HunkDiff::Modified {
                                 path: m.path.clone(),
                                 header: header.clone(),
                                 old: m.old.clone(),
                                 new: m.new.clone(),
                                 hunk: Some(hunk),
                                 _stats: Some(stats),
                             });
                         }
                         if let EofNewLine::OldMissing | EofNewLine::NewMissing = eof {
                             self.add_item(
                                 HunkDiff::EofChanged {
                                     path: m.path.clone(),
                                     header: header.clone(),
                                     old: m.old.clone(),
                                     new: m.new.clone(),
                                     _eof: eof,
                                 },
                                 state,
                             )
                             self.add_item(HunkDiff::EofChanged {
                                 path: m.path.clone(),
                                 header: header.clone(),
                                 old: m.old.clone(),
                                 new: m.new.clone(),
                                 _eof: eof,
                             })
                         }
                     }
                 }

@@ -171,35 +149,38 @@ impl ReviewQueue {

         }
     }
     fn add_item(&mut self, item: HunkDiff, state: HunkState) {
         self.queue.push_back((self.queue.len(), item, state));
     fn add_item(&mut self, item: HunkDiff) {
         self.hunks.push(item);
     }
 }
 impl std::ops::Deref for ReviewQueue {
     type Target = VecDeque<(usize, HunkDiff, HunkState)>;
     pub fn contains(&self, other: &HunkDiff) -> bool {
         for item in &self.hunks {
             if item.path() == other.path() && item.hunk() == other.hunk() {
                 return true;
             }
         }
     fn deref(&self) -> &Self::Target {
         &self.queue
         false
     }
 }
 impl std::ops::DerefMut for ReviewQueue {
     fn deref_mut(&mut self) -> &mut Self::Target {
         &mut self.queue
 impl std::ops::Deref for Hunks {
     type Target = Vec<HunkDiff>;
     fn deref(&self) -> &Self::Target {
         &self.hunks
     }
 }
 impl Iterator for ReviewQueue {
     type Item = (usize, HunkDiff, HunkState);
     fn next(&mut self) -> Option<Self::Item> {
         self.queue.pop_front()
 impl std::ops::DerefMut for Hunks {
     fn deref_mut(&mut self) -> &mut Self::Target {
         &mut self.hunks
     }
 }
 /// Builds a review for a single file.
 /// Adjusts line deltas when a hunk is ignored.
 #[derive(Debug)]
 pub struct FileReviewBuilder {
     delta: i32,
     header: FileHeader,

@@ -222,6 +203,12 @@ impl FileReviewBuilder {

         self
     }
     pub fn ignore_item(&mut self, item: &HunkDiff) {
         if let Some(h) = item.hunk_header() {
             self.delta += h.new_size as i32 - h.old_size as i32;
         }
     }
     pub fn item_diff(&mut self, item: &HunkDiff) -> Result<git::raw::Diff, Error> {
         let mut buf = Vec::new();
         let mut writer = unified_diff::Writer::new(&mut buf);

@@ -241,13 +228,14 @@ impl FileReviewBuilder {

         }
         drop(writer);
         log::debug!("Building item diff ({:?})", String::from_utf8(buf.clone()));
         git::raw::Diff::from_buffer(&buf).map_err(Error::from)
     }
 }
 /// Represents the reviewer's brain, ie. what they have seen or not seen in terms
 /// of changes introduced by a patch.
 #[derive(Clone)]
 #[derive(Clone, Debug)]
 pub struct Brain<'a> {
     /// Where the review draft is being stored.
     refname: git::Namespaced<'a>,

@@ -398,11 +386,7 @@ impl<'a> DiffUtil<'a> {

         Self { repo }
     }
     pub fn base_queue(
         &self,
         brain: Brain<'a>,
         revision: &Revision,
     ) -> anyhow::Result<(Diff, Diff)> {
     pub fn all_diffs(&self, revision: &Revision) -> anyhow::Result<Diff> {
         let repo = self.repo.raw();
         let base = repo.find_commit((*revision.base()).into())?.tree()?;

@@ -415,9 +399,23 @@ impl<'a> DiffUtil<'a> {

         opts.patience(true).minimal(true).context_lines(3_u32);
         let base_diff = self.diff(&base, &revision, repo, &mut opts)?;
         let queue_diff = self.diff(brain.accepted(), &revision, repo, &mut opts)?;
         Ok((base_diff, queue_diff))
         Ok(base_diff)
     }
     pub fn rejected_diffs(&self, brain: &Brain<'a>, revision: &Revision) -> anyhow::Result<Diff> {
         let repo = self.repo.raw();
         let revision = {
             let commit = repo.find_commit(revision.head().into())?;
             commit.tree()?
         };
         let mut opts = git::raw::DiffOptions::new();
         opts.patience(true).minimal(true).context_lines(3_u32);
         let rejected = self.diff(brain.accepted(), &revision, repo, &mut opts)?;
         Ok(rejected)
     }
     pub fn diff(

@@ -453,11 +451,9 @@ impl<'a> ReviewBuilder<'a> {

         Self { repo }
     }
     /// Assemble the review for the given revision.
     pub fn hunks(&self, brain: &'a Brain<'a>, revision: &Revision) -> anyhow::Result<ReviewQueue> {
         DiffUtil::new(self.repo)
             .base_queue(brain.clone(), revision)
             .map(|(base, queue)| Ok(ReviewQueue::new(base, queue)))?
     pub fn hunks(&self, revision: &Revision) -> anyhow::Result<Hunks> {
         let diff = DiffUtil::new(self.repo).all_diffs(revision)?;
         Ok(Hunks::new(diff))
     }
 }

modified bin/git.rs

@@ -1,13 +1,18 @@

 use std::fmt;
 use std::fmt::Debug;
 use std::path::Path;
 use std::{fs, path::PathBuf};
 use ratatui::text::Line;
 use radicle_surf::diff::{Copied, DiffFile, EofNewLine, FileStats, Hunk, Modification, Moved};
 use radicle::git;
 use radicle::git::Oid;
 use radicle_cli::git::unified_diff::{FileHeader, HunkHeader};
 use radicle_cli::terminal;
 use radicle_cli::terminal::highlight::Highlighter;
 use radicle_surf::diff::{Copied, DiffFile, EofNewLine, FileStats, Hunk, Modification, Moved};
 use ratatui::text::Line;
 pub type FilePaths<'a> = (Option<(&'a Path, Oid)>, Option<(&'a Path, Oid)>);

@@ -215,13 +220,14 @@ impl From<&Hunk<Modification>> for HunkStats {

 #[derive(Clone, Default, Debug, PartialEq)]
 pub enum HunkState {
     #[default]
     Unknown,
     Rejected,
     Accepted,
 }
 /// A single review item. Can be a hunk or eg. a file move.
 /// Files are usually split into multiple review items.
 #[derive(Clone, Debug)]
 #[derive(Clone, PartialEq)]
 pub enum HunkDiff {
     Added {
         path: PathBuf,

@@ -276,6 +282,18 @@ impl HunkDiff {

         }
     }
     pub fn path(&self) -> &PathBuf {
         match self {
             Self::Added { path, .. } => path,
             Self::Deleted { path, .. } => path,
             Self::Moved { moved } => &moved.new_path,
             Self::Copied { copied } => &copied.new_path,
             Self::Modified { path, .. } => path,
             Self::EofChanged { path, .. } => path,
             Self::ModeChanged { path, .. } => path,
         }
     }
     pub fn file_header(&self) -> FileHeader {
         match self {
             Self::Added { header, .. } => header.clone(),

@@ -326,6 +344,29 @@ impl HunkDiff {

     }
 }
 impl Debug for HunkDiff {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         let (name, path, hunk) = match self {
             Self::Added { path, hunk, .. } => ("Added", path, hunk),
             Self::Deleted { path, hunk, .. } => ("Deleted", path, hunk),
             Self::Moved { moved } => ("Moved", &moved.new_path, &None),
             Self::Copied { copied } => ("Copied", &copied.new_path, &None),
             Self::Modified { path, hunk, .. } => ("Modified", path, hunk),
             Self::EofChanged { path, .. } => ("EofChanged", path, &None),
             Self::ModeChanged { path, .. } => ("ModeChanged", path, &None),
         };
         match hunk {
             Some(hunk) => f
                 .debug_struct(name)
                 .field("path", path)
                 .field("hunk", &(hunk.old.clone(), hunk.new.clone()))
                 .finish(),
             _ => f.debug_struct(name).field("path", path).finish(),
         }
     }
 }
 #[derive(Clone, Debug)]
 pub struct StatefulHunkDiff(HunkDiff, HunkState);

@@ -343,8 +384,8 @@ impl StatefulHunkDiff {

     }
 }
 impl From<(&HunkDiff, &HunkState)> for StatefulHunkDiff {
     fn from(value: (&HunkDiff, &HunkState)) -> Self {
         Self(value.0.clone(), value.1.clone())
 impl From<&HunkDiff> for StatefulHunkDiff {
     fn from(diff: &HunkDiff) -> Self {
         Self(diff.clone(), HunkState::Unknown)
     }
 }

modified bin/test.rs

@@ -1,20 +1,213 @@

 pub mod setup {
     use std::path::Path;
     use radicle::git;
     use radicle::rad;
     use radicle::storage::git::Repository;
     use radicle::test::setup::{BranchWith, Node};
     /// A node with a repository.
     pub struct NodeWithRepo {
         pub node: Node,
         pub repo: NodeRepo,
     }
     impl std::ops::Deref for NodeWithRepo {
         type Target = Node;
         fn deref(&self) -> &Self::Target {
             &self.node
         }
     }
     impl std::ops::DerefMut for NodeWithRepo {
         fn deref_mut(&mut self) -> &mut Self::Target {
             &mut self.node
         }
     }
     /// A node repository with an optional checkout.
     pub struct NodeRepo {
         pub repo: Repository,
         pub checkout: Option<NodeRepoCheckout>,
     }
     impl NodeRepo {
         #[track_caller]
         pub fn checkout(&self) -> &NodeRepoCheckout {
             self.checkout.as_ref().unwrap()
         }
     }
     impl std::ops::Deref for NodeRepo {
         type Target = Repository;
         fn deref(&self) -> &Self::Target {
             &self.repo
         }
     }
     impl std::ops::DerefMut for NodeRepo {
         fn deref_mut(&mut self) -> &mut Self::Target {
             &mut self.repo
         }
     }
     /// A repository checkout.
     pub struct NodeRepoCheckout {
         pub checkout: git::raw::Repository,
     }
     impl NodeRepoCheckout {
         pub fn branch_with<S: AsRef<Path>, T: AsRef<[u8]>>(
             &self,
             blobs: impl IntoIterator<Item = (S, T)>,
         ) -> BranchWith {
             let refname = git::Qualified::from(git::lit::refs_heads(git::refname!("master")));
             let base = self.checkout.refname_to_id(refname.as_str()).unwrap();
             let parent = self.checkout.find_commit(base).unwrap();
             let oid = commit(&self.checkout, &refname, blobs, &[&parent]);
             git::push(&self.checkout, &rad::REMOTE_NAME, [(&refname, &refname)]).unwrap();
             BranchWith {
                 base: base.into(),
                 oid,
             }
         }
     }
     impl std::ops::Deref for NodeRepoCheckout {
         type Target = git::raw::Repository;
         fn deref(&self) -> &Self::Target {
             &self.checkout
         }
     }
     pub fn commit<S: AsRef<Path>, T: AsRef<[u8]>>(
         repo: &git2::Repository,
         refname: &git::Qualified,
         blobs: impl IntoIterator<Item = (S, T)>,
         parents: &[&git2::Commit<'_>],
     ) -> git::Oid {
         let tree = {
             let mut tb = repo.treebuilder(None).unwrap();
             for (name, blob) in blobs.into_iter() {
                 let oid = repo.blob(blob.as_ref()).unwrap();
                 tb.insert(name.as_ref(), oid, git2::FileMode::Blob.into())
                     .unwrap();
             }
             tb.write().unwrap()
         };
         let tree = repo.find_tree(tree).unwrap();
         let author = git2::Signature::now("anonymous", "anonymous@example.com").unwrap();
         repo.commit(
             Some(refname.as_str()),
             &author,
             &author,
             "Making changes",
             &tree,
             parents,
         )
         .unwrap()
         .into()
     }
 }
 pub mod fixtures {
     use std::path::Path;
     use anyhow::Result;
     use radicle::cob::cache::NoCache;
     use radicle::crypto::{Signer, Verified};
     use radicle::git;
     use radicle::identity::{RepoId, Visibility};
     use radicle::patch::{Cache, MergeTarget, PatchMut, Patches};
     use radicle::rad;
     use radicle::storage::git::Repository;
     use radicle::test::setup::{BranchWith, NodeWithRepo};
     use radicle::storage::refs::SignedRefs;
     use radicle::storage::ReadStorage;
     use radicle::test::setup::{BranchWith, Node};
     use radicle::Storage;
     use super::setup::{NodeRepo, NodeRepoCheckout, NodeWithRepo};
     /// The birth of the radicle project, January 1st, 2018.
     pub const RADICLE_EPOCH: i64 = 1514817556;
     pub const MAIN_RS: &str = r#"// This is a comment, and is ignored by the compiler.
 // You can test this code by clicking the "Run" button over there ->
 // or if you prefer to use your keyboard, you can use the "Ctrl + Enter"
 // shortcut.
 // This code is editable, feel free to hack it!
 // You can always return to the original code by clicking the "Reset" button ->
 // This is the main function.
 fn main() {
     // Statements here are executed when the compiled binary is called.
     // Print text to the console.
     println!("Hello World!");
 }
 "#;
     pub fn node_with_repo() -> NodeWithRepo {
         NodeWithRepo::default()
         let node = Node::default();
         let (id, _, checkout, _) =
             project(node.root.join("working"), &node.storage, &node.signer).unwrap();
         let repo = node.storage.repository(id).unwrap();
         let checkout = Some(NodeRepoCheckout { checkout });
         NodeWithRepo {
             node,
             repo: NodeRepo { repo, checkout },
         }
     }
     pub fn branch(node: &NodeWithRepo) -> BranchWith {
     pub fn branch_with_eof_removed(node: &NodeWithRepo) -> BranchWith {
         let checkout = node.repo.checkout();
         checkout.branch_with([("README", b"Hello World!")])
     }
     pub fn branch_with_main_changed(node: &NodeWithRepo) -> BranchWith {
         let checkout = node.repo.checkout();
         let main_rs = r#"// This is a comment, and is ignored by the compiler.
 // You can test this code by clicking the "Run" button over there ->
 // or if you prefer to use your keyboard, you can use the "Ctrl + Enter"
 // shortcut.
 // This is a new comment.
 // This code is editable, feel free to hack it!
 // You can always return to the original code by clicking the "Reset" button ->
 // This is the main function.
 fn main() {
     // Statements here are executed when the compiled binary is called.
     // Print text to the console.
     println!("Hello World!");
     println!("Hello again");
 }
 "#;
         checkout.branch_with([("main.rs", main_rs.as_bytes())])
     }
     pub fn branch_with_main_emptied(node: &NodeWithRepo) -> BranchWith {
         let checkout = node.repo.checkout();
         checkout.branch_with([("main.rs", b"")])
     }
     pub fn branch_with_main_deleted_and_file_added(node: &NodeWithRepo) -> BranchWith {
         let checkout = node.repo.checkout();
         checkout.branch_with([("CONTRIBUTE", b"TBD\n")])
     }
     pub fn patch<'a, 'g>(
         node: &'a NodeWithRepo,
         branch: &BranchWith,

@@ -32,4 +225,59 @@ pub mod fixtures {

         Ok(patch)
     }
     /// Create a new repository at the given path, and initialize it into a project.
     pub fn project<P: AsRef<Path>, G: Signer>(
         path: P,
         storage: &Storage,
         signer: &G,
     ) -> Result<(RepoId, SignedRefs<Verified>, git2::Repository, git2::Oid), rad::InitError> {
         radicle::storage::git::transport::local::register(storage.clone());
         let (working, head) = repository(path);
         let (id, _, refs) = rad::init(
             &working,
             "acme".try_into().unwrap(),
             "Acme's repository",
             git::refname!("master"),
             Visibility::default(),
             signer,
             storage,
         )?;
         Ok((id, refs, working, head))
     }
     /// Creates a regular repository at the given path with a couple of commits.
     pub fn repository<P: AsRef<Path>>(path: P) -> (git2::Repository, git2::Oid) {
         let repo = git2::Repository::init(path).unwrap();
         let sig = git2::Signature::new(
             "anonymous",
             "anonymous@radicle.xyz",
             &git2::Time::new(RADICLE_EPOCH, 0),
         )
         .unwrap();
         let head = git::initial_commit(&repo, &sig).unwrap();
         let tree = git::write_tree(Path::new("main.rs"), MAIN_RS.as_bytes(), &repo).unwrap();
         let oid = {
             let commit = git::commit(
                 &repo,
                 &head,
                 git::refname!("refs/heads/master").as_refstr(),
                 "Second commit",
                 &sig,
                 &tree,
             )
             .unwrap();
             commit.id()
         };
         repo.set_head("refs/heads/master").unwrap();
         repo.checkout_head(None).unwrap();
         drop(tree);
         drop(head);
         (repo, oid)
     }
 }

modified bin/ui/items.rs

@@ -1,4 +1,6 @@

 use std::collections::HashMap;
 use std::fmt;
 use std::fmt::Debug;
 use std::str::FromStr;
 use nom::bytes::complete::{tag, take};

@@ -1090,7 +1092,7 @@ impl From<Vec<(EntryId, Comment<CodeLocation>)>> for HunkComments {

 /// A [`HunkItem`] that can be rendered. Hunk items are indexed sequentially and
 /// provide access to the underlying hunk type.
 #[derive(Clone, Debug)]
 #[derive(Clone)]
 pub struct HunkItem<'a> {
     /// The underlying hunk type and its current state (accepted / rejected).
     pub inner: StatefulHunkDiff,

@@ -1580,6 +1582,15 @@ impl<'a> HunkItem<'a> {

     }
 }
 impl<'a> Debug for HunkItem<'a> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         f.debug_struct("HunkItem")
             .field("inner", &self.inner)
             .field("comments", &self.comments)
             .finish()
     }
 }
 pub struct HighlightedLine<'a>(Line<'a>);
 impl<'a> From<Line<'a>> for HighlightedLine<'a> {

modified bin/ui/span.rs

@@ -11,8 +11,8 @@ use tui::ui::span;

 pub fn hunk_state(state: &HunkState) -> Span<'static> {
     match state {
         HunkState::Unknown => span::secondary("?"),
         HunkState::Accepted => span::positive("✓"),
         // HunkState::Rejected => span::secondary("?"),
         HunkState::Rejected => span::negative("✗"),
     }
 }