Migrate CLI config to structopt

While structopt also uses clap under the hood, the configuration through
annotations allows for significantly more maintainable and concise CLI
definition.

This will also make it far easier to have platform-specific options,
which is problematic with clap since no individual methods can be
removed from its builder.

The change in Alacritty's CLI has been kept to a minimum with the only
significant changes being the `--version` flag listed before the
`-v` flag and the authors all on the same line.

6 files changed, 210 insertions, 201 deletions

diff --git a/Cargo.lock b/Cargo.lock
index adaa9409..38a44a21 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -21,7 +21,6 @@ dependencies = [
  "alacritty_config_derive",
  "alacritty_terminal",
  "bitflags",
- "clap",
  "cocoa 0.24.0",
  "copypasta",
  "crossfont",
@@ -40,6 +39,7 @@ dependencies = [
  "serde",
  "serde_json",
  "serde_yaml",
+ "structopt",
  "time",
  "unicode-width",
  "wayland-client",
@@ -754,6 +754,15 @@ dependencies = [
 ]
 
 [[package]]
+name = "heck"
+version = "0.3.3"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "6d621efb26863f0e9924c6ac577e8275e5e6b77455db64ffa6c65c904e9e132c"
+dependencies = [
+ "unicode-segmentation",
+]
+
+[[package]]
 name = "hermit-abi"
 version = "0.1.19"
 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -1279,6 +1288,30 @@ dependencies = [
 ]
 
 [[package]]
+name = "proc-macro-error"
+version = "1.0.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "da25490ff9892aab3fcf7c36f08cfb902dd3e71ca0f9f9517bea02a73a5ce38c"
+dependencies = [
+ "proc-macro-error-attr",
+ "proc-macro2",
+ "quote",
+ "syn",
+ "version_check",
+]
+
+[[package]]
+name = "proc-macro-error-attr"
+version = "1.0.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a1be40180e52ecc98ad80b184934baf3d0d29f979574e439af5a55274b35f869"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "version_check",
+]
+
+[[package]]
 name = "proc-macro2"
 version = "1.0.28"
 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -1547,6 +1580,30 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "6446ced80d6c486436db5c078dde11a9f73d42b57fb273121e160b84f63d894c"
 
 [[package]]
+name = "structopt"
+version = "0.3.22"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "69b041cdcb67226aca307e6e7be44c8806423d83e018bd662360a93dabce4d71"
+dependencies = [
+ "clap",
+ "lazy_static",
+ "structopt-derive",
+]
+
+[[package]]
+name = "structopt-derive"
+version = "0.4.15"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "7813934aecf5f51a54775e00068c237de98489463968231a51746bbbc03f9c10"
+dependencies = [
+ "heck",
+ "proc-macro-error",
+ "proc-macro2",
+ "quote",
+ "syn",
+]
+
+[[package]]
 name = "syn"
 version = "1.0.74"
 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -1612,6 +1669,12 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "3e5d7cd7ab3e47dda6e56542f4bbf3824c15234958c6e1bd6aaa347e93499fdc"
 
 [[package]]
+name = "unicode-segmentation"
+version = "1.8.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "8895849a949e7845e06bd6dc1aa51731a103c42707010a5b591c0038fb73385b"
+
+[[package]]
 name = "unicode-width"
 version = "0.1.8"
 source = "registry+https://github.com/rust-lang/crates.io-index"
diff --git a/alacritty/Cargo.toml b/alacritty/Cargo.toml
index 6984728a..b1fee0c2 100644
--- a/alacritty/Cargo.toml
+++ b/alacritty/Cargo.toml
@@ -18,7 +18,7 @@ path = "../alacritty_config_derive"
 version = "0.1.0"
 
 [dependencies]
-clap = "2"
+structopt = "0.3.22"
 log = { version = "0.4", features = ["std", "serde"] }
 time = "0.1.40"
 fnv = "1"
diff --git a/alacritty/build.rs b/alacritty/build.rs
index c093afca..e81da150 100644
--- a/alacritty/build.rs
+++ b/alacritty/build.rs
@@ -6,7 +6,11 @@ use std::process::Command;
 use gl_generator::{Api, Fallbacks, GlobalGenerator, Profile, Registry};
 
 fn main() {
-    println!("cargo:rustc-env=GIT_HASH={}", commit_hash());
+    let mut version = String::from(env!("CARGO_PKG_VERSION"));
+    if let Some(commit_hash) = commit_hash() {
+        version = format!("{} ({})", version, commit_hash);
+    }
+    println!("cargo:rustc-env=VERSION={}", version);
 
     let dest = env::var("OUT_DIR").unwrap();
     let mut file = File::create(&Path::new(&dest).join("gl_bindings.rs")).unwrap();
@@ -19,11 +23,11 @@ fn main() {
     embed_resource::compile("./windows/windows.rc");
 }
 
-fn commit_hash() -> String {
+fn commit_hash() -> Option<String> {
     Command::new("git")
         .args(&["rev-parse", "--short", "HEAD"])
         .output()
         .ok()
         .and_then(|output| String::from_utf8(output.stdout).ok())
-        .unwrap_or_default()
+        .map(|hash| hash.trim().into())
 }
diff --git a/alacritty/src/cli.rs b/alacritty/src/cli.rs
index 682bdde3..a1480807 100644
--- a/alacritty/src/cli.rs
+++ b/alacritty/src/cli.rs
@@ -1,219 +1,101 @@
 use std::cmp::max;
 use std::path::PathBuf;
 
-use clap::{crate_authors, crate_description, crate_name, crate_version, App, Arg};
 use log::{self, error, LevelFilter};
 use serde_yaml::Value;
+use structopt::StructOpt;
 
 use alacritty_terminal::config::Program;
 
 use crate::config::serde_utils;
-use crate::config::window::DEFAULT_NAME;
+use crate::config::window::{Class, DEFAULT_NAME};
 use crate::config::Config;
 
-#[cfg(not(any(target_os = "macos", windows)))]
-const CONFIG_PATH: &str = "$XDG_CONFIG_HOME/alacritty/alacritty.yml";
-#[cfg(windows)]
-const CONFIG_PATH: &str = "%APPDATA%\\alacritty\\alacritty.yml";
-#[cfg(target_os = "macos")]
-const CONFIG_PATH: &str = "$HOME/.config/alacritty/alacritty.yml";
-
 /// Options specified on the command line.
+#[derive(StructOpt, Debug)]
+#[structopt(author, about, version = env!("VERSION"))]
 pub struct Options {
+    /// Print all events to stdout.
+    #[structopt(long)]
     pub print_events: bool,
+
+    /// Generates ref test.
+    #[structopt(long)]
     pub ref_test: bool,
+
+    /// Defines the window title [default: Alacritty].
+    #[structopt(short, long)]
     pub title: Option<String>,
-    pub class_instance: Option<String>,
-    pub class_general: Option<String>,
-    pub embed: Option<String>,
-    pub log_level: LevelFilter,
-    pub command: Option<Program>,
-    pub hold: bool,
-    pub working_directory: Option<PathBuf>,
-    pub config_path: Option<PathBuf>,
-    pub config_options: Value,
-}
 
-impl Default for Options {
-    fn default() -> Options {
-        Options {
-            print_events: false,
-            ref_test: false,
-            title: None,
-            class_instance: None,
-            class_general: None,
-            embed: None,
-            log_level: LevelFilter::Warn,
-            command: None,
-            hold: false,
-            working_directory: None,
-            config_path: None,
-            config_options: Value::Null,
-        }
-    }
-}
+    /// Defines window class/app_id on X11/Wayland [default: Alacritty].
+    #[structopt(long, value_name = "instance> | <instance>,<general", parse(try_from_str = parse_class))]
+    pub class: Option<Class>,
 
-impl Options {
-    /// Build `Options` from command line arguments.
-    pub fn new() -> Self {
-        let mut version = crate_version!().to_owned();
-        let commit_hash = env!("GIT_HASH");
-        if !commit_hash.is_empty() {
-            version = format!("{} ({})", version, commit_hash);
-        }
+    /// Defines the X11 window ID (as a decimal integer) to embed Alacritty within.
+    #[structopt(long)]
+    pub embed: Option<String>,
 
-        let mut options = Options::default();
-
-        let matches = App::new(crate_name!())
-            .version(version.as_str())
-            .author(crate_authors!("\n"))
-            .about(crate_description!())
-            .arg(Arg::with_name("ref-test").long("ref-test").help("Generates ref test"))
-            .arg(
-                Arg::with_name("print-events")
-                    .long("print-events")
-                    .help("Print all events to stdout"),
-            )
-            .arg(
-                Arg::with_name("title")
-                    .long("title")
-                    .short("t")
-                    .takes_value(true)
-                    .help(&format!("Defines the window title [default: {}]", DEFAULT_NAME)),
-            )
-            .arg(
-                Arg::with_name("class")
-                    .long("class")
-                    .value_name("instance> | <instance>,<general")
-                    .takes_value(true)
-                    .use_delimiter(true)
-                    .help(&format!(
-                        "Defines window class/app_id on X11/Wayland [default: {}]",
-                        DEFAULT_NAME
-                    )),
-            )
-            .arg(
-                Arg::with_name("embed").long("embed").takes_value(true).help(
-                    "Defines the X11 window ID (as a decimal integer) to embed Alacritty within",
-                ),
-            )
-            .arg(
-                Arg::with_name("q")
-                    .short("q")
-                    .multiple(true)
-                    .conflicts_with("v")
-                    .help("Reduces the level of verbosity (the min level is -qq)"),
-            )
-            .arg(
-                Arg::with_name("v")
-                    .short("v")
-                    .multiple(true)
-                    .conflicts_with("q")
-                    .help("Increases the level of verbosity (the max level is -vvv)"),
-            )
-            .arg(
-                Arg::with_name("working-directory")
-                    .long("working-directory")
-                    .takes_value(true)
-                    .help("Start the shell in the specified working directory"),
-            )
-            .arg(Arg::with_name("config-file").long("config-file").takes_value(true).help(
-                &format!("Specify alternative configuration file [default: {}]", CONFIG_PATH),
-            ))
-            .arg(
-                Arg::with_name("command")
-                    .long("command")
-                    .short("e")
-                    .multiple(true)
-                    .takes_value(true)
-                    .allow_hyphen_values(true)
-                    .help("Command and args to execute (must be last argument)"),
-            )
-            .arg(Arg::with_name("hold").long("hold").help("Remain open after child process exits"))
-            .arg(
-                Arg::with_name("option")
-                    .long("option")
-                    .short("o")
-                    .multiple(true)
-                    .takes_value(true)
-                    .help("Override configuration file options [example: cursor.style=Beam]"),
-            )
-            .get_matches();
-
-        if matches.is_present("ref-test") {
-            options.ref_test = true;
-        }
+    /// Start the shell in the specified working directory.
+    #[structopt(long)]
+    pub working_directory: Option<PathBuf>,
 
-        if matches.is_present("print-events") {
-            options.print_events = true;
-        }
+    /// Specify alternative configuration file [default: $XDG_CONFIG_HOME/alacritty/alacritty.yml].
+    #[cfg(not(any(target_os = "macos", windows)))]
+    #[structopt(long)]
+    pub config_file: Option<PathBuf>,
 
-        if let Some(mut class) = matches.values_of("class") {
-            options.class_instance = class.next().map(|instance| instance.to_owned());
-            options.class_general = class.next().map(|general| general.to_owned());
-        }
+    /// Specify alternative configuration file [default: %APPDATA%\alacritty\alacritty.yml].
+    #[cfg(windows)]
+    #[structopt(long)]
+    pub config_file: Option<PathBuf>,
 
-        options.title = matches.value_of("title").map(ToOwned::to_owned);
-        options.embed = matches.value_of("embed").map(ToOwned::to_owned);
+    /// Specify alternative configuration file [default: $HOME/.config/alacritty/alacritty.yml].
+    #[cfg(target_os = "macos")]
+    #[structopt(long)]
+    pub config_file: Option<PathBuf>,
 
-        match matches.occurrences_of("q") {
-            0 => (),
-            1 => options.log_level = LevelFilter::Error,
-            _ => options.log_level = LevelFilter::Off,
-        }
+    /// Remain open after child process exits.
+    #[structopt(long)]
+    pub hold: bool,
 
-        match matches.occurrences_of("v") {
-            0 if !options.print_events => options.log_level = LevelFilter::Warn,
-            0 | 1 => options.log_level = LevelFilter::Info,
-            2 => options.log_level = LevelFilter::Debug,
-            _ => options.log_level = LevelFilter::Trace,
-        }
+    /// CLI options for config overrides.
+    #[structopt(skip)]
+    pub config_options: Value,
 
-        if let Some(dir) = matches.value_of("working-directory") {
-            options.working_directory = Some(PathBuf::from(dir.to_string()));
-        }
+    /// Reduces the level of verbosity (the min level is -qq).
+    #[structopt(short, conflicts_with("verbose"), parse(from_occurrences))]
+    quiet: u8,
 
-        if let Some(path) = matches.value_of("config-file") {
-            options.config_path = Some(PathBuf::from(path.to_string()));
-        }
+    /// Increases the level of verbosity (the max level is -vvv).
+    #[structopt(short, conflicts_with("quiet"), parse(from_occurrences))]
+    verbose: u8,
 
-        if let Some(mut args) = matches.values_of("command") {
-            // The following unwrap is guaranteed to succeed.
-            // If `command` exists it must also have a first item since
-            // `Arg::min_values(1)` is set.
-            let program = String::from(args.next().unwrap());
-            let args = args.map(String::from).collect();
-            options.command = Some(Program::WithArgs { program, args });
-        }
+    /// Command and args to execute (must be last argument).
+    #[structopt(short = "e", long, allow_hyphen_values = true)]
+    command: Vec<String>,
 
-        if matches.is_present("hold") {
-            options.hold = true;
-        }
+    /// Override configuration file options [example: cursor.style=Beam].
+    #[structopt(short = "o", long)]
+    option: Vec<String>,
+}
 
-        if let Some(config_options) = matches.values_of("option") {
-            for option in config_options {
-                match option_as_value(option) {
-                    Ok(value) => {
-                        options.config_options = serde_utils::merge(options.config_options, value);
-                    },
-                    Err(_) => eprintln!("Invalid CLI config option: {:?}", option),
-                }
+impl Options {
+    pub fn new() -> Self {
+        let mut options = Self::from_args();
+
+        // Convert `--option` flags into serde `Value`.
+        for option in &options.option {
+            match option_as_value(option) {
+                Ok(value) => {
+                    options.config_options = serde_utils::merge(options.config_options, value);
+                },
+                Err(_) => eprintln!("Invalid CLI config option: {:?}", option),
             }
         }
 
         options
     }
 
-    /// Configuration file path.
-    pub fn config_path(&self) -> Option<PathBuf> {
-        self.config_path.clone()
-    }
-
-    /// CLI config options as deserializable serde value.
-    pub fn config_options(&self) -> &Value {
-        &self.config_options
-    }
-
     /// Override configuration file with options from the CLI.
     pub fn override_config(&self, config: &mut Config) {
         if let Some(working_directory) = &self.working_directory {
@@ -224,8 +106,8 @@ impl Options {
             }
         }
 
-        if let Some(command) = &self.command {
-            config.shell = Some(command.clone());
+        if let Some(command) = self.command() {
+            config.shell = Some(command);
         }
 
         config.hold = self.hold;
@@ -233,17 +115,14 @@ impl Options {
         if let Some(title) = self.title.clone() {
             config.ui_config.window.title = title
         }
-        if let Some(class_instance) = self.class_instance.clone() {
-            config.ui_config.window.class.instance = class_instance;
-        }
-        if let Some(class_general) = self.class_general.clone() {
-            config.ui_config.window.class.general = class_general;
+        if let Some(class) = &self.class {
+            config.ui_config.window.class = class.clone();
         }
 
         config.ui_config.window.dynamic_title &= self.title.is_none();
         config.ui_config.window.embed = self.embed.as_ref().and_then(|embed| embed.parse().ok());
         config.ui_config.debug.print_events |= self.print_events;
-        config.ui_config.debug.log_level = max(config.ui_config.debug.log_level, self.log_level);
+        config.ui_config.debug.log_level = max(config.ui_config.debug.log_level, self.log_level());
         config.ui_config.debug.ref_test |= self.ref_test;
 
         if config.ui_config.debug.print_events {
@@ -251,6 +130,32 @@ impl Options {
                 max(config.ui_config.debug.log_level, LevelFilter::Info);
         }
     }
+
+    /// Logging filter level.
+    pub fn log_level(&self) -> LevelFilter {
+        match (self.quiet, self.verbose) {
+            // Force at least `Info` level for `--print-events`.
+            (_, 0) if self.print_events => LevelFilter::Info,
+
+            // Default.
+            (0, 0) => LevelFilter::Warn,
+
+            // Verbose.
+            (_, 1) => LevelFilter::Info,
+            (_, 2) => LevelFilter::Debug,
+            (0, _) => LevelFilter::Trace,
+
+            // Quiet.
+            (1, _) => LevelFilter::Error,
+            (..) => LevelFilter::Off,
+        }
+    }
+
+    /// Shell override passed through the CLI.
+    pub fn command(&self) -> Option<Program> {
+        let (program, args) = self.command.split_first()?;
+        Some(Program::WithArgs { program: program.clone(), args: args.to_vec() })
+    }
 }
 
 /// Format an option in the format of `parent.field=value` to a serde Value.
@@ -278,6 +183,23 @@ fn option_as_value(option: &str) -> Result<Value, serde_yaml::Error> {
     serde_yaml::from_str(&yaml_text)
 }
 
+/// Parse the class CLI parameter.
+fn parse_class(input: &str) -> Result<Class, String> {
+    match input.find(',') {
+        Some(position) => {
+            let general = input[position + 1..].to_owned();
+
+            // Warn the user if they've passed too many values.
+            if general.contains(',') {
+                return Err(String::from("Too many parameters"));
+            }
+
+            Ok(Class { instance: input[..position].into(), general })
+        },
+        None => Ok(Class { instance: input.into(), general: DEFAULT_NAME.into() }),
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -289,7 +211,7 @@ mod tests {
         let mut config = Config::default();
         let old_dynamic_title = config.ui_config.window.dynamic_title;
 
-        Options::default().override_config(&mut config);
+        Options::new().override_config(&mut config);
 
         assert_eq!(old_dynamic_title, config.ui_config.window.dynamic_title);
     }
@@ -298,7 +220,7 @@ mod tests {
     fn dynamic_title_overridden_by_options() {
         let mut config = Config::default();
 
-        let options = Options { title: Some("foo".to_owned()), ..Options::default() };
+        let options = Options { title: Some("foo".to_owned()), ..Options::new() };
         options.override_config(&mut config);
 
         assert!(!config.ui_config.window.dynamic_title);
@@ -309,7 +231,7 @@ mod tests {
         let mut config = Config::default();
 
         config.ui_config.window.title = "foo".to_owned();
-        Options::default().override_config(&mut config);
+        Options::new().override_config(&mut config);
 
         assert!(config.ui_config.window.dynamic_title);
     }
@@ -350,4 +272,24 @@ mod tests {
 
         assert_eq!(value, Value::Mapping(expected));
     }
+
+    #[test]
+    fn parse_instance_class() {
+        let class = parse_class("one").unwrap();
+        assert_eq!(class.instance, "one");
+        assert_eq!(class.general, DEFAULT_NAME);
+    }
+
+    #[test]
+    fn parse_general_class() {
+        let class = parse_class("one,two").unwrap();
+        assert_eq!(class.instance, "one");
+        assert_eq!(class.general, "two");
+    }
+
+    #[test]
+    fn parse_invalid_class() {
+        let class = parse_class("one,two,three");
+        assert!(class.is_err());
+    }
 }
diff --git a/alacritty/src/config/mod.rs b/alacritty/src/config/mod.rs
index 43cade6d..4a3c0ae9 100644
--- a/alacritty/src/config/mod.rs
+++ b/alacritty/src/config/mod.rs
@@ -101,8 +101,8 @@ impl From<serde_yaml::Error> for Error {
 
 /// Load the configuration file.
 pub fn load(options: &Options) -> Config {
-    let config_options = options.config_options().clone();
-    let config_path = options.config_path().or_else(installed_config);
+    let config_options = options.config_options.clone();
+    let config_path = options.config_file.clone().or_else(installed_config);
 
     // Load the config using the following fallback behavior:
     //  - Config path + CLI overrides
@@ -128,7 +128,7 @@ pub fn load(options: &Options) -> Config {
 /// Attempt to reload the configuration file.
 pub fn reload(config_path: &Path, options: &Options) -> Result<Config> {
     // Load config, propagating errors.
-    let config_options = options.config_options().clone();
+    let config_options = options.config_options.clone();
     let mut config = load_from(config_path, config_options)?;
 
     after_loading(&mut config, options);
diff --git a/alacritty/src/logging.rs b/alacritty/src/logging.rs
index 545905a2..7cef3887 100644
--- a/alacritty/src/logging.rs
+++ b/alacritty/src/logging.rs
@@ -29,7 +29,7 @@ pub fn initialize(
     options: &Options,
     event_proxy: EventLoopProxy<Event>,
 ) -> Result<Option<PathBuf>, log::SetLoggerError> {
-    log::set_max_level(options.log_level);
+    log::set_max_level(options.log_level());
 
     let logger = Logger::new(event_proxy);
     let path = logger.file_path();