Skip to content

Commit

Permalink
other: take ownership of config and matches when building App
Browse files Browse the repository at this point in the history
The clap matches/config struct was taken in as a reference when building
the App structure. However, we do not need to keep these around after
this, so we can instead take ownership of the match/config and drop it
after to save a bit of memory.
  • Loading branch information
ClementTsang committed Jun 19, 2023
1 parent 45840d4 commit 87075d1
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 19 deletions.
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

0 comments on commit 87075d1

Please sign in to comment.