refactor: Switch from is_present to actions

Because "diff" is conditionally present, we need to check for it to
avoid some of clap's stricter checks meant to prevent bugs.
This commit is contained in:
Ed Page 2022-09-01 20:53:37 -05:00 committed by David Peter
parent 6099f2c146
commit 3d398b35c3
3 changed files with 46 additions and 23 deletions

View File

@ -85,7 +85,7 @@ impl App {
Some("auto") | None => { Some("auto") | None => {
// If we have -pp as an option when in auto mode, the pager should be disabled. // If we have -pp as an option when in auto mode, the pager should be disabled.
let extra_plain = self.matches.get_count("plain") > 1; let extra_plain = self.matches.get_count("plain") > 1;
if extra_plain || self.matches.is_present("no-paging") { if extra_plain || self.matches.get_flag("no-paging") {
PagingMode::Never PagingMode::Never
} else if inputs.iter().any(Input::is_stdin) { } else if inputs.iter().any(Input::is_stdin) {
// If we are reading from stdin, only enable paging if we write to an // If we are reading from stdin, only enable paging if we write to an
@ -153,13 +153,13 @@ impl App {
.get_one::<String>("language") .get_one::<String>("language")
.map(|s| s.as_str()) .map(|s| s.as_str())
.or_else(|| { .or_else(|| {
if self.matches.is_present("show-all") { if self.matches.get_flag("show-all") {
Some("show-nonprintable") Some("show-nonprintable")
} else { } else {
None None
} }
}), }),
show_nonprintable: self.matches.is_present("show-all"), show_nonprintable: self.matches.get_flag("show-all"),
wrapping_mode: if self.interactive_output || maybe_term_width.is_some() { wrapping_mode: if self.interactive_output || maybe_term_width.is_some() {
match self.matches.get_one::<String>("wrap").map(|s| s.as_str()) { match self.matches.get_one::<String>("wrap").map(|s| s.as_str()) {
Some("character") => WrappingMode::Character, Some("character") => WrappingMode::Character,
@ -178,7 +178,7 @@ impl App {
// There's no point in wrapping when this is the case. // There's no point in wrapping when this is the case.
WrappingMode::NoWrapping(false) WrappingMode::NoWrapping(false)
}, },
colored_output: self.matches.is_present("force-colorization") colored_output: self.matches.get_flag("force-colorization")
|| match self.matches.get_one::<String>("color").map(|s| s.as_str()) { || match self.matches.get_one::<String>("color").map(|s| s.as_str()) {
Some("always") => true, Some("always") => true,
Some("never") => false, Some("never") => false,
@ -194,7 +194,7 @@ impl App {
.get_one::<String>("decorations") .get_one::<String>("decorations")
.map(|s| s.as_str()) .map(|s| s.as_str())
== Some("always") == Some("always")
|| self.matches.is_present("force-colorization")), || self.matches.get_flag("force-colorization")),
tab_width: self tab_width: self
.matches .matches
.get_one::<String>("tabs") .get_one::<String>("tabs")
@ -221,7 +221,7 @@ impl App {
} }
}) })
.unwrap_or_else(|| String::from(HighlightingAssets::default_theme())), .unwrap_or_else(|| String::from(HighlightingAssets::default_theme())),
visible_lines: match self.matches.is_present("diff") { visible_lines: match self.matches.contains_id("diff") && self.matches.get_flag("diff") {
#[cfg(feature = "git")] #[cfg(feature = "git")]
true => VisibleLines::DiffContext( true => VisibleLines::DiffContext(
self.matches self.matches
@ -255,7 +255,7 @@ impl App {
.map(LineRanges::from) .map(LineRanges::from)
.map(HighlightedLineRanges) .map(HighlightedLineRanges)
.unwrap_or_default(), .unwrap_or_default(),
use_custom_assets: !self.matches.is_present("no-custom-assets"), use_custom_assets: !self.matches.get_flag("no-custom-assets"),
}) })
} }
@ -310,7 +310,7 @@ impl App {
let mut styled_components = StyleComponents( let mut styled_components = StyleComponents(
if matches.get_one::<String>("decorations").map(|s| s.as_str()) == Some("never") { if matches.get_one::<String>("decorations").map(|s| s.as_str()) == Some("never") {
HashSet::new() HashSet::new()
} else if matches.is_present("number") { } else if matches.get_flag("number") {
[StyleComponent::LineNumbers].iter().cloned().collect() [StyleComponent::LineNumbers].iter().cloned().collect()
} else if 0 < matches.get_count("plain") { } else if 0 < matches.get_count("plain") {
[StyleComponent::Plain].iter().cloned().collect() [StyleComponent::Plain].iter().cloned().collect()

View File

@ -60,6 +60,7 @@ pub fn build_app(interactive_output: bool) -> Command<'static> {
.long("show-all") .long("show-all")
.alias("show-nonprintable") .alias("show-nonprintable")
.short('A') .short('A')
.action(ArgAction::SetTrue)
.conflicts_with("language") .conflicts_with("language")
.help("Show non-printable characters (space, tab, newline, ..).") .help("Show non-printable characters (space, tab, newline, ..).")
.long_help( .long_help(
@ -137,6 +138,7 @@ pub fn build_app(interactive_output: bool) -> Command<'static> {
Arg::new("diff") Arg::new("diff")
.long("diff") .long("diff")
.short('d') .short('d')
.action(ArgAction::SetTrue)
.conflicts_with("line-range") .conflicts_with("line-range")
.help("Only show lines that have been added/removed/modified.") .help("Only show lines that have been added/removed/modified.")
.long_help( .long_help(
@ -229,6 +231,7 @@ pub fn build_app(interactive_output: bool) -> Command<'static> {
.long("number") .long("number")
.overrides_with("number") .overrides_with("number")
.short('n') .short('n')
.action(ArgAction::SetTrue)
.help("Show line numbers (alias for '--style=numbers').") .help("Show line numbers (alias for '--style=numbers').")
.long_help( .long_help(
"Only show line numbers, no other decorations. This is an alias for \ "Only show line numbers, no other decorations. This is an alias for \
@ -283,6 +286,7 @@ pub fn build_app(interactive_output: bool) -> Command<'static> {
Arg::new("force-colorization") Arg::new("force-colorization")
.long("force-colorization") .long("force-colorization")
.short('f') .short('f')
.action(ArgAction::SetTrue)
.conflicts_with("color") .conflicts_with("color")
.conflicts_with("decorations") .conflicts_with("decorations")
.overrides_with("force-colorization") .overrides_with("force-colorization")
@ -314,6 +318,7 @@ pub fn build_app(interactive_output: bool) -> Command<'static> {
.short('P') .short('P')
.long("no-paging") .long("no-paging")
.alias("no-pager") .alias("no-pager")
.action(ArgAction::SetTrue)
.overrides_with("no-paging") .overrides_with("no-paging")
.hide(true) .hide(true)
.hide_short_help(true) .hide_short_help(true)
@ -379,6 +384,7 @@ pub fn build_app(interactive_output: bool) -> Command<'static> {
.arg( .arg(
Arg::new("list-themes") Arg::new("list-themes")
.long("list-themes") .long("list-themes")
.action(ArgAction::SetTrue)
.help("Display all supported highlighting themes.") .help("Display all supported highlighting themes.")
.long_help("Display a list of supported themes for syntax highlighting."), .long_help("Display a list of supported themes for syntax highlighting."),
) )
@ -469,6 +475,7 @@ pub fn build_app(interactive_output: bool) -> Command<'static> {
Arg::new("list-languages") Arg::new("list-languages")
.long("list-languages") .long("list-languages")
.short('L') .short('L')
.action(ArgAction::SetTrue)
.conflicts_with("list-themes") .conflicts_with("list-themes")
.help("Display all supported languages.") .help("Display all supported languages.")
.long_help("Display a list of supported languages for syntax highlighting."), .long_help("Display a list of supported languages for syntax highlighting."),
@ -493,12 +500,14 @@ pub fn build_app(interactive_output: bool) -> Command<'static> {
.arg( .arg(
Arg::new("no-custom-assets") Arg::new("no-custom-assets")
.long("no-custom-assets") .long("no-custom-assets")
.action(ArgAction::SetTrue)
.hide(true) .hide(true)
.help("Do not load custom assets"), .help("Do not load custom assets"),
) )
.arg( .arg(
Arg::new("config-file") Arg::new("config-file")
.long("config-file") .long("config-file")
.action(ArgAction::SetTrue)
.conflicts_with("list-languages") .conflicts_with("list-languages")
.conflicts_with("list-themes") .conflicts_with("list-themes")
.hide(true) .hide(true)
@ -507,6 +516,7 @@ pub fn build_app(interactive_output: bool) -> Command<'static> {
.arg( .arg(
Arg::new("generate-config-file") Arg::new("generate-config-file")
.long("generate-config-file") .long("generate-config-file")
.action(ArgAction::SetTrue)
.conflicts_with("list-languages") .conflicts_with("list-languages")
.conflicts_with("list-themes") .conflicts_with("list-themes")
.hide(true) .hide(true)
@ -515,12 +525,14 @@ pub fn build_app(interactive_output: bool) -> Command<'static> {
.arg( .arg(
Arg::new("config-dir") Arg::new("config-dir")
.long("config-dir") .long("config-dir")
.action(ArgAction::SetTrue)
.hide(true) .hide(true)
.help("Show bat's configuration directory."), .help("Show bat's configuration directory."),
) )
.arg( .arg(
Arg::new("cache-dir") Arg::new("cache-dir")
.long("cache-dir") .long("cache-dir")
.action(ArgAction::SetTrue)
.hide(true) .hide(true)
.help("Show bat's cache directory."), .help("Show bat's cache directory."),
) )
@ -528,12 +540,14 @@ pub fn build_app(interactive_output: bool) -> Command<'static> {
Arg::new("diagnostic") Arg::new("diagnostic")
.long("diagnostic") .long("diagnostic")
.alias("diagnostics") .alias("diagnostics")
.action(ArgAction::SetTrue)
.hide_short_help(true) .hide_short_help(true)
.help("Show diagnostic information for bug reports.") .help("Show diagnostic information for bug reports.")
) )
.arg( .arg(
Arg::new("acknowledgements") Arg::new("acknowledgements")
.long("acknowledgements") .long("acknowledgements")
.action(ArgAction::SetTrue)
.hide_short_help(true) .hide_short_help(true)
.help("Show acknowledgements."), .help("Show acknowledgements."),
) )
@ -551,6 +565,7 @@ pub fn build_app(interactive_output: bool) -> Command<'static> {
Arg::new("build") Arg::new("build")
.long("build") .long("build")
.short('b') .short('b')
.action(ArgAction::SetTrue)
.help("Initialize (or update) the syntax/theme cache.") .help("Initialize (or update) the syntax/theme cache.")
.long_help( .long_help(
"Initialize (or update) the syntax/theme cache by loading from \ "Initialize (or update) the syntax/theme cache by loading from \
@ -561,6 +576,7 @@ pub fn build_app(interactive_output: bool) -> Command<'static> {
Arg::new("clear") Arg::new("clear")
.long("clear") .long("clear")
.short('c') .short('c')
.action(ArgAction::SetTrue)
.help("Remove the cached syntax definitions and themes."), .help("Remove the cached syntax definitions and themes."),
) )
.group( .group(
@ -586,13 +602,20 @@ pub fn build_app(interactive_output: bool) -> Command<'static> {
"Use a different directory to store the cached syntax and theme set.", "Use a different directory to store the cached syntax and theme set.",
), ),
) )
.arg(Arg::new("blank").long("blank").requires("build").help( .arg(
"Create completely new syntax and theme sets \ Arg::new("blank")
.long("blank")
.action(ArgAction::SetTrue)
.requires("build")
.help(
"Create completely new syntax and theme sets \
(instead of appending to the default sets).", (instead of appending to the default sets).",
)) ),
)
.arg( .arg(
Arg::new("acknowledgements") Arg::new("acknowledgements")
.long("acknowledgements") .long("acknowledgements")
.action(ArgAction::SetTrue)
.requires("build") .requires("build")
.help("Build acknowledgements.bin."), .help("Build acknowledgements.bin."),
), ),

View File

@ -49,20 +49,20 @@ fn build_assets(matches: &clap::ArgMatches) -> Result<()> {
bat::assets::build( bat::assets::build(
source_dir, source_dir,
!matches.is_present("blank"), !matches.get_flag("blank"),
matches.is_present("acknowledgements"), matches.get_flag("acknowledgements"),
target_dir, target_dir,
clap::crate_version!(), clap::crate_version!(),
) )
} }
fn run_cache_subcommand(matches: &clap::ArgMatches) -> Result<()> { fn run_cache_subcommand(matches: &clap::ArgMatches) -> Result<()> {
if matches.is_present("build") { if matches.get_flag("build") {
#[cfg(feature = "build-assets")] #[cfg(feature = "build-assets")]
build_assets(matches)?; build_assets(matches)?;
#[cfg(not(feature = "build-assets"))] #[cfg(not(feature = "build-assets"))]
println!("bat has been built without the 'build-assets' feature. The 'cache --build' option is not available."); println!("bat has been built without the 'build-assets' feature. The 'cache --build' option is not available.");
} else if matches.is_present("clear") { } else if matches.get_flag("clear") {
clear_assets(); clear_assets();
} }
@ -286,7 +286,7 @@ fn invoke_bugreport(app: &App) {
fn run() -> Result<bool> { fn run() -> Result<bool> {
let app = App::new()?; let app = App::new()?;
if app.matches.is_present("diagnostic") { if app.matches.get_flag("diagnostic") {
#[cfg(feature = "bugreport")] #[cfg(feature = "bugreport")]
invoke_bugreport(&app); invoke_bugreport(&app);
#[cfg(not(feature = "bugreport"))] #[cfg(not(feature = "bugreport"))]
@ -313,7 +313,7 @@ fn run() -> Result<bool> {
let inputs = app.inputs()?; let inputs = app.inputs()?;
let config = app.config(&inputs)?; let config = app.config(&inputs)?;
if app.matches.is_present("list-languages") { if app.matches.get_flag("list-languages") {
let languages: String = get_languages(&config)?; let languages: String = get_languages(&config)?;
let inputs: Vec<Input> = vec![Input::from_reader(Box::new(languages.as_bytes()))]; let inputs: Vec<Input> = vec![Input::from_reader(Box::new(languages.as_bytes()))];
let plain_config = Config { let plain_config = Config {
@ -322,22 +322,22 @@ fn run() -> Result<bool> {
..Default::default() ..Default::default()
}; };
run_controller(inputs, &plain_config) run_controller(inputs, &plain_config)
} else if app.matches.is_present("list-themes") { } else if app.matches.get_flag("list-themes") {
list_themes(&config)?; list_themes(&config)?;
Ok(true) Ok(true)
} else if app.matches.is_present("config-file") { } else if app.matches.get_flag("config-file") {
println!("{}", config_file().to_string_lossy()); println!("{}", config_file().to_string_lossy());
Ok(true) Ok(true)
} else if app.matches.is_present("generate-config-file") { } else if app.matches.get_flag("generate-config-file") {
generate_config_file()?; generate_config_file()?;
Ok(true) Ok(true)
} else if app.matches.is_present("config-dir") { } else if app.matches.get_flag("config-dir") {
writeln!(io::stdout(), "{}", config_dir())?; writeln!(io::stdout(), "{}", config_dir())?;
Ok(true) Ok(true)
} else if app.matches.is_present("cache-dir") { } else if app.matches.get_flag("cache-dir") {
writeln!(io::stdout(), "{}", cache_dir())?; writeln!(io::stdout(), "{}", cache_dir())?;
Ok(true) Ok(true)
} else if app.matches.is_present("acknowledgements") { } else if app.matches.get_flag("acknowledgements") {
writeln!(io::stdout(), "{}", bat::assets::get_acknowledgements())?; writeln!(io::stdout(), "{}", bat::assets::get_acknowledgements())?;
Ok(true) Ok(true)
} else { } else {