Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add reply w/ Reviewed-by tag of subset of patches from a series #80

Open
wants to merge 4 commits into
base: unstable
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 29 additions & 18 deletions src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use screens::{
mail_list::MailingListSelectionState,
CurrentScreen,
};
use std::collections::HashMap;
use std::collections::{HashMap, HashSet};

use crate::utils;

Expand All @@ -30,7 +30,7 @@ pub struct App {
pub latest_patchsets_state: Option<LatestPatchsetsState>,
pub patchset_details_and_actions_state: Option<PatchsetDetailsAndActionsState>,
pub edit_config_state: Option<EditConfigState>,
pub reviewed_patchsets: HashMap<String, Vec<usize>>,
pub reviewed_patchsets: HashMap<String, HashSet<usize>>,
pub config: Config,
pub lore_api_client: BlockingLoreAPIClient,
}
Expand Down Expand Up @@ -153,10 +153,14 @@ impl App {
.into_text()?;
patches_preview.push(patch_preview);
}
let has_cover_letter = representative_patch.number_in_series() == 0;
let patches_to_reply = vec![false; raw_patches.len()];
self.patchset_details_and_actions_state = Some(PatchsetDetailsAndActionsState {
representative_patch,
raw_patches,
patches_preview,
patches_to_reply,
has_cover_letter,
preview_index: 0,
preview_scroll_offset: 0,
preview_pan: 0,
Expand Down Expand Up @@ -203,26 +207,33 @@ impl App {
self.config.bookmarked_patchsets_path(),
)?;

if *actions.get(&PatchsetAction::ReplyWithReviewedBy).unwrap() {
let successful_indexes = details_and_actions
.reply_patchset_with_reviewed_by("all", self.config.git_send_email_options())?;

if !successful_indexes.is_empty() {
self.reviewed_patchsets.insert(
representative_patch.message_id().href.clone(),
successful_indexes,
);

lore_session::save_reviewed_patchsets(
&self.reviewed_patchsets,
self.config.reviewed_patchsets_path(),
)?;
}
if let Some(true) = details_and_actions
.patchset_actions
.get(&PatchsetAction::ReplyWithReviewedBy)
{
let mut successful_indexes = self
.reviewed_patchsets
.remove(&representative_patch.message_id().href)
.unwrap_or_default();
details_and_actions.reply_patchset_with_reviewed_by(
"all",
self.config.git_send_email_options(),
&mut successful_indexes,
)?;
self.reviewed_patchsets.insert(
representative_patch.message_id().href.clone(),
successful_indexes,
);

lore_session::save_reviewed_patchsets(
&self.reviewed_patchsets,
self.config.reviewed_patchsets_path(),
)?;

self.patchset_details_and_actions_state
.as_mut()
.unwrap()
.toggle_action(PatchsetAction::ReplyWithReviewedBy);
.reset_reply_with_reviewed_by_action();
}

Ok(())
Expand Down
61 changes: 48 additions & 13 deletions src/app/screens/details_actions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,22 @@ use super::CurrentScreen;
use ::patch_hub::lore::{lore_api_client::BlockingLoreAPIClient, lore_session, patch::Patch};
use color_eyre::eyre::bail;
use ratatui::text::Text;
use std::{collections::HashMap, path::Path, process::Command};
use std::{
collections::{HashMap, HashSet},
path::Path,
process::Command,
};

pub struct PatchsetDetailsAndActionsState {
pub representative_patch: Patch,
/// Raw patches as plain text files
pub raw_patches: Vec<String>,
/// Patches in the format to be displayed as preview
pub patches_preview: Vec<Text<'static>>,
/// Indicates if patchset has a cover letter
pub has_cover_letter: bool,
/// Which patches to reply
pub patches_to_reply: Vec<bool>,
pub preview_index: usize,
pub preview_scroll_offset: usize,
/// Horizontal offset
Expand Down Expand Up @@ -100,8 +108,31 @@ impl PatchsetDetailsAndActionsState {
self.toggle_action(PatchsetAction::Bookmark);
}

pub fn toggle_reply_with_reviewed_by_action(&mut self) {
self.toggle_action(PatchsetAction::ReplyWithReviewedBy);
pub fn toggle_reply_with_reviewed_by_action(&mut self, all: bool) {
if all {
if self.patches_to_reply.contains(&false) {
// If there is at least one patch not to be replied, set all to be
self.patches_to_reply = vec![true; self.patches_to_reply.len()];
} else {
// If all patches are set to be replied, set none to be
self.patches_to_reply = vec![false; self.patches_to_reply.len()];
}
} else if let Some(entry) = self.patches_to_reply.get_mut(self.preview_index) {
*entry = !*entry;
}
if self.patches_to_reply.contains(&true) {
self.patchset_actions
.insert(PatchsetAction::ReplyWithReviewedBy, true);
} else {
self.patchset_actions
.insert(PatchsetAction::ReplyWithReviewedBy, false);
}
}

pub fn reset_reply_with_reviewed_by_action(&mut self) {
self.patches_to_reply = vec![false; self.patches_to_reply.len()];
self.patchset_actions
.insert(PatchsetAction::ReplyWithReviewedBy, false);
}

pub fn toggle_action(&mut self, patchset_action: PatchsetAction) {
Expand All @@ -111,23 +142,20 @@ impl PatchsetDetailsAndActionsState {
}

pub fn actions_require_user_io(&self) -> bool {
*self
.patchset_actions
.get(&PatchsetAction::ReplyWithReviewedBy)
.unwrap()
self.patches_to_reply.contains(&true)
}

pub fn reply_patchset_with_reviewed_by(
&self,
target_list: &str,
git_send_email_options: &str,
) -> color_eyre::Result<Vec<usize>> {
successful_indexes: &mut HashSet<usize>,
) -> color_eyre::Result<()> {
let (git_user_name, git_user_email) = lore_session::get_git_signature("");
let mut successful_indexes = Vec::new();

if git_user_name.is_empty() || git_user_email.is_empty() {
println!("`git config user.name` or `git config user.email` not set\nAborting...");
return Ok(successful_indexes);
return Ok(());
}

let tmp_dir = Command::new("mktemp").arg("--directory").output().unwrap();
Expand All @@ -138,6 +166,7 @@ impl PatchsetDetailsAndActionsState {
tmp_dir,
target_list,
&self.raw_patches,
&self.patches_to_reply,
&format!("{git_user_name} <{git_user_email}>"),
git_send_email_options,
) {
Expand All @@ -147,14 +176,20 @@ impl PatchsetDetailsAndActionsState {
}
};

for (index, mut command) in git_reply_commands.into_iter().enumerate() {
let reply_indexes: Vec<usize> = self
.patches_to_reply
.iter()
.enumerate()
.filter_map(|(i, &val)| if val { Some(i) } else { None })
.collect();
for (i, mut command) in git_reply_commands.into_iter().enumerate() {
let mut child = command.spawn().unwrap();
let exit_status = child.wait().unwrap();
if exit_status.success() {
successful_indexes.push(index);
successful_indexes.insert(reply_indexes[i]);
}
}

Ok(successful_indexes)
Ok(())
}
}
10 changes: 7 additions & 3 deletions src/handler/details_actions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,12 @@ pub fn handle_patchset_details<B: Backend>(
let patchset_details_and_actions = app.patchset_details_and_actions_state.as_mut().unwrap();

if key.modifiers.contains(KeyModifiers::SHIFT) {
if let KeyCode::Char('G') = key.code {
patchset_details_and_actions.go_to_last_line()
match key.code {
KeyCode::Char('G') => patchset_details_and_actions.go_to_last_line(),
KeyCode::Char('R') => {
patchset_details_and_actions.toggle_reply_with_reviewed_by_action(true);
}
_ => {}
}
return Ok(());
}
Expand Down Expand Up @@ -86,7 +90,7 @@ pub fn handle_patchset_details<B: Backend>(
patchset_details_and_actions.toggle_bookmark_action();
}
KeyCode::Char('r') => {
patchset_details_and_actions.toggle_reply_with_reviewed_by_action();
patchset_details_and_actions.toggle_reply_with_reviewed_by_action(false);
}
KeyCode::Enter => {
if patchset_details_and_actions.actions_require_user_io() {
Expand Down
13 changes: 9 additions & 4 deletions src/lore/lore_session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::lore::patch::{Patch, PatchFeed, PatchRegex};
use derive_getters::Getters;
use regex::Regex;
use serde_xml_rs::from_str;
use std::collections::HashMap;
use std::collections::{HashMap, HashSet};
use std::io::{BufRead, BufReader};
use std::mem::swap;
use std::path::Path;
Expand Down Expand Up @@ -372,6 +372,7 @@ pub fn prepare_reply_patchset_with_reviewed_by<T>(
tmp_dir: &Path,
target_list: &str,
patches: &[String],
patches_to_reply: &[bool],
git_signature: &str,
git_send_email_options: &str,
) -> Result<Vec<Command>, LoreSessionError>
Expand All @@ -383,7 +384,11 @@ where
static RE_MESSAGE_ID: LazyLock<Regex> =
LazyLock::new(|| Regex::new(r#"(?m)^Message-Id: <(.*?)>"#).unwrap());

for patch in patches.iter() {
for (i, patch) in patches.iter().enumerate() {
if !patches_to_reply[i] {
continue;
}

let message_id = RE_MESSAGE_ID
.captures(patch)
.unwrap()
Expand Down Expand Up @@ -499,7 +504,7 @@ pub fn get_git_signature(git_repo_path: &str) -> (String, String) {
}

pub fn save_reviewed_patchsets(
reviewed_patchsets: &HashMap<String, Vec<usize>>,
reviewed_patchsets: &HashMap<String, HashSet<usize>>,
filepath: &str,
) -> io::Result<()> {
if let Some(parent) = Path::new(filepath).parent() {
Expand All @@ -515,7 +520,7 @@ pub fn save_reviewed_patchsets(
Ok(())
}

pub fn load_reviewed_patchsets(filepath: &str) -> io::Result<HashMap<String, Vec<usize>>> {
pub fn load_reviewed_patchsets(filepath: &str) -> io::Result<HashMap<String, HashSet<usize>>> {
let reviewed_patchsets_file = File::open(filepath)?;
let reviewed_patchsets = serde_json::from_reader(reviewed_patchsets_file)?;
Ok(reviewed_patchsets)
Expand Down
3 changes: 3 additions & 0 deletions src/lore/lore_session/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -534,11 +534,14 @@ fn should_prepare_reply_patchset_with_reviewed_by() {
.unwrap(),
];

let patches_to_reply = vec![true; patches.len()];

let git_reply_commands = prepare_reply_patchset_with_reviewed_by(
&lore_api_client,
tmp_dir,
target_list,
&patches,
&patches_to_reply,
"Bar Foo <[email protected]>",
"--dry-run --suppress-cc=all",
)
Expand Down
Loading