Skip to content

Commit 8f1157d

Browse files
feat: show list of patches that already have been Reviewed-by
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]>
1 parent 7bdb076 commit 8f1157d

File tree

1 file changed

+215
-18
lines changed

1 file changed

+215
-18
lines changed

src/ui/details_actions.rs

Lines changed: 215 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,91 @@
1+
use std::cmp::Ordering;
2+
13
use ratatui::{
24
layout::{Constraint, Direction, Layout, Rect},
35
style::{Color, Modifier, Style},
4-
text::{Line, Span},
6+
text::{Line, Span, ToSpan},
57
widgets::{Block, Borders, Padding, Paragraph, Wrap},
68
Frame,
79
};
810

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

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

14-
let mut patches_to_reply = String::new();
29+
let number_offset = if patchset_details_and_actions.has_cover_letter {
30+
0
31+
} else {
32+
1
33+
};
34+
35+
let mut replied_inds: Vec<usize> = Vec::new();
36+
if let Some(already_reviewed_by) = app.reviewed_patchsets.get(
37+
&patchset_details_and_actions
38+
.representative_patch
39+
.message_id()
40+
.href,
41+
) {
42+
replied_inds = already_reviewed_by
43+
.iter()
44+
.cloned()
45+
.map(|i| i + number_offset)
46+
.collect();
47+
replied_inds.sort_unstable();
48+
}
49+
50+
let mut staged_inds: Vec<usize> = Vec::new();
1551
if let Some(true) = patchset_details_and_actions
1652
.patchset_actions
1753
.get(&PatchsetAction::ReplyWithReviewedBy)
1854
{
19-
patches_to_reply.push('(');
20-
let number_offset = if patchset_details_and_actions.has_cover_letter {
21-
0
22-
} else {
23-
1
24-
};
25-
let patches_to_reply_numbers: Vec<usize> = patchset_details_and_actions
55+
staged_inds = patchset_details_and_actions
2656
.patches_to_reply
2757
.iter()
2858
.enumerate()
2959
.filter_map(|(i, &val)| if val { Some(i + number_offset) } else { None })
3060
.collect();
31-
for number in patches_to_reply_numbers {
32-
patches_to_reply.push_str(&format!("{number},"));
61+
staged_inds.sort_unstable();
62+
}
63+
64+
let reviewed_by_inds = merge_reviewed_by_inds(replied_inds, staged_inds);
65+
66+
let mut reviewed_by_list = Vec::new();
67+
// let mut reviewed_by_list = String::new();
68+
69+
for (index, is_replied) in reviewed_by_inds {
70+
let mut entry = index.to_string();
71+
let mut entry_color = Style::default().fg(Color::White);
72+
if !is_replied {
73+
entry.push('*');
74+
entry_color = entry_color.fg(Color::DarkGray);
3375
}
34-
patches_to_reply = format!("{})", &patches_to_reply[..patches_to_reply.len() - 1]);
76+
reviewed_by_list.push(Span::styled(entry, entry_color));
77+
reviewed_by_list.push(Span::styled(
78+
", ".to_string(),
79+
Style::default().fg(Color::White),
80+
));
3581
}
82+
reviewed_by_list.pop();
83+
84+
reviewed_by_list
85+
}
86+
87+
fn render_details_and_actions(f: &mut Frame, app: &App, details_chunk: Rect, actions_chunk: Rect) {
88+
let patchset_details_and_actions = app.details_actions.as_ref().unwrap();
3689

3790
let patchset_details = &patchset_details_and_actions.representative_patch;
3891
let mut patchset_details = vec![
@@ -72,11 +125,15 @@ fn render_details_and_actions(f: &mut Frame, app: &App, details_chunk: Rect, act
72125
),
73126
]),
74127
];
75-
if !patches_to_reply.is_empty() {
76-
patchset_details.push(Line::from(vec![
77-
Span::styled("Reviewed-by*: ", Style::default().fg(Color::Cyan)),
78-
Span::styled(patches_to_reply, Style::default().fg(Color::DarkGray)),
79-
]));
128+
let mut reviewed_by = reviewed_by_list(app);
129+
if !reviewed_by.is_empty() {
130+
let mut reviewed_by_line = vec![
131+
Span::styled("Reviewed-by: ", Style::default().fg(Color::Cyan)),
132+
'('.to_span(),
133+
];
134+
reviewed_by_line.append(&mut reviewed_by);
135+
reviewed_by_line.push(')'.to_span());
136+
patchset_details.push(Line::from(reviewed_by_line));
80137
}
81138

82139
let patchset_details = Paragraph::new(patchset_details)
@@ -224,3 +281,143 @@ pub fn keys_hint() -> Span<'static> {
224281
Style::default().fg(Color::Red),
225282
)
226283
}
284+
285+
/// Receives two `usize` slices representing the patch indexes tagged w/
286+
/// "Reviewed-by", preceding those that were replied over those that are only
287+
/// staged.
288+
///
289+
/// # Arguments
290+
///
291+
/// * `replied_inds`: Ordered slice of patch indexes that were replied
292+
/// * `staged_inds`: Ordered slice of patch indexes that staged to be replied
293+
///
294+
/// # Returns
295+
///
296+
/// This function always returns a `Vec<(usize, bool)>` (empty or not). The
297+
/// tuples `(index, is_replied)` encode the precedence, in which `is_replied ==
298+
/// true` means that patch of number `index` was replied w/ the tag.
299+
fn merge_reviewed_by_inds(
300+
mut replied_inds: Vec<usize>,
301+
mut staged_inds: Vec<usize>,
302+
) -> Vec<(usize, bool)> {
303+
let mut reviewed_by_inds = Vec::new();
304+
let mut i = 0;
305+
let mut j = 0;
306+
307+
// Don't assume the caller ordered the vectors beforehand
308+
replied_inds.sort();
309+
staged_inds.sort();
310+
311+
while (i != replied_inds.len()) && (j != staged_inds.len()) {
312+
match replied_inds[i].cmp(&staged_inds[j]) {
313+
Ordering::Less => {
314+
reviewed_by_inds.push((replied_inds[i], true));
315+
i += 1;
316+
}
317+
Ordering::Equal => {
318+
reviewed_by_inds.push((replied_inds[i], true));
319+
i += 1;
320+
j += 1;
321+
}
322+
Ordering::Greater => {
323+
reviewed_by_inds.push((staged_inds[j], false));
324+
j += 1;
325+
}
326+
}
327+
}
328+
329+
let mut remaining_inds: Vec<(usize, bool)>;
330+
if i != replied_inds.len() {
331+
remaining_inds = replied_inds[i..]
332+
.iter()
333+
.map(|&index| (index, true))
334+
.collect();
335+
} else {
336+
remaining_inds = staged_inds[j..]
337+
.iter()
338+
.map(|&index| (index, false))
339+
.collect();
340+
}
341+
reviewed_by_inds.append(&mut remaining_inds);
342+
343+
reviewed_by_inds
344+
}
345+
346+
#[cfg(test)]
347+
mod tests {
348+
use super::merge_reviewed_by_inds;
349+
350+
#[test]
351+
fn test_merge_reviewed_by_inds() {
352+
let replied_inds = vec![];
353+
let staged_inds = vec![];
354+
let reviewed_by_inds = merge_reviewed_by_inds(replied_inds, staged_inds);
355+
assert_eq!(
356+
reviewed_by_inds.len(),
357+
0,
358+
"No replied neither staged patches should result in empty reviewed-by"
359+
);
360+
361+
let replied_inds = vec![0, 1, 2, 3, 4];
362+
let staged_inds = vec![];
363+
let reviewed_by_inds = merge_reviewed_by_inds(replied_inds.clone(), staged_inds);
364+
assert_eq!(
365+
reviewed_by_inds.len(),
366+
5,
367+
"No staged patches should result in reviewed-by w/ same size as replied"
368+
);
369+
assert_eq!(
370+
reviewed_by_inds
371+
.clone()
372+
.into_iter()
373+
.map(|(i, _)| i)
374+
.collect::<Vec<usize>>(),
375+
replied_inds,
376+
"No staged patches should result in reviewed-by equal to replied"
377+
);
378+
assert!(
379+
reviewed_by_inds.iter().all(|(_, b)| *b),
380+
"No staged patches should result in reviewed-by equal to replied"
381+
);
382+
383+
let replied_inds = vec![];
384+
let staged_inds = vec![1, 3, 10];
385+
let reviewed_by_inds = merge_reviewed_by_inds(replied_inds, staged_inds.clone());
386+
assert_eq!(
387+
reviewed_by_inds.len(),
388+
3,
389+
"No staged patches should result in reviewed-by w/ same size as staged"
390+
);
391+
assert_eq!(
392+
reviewed_by_inds
393+
.clone()
394+
.into_iter()
395+
.map(|(i, _)| i)
396+
.collect::<Vec<usize>>(),
397+
staged_inds,
398+
"No staged patches should result in reviewed-by equal to staged"
399+
);
400+
assert!(
401+
reviewed_by_inds.iter().all(|(_, b)| !*b),
402+
"No staged patches should result in reviewed-by equal to staged"
403+
);
404+
405+
let replied_inds = vec![0, 1, 2, 3, 4];
406+
let staged_inds = vec![1, 3, 10];
407+
let expected_reviewed_by_inds = vec![
408+
(0, true),
409+
(1, true),
410+
(2, true),
411+
(3, true),
412+
(4, true),
413+
(10, false),
414+
];
415+
let reviewed_by_inds = merge_reviewed_by_inds(replied_inds, staged_inds);
416+
assert_eq!(
417+
reviewed_by_inds.len(),
418+
6,
419+
"No staged patches should result in reviewed-by w/ same size as staged"
420+
);
421+
assert_eq!(reviewed_by_inds, expected_reviewed_by_inds,);
422+
}
423+
}

0 commit comments

Comments
 (0)