start working on a cli app #7
Loading…
Reference in a new issue
No description provided.
Delete branch ":app"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
I've started work on a cli app and some config file support stuff...
The end goal for this PR is, that this becomes the primary iOwO binary.
@ -1,5 +1,5 @@
[workspace]
members = [
members = [ "crates/app",
Probably unintentional formatting.
that was automatically modified to be like that by cargo
@ -10,1 +10,4 @@
clap = { version = "4", features = [ "derive" ] }
serde = { version = "1.0", features = [ "derive" ] }
ron = "0.8"
serde_json = "1.0"
Is there a reason
ron
andserde_json
are included workspace-wide?i wanted the version synced workspace wide, but ye, thinking about it the only place where they actually should get used is
app
@ -0,0 +41,4 @@
json_path
} else {
eprintln!("{}: couldn't find config file", "Fatal error".red());
process::exit(1)
For later, not now: Should use proper error handling.
put that on the non-existing todo list for when we have a proper error handling and reporting system
@ -0,0 +11,4 @@
report_serde_err(src, line, col, err.to_string())
}
pub fn report_serde_err(src: &str, line: usize, col: usize, msg: String) -> ! {
Maybe
report_serde_err
could take a custom error, which the errors fromron
andserde_json
are converted into.Then that streamlined error could report a bit more specific using
serde_error::Error::classify
andron::Error
.ye, that's a hack bc i was too lazy to implement a proper error type ^^'
@ -0,0 +11,4 @@
fn main() {
let args = Args::parse();
let cfg = Configs::read(args.config_file);
Might want to streamline those two, in order to avoid accidentally checking only one of them in future.
The
figment
crate might be worth looking at in that case.i'd prefer to try at least to some degree reduce dependencies, but i'll work on something like that then
You don't need to use
figment
. Custom merge logic works fine, too.@ -0,0 +2,4 @@
pub fn print_startup_msg() {
// now or fallback to utc
let now = OffsetDateTime::now_local().unwrap_or_else(|_| OffsetDateTime::now_utc());
Can always use
now_utc
, since no calculations are performed based on the timezone.the day might very well be timezone dependent, depending on where you live though?
Nevermind, I misinterpreted
now_utc
. I thought it'd return the current system time and assume it to be UTC (likeSystemTime::now
does), but it doesn't:app
1c843c5c02A few questions to be answered:
If there's no config file, should we fall back to defaults? What if we have one, but reading it fails?
Also, what about invalid values? Or if it's empty?
Yes, imo. Allows trying out iowo quickly without actually leaving permanent traces.
Yield a warning and continue running. Chances are the user still wants their pipeline executed. Or perhaps the config file was written for a newer version, or a different installation.
An empty config file is not an error, imo. Just assume defaults.
@ -0,0 +7,4 @@
/// Read this config file.
#[arg(short, long)]
pub config_path: Option<PathBuf>,
#[arg(long, env = "NO_STARTUP_MESSAGE", default_value = "false")]
Might want a
clap::builder::BoolishValueParser
here.How do I use that with claps derive api?
Through
@ -0,0 +11,4 @@
pub struct Configs {
#[serde(default = "default_example_value")]
pub example_value: i32,
#[serde(default = "default_no_startup_msg")]
Since the default is
false
which is also returned by `bool::default, the extra function is not necessary:If you do desire to keep the functions to allow more detailed behavior in future, I suggest renaming
default_no_startup_msg
to to justbool_false
and creating abool_true
counterpart once necessary.Thank you!! Feel free to merge if you're finished with this.
WIP: start working on a cli appto start working on a cli appI think this one also depends on #2, so lets get that finally merged first :)
Chances are you'll want to
git remote update; git rebase origin/main; git push --force-with-lease
after #2 is merged32c981f245
to746c81ab60