Skip to content

Commit

Permalink
feat: show list of patches that already have been Reviewed-by
Browse files Browse the repository at this point in the history
In the last commit, we updated the UI to reflect the new functionality
of replying to a subset of patches from a series w/ the "Reviewed-by"
tag.

Make the line "Reviewed-by:" in the "Details" tab, display information
not only about the staged patches, but the ones recorded as already
replied with the tag, differentiating them by the color and the
appended `*`.

Following the example of the last commit, if a patches 1, 2, and 7 are
staged, and 0, 3, and 10 have already been replied with the tag, the
line will display as

```
Reviewed-by: (0, 1*, 2*, 3, 7*, 10)
```

Closes: #78

Signed-off-by: David Tadokoro <[email protected]>
  • Loading branch information
davidbtadokoro committed Nov 11, 2024
1 parent 8c4c457 commit 59b3b1a
Showing 1 changed file with 215 additions and 18 deletions.
233 changes: 215 additions & 18 deletions src/ui/details_actions.rs
Original file line number Diff line number Diff line change
@@ -1,38 +1,91 @@
use std::cmp::Ordering;

use ratatui::{
layout::{Constraint, Direction, Layout, Rect},
style::{Color, Modifier, Style},
text::{Line, Span},
text::{Line, Span, ToSpan},
widgets::{Block, Borders, Padding, Paragraph, Wrap},
Frame,
};

use crate::app::{screens::details_actions::PatchsetAction, App};

fn render_details_and_actions(f: &mut Frame, app: &App, details_chunk: Rect, actions_chunk: Rect) {
/// Get list of indexes representing the patches that were replied w/ the
/// "Reviewed-by:" tag, or that are staged to.
///
/// # Arguments
///
/// * `app`: Immutable reference to `patch-hub` model
///
/// # Returns
///
/// An owned `Vec<Span>` that is an ordered comma-separated list of the indexes.
/// Staged indexes are appended w/ `*` and colored `Color::DarkGrey`. If there
/// are no patches that were neither replied or staged w/ the tag, the return
/// string will be empty.
fn reviewed_by_list(app: &App) -> Vec<Span> {
let patchset_details_and_actions = app.patchset_details_and_actions_state.as_ref().unwrap();

let mut patches_to_reply = String::new();
let number_offset = if patchset_details_and_actions.has_cover_letter {
0
} else {
1
};

let mut replied_inds: Vec<usize> = Vec::new();
if let Some(already_reviewed_by) = app.reviewed_patchsets.get(
&patchset_details_and_actions
.representative_patch
.message_id()
.href,
) {
replied_inds = already_reviewed_by
.iter()
.cloned()
.map(|i| i + number_offset)
.collect();
replied_inds.sort_unstable();
}

let mut staged_inds: Vec<usize> = Vec::new();
if let Some(true) = patchset_details_and_actions
.patchset_actions
.get(&PatchsetAction::ReplyWithReviewedBy)
{
patches_to_reply.push('(');
let number_offset = if patchset_details_and_actions.has_cover_letter {
0
} else {
1
};
let patches_to_reply_numbers: Vec<usize> = patchset_details_and_actions
staged_inds = patchset_details_and_actions
.patches_to_reply
.iter()
.enumerate()
.filter_map(|(i, &val)| if val { Some(i + number_offset) } else { None })
.collect();
for number in patches_to_reply_numbers {
patches_to_reply.push_str(&format!("{number},"));
staged_inds.sort_unstable();
}

let reviewed_by_inds = merge_reviewed_by_inds(replied_inds, staged_inds);

let mut reviewed_by_list = Vec::new();
// let mut reviewed_by_list = String::new();

for (index, is_replied) in reviewed_by_inds {
let mut entry = index.to_string();
let mut entry_color = Style::default().fg(Color::White);
if !is_replied {
entry.push('*');
entry_color = entry_color.fg(Color::DarkGray);
}
patches_to_reply = format!("{})", &patches_to_reply[..patches_to_reply.len() - 1]);
reviewed_by_list.push(Span::styled(entry, entry_color));
reviewed_by_list.push(Span::styled(
", ".to_string(),
Style::default().fg(Color::White),
));
}
reviewed_by_list.pop();

reviewed_by_list
}

fn render_details_and_actions(f: &mut Frame, app: &App, details_chunk: Rect, actions_chunk: Rect) {
let patchset_details_and_actions = app.patchset_details_and_actions_state.as_ref().unwrap();

let patchset_details = &patchset_details_and_actions.representative_patch;
let mut patchset_details = vec![
Expand Down Expand Up @@ -72,11 +125,15 @@ fn render_details_and_actions(f: &mut Frame, app: &App, details_chunk: Rect, act
),
]),
];
if !patches_to_reply.is_empty() {
patchset_details.push(Line::from(vec![
Span::styled("Reviewed-by*: ", Style::default().fg(Color::Cyan)),
Span::styled(patches_to_reply, Style::default().fg(Color::DarkGray)),
]));
let mut reviewed_by = reviewed_by_list(app);
if !reviewed_by.is_empty() {
let mut reviewed_by_line = vec![
Span::styled("Reviewed-by: ", Style::default().fg(Color::Cyan)),
'('.to_span(),
];
reviewed_by_line.append(&mut reviewed_by);
reviewed_by_line.push(')'.to_span());
patchset_details.push(Line::from(reviewed_by_line));
}

let patchset_details = Paragraph::new(patchset_details)
Expand Down Expand Up @@ -224,3 +281,143 @@ pub fn keys_hint() -> Span<'static> {
Style::default().fg(Color::Red),
)
}

/// Receives two `usize` slices representing the patch indexes tagged w/
/// "Reviewed-by", preceding those that were replied over those that are only
/// staged.
///
/// # Arguments
///
/// * `replied_inds`: Ordered slice of patch indexes that were replied
/// * `staged_inds`: Ordered slice of patch indexes that staged to be replied
///
/// # Returns
///
/// This function always returns a `Vec<(usize, bool)>` (empty or not). The
/// tuples `(index, is_replied)` encode the precedence, in which `is_replied ==
/// true` means that patch of number `index` was replied w/ the tag.
fn merge_reviewed_by_inds(
mut replied_inds: Vec<usize>,
mut staged_inds: Vec<usize>,
) -> Vec<(usize, bool)> {
let mut reviewed_by_inds = Vec::new();
let mut i = 0;
let mut j = 0;

// Don't assume the caller ordered the vectors beforehand
replied_inds.sort();
staged_inds.sort();

while (i != replied_inds.len()) && (j != staged_inds.len()) {
match replied_inds[i].cmp(&staged_inds[j]) {
Ordering::Less => {
reviewed_by_inds.push((replied_inds[i], true));
i += 1;
}
Ordering::Equal => {
reviewed_by_inds.push((replied_inds[i], true));
i += 1;
j += 1;
}
Ordering::Greater => {
reviewed_by_inds.push((staged_inds[j], false));
j += 1;
}
}
}

let mut remaining_inds: Vec<(usize, bool)>;
if i != replied_inds.len() {
remaining_inds = replied_inds[i..]
.iter()
.map(|&index| (index, true))
.collect();
} else {
remaining_inds = staged_inds[j..]
.iter()
.map(|&index| (index, false))
.collect();
}
reviewed_by_inds.append(&mut remaining_inds);

reviewed_by_inds
}

#[cfg(test)]
mod tests {
use super::merge_reviewed_by_inds;

#[test]
fn test_merge_reviewed_by_inds() {
let replied_inds = vec![];
let staged_inds = vec![];
let reviewed_by_inds = merge_reviewed_by_inds(replied_inds, staged_inds);
assert_eq!(
reviewed_by_inds.len(),
0,
"No replied neither staged patches should result in empty reviewed-by"
);

let replied_inds = vec![0, 1, 2, 3, 4];
let staged_inds = vec![];
let reviewed_by_inds = merge_reviewed_by_inds(replied_inds.clone(), staged_inds);
assert_eq!(
reviewed_by_inds.len(),
5,
"No staged patches should result in reviewed-by w/ same size as replied"
);
assert_eq!(
reviewed_by_inds
.clone()
.into_iter()
.map(|(i, _)| i)
.collect::<Vec<usize>>(),
replied_inds,
"No staged patches should result in reviewed-by equal to replied"
);
assert!(
reviewed_by_inds.iter().all(|(_, b)| *b),
"No staged patches should result in reviewed-by equal to replied"
);

let replied_inds = vec![];
let staged_inds = vec![1, 3, 10];
let reviewed_by_inds = merge_reviewed_by_inds(replied_inds, staged_inds.clone());
assert_eq!(
reviewed_by_inds.len(),
3,
"No staged patches should result in reviewed-by w/ same size as staged"
);
assert_eq!(
reviewed_by_inds
.clone()
.into_iter()
.map(|(i, _)| i)
.collect::<Vec<usize>>(),
staged_inds,
"No staged patches should result in reviewed-by equal to staged"
);
assert!(
reviewed_by_inds.iter().all(|(_, b)| !*b),
"No staged patches should result in reviewed-by equal to staged"
);

let replied_inds = vec![0, 1, 2, 3, 4];
let staged_inds = vec![1, 3, 10];
let expected_reviewed_by_inds = vec![
(0, true),
(1, true),
(2, true),
(3, true),
(4, true),
(10, false),
];
let reviewed_by_inds = merge_reviewed_by_inds(replied_inds, staged_inds);
assert_eq!(
reviewed_by_inds.len(),
6,
"No staged patches should result in reviewed-by w/ same size as staged"
);
assert_eq!(reviewed_by_inds, expected_reviewed_by_inds,);
}
}

0 comments on commit 59b3b1a

Please sign in to comment.