Skip to content

Commit e42de1b

Browse files
committed
fix: download_patchset checks if patch exists at expected path
Seeking to fix #69, this commit introduces a check to the function download_patchset and a new enum. If the function fails to create a file with the newly retrieved patch, this result is passed on and eventually treated as an alert popup. Fixes: #69 Co-developed-by: Bruno Stephan <[email protected]> Signed-off-by: Bruno Stephan <[email protected]> Co-developed-by: Andre de Lima <[email protected]> Signed-off-by: Andre de Lima <[email protected]> Signed-off-by: Arthur Pilone <[email protected]>
1 parent e12cadf commit e42de1b

File tree

3 files changed

+48
-15
lines changed

3 files changed

+48
-15
lines changed

src/app.rs

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use cover_renderer::render_cover;
99
use logging::Logger;
1010
use patch_hub::lore::{
1111
lore_api_client::BlockingLoreAPIClient,
12-
lore_session,
12+
lore_session::{self, B4Result},
1313
patch::{Author, Patch},
1414
};
1515
use patch_renderer::{render_patch_preview, PatchRenderer};
@@ -132,7 +132,7 @@ impl App {
132132
/// Initializes field [App::details_actions], from currently selected
133133
/// patchset in [App::bookmarked_patchsets] or [App::latest_patchsets],
134134
/// depending on the value of [App::current_screen].
135-
pub fn init_details_actions(&mut self) -> color_eyre::Result<()> {
135+
pub fn init_details_actions(&mut self) -> color_eyre::Result<B4Result> {
136136
let representative_patch: Patch;
137137
let mut is_patchset_bookmarked = true;
138138
let mut reviewed_by = Vec::new();
@@ -160,12 +160,14 @@ impl App {
160160
screen => bail!(format!("Invalid screen passed as argument {screen:?}")),
161161
};
162162

163-
let patchset_path: String = match log_on_error!(lore_session::download_patchset(
163+
let patchset_path: String = match lore_session::download_patchset(
164164
self.config.patchsets_cache_dir(),
165165
&representative_patch,
166-
)) {
167-
Ok(result) => result,
168-
Err(io_error) => bail!("{io_error}"),
166+
) {
167+
lore_session::B4Result::PatchFound(path) => path,
168+
lore_session::B4Result::PatchNotFound(error) => {
169+
return Ok(lore_session::B4Result::PatchNotFound(error))
170+
}
169171
};
170172

171173
match log_on_error!(lore_session::split_patchset(&patchset_path)) {
@@ -250,7 +252,7 @@ impl App {
250252
lore_api_client: self.lore_api_client.clone(),
251253
patchset_path,
252254
});
253-
Ok(())
255+
Ok(B4Result::PatchFound("Patch found".to_string()))
254256
}
255257
Err(message) => bail!(message),
256258
}

src/handler/latest.rs

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,17 @@ use std::ops::ControlFlow;
33
use crate::{
44
app::{screens::CurrentScreen, App},
55
loading_screen,
6-
ui::popup::{help::HelpPopUpBuilder, PopUp},
6+
ui::popup::{help::HelpPopUpBuilder, info_popup::InfoPopUp, PopUp},
77
};
8+
use color_eyre::eyre::Ok;
89
use ratatui::{
910
crossterm::event::{KeyCode, KeyEvent},
1011
prelude::Backend,
1112
Terminal,
1213
};
1314

15+
use patch_hub::lore::lore_session::B4Result;
16+
1417
pub fn handle_latest_patchsets<B>(
1518
app: &mut App,
1619
key: KeyEvent,
@@ -55,9 +58,18 @@ where
5558
"Loading patchset" => {
5659
let result = app.init_details_actions();
5760
if result.is_ok() {
58-
app.set_current_screen(CurrentScreen::PatchsetDetails);
61+
match result.unwrap() {
62+
B4Result::PatchFound(_) => {
63+
app.set_current_screen(CurrentScreen::PatchsetDetails);
64+
}
65+
66+
B4Result::PatchNotFound(err_cause) => {
67+
app.popup = Some(InfoPopUp::generate_info_popup("Error",&format!("The selected patchset couldn't be retrieved.\nReason: {err_cause}\nPlease choose another patchset.")));
68+
app.set_current_screen(CurrentScreen::LatestPatchsets);
69+
}
70+
}
5971
}
60-
result
72+
Ok(())
6173
}
6274
};
6375
}

src/lore/lore_session.rs

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use derive_getters::Getters;
77
use regex::Regex;
88
use serde_xml_rs::from_str;
99
use std::collections::{HashMap, HashSet};
10+
use std::ffi::OsStr;
1011
use std::io::{BufRead, BufReader};
1112
use std::mem::swap;
1213
use std::path::Path;
@@ -18,6 +19,11 @@ use std::{
1819
};
1920
use thiserror::Error;
2021

22+
pub enum B4Result {
23+
PatchFound(String),
24+
PatchNotFound(String),
25+
}
26+
2127
#[cfg(test)]
2228
mod tests;
2329

@@ -152,17 +158,20 @@ impl LoreSession {
152158
}
153159
}
154160

155-
pub fn download_patchset(output_dir: &str, patch: &Patch) -> io::Result<String> {
161+
pub fn download_patchset(output_dir: &str, patch: &Patch) -> B4Result {
156162
let message_id: &str = &patch.message_id().href;
157163
let mbox_name: String = extract_mbox_name_from_message_id(message_id);
158164

159165
if !Path::new(output_dir).exists() {
160-
fs::create_dir_all(output_dir)?;
166+
match fs::create_dir_all(output_dir) {
167+
Ok(_) => {}
168+
Err(_) => return B4Result::PatchNotFound("Couldn't create patches dir.".to_string()),
169+
};
161170
}
162171

163172
let filepath: String = format!("{output_dir}/{mbox_name}");
164173
if !Path::new(&filepath).exists() {
165-
Command::new("b4")
174+
match Command::new("b4")
166175
.arg("--quiet")
167176
.arg("am")
168177
.arg("--use-version")
@@ -174,10 +183,20 @@ pub fn download_patchset(output_dir: &str, patch: &Patch) -> io::Result<String>
174183
.arg(&mbox_name)
175184
.stdout(Stdio::null())
176185
.stderr(Stdio::null())
177-
.status()?;
186+
.status()
187+
{
188+
Ok(_) => {}
189+
Err(_) => return B4Result::PatchNotFound("Couldn't create patch file.".to_string()),
190+
};
191+
}
192+
193+
let path = Path::new(OsStr::new(&filepath));
194+
195+
if !path.exists() {
196+
return B4Result::PatchNotFound("Couldn't create patch file.".to_string());
178197
}
179198

180-
Ok(filepath)
199+
B4Result::PatchFound(filepath)
181200
}
182201

183202
fn extract_mbox_name_from_message_id(message_id: &str) -> String {

0 commit comments

Comments
 (0)