Radish alpha
r
rad:z39mP9rQAaGmERfUMPULfPUi473tY
Radicle terminal user interface
Radicle
Git
bin(patch): Rework accepted / rejected review hunks
Erik Kundt committed 1 year ago
commit dcc4aa657207919a056e43a5617f9ed417b114f2
parent fdf29a6
3 files changed +76 -63
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)?;
modified bin/commands/patch/review.rs
@@ -45,7 +45,7 @@ use crate::ui::layout;

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

/// The actions that a user can carry out on a review item.
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
@@ -71,7 +71,7 @@ pub struct Tui {
    pub title: String,
    pub revision: Revision,
    pub review: Review,
-
    pub hunks: ReviewQueue,
+
    pub hunks: Hunks,
}

impl Tui {
@@ -84,7 +84,7 @@ impl Tui {
        title: String,
        revision: Revision,
        review: Review,
-
        hunks: ReviewQueue,
+
        hunks: Hunks,
    ) -> Self {
        Self {
            storage,
@@ -196,7 +196,7 @@ 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
@@ -269,31 +269,40 @@ impl<'a> App<'a> {
        let signer: &Box<dyn Signer> = &self.signer.lock().unwrap();

        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 rejected_hunks =
+
            Hunks::new(DiffUtil::new(&repo).rejected_diffs(&brain, &self.revision)?);

-
        let states = base_files
-
            .iter()
-
            .map(|file| {
-
                if !queue_files.contains(file) {
-
                    HunkState::Accepted
-
                } else {
-
                    HunkState::Rejected
-
                }
-
            })
-
            .collect::<Vec<_>>();
+
        let mut hunks = self.hunks.lock().unwrap();
+
        for hunk in &hunks.0 {

-
        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 (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();
+
        //     }
+
        // }
+

        Ok(())
    }

@@ -627,12 +636,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 super::builder::{Brain, ReviewBuilder};
+
        use super::builder::ReviewBuilder;
        use super::App;

        pub fn app<'a>(
@@ -646,8 +654,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(),
modified bin/commands/patch/review/builder.rs
@@ -32,12 +32,11 @@ 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)>,
+
pub struct Hunks {
+
    hunks: Vec<(usize, HunkDiff)>,
}

-
impl ReviewQueue {
+
impl Hunks {
    pub fn new(base: Diff) -> Self {
        let base_files = base.into_files();

@@ -152,29 +151,21 @@ impl ReviewQueue {
    }

    fn add_item(&mut self, item: HunkDiff) {
-
        self.queue.push_back((self.queue.len(), item));
+
        self.hunks.push((self.hunks.len(), item));
    }
}

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

    fn deref(&self) -> &Self::Target {
-
        &self.queue
+
        &self.hunks
    }
}

-
impl std::ops::DerefMut for ReviewQueue {
+
impl std::ops::DerefMut for Hunks {
    fn deref_mut(&mut self) -> &mut Self::Target {
-
        &mut self.queue
-
    }
-
}
-

-
impl Iterator for ReviewQueue {
-
    type Item = (usize, HunkDiff);
-

-
    fn next(&mut self) -> Option<Self::Item> {
-
        self.queue.pop_front()
+
        &mut self.hunks
    }
}

@@ -378,11 +369,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()?;
@@ -395,9 +382,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(
@@ -433,12 +434,19 @@ 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, _)| Ok(ReviewQueue::new(base)))?
+
    pub fn hunks(&self, revision: &Revision) -> anyhow::Result<Hunks> {
+
        let diff = DiffUtil::new(self.repo).all_diffs(revision)?;
+
        Ok(Hunks::new(diff))
    }
+

+
    // pub fn rejected_hunks(
+
    //     &self,
+
    //     brain: &'a Brain<'a>,
+
    //     revision: &Revision,
+
    // ) -> anyhow::Result<Hunks> {
+
    //     let diff = DiffUtil::new(self.repo).rejected_diffs(brain, revision)?;
+
    //     Ok(Hunks::new(diff))
+
    // }
}

#[derive(Debug, PartialEq, Eq)]