-
Notifications
You must be signed in to change notification settings - Fork 12
fix: patch-hub crashes when loading patches after b4 fails #142
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
base: unstable
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -3,7 +3,7 @@ use crate::{ | |||||||||||||||||
ui::popup::{info_popup::InfoPopUp, PopUp}, | ||||||||||||||||||
}; | ||||||||||||||||||
use ansi_to_tui::IntoText; | ||||||||||||||||||
use color_eyre::eyre::bail; | ||||||||||||||||||
use color_eyre::eyre::{bail}; | ||||||||||||||||||
use config::Config; | ||||||||||||||||||
use cover_renderer::render_cover; | ||||||||||||||||||
use logging::Logger; | ||||||||||||||||||
|
@@ -23,6 +23,8 @@ use screens::{ | |||||||||||||||||
CurrentScreen, | ||||||||||||||||||
}; | ||||||||||||||||||
use std::collections::{HashMap, HashSet}; | ||||||||||||||||||
use std::ffi::OsStr; | ||||||||||||||||||
use std::path::Path; | ||||||||||||||||||
|
||||||||||||||||||
use crate::utils; | ||||||||||||||||||
|
||||||||||||||||||
|
@@ -32,6 +34,11 @@ pub mod logging; | |||||||||||||||||
pub mod patch_renderer; | ||||||||||||||||||
pub mod screens; | ||||||||||||||||||
|
||||||||||||||||||
pub enum PatchFound { | ||||||||||||||||||
Found, | ||||||||||||||||||
NotFound | ||||||||||||||||||
} | ||||||||||||||||||
Comment on lines
+37
to
+40
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe this naming is a bit unidiomatic. I would like to propose a change that goes a bit beyond just changing the name.
Suggested change
|
||||||||||||||||||
|
||||||||||||||||||
/// Type that represents the overall state of the application. It can be viewed | ||||||||||||||||||
/// as the **Model** component of `patch-hub`. | ||||||||||||||||||
pub struct App { | ||||||||||||||||||
|
@@ -132,7 +139,7 @@ impl App { | |||||||||||||||||
/// Initializes field [App::details_actions], from currently selected | ||||||||||||||||||
/// patchset in [App::bookmarked_patchsets] or [App::latest_patchsets], | ||||||||||||||||||
/// depending on the value of [App::current_screen]. | ||||||||||||||||||
pub fn init_details_actions(&mut self) -> color_eyre::Result<()> { | ||||||||||||||||||
pub fn init_details_actions(&mut self) -> color_eyre::Result<PatchFound> { | ||||||||||||||||||
let representative_patch: Patch; | ||||||||||||||||||
let mut is_patchset_bookmarked = true; | ||||||||||||||||||
let mut reviewed_by = Vec::new(); | ||||||||||||||||||
|
@@ -164,7 +171,15 @@ impl App { | |||||||||||||||||
self.config.patchsets_cache_dir(), | ||||||||||||||||||
&representative_patch, | ||||||||||||||||||
)) { | ||||||||||||||||||
Ok(result) => result, | ||||||||||||||||||
Ok(result) => { | ||||||||||||||||||
let path = Path::new(OsStr::new(&result)); | ||||||||||||||||||
|
||||||||||||||||||
if ! path.exists() { | ||||||||||||||||||
return Ok(PatchFound::NotFound); | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
result | ||||||||||||||||||
} | ||||||||||||||||||
Err(io_error) => bail!("{io_error}"), | ||||||||||||||||||
}; | ||||||||||||||||||
|
||||||||||||||||||
|
@@ -250,7 +265,7 @@ impl App { | |||||||||||||||||
lore_api_client: self.lore_api_client.clone(), | ||||||||||||||||||
patchset_path, | ||||||||||||||||||
}); | ||||||||||||||||||
Ok(()) | ||||||||||||||||||
Ok(PatchFound::Found) | ||||||||||||||||||
} | ||||||||||||||||||
Err(message) => bail!(message), | ||||||||||||||||||
} | ||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,10 +1,11 @@ | ||||||||||||||
use std::ops::ControlFlow; | ||||||||||||||
|
||||||||||||||
use crate::{ | ||||||||||||||
app::{screens::CurrentScreen, App}, | ||||||||||||||
app::{screens::CurrentScreen, App, PatchFound}, | ||||||||||||||
loading_screen, | ||||||||||||||
ui::popup::{help::HelpPopUpBuilder, PopUp}, | ||||||||||||||
ui::popup::{help::HelpPopUpBuilder, info_popup::InfoPopUp, PopUp}, | ||||||||||||||
}; | ||||||||||||||
use color_eyre::eyre::Ok; | ||||||||||||||
use ratatui::{ | ||||||||||||||
crossterm::event::{KeyCode, KeyEvent}, | ||||||||||||||
prelude::Backend, | ||||||||||||||
|
@@ -55,10 +56,19 @@ where | |||||||||||||
"Loading patchset" => { | ||||||||||||||
let result = app.init_details_actions(); | ||||||||||||||
if result.is_ok() { | ||||||||||||||
app.set_current_screen(CurrentScreen::PatchsetDetails); | ||||||||||||||
match result.unwrap() { | ||||||||||||||
PatchFound::Found => { | ||||||||||||||
app.set_current_screen(CurrentScreen::PatchsetDetails); | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
PatchFound::NotFound => { | ||||||||||||||
app.popup = Some(InfoPopUp::generate_info_popup("Error","The selected patchset couldn't be retrieved.\nPlease choose another patchset.")); | ||||||||||||||
app.set_current_screen(CurrentScreen::LatestPatchsets); | ||||||||||||||
} | ||||||||||||||
Comment on lines
+65
to
+67
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fix formatting
Suggested change
|
||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
result | ||||||||||||||
} | ||||||||||||||
Ok(()) | ||||||||||||||
} | ||||||||||||||
Comment on lines
+70
to
+71
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fix formatting
Suggested change
|
||||||||||||||
}; | ||||||||||||||
} | ||||||||||||||
_ => {} | ||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unecessary braces. They should be used when importing different parts in the same package.