From 6bd4a3ee7684d2f86672df0482bef8a4c827ae75 Mon Sep 17 00:00:00 2001 From: Milad Alizadeh Date: Mon, 7 Sep 2026 18:25:47 +0100 Subject: [PATCH] Let a flag turn a boolean setting back on Both booleans could only be switched one way from the command line. There was a --no-outputs but no --outputs in the help, and no --labels at all, so a config file saying `outputs = no` could not be overridden for a single run: the only way back was to edit the file. "A flag always beats the file" was true of everything that takes a value and half true of the rest. So --outputs and --labels are the counterparts, --no-labels is the negative that matches them, and --hide-labels stays accepted for whatever it is already wired into. --outputs turned out to be parsed already and merely undocumented, which is its own kind of missing. Argument parsing moves behind parse(), taking the arguments as an iterator rather than reading the environment, so precedence is testable. The tests pin both directions and the fall-through to file then default, which is the part that quietly went wrong. --- README.md | 9 ++++--- src/cli.rs | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 75 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index f5e71d8..3655fc4 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,8 @@ is the one thing the rofi version had that this doesn't — see the roadmap. ``` wl-pick [--format tsv|json|portal] [--live all|current|none] [--fps N] - [--no-outputs] [--hide-labels] [--font FAMILY] [--font-size PX] + [--outputs|--no-outputs] [--labels|--no-labels] + [--font FAMILY] [--font-size PX] [--timeout SECS] [--verbose] ``` @@ -44,8 +45,10 @@ wl-pick [--format tsv|json|portal] [--live all|current|none] [--fps N] - `--live all|current|none` which tiles keep updating (default `all`; displays are always a single snapshot) - `--fps N` cap on live updates per tile per second (default 12) -- `--no-outputs` windows only; displays are included as tiles by default -- `--hide-labels` draws an icon-only grid +- `--outputs` / `--no-outputs` whether whole displays are tiles too (default + on). Both directions exist so either can override the config file +- `--labels` / `--no-labels` whether a label is drawn under each thumbnail + (default on); `--hide-labels` is the old spelling and still works - `--font FAMILY` label font family (default: the system monospace font) - `--font-size PX` label size in logical px - `--config PATH` config file (default `~/.config/wl-pick/config`) diff --git a/src/cli.rs b/src/cli.rs index e210502..7b2733a 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -20,8 +20,8 @@ usage: wl-pick [options] --live all|current|none which tiles keep updating live [all] (displays are always a single snapshot) --fps N cap on live updates per tile per second [12] - --no-outputs windows only; displays are included by default - --hide-labels draw an icon-only grid + --outputs, --no-outputs include whole displays as tiles [yes] + --labels, --no-labels a label under each thumbnail [yes] --font FAMILY label font family [the system monospace font] --font-size PX label size in logical px [13.3] --timeout SECS exit anyway after SECS, in case the keyboard @@ -198,8 +198,12 @@ impl Args { } pub fn parse_args() -> Result { + parse(std::env::args().skip(1)) +} + +fn parse(it: impl Iterator) -> Result { let mut args = Args::default(); - let mut it = std::env::args().skip(1); + let mut it = it; while let Some(arg) = it.next() { match arg.as_str() { "--format" => { @@ -212,7 +216,10 @@ pub fn parse_args() -> Result { args.config = Some(PathBuf::from(it.next().ok_or("--config needs a path")?)) } "-v" | "--verbose" => args.verbose = true, - "--hide-labels" => args.labels = Some(false), + "--labels" => args.labels = Some(true), + // --hide-labels was the only spelling before --labels existed, and + // is still accepted for whatever it is wired into. + "--no-labels" | "--hide-labels" => args.labels = Some(false), "--live" => { let v = it.next().ok_or("--live needs all|current|none")?; args.live = Some(Live::parse(&v)?); @@ -240,3 +247,61 @@ pub fn parse_args() -> Result { } Ok(args) } + +#[cfg(test)] +mod tests { + use super::*; + + fn args(flags: &[&str]) -> Args { + parse(flags.iter().map(|s| s.to_string())).expect("should parse") + } + + /// A config file that sets both booleans the same way. + fn file(on: bool) -> Config { + Config { + outputs: Some(on), + labels: Some(on), + ..Config::default() + } + } + + fn display() -> Display { + Display { + name: "DP-1".into(), + width: 2560, + height: 1440, + scale: 2, + focused: true, + } + } + + #[test] + fn every_boolean_can_be_set_both_ways() { + assert_eq!(args(&["--outputs"]).outputs, Some(true)); + assert_eq!(args(&["--no-outputs"]).outputs, Some(false)); + assert_eq!(args(&["--labels"]).labels, Some(true)); + assert_eq!(args(&["--no-labels"]).labels, Some(false)); + assert_eq!(args(&["--hide-labels"]).labels, Some(false), "old spelling"); + // Unset is what lets the file have its say. + assert_eq!(args(&[]).outputs, None); + assert_eq!(args(&[]).labels, None); + } + + #[test] + fn a_flag_beats_the_file_in_both_directions() { + // Turning something back on is the case that used to be unsayable: + // there was a --no-outputs but no --outputs, so a config saying no + // could not be overridden from the command line at all. + let on = args(&["--outputs", "--labels"]).resolve(&file(false), &display()); + assert!(on.outputs); + assert!(on.settings.theme.labels); + + let off = args(&["--no-outputs", "--no-labels"]).resolve(&file(true), &display()); + assert!(!off.outputs); + assert!(!off.settings.theme.labels); + + // With no flag the file decides, and with no file either, the default. + assert!(!args(&[]).resolve(&file(false), &display()).outputs); + assert!(args(&[]).resolve(&Config::default(), &display()).outputs); + } +}