Improve CLI design

This "improves" (and that is subjective) the design of the CLI. I am aiming to get some feedback on what people think of the new design:

```
Screenshot tool for wlroots based compositors implementing the zwlr_screencopy_v1 protocol.

Usage: wayshot [OPTIONS] [OUTPUT]

Arguments:
  [OUTPUT]
          Where to save the screenshot, "-" for stdout. Defaults to "$UNIX_TIMESTAMP-wayshot.$EXTENSION"

Options:
      --log-level <LOG_LEVEL>
          Log level to be used for printing to stderr

          [default: info]
          [possible values: trace, debug, info, warn, error]

  -s, --slurp <SLURP_ARGS>
          Arguments to call slurp with for selecting a region

  -c, --cursor
          Enable cursor in screenshots

      --encoding <FILE_EXTENSION>
          Set image encoder, if output file contains an extension, that will be used instead

          [default: png]
          [aliases: extension, format, output-format]

          Possible values:
          - jpg: JPG/JPEG encoder
          - png: PNG encoder
          - ppm: PPM encoder
          - qoi: Qut encoder

  -l, --list-outputs
          List all valid outputs

  -o, --output <OUTPUT>
          Choose a particular output/display to screenshot

      --choose-output
          Present a fuzzy selector for output/display selection

  -h, --help
          Print help (see a summary with '-h')

  -V, --version
          Print version
```

The main changes are:
1. `--debug` is now `--log-level` because this makes it easy to select more specifically what log level to use. I considered using `-v`, `-vv`... to increase verbosity but the `clap-verbosity-crate` uses `log` and not `tracing`. We could use it somewhat, but we'd pull in `log` (which seems fine) and we'd need to map from `log`'s Level to `tracing`'s Level enums (they use inverse ordering).
2.  `--stdout` and `--file` has been made an optional positional argument. This because it's what other CLIs often do and I wasn't sure what to call the option otherwise because `--output` and `-O`/`-o` is often what others use but we use it here to refer to displays/monitors/Wayland outputs. This avoids that confusion hopefully and I've also clarified this in the documentation.
    * Additionally if a path is given, its extension will always be used. So you cannot save `jpg` to `foo.png`. Perhaps this behaviour can be changed, though I don't see a reason to support this weird edge case? When is someone saving `png` to `jpg`?
3. `--extension` is `--encoding` with aliases like `extension`.

Again, let me know what you think.

.gitignore

@@ -2,3 +2,8 @@ target
 *.gz
 *.out
 .direnv
+*.jpg
+*.jpeg
+*.png
+*.ppm
+*.qoi

wayshot/src/cli.rs

@@ -1,43 +1,45 @@
+use std::path::PathBuf;
+
 use clap::arg;
 
 use clap::Parser;
 
+use crate::utils::EncodingFormat;
+use clap::builder::TypedValueParser;
+
 #[derive(Parser)]
 #[command(version, about)]
 pub struct Cli {
-    /// Enable debug mode
-    #[arg(short, long, action = clap::ArgAction::SetTrue)]
-    pub debug: bool,
+    /// Where to save the screenshot, "-" for stdout. Defaults to "$UNIX_TIMESTAMP-wayshot.$EXTENSION".
+    #[arg(value_name = "OUTPUT")]
+    pub file: Option<PathBuf>,
+
+    /// Log level to be used for printing to stderr
+    #[arg(long, default_value = "info", value_parser = clap::builder::PossibleValuesParser::new(["trace", "debug", "info", "warn", "error"]).map(|s| -> tracing::Level{ s.parse().unwrap()}))]
+    pub log_level: tracing::Level,
 
     /// Arguments to call slurp with for selecting a region
     #[arg(short, long, value_name = "SLURP_ARGS")]
     pub slurp: Option<String>,
 
-    /// Mention a custom file path
-    #[arg(short, long, conflicts_with = "stdout", value_name = "FILE_PATH")]
-    pub file: Option<String>,
-
-    /// Output the image data to standard out
-    #[arg(long, conflicts_with = "file", action = clap::ArgAction::SetTrue)]
-    pub stdout: bool,
-
     /// Enable cursor in screenshots
-    #[arg(short, long, action = clap::ArgAction::SetTrue)]
+    #[arg(short, long)]
     pub cursor: bool,
 
-    /// Set image encoder (Png is default)
-    #[arg(short, long, value_name = "FILE_EXTENSION")]
-    pub extension: Option<String>,
+    /// Set image encoder, by default uses the file extension from the OUTPUT
+    /// positional argument. Otherwise defaults to png.
+    #[arg(long, visible_aliases = ["extension", "format", "output-format"], value_name = "FILE_EXTENSION")]
+    pub encoding: Option<EncodingFormat>,
 
     /// List all valid outputs
-    #[arg(short, long, alias = "listoutputs", action = clap::ArgAction::SetTrue)]
+    #[arg(short, long, alias = "listoutputs")]
     pub list_outputs: bool,
 
-    /// Choose a particular display to screenshot
+    /// Choose a particular output/display to screenshot
     #[arg(short, long, conflicts_with = "slurp")]
     pub output: Option<String>,
 
-    /// Present a fuzzy selector for outputs
-    #[arg(long, alias = "chooseoutput", conflicts_with_all = ["slurp", "output"], action = clap::ArgAction::SetTrue)]
+    /// Present a fuzzy selector for output/display selection
+    #[arg(long, alias = "chooseoutput", conflicts_with_all = ["slurp", "output"])]
     pub choose_output: bool,
 }

wayshot/src/utils.rs

@@ -1,6 +1,11 @@
-use eyre::{ContextCompat, Result};
+use clap::ValueEnum;
+use eyre::{bail, ContextCompat, Error, Result};
+
 use std::{
+    fmt::Display,
+    path::PathBuf,
     process::exit,
+    str::FromStr,
     time::{SystemTime, UNIX_EPOCH},
 };
 
@@ -47,18 +52,24 @@ pub fn parse_geometry(g: &str) -> Result<LogicalRegion> {
 }
 
 /// Supported image encoding formats.
-#[derive(Debug, Copy, Clone, PartialEq, Eq)]
+#[derive(Debug, Copy, Clone, PartialEq, Eq, ValueEnum)]
 pub enum EncodingFormat {
-    /// Jpeg / jpg encoder.
+    /// JPG/JPEG encoder.
     Jpg,
-    /// Png encoder.
+    /// PNG encoder.
     Png,
-    /// Ppm encoder.
+    /// PPM encoder.
     Ppm,
-    /// Qoi encoder.
+    /// Qut encoder.
     Qoi,
 }
 
+impl Default for EncodingFormat {
+    fn default() -> Self {
+        Self::Png
+    }
+}
+
 impl From<EncodingFormat> for image::ImageOutputFormat {
     fn from(format: EncodingFormat) -> Self {
         match format {
@@ -70,6 +81,33 @@ impl From<EncodingFormat> for image::ImageOutputFormat {
     }
 }
 
+impl TryFrom<&PathBuf> for EncodingFormat {
+    type Error = Error;
+
+    fn try_from(value: &PathBuf) -> std::result::Result<Self, Self::Error> {
+        value
+            .extension()
+            .wrap_err_with(|| {
+                format!(
+                    "no extension in {} to deduce encoding format",
+                    value.display()
+                )
+            })
+            .and_then(|ext| {
+                ext.to_str().wrap_err_with(|| {
+                    format!("extension in {} is not valid unicode", value.display())
+                })
+            })
+            .and_then(|ext| ext.parse())
+    }
+}
+
+impl Display for EncodingFormat {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{}", Into::<&str>::into(*self))
+    }
+}
+
 impl From<EncodingFormat> for &str {
     fn from(format: EncodingFormat) -> Self {
         match format {
@@ -81,7 +119,21 @@ impl From<EncodingFormat> for &str {
     }
 }
 
-pub fn get_default_file_name(extension: EncodingFormat) -> String {
+impl FromStr for EncodingFormat {
+    type Err = Error;
+
+    fn from_str(s: &str) -> std::result::Result<Self, Self::Err> {
+        Ok(match s {
+            "jpg" | "jpeg" => Self::Jpg,
+            "png" => Self::Png,
+            "ppm" => Self::Ppm,
+            "qoi" => Self::Qoi,
+            _ => bail!("unsupported extension '{s}'"),
+        })
+    }
+}
+
+pub fn get_default_file_name(extension: EncodingFormat) -> PathBuf {
     let time = match SystemTime::now().duration_since(UNIX_EPOCH) {
         Ok(n) => n.as_secs().to_string(),
         Err(_) => {
@@ -90,5 +142,5 @@ pub fn get_default_file_name(extension: EncodingFormat) -> String {
         }
     };
 
-    time + "-wayshot." + extension.into()
+    (time + "-wayshot." + extension.into()).into()
 }

wayshot/src/wayshot.rs

@@ -11,9 +11,7 @@ mod cli;
 mod utils;
 
 use dialoguer::{theme::ColorfulTheme, FuzzySelect};
-use tracing::Level;
-
-use crate::utils::EncodingFormat;
+use utils::EncodingFormat;
 
 fn select_ouput<T>(ouputs: &[T]) -> Option<usize>
 where
@@ -32,41 +30,39 @@ where
 
 fn main() -> Result<()> {
     let cli = cli::Cli::parse();
-    let level = if cli.debug { Level::TRACE } else { Level::INFO };
     tracing_subscriber::fmt()
-        .with_max_level(level)
+        .with_max_level(cli.log_level)
         .with_writer(std::io::stderr)
         .init();
 
-    let extension = if let Some(extension) = cli.extension {
-        let ext = extension.trim().to_lowercase();
-        tracing::debug!("Using custom extension: {:#?}", ext);
-
-        match ext.as_str() {
-            "jpeg" | "jpg" => EncodingFormat::Jpg,
-            "png" => EncodingFormat::Png,
-            "ppm" => EncodingFormat::Ppm,
-            "qoi" => EncodingFormat::Qoi,
-            _ => {
-                tracing::error!("Invalid extension provided.\nValid extensions:\n1) jpeg\n2) jpg\n3) png\n4) ppm\n5) qoi");
-                exit(1);
+    let input_encoding = cli
+        .file
+        .as_ref()
+        .and_then(|pathbuf| pathbuf.try_into().ok());
+    let requested_encoding = cli
+        .encoding
+        .or(input_encoding)
+        .unwrap_or(EncodingFormat::default());
+
+    if let Some(input_encoding) = input_encoding {
+        if input_encoding != requested_encoding {
+            tracing::warn!(
+                "The encoding requested '{requested_encoding}' does not match the output file's encoding '{input_encoding}'. Still using the requested encoding however.",
+            );
+        }
+    }
+
+    let file = match cli.file {
+        Some(pathbuf) => {
+            if pathbuf.to_string_lossy() == "-" {
+                None
+            } else {
+                Some(pathbuf)
             }
         }
-    } else {
-        EncodingFormat::Png
+        None => Some(utils::get_default_file_name(requested_encoding)),
     };
 
-    let mut file_is_stdout: bool = false;
-    let mut file_path: Option<String> = None;
-
-    if cli.stdout {
-        file_is_stdout = true;
-    } else if let Some(filepath) = cli.file {
-        file_path = Some(filepath.trim().to_string());
-    } else {
-        file_path = Some(utils::get_default_file_name(extension));
-    }
-
     let wayshot_conn = WayshotConnection::new()?;
 
     if cli.list_outputs {
@@ -117,16 +113,16 @@ fn main() -> Result<()> {
         wayshot_conn.screenshot_all(cli.cursor)?
     };
 
-    if file_is_stdout {
+    if let Some(file) = file {
+        image_buffer.save(file)?;
+    } else {
         let stdout = stdout();
         let mut buffer = Cursor::new(Vec::new());
 
         let mut writer = BufWriter::new(stdout.lock());
-        image_buffer.write_to(&mut buffer, extension)?;
+        image_buffer.write_to(&mut buffer, requested_encoding)?;
 
         writer.write_all(buffer.get_ref())?;
-    } else {
-        image_buffer.save(file_path.unwrap())?;
     }
 
     Ok(())