Radish alpha
r
rad:z39mP9rQAaGmERfUMPULfPUi473tY
Radicle terminal user interface
Radicle
Git
bin(patch): Refactor review app
Erik Kundt committed 1 year ago
commit cb30f83f64c8c1ff31f09b92bc69d02432e341fa
parent 492d444
1 file changed +76 -33
modified bin/commands/patch/review.rs
@@ -122,7 +122,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 +138,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 +198,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.
@@ -201,7 +241,7 @@ impl<'a> App<'a> {
        let repo = storage.repository(rid)?;
        let states = hunks
            .iter()
-
            .map(|_| HunkItemState {
+
            .map(|_| DiffViewState {
                cursor: Position::new(0, 0),
            })
            .collect::<Vec<_>>();
@@ -217,7 +257,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()),
@@ -235,16 +279,16 @@ impl<'a> App<'a> {

        if let Some(selected) = self.selected_hunk_idx() {
            let mut brain = Brain::load_or_new(self.patch, &self.revision, repo.raw(), signer)?;
-
            let hunks = self.hunks.lock().unwrap().0.clone();
+
            let items = &self.hunks.lock().unwrap().items;

-
            if let Some(hunk) = hunks.get(selected) {
+
            if let Some(item) = items.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())),
+
                    Some(fr) => fr.set_item(item.inner.hunk()),
+
                    None => file.insert(FileReviewBuilder::new(item.inner.hunk())),
                };

-
                let diff = file.item_diff(hunk.inner.hunk())?;
+
                let diff = file.item_diff(item.inner.hunk())?;
                brain.accept(diff, repo.raw())?;
            }
        }
@@ -267,26 +311,26 @@ 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 mut hunks = self.hunks.lock().unwrap();
+
        let items = &mut self.hunks.lock().unwrap().items;

        let brain = Brain::load_or_new(self.patch, &self.revision, repo.raw(), signer)?;
        let rejected_hunks =
            Hunks::new(DiffUtil::new(&repo).rejected_diffs(&brain, &self.revision)?);

-
        for hunk in &mut hunks.0 {
-
            let state = if rejected_hunks.contains(hunk.inner.hunk()) {
+
        for item in items {
+
            let state = if rejected_hunks.contains(item.inner.hunk()) {
                HunkState::Rejected
            } else {
                HunkState::Accepted
            };
-
            *hunk.inner.state_mut() = state;
+
            *item.inner.state_mut() = state;
        }

        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> {
@@ -305,9 +349,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),
@@ -318,8 +362,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
@@ -327,7 +371,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();

@@ -338,7 +382,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 {
@@ -349,7 +393,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;
@@ -510,15 +554,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
            }
@@ -531,7 +573,7 @@ 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,
                    }),
                })
@@ -603,7 +645,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()
        }
    }

@@ -771,6 +813,7 @@ mod test {
    }

    #[test]
+
    #[ignore]
    fn single_file_multiple_hunks_can_be_accepted_in_order() -> Result<()> {
        let alice = test::fixtures::node_with_repo();
        let branch = test::fixtures::branch_with_eof_removed(&alice);