feat: recover message of last failed commit #424
Conversation
| use crate::recover::{ | ||
| ask_for_recovery, clear_recovery_file, read_recovery_file, write_recovery_file, | ||
| }; |
There was a problem hiding this comment.
i might rename the file to recovery.rs and call recovery::ask etc instead of doing this
| let CommitTitleDescription { title, description } = | ||
| ask_commit_title_description(&config, term).await?; | ||
| let commit = match read_recovery_file() { | ||
| Ok(Some(commit)) => match ask_for_recovery(term, commit.clone()) { |
There was a problem hiding this comment.
💭 thought: Maybe we could improve the UX if we use the recovery value as default value for title & description.
It's nearly as fast for the user, and allow update value if necessary
There was a problem hiding this comment.
I thought about it, but since the function that asks for gitmoji inputs is called through another one that only returns title+description, we can't really get out just the emoji
Do we make different prompts (title + description) when recovering?
I do like the idea of limiting the number of key presses needed to just "retry" a commit, clicking enter like 4 times would be a bit much imo
| ) | ||
| .await?; | ||
|
|
||
| match status.success().then_some(()).ok_or(Error::FailToCommit) { |
There was a problem hiding this comment.
🔨 todo: should be simplify, why not having a simple if-then-else here ?
|
|
||
| #[derive(Debug, derive_more::Error, derive_more::Display)] | ||
| #[display("Could not {action} recovery file: {source}")] | ||
| pub struct RecoveryError { |
There was a problem hiding this comment.
🎯 suggestion: turn it into an enum with a single variant.
It's better if later we want to add more error like splitting read and write error.
There was a problem hiding this comment.
Since the action field has just a few constant string values, i think it could be turned into a enum with just source inside, what do you think? Also, will the annotations work with enums? I'm not familiar with that way of organizing errors (i... Usually just slap anyhow lol)
| .join("lastmessage.json"); | ||
|
|
||
| if !path.exists() { | ||
| let dir = path.parent().ok_or(RecoveryError { |
There was a problem hiding this comment.
🔨 todo: create a private function that return the file to avoid duplication
| fs::create_dir_all(dir).expect("Failed to create recovery directory"); | ||
| } | ||
|
|
||
| let content = serde_json::to_string(¶ms).map_err(|source| RecoveryError { |
There was a problem hiding this comment.
🔨 todo: write it in TOML (see config module that already read/write data in toml)
| return Ok(None); | ||
| } | ||
|
|
||
| let content = fs::read_to_string(&path).map_err(|source| RecoveryError { |
There was a problem hiding this comment.
🔨 todo: write it in TOML (see config module that already read/write data in toml)
| @@ -33,6 +33,7 @@ indicatif = "0.17" | |||
| jiff = { version = "0.2.14", features = ["serde"] } | |||
| reqwest = { version = "0.12", features = ["json", "rustls-tls-native-roots"] } | |||
| serde = { version = "1", features = ["derive"] } | |||
| serde_json = "1.0.140" | |||
There was a problem hiding this comment.
💭 thought: we could use TOML to store the data instead, the dependency is already present
really useful when using precommit hooks :)