Skip to content

Commit

Permalink
refactor: extract app sub-states for better modularization
Browse files Browse the repository at this point in the history 
In `patch-hub`, the `App` represents the state of the application (the
model component), i.e., every action/event done/triggered by the
controller (currently completely defined at `src/main.rs`) changes the
state of the app, while the view component (`src/ui.rs`) queries it for
displaying screens to the user. For organization, the app state is
broken down into "sub-states" - types that have a `State` prefix - in
a way that we have a state for each screen.

However, all sub-states, as well as the top `App` state, are all defined
in the file `src/app.rs`. It goes without saying that this is a bad
engeneering choice and the big ball of mud keeps growing uncontrollably.

In this sense, extract all the sub-states to a dedicated file in the
directory `src/app/screens` to better modularize the app state.
Adaptations all throughout the project had to be made, so this commit
involves a lot of changes. However, most of them are represented by
moving files and correcting imports.

Helps: #7

[Maintainer edits]
- Fix some merge conflicts, which result in additional changes (adding
  of getters, for example)
- Rephrase commit message subject
- Add more context to commit message body
- Add tag "Helps"
- Format with `cargo fmt --all`

Signed-off-by: Thiago Duvanel <thiago.duvanel@usp.br>
Reviewed-by: David Tadokoro <davidbtadokoro@usp.br>
Signed-off-by: David Tadokoro <davidbtadokoro@usp.br>
  • Loading branch information
@th-duvanel @davidbtadokoro
th-duvanel authored and davidbtadokoro committed Oct 5, 2024
1 parent c1dd31f commit 676581e
Show file tree
Hide file tree
Showing 11 changed files with 365 additions and 346 deletions.
4 changes: 2 additions & 2 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ description = "patch-hub is a TUI that streamlines the interaction of Linux deve
[dependencies]
color-eyre = "0.6.3"
mockall = "0.13.0"
derive-getters = "0.4.0"
derive-getters = { version = "0.5.0", features = ["auto_copy_getters"] }
ratatui = "0.28.1"
regex = "1.10.5"
reqwest = { version = "0.12.5", default-features = false, features = ["blocking", "rustls-tls"] }
Expand Down
350 changes: 10 additions & 340 deletions src/app.rs
View file
Original file line number Diff line number Diff line change
@@ -1,351 +1,21 @@
use color_eyre::eyre::bail;
use config::Config;
use derive_getters::Getters;
use edit_config::EditConfigState;
use logging::Logger;
use patch_hub::{
lore_api_client::{BlockingLoreAPIClient, FailedFeedRequest},
lore_session::{self, LoreSession},
mailing_list::MailingList,
patch::Patch,
use patch_hub::{lore_session, patch::Patch};
use screens::{
bookmarked::BookmarkedPatchsetsState,
details::{PatchsetAction, PatchsetDetailsAndActionsState},
edit_config::EditConfigState,
latest::LatestPatchsetsState,
mail_list::MailingListSelectionState,
CurrentScreen,
};
use std::{collections::HashMap, path::Path, process::Command};
use std::collections::HashMap;

mod config;
mod edit_config;
pub mod logging;
pub mod patch_renderer;

pub struct BookmarkedPatchsetsState {
pub bookmarked_patchsets: Vec<Patch>,
pub patchset_index: usize,
}

impl BookmarkedPatchsetsState {
pub fn select_below_patchset(&mut self) {
if self.patchset_index + 1 < self.bookmarked_patchsets.len() {
self.patchset_index += 1;
}
}

pub fn select_above_patchset(&mut self) {
self.patchset_index = self.patchset_index.saturating_sub(1);
}

fn get_selected_patchset(&self) -> Patch {
self.bookmarked_patchsets
.get(self.patchset_index)
.unwrap()
.clone()
}

fn bookmark_selected_patch(&mut self, patch_to_bookmark: &Patch) {
if !self.bookmarked_patchsets.contains(patch_to_bookmark) {
self.bookmarked_patchsets.push(patch_to_bookmark.clone());
}
}

fn unbookmark_selected_patch(&mut self, patch_to_unbookmark: &Patch) {
if let Some(index) = self
.bookmarked_patchsets
.iter()
.position(|r| r == patch_to_unbookmark)
{
self.bookmarked_patchsets.remove(index);
}
}
}

#[derive(Getters)]
pub struct LatestPatchsetsState {
#[getter(skip)]
lore_session: LoreSession,
#[getter(skip)]
lore_api_client: BlockingLoreAPIClient,
target_list: String,
page_number: usize,
patchset_index: usize,
#[getter(skip)]
page_size: usize,
}

impl LatestPatchsetsState {
pub fn new(target_list: String, page_size: usize) -> LatestPatchsetsState {
LatestPatchsetsState {
lore_session: LoreSession::new(target_list.clone()),
lore_api_client: BlockingLoreAPIClient::new(),
target_list,
page_number: 1,
patchset_index: 0,
page_size,
}
}

pub fn fetch_current_page(&mut self) -> color_eyre::Result<()> {
if let Err(failed_feed_request) = self.lore_session.process_n_representative_patches(
&self.lore_api_client,
self.page_size * self.page_number,
) {
match failed_feed_request {
FailedFeedRequest::UnknownError(error) => bail!("[FailedFeedRequest::UnknownError]\n*\tFailed to request feed\n*\t{error:#?}"),
FailedFeedRequest::StatusNotOk(feed_response) => bail!("[FailedFeedRequest::StatusNotOk]\n*\tRequest returned with non-OK status\n*\t{feed_response:#?}"),
FailedFeedRequest::EndOfFeed => (),
}
};
Ok(())
}

pub fn select_below_patchset(&mut self) {
if self.patchset_index + 1 < self.lore_session.representative_patches_ids().len()
&& self.patchset_index + 1 < self.page_size * self.page_number
{
self.patchset_index += 1;
}
}

pub fn select_above_patchset(&mut self) {
if self.patchset_index == 0 {
return;
}
if self.patchset_index > self.page_size * (&self.page_number - 1) {
self.patchset_index -= 1;
}
}

pub fn increment_page(&mut self) {
let patchsets_processed: usize = self.lore_session.representative_patches_ids().len();
if self.page_size * self.page_number > patchsets_processed {
return;
}
self.page_number += 1;
self.patchset_index = self.page_size * (&self.page_number - 1);
}

pub fn decrement_page(&mut self) {
if self.page_number == 1 {
return;
}
self.page_number -= 1;
self.patchset_index = self.page_size * (&self.page_number - 1);
}

pub fn get_selected_patchset(&self) -> Patch {
let message_id: &str = self
.lore_session
.representative_patches_ids()
.get(self.patchset_index)
.unwrap();

self.lore_session
.get_processed_patch(message_id)
.unwrap()
.clone()
}

pub fn get_current_patch_feed_page(&self) -> Option<Vec<&Patch>> {
self.lore_session
.get_patch_feed_page(self.page_size, self.page_number)
}

pub fn get_number_of_processed_patchsets(&self) -> usize {
self.lore_session.representative_patches_ids().len()
}
}

pub struct PatchsetDetailsAndActionsState {
pub representative_patch: Patch,
pub patches: Vec<String>,
pub preview_index: usize,
pub preview_scroll_offset: usize,
pub patchset_actions: HashMap<PatchsetAction, bool>,
pub last_screen: CurrentScreen,
}

#[derive(Hash, Eq, PartialEq)]
pub enum PatchsetAction {
Bookmark,
ReplyWithReviewedBy,
}

impl PatchsetDetailsAndActionsState {
pub fn preview_next_patch(&mut self) {
if (self.preview_index + 1) < self.patches.len() {
self.preview_index += 1;
self.preview_scroll_offset = 0;
}
}

pub fn preview_previous_patch(&mut self) {
if self.preview_index > 0 {
self.preview_index -= 1;
self.preview_scroll_offset = 0;
}
}

pub fn preview_scroll_down(&mut self) {
let number_of_lines = self.patches[self.preview_index].lines().count();
if (self.preview_scroll_offset + 1) <= number_of_lines {
self.preview_scroll_offset += 1;
}
}

pub fn preview_scroll_up(&mut self) {
if self.preview_scroll_offset > 0 {
self.preview_scroll_offset -= 1;
}
}

pub fn toggle_bookmark_action(&mut self) {
self.toggle_action(PatchsetAction::Bookmark);
}

pub fn toggle_reply_with_reviewed_by_action(&mut self) {
self.toggle_action(PatchsetAction::ReplyWithReviewedBy);
}

fn toggle_action(&mut self, patchset_action: PatchsetAction) {
let current_value = *self.patchset_actions.get(&patchset_action).unwrap();
self.patchset_actions
.insert(patchset_action, !current_value);
}

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

pub fn reply_patchset_with_reviewed_by(
&self,
target_list: &str,
git_send_email_options: &str,
) -> color_eyre::Result<Vec<usize>> {
let lore_api_client = BlockingLoreAPIClient::new();
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);
}

let tmp_dir = Command::new("mktemp").arg("--directory").output().unwrap();
let tmp_dir = Path::new(std::str::from_utf8(&tmp_dir.stdout).unwrap().trim());

let git_reply_commands = match lore_session::prepare_reply_patchset_with_reviewed_by(
&lore_api_client,
tmp_dir,
target_list,
&self.patches,
&format!("{git_user_name} <{git_user_email}>"),
git_send_email_options,
) {
Ok(commands_vector) => commands_vector,
Err(failed_patch_html_request) => {
bail!(format!("{failed_patch_html_request:#?}"));
}
};

for (index, 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);
}
}

Ok(successful_indexes)
}
}

pub struct MailingListSelectionState {
pub mailing_lists: Vec<MailingList>,
pub target_list: String,
pub possible_mailing_lists: Vec<MailingList>,
pub highlighted_list_index: usize,
pub mailing_lists_path: String,
}

impl MailingListSelectionState {
pub fn refresh_available_mailing_lists(&mut self) -> color_eyre::Result<()> {
let lore_api_client = BlockingLoreAPIClient::new();

match lore_session::fetch_available_lists(&lore_api_client) {
Ok(available_mailing_lists) => {
self.mailing_lists = available_mailing_lists;
}
Err(failed_available_lists_request) => {
bail!(format!("{failed_available_lists_request:#?}"));
}
};

self.clear_target_list();

lore_session::save_available_lists(&self.mailing_lists, &self.mailing_lists_path)?;

Ok(())
}

pub fn remove_last_target_list_char(&mut self) {
if !self.target_list.is_empty() {
self.target_list.pop();
self.process_possible_mailing_lists();
}
}

pub fn push_char_to_target_list(&mut self, ch: char) {
self.target_list.push(ch);
self.process_possible_mailing_lists();
}

pub fn clear_target_list(&mut self) {
self.target_list.clear();
self.process_possible_mailing_lists();
}

fn process_possible_mailing_lists(&mut self) {
let mut possible_mailing_lists: Vec<MailingList> = Vec::new();

for mailing_list in &self.mailing_lists {
if mailing_list.name().starts_with(&self.target_list) {
possible_mailing_lists.push(mailing_list.clone());
}
}

self.possible_mailing_lists = possible_mailing_lists;
self.highlighted_list_index = 0;
}

pub fn highlight_below_list(&mut self) {
if self.highlighted_list_index + 1 < self.possible_mailing_lists.len() {
self.highlighted_list_index += 1;
}
}

pub fn highlight_above_list(&mut self) {
self.highlighted_list_index = self.highlighted_list_index.saturating_sub(1);
}

pub fn has_valid_target_list(&self) -> bool {
let list_length = self.possible_mailing_lists.len(); // Possible mailing list length
let list_index = self.highlighted_list_index; // Index of the selected mailing list

if list_index < list_length {
return true;
}
false
}
}

#[derive(Debug, Clone, PartialEq)]
pub enum CurrentScreen {
MailingListSelection,
BookmarkedPatchsets,
LatestPatchsets,
PatchsetDetails,
EditConfig,
}
pub mod screens;

pub struct App {
pub current_screen: CurrentScreen,
Expand Down
14 changes: 14 additions & 0 deletions src/app/screens.rs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
pub mod bookmarked;
pub mod details;
pub mod edit_config;
pub mod latest;
pub mod mail_list;

#[derive(Debug, Clone, PartialEq)]
pub enum CurrentScreen {
MailingListSelection,
BookmarkedPatchsets,
LatestPatchsets,
PatchsetDetails,
EditConfig,
}
Loading

0 comments on commit 676581e

Please sign in to comment.