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

@@ -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
+- `--outputs` / `--no-outputs` whether whole displays are tiles too (default
-- `--hide-labels` draws an icon-only grid
+  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`)

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
+  --outputs, --no-outputs   include whole displays as tiles [yes]
-  --hide-labels             draw an icon-only grid
+  --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<Args, String> {
+    parse(std::env::args().skip(1))
+}
+
+fn parse(it: impl Iterator<Item = String>) -> Result<Args, String> {
     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, String> {
                 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<Args, String> {
     }
     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);
+    }
+}