refactors; improved errors; accept -- arbitrary positionals

src/cli.rs

@@ -1,45 +1,50 @@
-use crate::convert::ConversionError;
+use crate::convert::{ConversionError, KdlVersion};
 use crate::convert::{convert_and_write_file_content, convert_file_content};
 use std::env;
-use std::path::Path;
+use std::ffi::OsString;
+use std::path::{Path, PathBuf};
 
 const HELP_TEXT: &str = "\
-Usage: jsonkdl <input> <output> [options]
+Usage: jsonkdl [options] [--] <input> [<output>]
 Converts JSON to KDL.
-By default, KDL spec v2 is used.
-
-Arguments:
-  <input>          Path to input JSON file
-  <output>         Path to output KDL file
 
 Options:
   -1, --kdl-v1     Convert to KDL v1
-  -2, --kdl-v2     Convert to KDL v2
+  -2, --kdl-v2     Convert to KDL v2 (default)
   -f, --force      Overwrite output if it exists
   -v, --verbose    Print extra information during conversion
   -h, --help       Show this help message
+
+Arguments:
+  <input>          Path to input JSON file
+  <output>         Path to output KDL file (optional)
+
+If no output file is provided, the resulting KDL document is printed to standard output.
 ";
 
 #[derive(Debug)]
 pub enum CliError {
     MissingInput,
     HelpRequested,
     MultipleKdlVersion,
-    InvalidInputPath(String),
-    FileExists(String),
-    InputNotFound(String),
+    UnknownOption(String),
+    TooManyPositionals,
+    NotUnicode(OsString),
+    InvalidInputPath(PathBuf),
+    FileExists(PathBuf),
+    InputNotFound(PathBuf),
     Conversion(ConversionError),
 }
 
 pub type Result<T> = std::result::Result<T, CliError>;
 
 #[derive(Debug)]
 pub struct Args {
-    pub input: String,
-    pub output: Option<String>,
+    pub input: PathBuf,
+    pub output: Option<PathBuf>,
     pub force: bool,
     pub verbose: bool,
-    pub kdl_version: i32,
+    pub kdl_version: KdlVersion,
 }
 
 impl std::fmt::Display for CliError {
@@ -48,11 +53,24 @@ impl std::fmt::Display for CliError {
             CliError::MissingInput => writeln!(f, "missing input path"),
             CliError::HelpRequested => writeln!(f, "help requested"),
             CliError::MultipleKdlVersion => writeln!(f, "specify only one of --kdl-v1 or --kdl-v2"),
-            CliError::InvalidInputPath(path) => writeln!(f, "not a file: {}", path),
+            CliError::UnknownOption(opt) => writeln!(
+                f,
+                "unknown command-line option {opt} (use `--` to pass arbitrary filenames)"
+            ),
+            CliError::TooManyPositionals => writeln!(f, "too many positional arguments"),
+            CliError::NotUnicode(arg) => writeln!(
+                f,
+                "the argument {arg:?} was not valid Unicode (use `--` to pass arbitrary filenames)"
+            ),
+            CliError::InvalidInputPath(path) => writeln!(f, "not a file: {}", path.display()),
             CliError::FileExists(path) => {
-                writeln!(f, "file exists: {} (use --force to overwrite)", path)
+                writeln!(
+                    f,
+                    "file exists: {} (use --force to overwrite)",
+                    path.display()
+                )
             }
-            CliError::InputNotFound(path) => writeln!(f, "no such file: {}", path),
+            CliError::InputNotFound(path) => writeln!(f, "no such file: {}", path.display()),
             CliError::Conversion(e) => writeln!(f, "conversion error: {}", e),
         }
     }
@@ -75,47 +93,71 @@ impl From<ConversionError> for CliError {
 
 impl Args {
     fn parse() -> Result<Self> {
-        let args: Vec<String> = env::args().collect();
-        let mut positional: Vec<String> = vec![];
-        let mut force = false;
-        let mut verbose = false;
-        let mut kdl_version = 0;
+        let args = env::args_os();
 
         if args.len() == 1 {
             return Err(CliError::HelpRequested);
         }
 
-        for arg in args.iter().skip(1) {
-            if !arg.starts_with('-') {
-                positional.push(arg.into());
-            } else if arg == "-f" || arg == "--force" {
-                force = true;
-            } else if arg == "-v" || arg == "--verbose" {
-                verbose = true;
-            } else if arg == "-1" || arg == "--kdl-v1" {
-                if kdl_version != 0 {
-                    return Err(CliError::MultipleKdlVersion);
-                }
+        let args = args.skip(1);
 
-                kdl_version = 1;
-            } else if arg == "-2" || arg == "--kdl-v2" {
-                if kdl_version != 0 {
-                    return Err(CliError::MultipleKdlVersion);
+        let mut force = false;
+        let mut verbose = false;
+        let mut kdl_version = None;
+
+        let mut positionals_only = false;
+
+        let mut input = None;
+        let mut output = None;
+
+        for arg in args {
+            let is_positional;
+
+            if positionals_only {
+                is_positional = true;
+            } else {
+                let Some(arg) = arg.to_str() else {
+                    return Err(CliError::NotUnicode(arg));
+                };
+
+                if arg.starts_with("-") {
+                    is_positional = false;
+                    match arg {
+                        "--" => positionals_only = true,
+                        "-f" | "--force" => force = true,
+                        "-v" | "--verbose" => verbose = true,
+                        "-1" | "--kdl-v1" => {
+                            if kdl_version.replace(KdlVersion::V1).is_some() {
+                                return Err(CliError::MultipleKdlVersion);
+                            }
+                        }
+                        "-2" | "--kdl-v2" => {
+                            if kdl_version.replace(KdlVersion::V2).is_some() {
+                                return Err(CliError::MultipleKdlVersion);
+                            }
+                        }
+                        "-h" | "--help" => return Err(CliError::HelpRequested),
+                        _ => return Err(CliError::UnknownOption(arg.to_string())),
+                    }
+                } else {
+                    is_positional = true;
                 }
+            }
 
-                kdl_version = 2;
-            } else if arg == "-h" || arg == "--help" {
-                return Err(CliError::HelpRequested);
+            if is_positional {
+                if input.is_none() {
+                    input = Some(PathBuf::from(arg));
+                } else if output.is_none() {
+                    output = Some(PathBuf::from(arg));
+                } else {
+                    return Err(CliError::TooManyPositionals);
+                }
             }
         }
 
-        if kdl_version == 0 {
-            kdl_version = 2;
-        };
-
-        let input = positional.get(0).ok_or(CliError::MissingInput)?.to_string();
+        let kdl_version = kdl_version.unwrap_or_default();
 
-        let output = positional.get(1).map(|s| s.to_string());
+        let input = input.ok_or(CliError::MissingInput)?;
 
         let result = Self {
             input,
@@ -143,13 +185,11 @@ pub fn run() -> Result<()> {
         Err(e) => return Err(e),
     };
 
-    let input_path = Path::new(&args.input);
-
-    if !input_path.exists() {
+    if !args.input.exists() {
         return Err(CliError::InputNotFound(args.input));
     }
 
-    if !input_path.is_file() {
+    if !args.input.is_file() {
         return Err(CliError::InvalidInputPath(args.input));
     }
 
@@ -160,9 +200,9 @@ pub fn run() -> Result<()> {
             return Err(CliError::FileExists(output));
         }
 
-        convert_and_write_file_content(input_path, output_path, args.verbose, args.kdl_version)?;
+        convert_and_write_file_content(&args.input, output_path, args.verbose, args.kdl_version)?;
     } else {
-        let kdl_content = convert_file_content(input_path, args.kdl_version)?;
+        let kdl_content = convert_file_content(&args.input, args.kdl_version)?;
 
         println!("{}", kdl_content);
     }