Skip to content
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

other: take ownership of config and matches when building App #1215

Merged
merged 1 commit into from
Jun 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::{

use clap_complete::{generate_to, shells::Shell};

include!("src/clap.rs");
include!("src/args.rs");

fn create_dir(dir: &Path) -> io::Result<()> {
let res = fs::create_dir_all(dir);
Expand Down
File renamed without changes.
18 changes: 11 additions & 7 deletions src/bin/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ use bottom::{
fn main() -> Result<()> {
// let _profiler = dhat::Profiler::new_heap();

let matches = clap::get_matches();
let matches = args::get_matches();

#[cfg(feature = "logging")]
{
if let Err(err) =
Expand All @@ -45,10 +46,13 @@ fn main() -> Result<()> {
}

// Read from config file.
let config_path = read_config(matches.get_one::<String>("config_location"))
.context("Unable to access the given config file location.")?;
let mut config: Config = create_or_get_config(&config_path)
.context("Unable to properly parse or create the config file.")?;
let config = {
let config_path = read_config(matches.get_one::<String>("config_location"))
.context("Unable to access the given config file location.")?;

create_or_get_config(&config_path)
.context("Unable to properly parse or create the config file.")?
};

// Get widget layout separately
let (widget_layout, default_widget_id, default_widget_type_option) =
Expand All @@ -63,8 +67,8 @@ fn main() -> Result<()> {

// Create an "app" struct, which will control most of the program and store settings/state
let mut app = build_app(
&matches,
&mut config,
matches,
config,
&widget_layout,
default_widget_id,
&default_widget_type_option,
Expand Down
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ pub mod utils {
pub mod gen_util;
pub mod logging;
}
pub mod args;
pub mod canvas;
pub mod clap;
pub mod components;
pub mod constants;
pub mod data_conversion;
Expand Down
23 changes: 13 additions & 10 deletions src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,12 +171,15 @@ macro_rules! is_flag_enabled {
}

pub fn build_app(
matches: &ArgMatches, config: &mut Config, widget_layout: &BottomLayout,
default_widget_id: u64, default_widget_type_option: &Option<BottomWidgetType>,
styling: &CanvasStyling,
matches: ArgMatches, config: Config, widget_layout: &BottomLayout, default_widget_id: u64,
default_widget_type_option: &Option<BottomWidgetType>, styling: &CanvasStyling,
) -> Result<App> {
use BottomWidgetType::*;

// Since everything takes a reference, but we want to take ownership here to drop matches/config later...
let matches = &matches;
let config = &config;

let retention_ms =
get_retention_ms(matches, config).context("Update `retention` in your config file.")?;
let autohide_time = is_flag_enabled!(autohide_time, matches, config);
Expand Down Expand Up @@ -911,7 +914,7 @@ mod test {
#[test]
fn matches_human_times() {
let config = Config::default();
let app = crate::clap::build_app();
let app = crate::args::build_app();

{
let app = app.clone();
Expand All @@ -938,7 +941,7 @@ mod test {
#[test]
fn matches_number_times() {
let config = Config::default();
let app = crate::clap::build_app();
let app = crate::args::build_app();

{
let app = app.clone();
Expand All @@ -964,7 +967,7 @@ mod test {

#[test]
fn config_human_times() {
let app = crate::clap::build_app();
let app = crate::args::build_app();
let matches = app.get_matches_from(["btm"]);

let mut config = Config::default();
Expand All @@ -989,7 +992,7 @@ mod test {

#[test]
fn config_number_times() {
let app = crate::clap::build_app();
let app = crate::args::build_app();
let matches = app.get_matches_from(["btm"]);

let mut config = Config::default();
Expand All @@ -1012,19 +1015,19 @@ mod test {
);
}

fn create_app(mut config: Config, matches: ArgMatches) -> App {
fn create_app(config: Config, matches: ArgMatches) -> App {
let (layout, id, ty) = get_widget_layout(&matches, &config).unwrap();
let styling =
CanvasStyling::new(get_color_scheme(&matches, &config).unwrap(), &config).unwrap();

super::build_app(&matches, &mut config, &layout, id, &ty, &styling).unwrap()
super::build_app(matches, config, &layout, id, &ty, &styling).unwrap()
}

// TODO: There's probably a better way to create clap options AND unify together to avoid the possibility of
// typos/mixing up. Use proc macros to unify on one struct?
#[test]
fn verify_cli_options_build() {
let app = crate::clap::build_app();
let app = crate::args::build_app();

let default_app = {
let app = app.clone();
Expand Down