Radish alpha
r
rad:z39mP9rQAaGmERfUMPULfPUi473tY
Radicle terminal user interface
Radicle
Git
bin(patch-review): Support for accepting hunks
Erik Kundt committed 1 year ago
commit f6fe140aba749bcc9a2531428b48b7e4f1d0faa0
parent 6c36e7a
5 files changed +543 -388
modified bin/cob.rs
@@ -4,6 +4,7 @@ use anyhow::Result;

use radicle::cob::Label;
use radicle::prelude::Did;
+
use radicle_cli::git::unified_diff::FileHeader;

use std::path::{Path, PathBuf};

@@ -21,7 +22,7 @@ pub mod inbox;
pub mod issue;
pub mod patch;

-
pub type IndexedHunkItem = (usize, crate::cob::HunkItem);
+
pub type IndexedHunkItem = (usize, crate::cob::HunkItem, HunkState);

#[allow(dead_code)]
pub fn parse_labels(input: String) -> Result<Vec<Label>> {
@@ -90,24 +91,34 @@ impl From<&Hunk<Modification>> for HunkStats {
    }
}

+
#[derive(Clone, Default, Debug, PartialEq)]
+
pub enum HunkState {
+
    #[default]
+
    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)]
pub enum HunkItem {
    FileAdded {
        path: PathBuf,
+
        header: FileHeader,
        new: DiffFile,
        hunk: Option<Hunk<Modification>>,
        _stats: Option<FileStats>,
    },
    FileDeleted {
        path: PathBuf,
+
        header: FileHeader,
        old: DiffFile,
        hunk: Option<Hunk<Modification>>,
        _stats: Option<FileStats>,
    },
    FileModified {
        path: PathBuf,
+
        header: FileHeader,
        old: DiffFile,
        new: DiffFile,
        hunk: Option<Hunk<Modification>>,
@@ -121,12 +132,14 @@ pub enum HunkItem {
    },
    FileEofChanged {
        path: PathBuf,
+
        header: FileHeader,
        old: DiffFile,
        new: DiffFile,
        _eof: EofNewLine,
    },
    FileModeChanged {
        path: PathBuf,
+
        header: FileHeader,
        old: DiffFile,
        new: DiffFile,
    },
@@ -142,6 +155,24 @@ impl HunkItem {
        }
    }

+
    pub 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(),
+
        }
+
    }
+

    pub fn hunk_header(&self) -> Option<HunkHeader> {
        self.hunk().and_then(|h| HunkHeader::try_from(h).ok())
    }
modified bin/commands/patch.rs
@@ -232,27 +232,6 @@ pub async fn run(options: Options, ctx: impl terminal::Context) -> anyhow::Resul
            let rid = options.repo.unwrap_or(rid);
            let repo = profile.storage.repository(rid).unwrap();

-
            // Load patch
-
            // let patch_id = if let Some(patch_id) = &opts.patch_id {
-
            //     patch_id.resolve(&repo.backend)?
-
            // } else {
-
            //     let opts = SelectOptions {
-
            //         mode: common::Mode::Id,
-
            //         ..SelectOptions::default()
-
            //     };
-

-
            //     // Run TUI with patch selection interface
-
            //     let selection = interface::select(opts, profile.clone(), rid).await?;
-
            //     let patch_id = selection
-
            //         .and_then(|selection| selection.ids.first().cloned())
-
            //         .map(|id| *id);
-

-
            //     if patch_id.is_none() {
-
            //         anyhow::bail!("a patch id must be provided");
-
            //     }
-

-
            //     patch_id.unwrap()
-
            // };
            let patch_id: ObjectId = if let Some(patch_id) = &opts.patch_id {
                patch_id.resolve(&repo.backend)?
            } else {
@@ -327,8 +306,8 @@ mod interface {
        let (_, revision) = opts.revision_or_latest(&patch, &repo)?;

        let brain = Brain::load_or_new(patch_id, &revision, repo.raw(), &signer)?;
-
        let builder = ReviewBuilder::new(patch_id.into(), &signer, &repo);
-
        let hunks = builder.all_hunks(&brain, &revision)?;
+
        let builder = ReviewBuilder::new(&repo).hunks(&brain, &revision)?;
+
        let hunks = builder;

        let drafts = DraftStore::new(&repo, *signer.public_key());
        let mut patches = cob::patch::Cache::no_cache(&drafts)?;
@@ -368,29 +347,25 @@ mod interface {
            let (review_id, review) = patch::find_review(&patch, revision, &signer)
                .ok_or_else(|| anyhow!("Could not find review."))?;

-
            log::info!(
-
                "Found comments for {review_id}: {:?}",
-
                review.comments().collect::<Vec<_>>()
-
            );
-

-
            let selection = review::Tui::new(profile.clone(), rid, review.clone(), hunks.clone())
-
                .run()
-
                .await?;
+
            let selection = review::Tui::new(
+
                patch_id,
+
                revision.clone(),
+
                review.clone(),
+
                hunks.clone(),
+
                profile.clone(),
+
                rid,
+
            )
+
            .run()
+
            .await?;
            log::info!("Received selection from TUI: {:?}", selection);

            if let Some(selection) = selection.as_ref() {
                match ReviewAction::try_from(selection.action)? {
-
                    ReviewAction::Accept => {
-
                        // brain accept
-
                    }
-
                    ReviewAction::Ignore => {
-
                        // next hunk
-
                    }
                    ReviewAction::Comment => {
                        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
@@ -8,18 +8,24 @@ use std::sync::Mutex;

use anyhow::Result;

+
use termion::event::Key;
+

use ratatui::layout::Position;
use ratatui::layout::{Constraint, Layout};
use ratatui::style::Stylize;
use ratatui::text::Text;
use ratatui::{Frame, Viewport};
-
use termion::event::Key;

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::Profile;

+
use radicle_cli::terminal;
+

use radicle_tui as tui;

use tui::store;
@@ -31,41 +37,20 @@ use tui::ui::span;
use tui::ui::Column;
use tui::{Channel, Exit};

+
use crate::cob::HunkState;
+
use crate::tui_patch::review::builder::DiffUtil;
use crate::ui::items::HunkItem;

+
use self::builder::Brain;
+
use self::builder::FileReviewBuilder;
use self::builder::ReviewQueue;

/// The actions that a user can carry out on a review item.
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub enum ReviewAction {
-
    Accept,
-
    Ignore,
    Comment,
}

-
impl std::fmt::Display for ReviewAction {
-
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-
        match self {
-
            Self::Accept => write!(f, "accept"),
-
            Self::Ignore => write!(f, "ignore"),
-
            Self::Comment => write!(f, "comment"),
-
        }
-
    }
-
}
-

-
impl TryFrom<&str> for ReviewAction {
-
    type Error = anyhow::Error;
-

-
    fn try_from(value: &str) -> Result<Self, Self::Error> {
-
        match value {
-
            "accept" => Ok(ReviewAction::Accept),
-
            "ignore" => Ok(ReviewAction::Ignore),
-
            "comment" => Ok(ReviewAction::Comment),
-
            _ => anyhow::bail!("Unknown review action"),
-
        }
-
    }
-
}
-

#[derive(Clone, Debug, Eq, PartialEq)]
pub struct Args(String);

@@ -77,31 +62,45 @@ pub struct Selection {
}

pub struct Tui {
-
    pub profile: Profile,
-
    pub rid: RepoId,
+
    pub patch: PatchId,
+
    pub revision: Revision,
    pub review: Review,
    pub queue: ReviewQueue,
+
    pub profile: Profile,
+
    pub rid: RepoId,
}

impl Tui {
-
    pub fn new(profile: Profile, rid: RepoId, review: Review, queue: ReviewQueue) -> Self {
+
    pub fn new(
+
        patch: PatchId,
+
        revision: Revision,
+
        review: Review,
+
        queue: ReviewQueue,
+
        profile: Profile,
+
        rid: RepoId,
+
    ) -> Self {
        Self {
-
            rid,
-
            profile,
+
            patch,
+
            revision,
            review,
            queue,
+
            rid,
+
            profile,
        }
    }

    pub async fn run(&self) -> Result<Option<Selection>> {
        let viewport = Viewport::Fullscreen;
+
        let _ = self.profile.signer()?;

        let channel = Channel::default();
        let state = App::new(
-
            self.profile.clone(),
-
            self.rid,
+
            self.patch,
+
            self.revision.clone(),
            self.review.clone(),
            self.queue.clone(),
+
            self.profile.clone(),
+
            self.rid,
        )?;

        tui::im(state, viewport, channel).await
@@ -114,8 +113,9 @@ pub enum Message {
    ItemChanged { state: TableState },
    ItemViewChanged { state: ReviewItemState },
    Quit,
-
    Accept,
    Comment,
+
    Accept,
+
    Discard,
    ShowMain,
    ShowHelp,
}
@@ -138,8 +138,12 @@ pub struct ReviewItemState {

#[derive(Clone)]
pub struct App<'a> {
+
    patch: PatchId,
+
    revision: Revision,
    queue: Arc<Mutex<(Vec<HunkItem<'a>>, TableState)>>,
    items: HashMap<usize, ReviewItemState>,
+
    profile: Profile,
+
    rid: RepoId,
    page: AppPage,
    windows: GroupState,
    help: HelpState<'a>,
@@ -150,26 +154,29 @@ impl<'a> TryFrom<&Tui> for App<'a> {

    fn try_from(tui: &Tui) -> Result<Self, Self::Error> {
        App::new(
-
            tui.profile.clone(),
-
            tui.rid,
+
            tui.patch,
+
            tui.revision.clone(),
            tui.review.clone(),
            tui.queue.clone(),
+
            tui.profile.clone(),
+
            tui.rid,
        )
    }
}

impl<'a> App<'a> {
    pub fn new(
-
        profile: Profile,
-
        rid: RepoId,
+
        patch: PatchId,
+
        revision: Revision,
        review: Review,
        queue: ReviewQueue,
+
        profile: Profile,
+
        rid: RepoId,
    ) -> Result<Self, anyhow::Error> {
-
        let repository = profile.storage.repository(rid)?;
-

+
        let repo = profile.storage.repository(rid)?;
        let queue = queue
            .iter()
-
            .map(|item| HunkItem::from((&repository, &review, item)))
+
            .map(|item| HunkItem::from((&repo, &review, item)))
            .collect::<Vec<_>>();

        let mut items = HashMap::new();
@@ -183,15 +190,93 @@ impl<'a> App<'a> {
        }

        Ok(Self {
+
            profile,
+
            rid,
+
            patch,
+
            revision,
+
            queue: Arc::new(Mutex::new((queue, TableState::new(Some(0))))),
+
            items,
            page: AppPage::Main,
            windows: GroupState::new(2, Some(0)),
            help: HelpState {
                text: TextViewState::new(help_text(), Position::default()),
            },
-
            queue: Arc::new(Mutex::new((queue, TableState::new(Some(0))))),
-
            items,
        })
    }
+

+
    pub fn accept_current_hunk(&mut self) -> Result<()> {
+
        let repo = self.profile.storage.repository(self.rid).unwrap();
+
        let signer = terminal::signer(&self.profile)?;
+

+
        let mut brain = Brain::load_or_new(self.patch, &self.revision, repo.raw(), &signer)?;
+
        let selected = self.queue.lock().unwrap().1.selected();
+

+
        if let Some(selected) = selected {
+
            let hunks = self.queue.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.1),
+
                    None => file.insert(FileReviewBuilder::new(&hunk.inner.1)),
+
                };
+

+
                let diff = file.item_diff(&hunk.inner.1)?;
+
                brain.accept(diff, repo.raw())?;
+

+
                self.reload_states()?;
+
            }
+
        }
+

+
        Ok(())
+
    }
+

+
    pub fn discard_accepted_hunks(&mut self) -> Result<()> {
+
        let repo = self.profile.storage.repository(self.rid).unwrap();
+
        let signer = terminal::signer(&self.profile)?;
+

+
        let mut brain = Brain::load_or_new(self.patch, &self.revision, repo.raw(), &signer)?;
+
        brain.discard_accepted(repo.raw())?;
+

+
        self.reload_states()?;
+

+
        Ok(())
+
    }
+

+
    pub fn reload_states(&mut self) -> anyhow::Result<()> {
+
        let repo = self.profile.storage.repository(self.rid).unwrap();
+
        let signer = terminal::signer(&self.profile)?;
+

+
        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.queue.lock().unwrap();
+
        for (idx, new_state) in states.iter().enumerate() {
+
            if let Some(hunk) = queue.0.get_mut(idx) {
+
                let (_, _, ref mut state) = hunk.inner;
+
                *state = new_state.clone();
+
            }
+
        }
+

+
        Ok(())
+
    }
}

impl<'a> App<'a> {
@@ -252,6 +337,45 @@ impl<'a> App<'a> {
            );
        }
    }
+

+
    fn show_context_bar(&self, ui: &mut Ui<Message>, frame: &mut Frame) {
+
        let queue = &self.queue.lock().unwrap().0;
+

+
        let hunks_total = queue.len();
+
        let hunks_accepted = queue
+
            .iter()
+
            .filter(|item| item.inner.2 == HunkState::Accepted)
+
            .collect::<Vec<_>>()
+
            .len();
+

+
        let accepted_stats = format!(" Accepted {hunks_accepted}/{hunks_total} ");
+

+
        ui.bar(
+
            frame,
+
            [
+
                Column::new(
+
                    span::default(" Review ").cyan().dim().reversed(),
+
                    Constraint::Length(8),
+
                ),
+
                Column::new(
+
                    span::default(&" ".to_string())
+
                        .into_left_aligned_line()
+
                        .style(ui.theme().bar_on_black_style),
+
                    Constraint::Fill(1),
+
                ),
+
                Column::new(
+
                    span::default(&accepted_stats)
+
                        .into_right_aligned_line()
+
                        .cyan()
+
                        .dim()
+
                        .reversed(),
+
                    Constraint::Length(accepted_stats.chars().count() as u16),
+
                ),
+
            ]
+
            .to_vec(),
+
            Some(Borders::None),
+
        );
+
    }
}

impl<'a> Show<Message> for App<'a> {
@@ -259,10 +383,6 @@ impl<'a> Show<Message> for App<'a> {
        Window::default().show(ctx, |ui| {
            let mut page_focus = self.windows.focus();

-
            let hunks_total = 10;
-
            let hunks_accepted = 5;
-
            let accepted_stats = format!(" Accepted {hunks_accepted}/{hunks_total} ");
-

            match self.page {
                AppPage::Main => {
                    ui.layout(
@@ -290,37 +410,14 @@ impl<'a> Show<Message> for App<'a> {
                                });
                            }

-
                            ui.bar(
-
                                frame,
-
                                [
-
                                    Column::new(
-
                                        span::default(" Review ").cyan().dim().reversed(),
-
                                        Constraint::Length(8),
-
                                    ),
-
                                    Column::new(
-
                                        span::default(&" ".to_string())
-
                                            .into_left_aligned_line()
-
                                            .style(ui.theme().bar_on_black_style),
-
                                        Constraint::Fill(1),
-
                                    ),
-
                                    Column::new(
-
                                        span::default(&accepted_stats)
-
                                            .into_right_aligned_line()
-
                                            .cyan()
-
                                            .dim()
-
                                            .reversed(),
-
                                        Constraint::Length(accepted_stats.chars().count() as u16),
-
                                    ),
-
                                ]
-
                                .to_vec(),
-
                                Some(Borders::None),
-
                            );
+
                            self.show_context_bar(ui, frame);

                            ui.shortcuts(
                                frame,
                                &[
-
                                    ("a", "accept"),
                                    ("c", "comment"),
+
                                    ("a", "accept"),
+
                                    ("d", "discard accepted"),
                                    ("?", "help"),
                                    ("q", "quit"),
                                ],
@@ -330,11 +427,14 @@ impl<'a> Show<Message> for App<'a> {
                            if ui.input_global(|key| key == Key::Char('?')) {
                                ui.send_message(Message::ShowHelp);
                            }
+
                            if ui.input_global(|key| key == Key::Char('c')) {
+
                                ui.send_message(Message::Comment);
+
                            }
                            if ui.input_global(|key| key == Key::Char('a')) {
                                ui.send_message(Message::Accept);
                            }
-
                            if ui.input_global(|key| key == Key::Char('c')) {
-
                                ui.send_message(Message::Comment);
+
                            if ui.input_global(|key| key == Key::Char('d')) {
+
                                ui.send_message(Message::Discard);
                            }
                        },
                    );
@@ -401,37 +501,64 @@ impl<'a> store::Update<Message> for App<'a> {
    fn update(&mut self, message: Message) -> Option<Exit<Self::Return>> {
        log::info!("Received message: {:?}", message);

-
        let mut queue = self.queue.lock().unwrap();
        match message {
            Message::WindowsChanged { state } => {
                self.windows = state;
                None
            }
            Message::ItemChanged { state } => {
+
                let mut queue = self.queue.lock().unwrap();
                queue.1 = state;
                None
            }
            Message::ItemViewChanged { state } => {
+
                let queue = self.queue.lock().unwrap();
                if let Some(selected) = queue.1.selected() {
                    self.items.insert(selected, state);
                }
                None
            }
            Message::Quit => Some(Exit { value: None }),
-
            Message::Accept => Some(Exit {
-
                value: Some(Selection {
-
                    action: ReviewAction::Accept,
-
                    hunk: queue.1.selected(),
-
                    args: None,
-
                }),
-
            }),
-
            Message::Comment => Some(Exit {
-
                value: Some(Selection {
-
                    action: ReviewAction::Comment,
-
                    hunk: queue.1.selected(),
-
                    args: None,
-
                }),
-
            }),
+
            Message::Comment => {
+
                let queue = self.queue.lock().unwrap();
+
                Some(Exit {
+
                    value: Some(Selection {
+
                        action: ReviewAction::Comment,
+
                        hunk: queue.1.selected(),
+
                        args: None,
+
                    }),
+
                })
+
            }
+
            // Message::Accept => Some(Exit {
+
            //     value: Some(Selection {
+
            //         action: ReviewAction::Accept,
+
            //         hunk: queue.1.selected(),
+
            //         args: None,
+
            //     }),
+
            // }),
+
            Message::Accept => {
+
                match self.accept_current_hunk() {
+
                    Ok(()) => log::info!("Accepted hunk."),
+
                    Err(err) => log::info!("An error occured while accepting hunk: {}", err),
+
                }
+

+
                None
+
            }
+
            Message::Discard => {
+
                match self.discard_accepted_hunks() {
+
                    Ok(()) => log::info!("Discarded all hunks."),
+
                    Err(err) => log::info!("An error occured while discarding hunks: {}", err),
+
                }
+

+
                None
+
            }
+
            // Message::Discard => Some(Exit {
+
            //     value: Some(Selection {
+
            //         action: ReviewAction::Discard,
+
            //         hunk: None,
+
            //         args: None,
+
            //     }),
+
            // }),
            Message::ShowMain => {
                self.page = AppPage::Main;
                None
modified bin/commands/patch/review/builder.rs
@@ -13,201 +13,128 @@
//!
use std::collections::VecDeque;
use std::fmt::Write as _;
-
use std::ops::{Deref, Not, Range};
-
use std::path::{Path, PathBuf};
-
use std::str::FromStr;
-
use std::{fmt, io};
-

-
use radicle::cob;
-
use radicle::cob::cache::NoCache;
-
use radicle::cob::patch::{PatchId, Revision, Verdict};
+
use std::io;
+
use std::ops::{Not, Range};
+
use std::path::PathBuf;
+

+
use radicle::cob::patch::{PatchId, Revision};
use radicle::cob::{CodeLocation, CodeRange};
use radicle::git;
use radicle::git::Oid;
-
use radicle::patch::PatchMut;
use radicle::prelude::*;
-
use radicle::storage::git::{cob::DraftStore, Repository};
+
use radicle::storage::git::Repository;
use radicle_surf::diff::*;
-
use radicle_term::{Element, VStack};

-
use radicle_cli::git::pretty_diff::ToPretty;
-
use radicle_cli::git::pretty_diff::{Blob, Blobs, Repo};
use radicle_cli::git::unified_diff::{self, FileHeader};
use radicle_cli::git::unified_diff::{Encode, HunkHeader};
use radicle_cli::terminal as term;
-
use radicle_cli::terminal::highlight::Highlighter;
-

-
use crate::cob::HunkItem;
-

-
/// Help message shown to user.
-
const HELP: &str = "\
-
y - accept this hunk
-
n - ignore this hunk
-
c - comment on this hunk
-
j - leave this hunk undecided, see next hunk
-
k - leave this hunk undecided, see previous hunk
-
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 {
-
    Accept,
-
    Ignore,
-
    Comment,
-
    Split,
-
    Next,
-
    Previous,
-
    Help,
-
    Quit,
-
}
-

-
impl ReviewAction {
-
    /// Ask the user what action to take.
-
    fn prompt(
-
        mut input: impl io::BufRead,
-
        mut output: impl io::Write,
-
        prompt: impl fmt::Display,
-
    ) -> io::Result<Option<Self>> {
-
        write!(&mut output, "{prompt} ")?;
-

-
        let mut s = String::new();
-
        input.read_line(&mut s)?;
-

-
        if s.trim().is_empty() {
-
            return Ok(None);
-
        }
-
        Self::from_str(s.trim())
-
            .map(Some)
-
            .map_err(|e| io::Error::new(io::ErrorKind::InvalidInput, e))
-
    }
-
}

-
impl std::fmt::Display for ReviewAction {
-
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-
        match self {
-
            Self::Accept => write!(f, "y"),
-
            Self::Ignore => write!(f, "n"),
-
            Self::Comment => write!(f, "c"),
-
            Self::Split => write!(f, "s"),
-
            Self::Next => write!(f, "j"),
-
            Self::Previous => write!(f, "k"),
-
            Self::Help => write!(f, "?"),
-
            Self::Quit => write!(f, "q"),
-
        }
-
    }
-
}
-

-
impl FromStr for ReviewAction {
-
    type Err = io::Error;
-

-
    fn from_str(s: &str) -> Result<Self, Self::Err> {
-
        match s {
-
            "y" => Ok(Self::Accept),
-
            "n" => Ok(Self::Ignore),
-
            "c" => Ok(Self::Comment),
-
            "s" => Ok(Self::Split),
-
            "j" => Ok(Self::Next),
-
            "k" => Ok(Self::Previous),
-
            "?" => Ok(Self::Help),
-
            "q" => Ok(Self::Quit),
-
            _ => Err(io::Error::new(
-
                io::ErrorKind::InvalidInput,
-
                format!("invalid action '{s}'"),
-
            )),
-
        }
-
    }
-
}
+
use crate::cob::{HunkItem, HunkState};

/// Queue of items (usually hunks) left to review.
#[derive(Clone, Default)]
pub struct ReviewQueue {
    /// Hunks left to review.
-
    queue: VecDeque<(usize, HunkItem)>,
+
    queue: VecDeque<(usize, HunkItem, HunkState)>,
}

impl ReviewQueue {
+
    pub fn new(base: Diff, queue: 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 a file to the queue.
    /// Mostly splits files into individual review items (eg. hunks) to review.
-
    fn add_file(&mut self, file: FileDiff) {
+
    fn add_file(&mut self, file: FileDiff, state: HunkState) {
+
        let header = FileHeader::from(&file);
+

        match file {
            FileDiff::Moved(moved) => {
-
                self.add_item(HunkItem::FileMoved { moved });
+
                self.add_item(HunkItem::FileMoved { moved }, state);
            }
            FileDiff::Copied(copied) => {
-
                self.add_item(HunkItem::FileCopied { copied });
+
                self.add_item(HunkItem::FileCopied { copied }, state);
            }
            FileDiff::Added(a) => {
-
                self.add_item(HunkItem::FileAdded {
-
                    path: a.path,
-
                    new: a.new,
-
                    hunk: if let DiffContent::Plain {
-
                        hunks: Hunks(mut hs),
-
                        ..
-
                    } = a.diff.clone()
-
                    {
-
                        hs.pop()
-
                    } else {
-
                        None
+
                self.add_item(
+
                    HunkItem::FileAdded {
+
                        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(),
                    },
-
                    _stats: a.diff.stats().cloned(),
-
                });
+
                    state,
+
                );
            }
            FileDiff::Deleted(d) => {
-
                self.add_item(HunkItem::FileDeleted {
-
                    path: d.path,
-
                    old: d.old,
-
                    hunk: if let DiffContent::Plain {
-
                        hunks: Hunks(mut hs),
-
                        ..
-
                    } = d.diff.clone()
-
                    {
-
                        hs.pop()
-
                    } else {
-
                        None
+
                self.add_item(
+
                    HunkItem::FileDeleted {
+
                        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(),
                    },
-
                    _stats: d.diff.stats().cloned(),
-
                });
+
                    state,
+
                );
            }
            FileDiff::Modified(m) => {
                if m.old.mode != m.new.mode {
-
                    self.add_item(HunkItem::FileModeChanged {
-
                        path: m.path.clone(),
-
                        old: m.old.clone(),
-
                        new: m.new.clone(),
-
                    });
+
                    self.add_item(
+
                        HunkItem::FileModeChanged {
+
                            path: m.path.clone(),
+
                            header: header.clone(),
+
                            old: m.old.clone(),
+
                            new: m.new.clone(),
+
                        },
+
                        state.clone(),
+
                    );
                }
                match m.diff {
                    DiffContent::Empty => {
                        // Likely a file mode change, which is handled above.
                    }
                    DiffContent::Binary => {
-
                        self.add_item(HunkItem::FileModified {
-
                            path: m.path.clone(),
-
                            old: m.old.clone(),
-
                            new: m.new.clone(),
-
                            hunk: None,
-
                            _stats: m.diff.stats().cloned(),
-
                        });
+
                        self.add_item(
+
                            HunkItem::FileModified {
+
                                path: m.path.clone(),
+
                                header: header.clone(),
+
                                old: m.old.clone(),
+
                                new: m.new.clone(),
+
                                hunk: None,
+
                                _stats: m.diff.stats().cloned(),
+
                            },
+
                            state,
+
                        );
                    }
                    DiffContent::Plain {
                        hunks: Hunks(hunks),
@@ -215,21 +142,29 @@ impl ReviewQueue {
                        stats,
                    } => {
                        for hunk in hunks {
-
                            self.add_item(HunkItem::FileModified {
-
                                path: m.path.clone(),
-
                                old: m.old.clone(),
-
                                new: m.new.clone(),
-
                                hunk: Some(hunk),
-
                                _stats: Some(stats),
-
                            });
+
                            self.add_item(
+
                                HunkItem::FileModified {
+
                                    path: m.path.clone(),
+
                                    header: header.clone(),
+
                                    old: m.old.clone(),
+
                                    new: m.new.clone(),
+
                                    hunk: Some(hunk),
+
                                    _stats: Some(stats),
+
                                },
+
                                state.clone(),
+
                            );
                        }
                        if let EofNewLine::OldMissing | EofNewLine::NewMissing = eof {
-
                            self.add_item(HunkItem::FileEofChanged {
-
                                path: m.path.clone(),
-
                                old: m.old.clone(),
-
                                new: m.new.clone(),
-
                                _eof: eof,
-
                            })
+
                            self.add_item(
+
                                HunkItem::FileEofChanged {
+
                                    path: m.path.clone(),
+
                                    header: header.clone(),
+
                                    old: m.old.clone(),
+
                                    new: m.new.clone(),
+
                                    _eof: eof,
+
                                },
+
                                state,
+
                            )
                        }
                    }
                }
@@ -237,23 +172,13 @@ impl ReviewQueue {
        }
    }

-
    fn add_item(&mut self, item: HunkItem) {
-
        self.queue.push_back((self.queue.len(), item));
-
    }
-
}
-

-
impl From<Diff> for ReviewQueue {
-
    fn from(diff: Diff) -> Self {
-
        let mut queue = Self::default();
-
        for file in diff.into_files() {
-
            queue.add_file(file);
-
        }
-
        queue
+
    fn add_item(&mut self, item: HunkItem, state: HunkState) {
+
        self.queue.push_back((self.queue.len(), item, state));
    }
}

impl std::ops::Deref for ReviewQueue {
-
    type Target = VecDeque<(usize, HunkItem)>;
+
    type Target = VecDeque<(usize, HunkItem, HunkState)>;

    fn deref(&self) -> &Self::Target {
        &self.queue
@@ -267,7 +192,7 @@ impl std::ops::DerefMut for ReviewQueue {
}

impl Iterator for ReviewQueue {
-
    type Item = (usize, HunkItem);
+
    type Item = (usize, HunkItem, HunkState);

    fn next(&mut self) -> Option<Self::Item> {
        self.queue.pop_front()
@@ -278,34 +203,59 @@ impl Iterator for ReviewQueue {
/// Adjusts line deltas when a hunk is ignored.
pub struct FileReviewBuilder {
    delta: i32,
+
    header: FileHeader,
}

impl FileReviewBuilder {
-
    fn new(item: &HunkItem) -> Self {
-
        Self { delta: 0 }
+
    pub fn new(item: &HunkItem) -> Self {
+
        Self {
+
            delta: 0,
+
            header: item.file_header(),
+
        }
    }

-
    fn set_item(&mut self, item: &HunkItem) -> &mut Self {
-
        self.delta = 0;
+
    pub fn set_item(&mut self, item: &HunkItem) -> &mut Self {
+
        let header = item.file_header();
+
        if self.header != header {
+
            self.header = header;
+
            self.delta = 0;
+
        }
        self
    }

-
    fn ignore_item(&mut self, item: &HunkItem) {
-
        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: &HunkItem) -> Result<git::raw::Diff, 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);
+

+
        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)]
pub struct Brain<'a> {
    /// Where the review draft is being stored.
    refname: git::Namespaced<'a>,
+
    /// The merge base
+
    base: git::raw::Commit<'a>,
    /// The commit pointed to by the ref.
    head: git::raw::Commit<'a>,
-
    /// The merge base of this revision.
-
    base: git::raw::Commit<'a>,
    /// The tree of accepted changes pointed to by the head commit.
    accepted: git::raw::Tree<'a>,
}
@@ -334,18 +284,12 @@ impl<'a> Brain<'a> {

        Ok(Self {
            refname,
-
            head,
            base,
+
            head,
            accepted: tree,
        })
    }

-
    /// Return the content identifier of this brain. This represents the state of the
-
    /// accepted hunks, ie. the git tree.
-
    pub fn cid(&self) -> Oid {
-
        self.accepted.id().into()
-
    }
-

    /// Load an existing brain from the repository.
    pub fn load(
        patch: PatchId,
@@ -361,8 +305,8 @@ impl<'a> Brain<'a> {

        Ok(Self {
            refname,
-
            head,
            base,
+
            head,
            accepted: tree,
        })
    }
@@ -390,6 +334,25 @@ impl<'a> Brain<'a> {
        Ok(brain)
    }

+
    pub fn discard_accepted(
+
        &mut self,
+
        repo: &'a git::raw::Repository,
+
    ) -> Result<(), git::raw::Error> {
+
        // Reset brain
+
        let head = self.head.amend(
+
            Some(&self.refname),
+
            None,
+
            None,
+
            None,
+
            None,
+
            Some(&self.base.tree()?),
+
        )?;
+
        self.head = repo.find_commit(head)?;
+
        self.accepted = self.head.tree()?;
+

+
        Ok(())
+
    }
+

    /// Accept changes to the brain.
    pub fn accept(
        &mut self,
@@ -428,41 +391,20 @@ impl<'a> Brain<'a> {
    }
}

-
/// Builds a patch review interactively, across multiple files.
-
pub struct ReviewBuilder<'a, G> {
-
    /// Patch being reviewed.
-
    patch_id: PatchId,
-
    /// Signer.
-
    signer: &'a G,
-
    /// Stored copy of repository.
+
pub struct DiffUtil<'a> {
    repo: &'a Repository,
-
    /// Verdict for review items.
-
    verdict: Option<Verdict>,
}

-
impl<'a, G: Signer> ReviewBuilder<'a, G> {
-
    /// Create a new review builder.
-
    pub fn new(patch_id: PatchId, signer: &'a G, repo: &'a Repository) -> Self {
-
        Self {
-
            patch_id,
-
            signer,
-
            repo,
-
            verdict: None,
-
        }
-
    }
-

-
    /// Give this verdict to all review items. Set to `None` to not give a verdict.
-
    pub fn verdict(mut self, verdict: Option<Verdict>) -> Self {
-
        self.verdict = verdict;
-
        self
+
impl<'a> DiffUtil<'a> {
+
    pub fn new(repo: &'a Repository) -> Self {
+
        Self { repo }
    }

-
    /// Assemble the review for the given revision.
-
    pub fn all_hunks(
+
    pub fn base_queue(
        &self,
-
        _brain: &'a Brain<'a>,
+
        brain: Brain<'a>,
        revision: &Revision,
-
    ) -> anyhow::Result<ReviewQueue> {
+
    ) -> anyhow::Result<(Diff, Diff)> {
        let repo = self.repo.raw();

        let base = repo.find_commit((*revision.base()).into())?.tree()?;
@@ -474,9 +416,10 @@ impl<'a, G: Signer> ReviewBuilder<'a, G> {
        let mut opts = git::raw::DiffOptions::new();
        opts.patience(true).minimal(true).context_lines(3_u32);

-
        let diff = self.diff(&base, &revision, repo, &mut opts)?;
+
        let base_diff = self.diff(&base, &revision, repo, &mut opts)?;
+
        let queue_diff = self.diff(&brain.accepted(), &revision, repo, &mut opts)?;

-
        Ok(ReviewQueue::from(diff))
+
        Ok((base_diff, queue_diff))
    }

    pub fn diff(
@@ -500,6 +443,30 @@ impl<'a, G: Signer> ReviewBuilder<'a, G> {
    }
}

+
/// Builds a patch review interactively, across multiple files.
+
pub struct ReviewBuilder<'a> {
+
    /// Stored copy of repository.
+
    repo: &'a Repository,
+
}
+

+
impl<'a> ReviewBuilder<'a> {
+
    /// Create a new review builder.
+
    pub fn new(repo: &'a Repository) -> Self {
+
        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)))?
+
    }
+
}
+

#[derive(Debug, PartialEq, Eq)]
pub struct ReviewComment {
    pub location: CodeLocation,
@@ -641,6 +608,7 @@ impl CommentBuilder {
#[cfg(test)]
mod tests {
    use super::*;
+
    use std::str::FromStr;

    #[test]
    fn test_review_comments_basic() {
modified bin/ui/items.rs
@@ -42,7 +42,7 @@ use tui::ui::utils::LineMerger;
use tui::ui::{span, Column};
use tui::ui::{ToRow, ToTree};

-
use crate::cob::{DiffStats, HunkStats, IndexedHunkItem};
+
use crate::cob::{DiffStats, HunkState, HunkStats, IndexedHunkItem};
use crate::git::{Blob, Repo};

use super::super::git;
@@ -1181,11 +1181,17 @@ impl<'a> ToRow<3> for HunkItem<'a> {
                _,
                Item::FileAdded {
                    path,
+
                    header: _,
                    new: _,
                    hunk,
                    _stats: _,
                },
+
                state,
            ) => {
+
                let state = match state {
+
                    HunkState::Accepted => span::positive("✓"),
+
                    HunkState::Rejected => span::secondary("?"),
+
                };
                let stats = hunk
                    .as_ref()
                    .map(|hunk| HunkStats::from(hunk))
@@ -1197,7 +1203,7 @@ impl<'a> ToRow<3> for HunkItem<'a> {
                .concat();

                [
-
                    span::secondary("?").into(),
+
                    state.into(),
                    HunkItem::pretty_path(path, false).into(),
                    Line::from(stats_cell).right_aligned().into(),
                ]
@@ -1206,12 +1212,18 @@ impl<'a> ToRow<3> for HunkItem<'a> {
                _,
                Item::FileModified {
                    path,
+
                    header: _,
                    old: _,
                    new: _,
                    hunk,
                    _stats: _,
                },
+
                state,
            ) => {
+
                let state = match state {
+
                    HunkState::Accepted => span::positive("✓"),
+
                    HunkState::Rejected => span::secondary("?"),
+
                };
                let stats = hunk
                    .as_ref()
                    .map(|hunk| HunkStats::from(hunk))
@@ -1223,7 +1235,7 @@ impl<'a> ToRow<3> for HunkItem<'a> {
                .concat();

                [
-
                    span::secondary("?").into(),
+
                    state.into(),
                    HunkItem::pretty_path(path, false).into(),
                    Line::from(stats_cell).right_aligned().into(),
                ]
@@ -1232,11 +1244,17 @@ impl<'a> ToRow<3> for HunkItem<'a> {
                _,
                Item::FileDeleted {
                    path,
+
                    header: _,
                    old: _,
                    hunk,
                    _stats: _,
                },
+
                state,
            ) => {
+
                let state = match state {
+
                    HunkState::Accepted => span::positive("✓"),
+
                    HunkState::Rejected => span::secondary("?"),
+
                };
                let stats = hunk
                    .as_ref()
                    .map(|hunk| HunkStats::from(hunk))
@@ -1248,12 +1266,16 @@ impl<'a> ToRow<3> for HunkItem<'a> {
                .concat();

                [
-
                    span::secondary("?").into(),
+
                    state.into(),
                    HunkItem::pretty_path(path, true).into(),
                    Line::from(stats_cell).right_aligned().into(),
                ]
            }
-
            (_, Item::FileCopied { copied }) => {
+
            (_, Item::FileCopied { copied }, state) => {
+
                let state = match state {
+
                    HunkState::Accepted => span::positive("✓"),
+
                    HunkState::Rejected => span::secondary("?"),
+
                };
                let stats = copied
                    .diff
                    .stats()
@@ -1266,12 +1288,16 @@ impl<'a> ToRow<3> for HunkItem<'a> {
                .concat();

                [
-
                    span::secondary("?").into(),
+
                    state.into(),
                    HunkItem::pretty_path(&copied.new_path, false).into(),
                    Line::from(stats_cell).right_aligned().into(),
                ]
            }
-
            (_, Item::FileMoved { moved }) => {
+
            (_, Item::FileMoved { moved }, state) => {
+
                let state = match state {
+
                    HunkState::Accepted => span::positive("✓"),
+
                    HunkState::Rejected => span::secondary("?"),
+
                };
                let stats = moved
                    .diff
                    .stats()
@@ -1284,7 +1310,7 @@ impl<'a> ToRow<3> for HunkItem<'a> {
                .concat();

                [
-
                    span::secondary("?").into(),
+
                    state.into(),
                    HunkItem::pretty_path(&moved.new_path, false).into(),
                    Line::from(stats_cell).right_aligned().into(),
                ]
@@ -1293,33 +1319,51 @@ impl<'a> ToRow<3> for HunkItem<'a> {
                _,
                Item::FileEofChanged {
                    path,
+
                    header: _,
                    old: _,
                    new: _,
                    _eof: _,
                },
-
            ) => [
-
                span::secondary("?").into(),
-
                HunkItem::pretty_path(path, false).into(),
-
                span::default("EOF ")
-
                    .light_blue()
-
                    .into_right_aligned_line()
-
                    .into(),
-
            ],
+
                state,
+
            ) => {
+
                let state = match state {
+
                    HunkState::Accepted => span::positive("✓"),
+
                    HunkState::Rejected => span::secondary("?"),
+
                };
+

+
                [
+
                    state.into(),
+
                    HunkItem::pretty_path(path, false).into(),
+
                    span::default("EOF ")
+
                        .light_blue()
+
                        .into_right_aligned_line()
+
                        .into(),
+
                ]
+
            }
            (
                _,
                Item::FileModeChanged {
                    path,
+
                    header: _,
                    old: _,
                    new: _,
                },
-
            ) => [
-
                span::secondary("?").into(),
-
                HunkItem::pretty_path(path, false).into(),
-
                span::default("FM ")
-
                    .light_blue()
-
                    .into_right_aligned_line()
-
                    .into(),
-
            ],
+
                state,
+
            ) => {
+
                let state = match state {
+
                    HunkState::Accepted => span::positive("✓"),
+
                    HunkState::Rejected => span::secondary("?"),
+
                };
+

+
                [
+
                    state.into(),
+
                    HunkItem::pretty_path(path, false).into(),
+
                    span::default("FM ")
+
                        .light_blue()
+
                        .into_right_aligned_line()
+
                        .into(),
+
                ]
+
            }
        }
    }
}
@@ -1374,10 +1418,12 @@ impl<'a> HunkItem<'a> {
                _,
                crate::cob::HunkItem::FileAdded {
                    path,
+
                    header: _,
                    new: _,
                    hunk: _,
                    _stats: _,
                },
+
                _,
            ) => {
                let path = HunkItem::pretty_path(path, false);
                let header = [
@@ -1403,11 +1449,13 @@ impl<'a> HunkItem<'a> {
                _,
                crate::cob::HunkItem::FileModified {
                    path,
+
                    header: _,
                    old: _,
                    new: _,
                    hunk: _,
                    _stats: _,
                },
+
                _,
            ) => {
                let path = HunkItem::pretty_path(path, false);
                let header = [
@@ -1433,10 +1481,12 @@ impl<'a> HunkItem<'a> {
                _,
                crate::cob::HunkItem::FileDeleted {
                    path,
+
                    header: _,
                    old: _,
                    hunk: _,
                    _stats: _,
                },
+
                _,
            ) => {
                let path = HunkItem::pretty_path(path, true);
                let header = [
@@ -1458,7 +1508,7 @@ impl<'a> HunkItem<'a> {

                header.to_vec()
            }
-
            (_, crate::cob::HunkItem::FileCopied { copied }) => {
+
            (_, crate::cob::HunkItem::FileCopied { copied }, _) => {
                let path = Line::from(
                    [
                        HunkItem::pretty_path(&copied.old_path, false).spans,
@@ -1483,7 +1533,7 @@ impl<'a> HunkItem<'a> {

                header.to_vec()
            }
-
            (_, crate::cob::HunkItem::FileMoved { moved }) => {
+
            (_, crate::cob::HunkItem::FileMoved { moved }, _) => {
                let path = Line::from(
                    [
                        HunkItem::pretty_path(&moved.old_path, false).spans,
@@ -1512,10 +1562,12 @@ impl<'a> HunkItem<'a> {
                _,
                crate::cob::HunkItem::FileEofChanged {
                    path,
+
                    header: _,
                    old: _,
                    new: _,
                    _eof: _,
                },
+
                _,
            ) => {
                let path = HunkItem::pretty_path(&path, false);
                let header = [
@@ -1536,9 +1588,11 @@ impl<'a> HunkItem<'a> {
                _,
                crate::cob::HunkItem::FileModeChanged {
                    path,
+
                    header: _,
                    old: _,
                    new: _,
                },
+
                _,
            ) => {
                let path = HunkItem::pretty_path(&path, false);
                let header = [
@@ -1562,9 +1616,9 @@ impl<'a> HunkItem<'a> {
        use crate::cob::HunkItem;

        match &self.inner {
-
            (_, HunkItem::FileAdded { hunk, .. })
-
            | (_, HunkItem::FileModified { hunk, .. })
-
            | (_, HunkItem::FileDeleted { hunk, .. }) => {
+
            (_, HunkItem::FileAdded { hunk, .. }, _)
+
            | (_, HunkItem::FileModified { hunk, .. }, _)
+
            | (_, HunkItem::FileDeleted { hunk, .. }, _) => {
                let mut lines = hunk
                    .as_ref()
                    .map(|hunk| Text::from(hunk.to_text(&self.lines)));