diff --git a/src/cli.rs b/src/cli.rs index 9578a49..faba4a5 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -9,13 +9,14 @@ type Error = crate::errors::CliError; const DEFAULT_MAX_CONN: usize = 8; -#[derive(derive_builder::Builder)] +#[derive(derive_builder::Builder, Debug)] pub struct AppConfig { /// Article urls pub urls: Vec, pub max_conn: usize, /// Path to file of multiple articles into a single article pub merged: Option, + // TODO: Change type to Path pub output_directory: Option, pub log_level: LogLevel, pub can_disable_progress_bar: bool, @@ -95,7 +96,7 @@ impl<'a> TryFrom> for AppConfig { None => DEFAULT_MAX_CONN, }) .merged(arg_matches.value_of("output-name").map(|name| { - let file_ext = format!(".{}", arg_matches.value_of("export").unwrap()); + let file_ext = format!(".{}", arg_matches.value_of("export").unwrap_or("epub")); if name.ends_with(&file_ext) { name.to_owned() } else { @@ -132,10 +133,11 @@ impl<'a> TryFrom> for AppConfig { ) .output_directory( arg_matches - .value_of("output_directory") + .value_of("output-directory") .map(|output_directory| { let path = Path::new(output_directory); if !path.exists() { + // TODO: Create the directory Err(Error::OutputDirectoryNotExists) } else if !path.is_dir() { Err(Error::WrongOutputDirectory) @@ -157,7 +159,7 @@ impl<'a> TryFrom> for AppConfig { }, ) .export_type({ - let export_type = arg_matches.value_of("export").unwrap(); + let export_type = arg_matches.value_of("export").unwrap_or("epub"); if export_type == "html" { ExportType::HTML } else { @@ -200,3 +202,134 @@ pub enum ExportType { HTML, EPUB, } + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn test_clap_config_errors() { + let yaml_config = load_yaml!("cli_config.yml"); + let app = App::from_yaml(yaml_config); + + // It returns Ok when only a url is passed + let result = app + .clone() + .get_matches_from_safe(vec!["paperoni", "http://example.org"]); + assert!(result.is_ok()); + + // It returns an error when no args are passed + let result = app.clone().get_matches_from_safe(vec!["paperoni"]); + assert!(result.is_err()); + assert_eq!( + clap::ErrorKind::MissingArgumentOrSubcommand, + result.unwrap_err().kind + ); + + // It returns an error when both output-dir and merge are used + let result = app.clone().get_matches_from_safe(vec![ + "paperoni", + "http://example.org", + "--merge", + "foo", + "--output-dir", + "~", + ]); + assert!(result.is_err()); + assert_eq!(clap::ErrorKind::ArgumentConflict, result.unwrap_err().kind); + + // It returns an error when both no-css and no-header-css are used + let result = app.clone().get_matches_from_safe(vec![ + "paperoni", + "http://example.org", + "--no-css", + "--no-header-css", + ]); + assert!(result.is_err()); + assert_eq!(clap::ErrorKind::ArgumentConflict, result.unwrap_err().kind); + + // It returns an error when inline-toc is used without merge + let result = app.clone().get_matches_from_safe(vec![ + "paperoni", + "http://example.org", + "--inline-toc", + ]); + assert!(result.is_err()); + assert_eq!( + clap::ErrorKind::MissingRequiredArgument, + result.unwrap_err().kind + ); + + // It returns an error when inline-images is used without export + let result = app.clone().get_matches_from_safe(vec![ + "paperoni", + "http://example.org", + "--inline-images", + ]); + assert!(result.is_err()); + assert_eq!( + clap::ErrorKind::MissingRequiredArgument, + result.unwrap_err().kind + ); + + // It returns an error when export is given an invalid value + let result = app.clone().get_matches_from_safe(vec![ + "paperoni", + "http://example.org", + "--export", + "pdf", + ]); + assert!(result.is_err()); + assert_eq!(clap::ErrorKind::InvalidValue, result.unwrap_err().kind); + + // It returns an error when a max-conn is given a negative number. + let result = app.clone().get_matches_from_safe(vec![ + "paperoni", + "http://example.org", + "--max-conn", + "-1", + ]); + assert!(result.is_err()); + // The cli is configured not to accept negative numbers so passing "-1" would have it be read as a flag called 1 + assert_eq!(clap::ErrorKind::UnknownArgument, result.unwrap_err().kind); + } + + #[test] + fn test_init_with_cli() { + let yaml_config = load_yaml!("cli_config.yml"); + let app = App::from_yaml(yaml_config); + + // It returns an error when the urls passed are whitespace + let matches = app.clone().get_matches_from(vec!["paperoni", ""]); + let app_config = AppConfig::try_from(matches); + assert!(app_config.is_err()); + assert_eq!(Error::NoUrls, app_config.unwrap_err()); + + // It returns an error when inline-toc is used when exporting to HTML + let matches = app.clone().get_matches_from(vec![ + "paperoni", + "http://example.org", + "--merge", + "foo", + "--export", + "html", + "--inline-toc", + ]); + let app_config = AppConfig::try_from(matches); + assert!(app_config.is_err()); + assert_eq!(Error::WrongExportInliningToC, app_config.unwrap_err()); + // It returns an Ok when inline-toc is used when exporting to epub + let matches = app.clone().get_matches_from(vec![ + "paperoni", + "http://example.org", + "--merge", + "foo", + "--export", + "epub", + "--inline-toc", + ]); + assert!(AppConfig::try_from(matches).is_ok()); + + // It returns an error when inline-images is used when exporting to epub + } +} diff --git a/src/cli_config.yml b/src/cli_config.yml index 4f86d52..8de70e1 100644 --- a/src/cli_config.yml +++ b/src/cli_config.yml @@ -12,7 +12,7 @@ args: long: file help: Input file containing links takes_value: true - - output_directory: + - output-directory: short: o long: output-dir help: Directory to store output epub documents @@ -70,7 +70,6 @@ args: possible_values: [html, epub] value_name: type takes_value: true - default_value: epub - inline-images: long: inline-images help: Inlines the article images when exporting to HTML using base64. Pass --help to learn more. diff --git a/src/errors.rs b/src/errors.rs index 5b1cc25..c615477 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -138,6 +138,14 @@ pub enum LogError { CreateLogDirectoryError(#[from] std::io::Error), } +// dumb hack to allow for comparing errors in testing. +// derive macros cannot be used because underlying errors like io::Error do not derive PartialEq +impl PartialEq for LogError { + fn eq(&self, other: &Self) -> bool { + format!("{:?}", self) == format!("{:?}", other) + } +} + #[derive(Debug, Error)] pub enum CliError { #[error("Failed to open file with urls: {0}")] @@ -161,3 +169,11 @@ pub enum CliError { #[error("The --inline-images flag can only be used when exporting to html")] WrongExportInliningImages, } + +// dumb hack to allow for comparing errors in testing. +// derive macros cannot be used because underlying errors like io::Error do not derive PartialEq +impl PartialEq for CliError { + fn eq(&self, other: &Self) -> bool { + format!("{:?}", self) == format!("{:?}", other) + } +}