bug 1526938: geckodriver: display help message on non-clap errors; r=whimboo
authorAndreas Tolfsen <ato@sny.no>
Mon, 18 Feb 2019 12:48:49 +0000
changeset 520654 16aac9625fdee7452d9cfdcf06115512db3d9410
parent 520653 e37346a0237371a6d707c27d0c7c3e4bf3dbdff1
child 520655 b2768473a2805ecf5b58aa0c341d1085d65bad96
push id2032
push userffxbld-merge
push dateMon, 13 May 2019 09:36:57 +0000
treeherdermozilla-release@455c1065dcbe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerswhimboo
bugs1526938
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
bug 1526938: geckodriver: display help message on non-clap errors; r=whimboo The help message is implicitly included in clap error descriptions, but they are not for errors originating from this file. By introducing a FatalError::help_included() function we make sure we print the help message in all cases. An alternative implementation, which perhaps would be more idiomatic, would be to prepare the help message within the fmt::Display trait implementation for FatalError, but I could find no way of passing in a reference to clap::App without storing it in an atomic global const. Depends on D19430 Differential Revision: https://phabricator.services.mozilla.com/D19431
testing/geckodriver/src/main.rs
--- a/testing/geckodriver/src/main.rs
+++ b/testing/geckodriver/src/main.rs
@@ -17,17 +17,17 @@ extern crate uuid;
 extern crate webdriver;
 extern crate zip;
 
 #[macro_use]
 extern crate log;
 
 use std::env;
 use std::fmt;
-use std::io::{self, Write};
+use std::io;
 use std::net::{IpAddr, SocketAddr};
 use std::path::PathBuf;
 use std::result;
 use std::str::FromStr;
 
 use clap::{App, Arg};
 
 macro_rules! try_opt {
@@ -66,16 +66,23 @@ enum FatalError {
 impl FatalError {
     fn exit_code(&self) -> i32 {
         use FatalError::*;
         match *self {
             Parsing(_) | Usage(_) => EXIT_USAGE,
             Server(_) => EXIT_UNAVAILABLE,
         }
     }
+
+    fn help_included(&self) -> bool {
+        match *self {
+            FatalError::Parsing(_) => true,
+            _ => false,
+        }
+    }
 }
 
 impl From<clap::Error> for FatalError {
     fn from(err: clap::Error) -> FatalError {
         FatalError::Parsing(err)
     }
 }
 
@@ -105,17 +112,94 @@ macro_rules! usage {
 
     ($fmt:expr, $($arg:tt)+) => {
         return Err(FatalError::Usage(format!($fmt, $($arg)+)));
     };
 }
 
 type ProgramResult<T> = result::Result<T, FatalError>;
 
-fn app<'a, 'b>() -> App<'a, 'b> {
+fn run(app: &mut App) -> ProgramResult<()> {
+    let matches = app.get_matches_from_safe_borrow(env::args())?;
+
+    if matches.is_present("version") {
+        print_version();
+        return Ok(());
+    }
+
+    let host = matches.value_of("webdriver_host").unwrap();
+    let port = match u16::from_str(matches.value_of("webdriver_port").unwrap()) {
+        Ok(x) => x,
+        Err(_) => usage!("invalid WebDriver port"),
+    };
+    let addr = match IpAddr::from_str(host) {
+        Ok(addr) => SocketAddr::new(addr, port),
+        Err(_) => usage!("invalid host address"),
+    };
+
+    let binary = matches.value_of("binary").map(PathBuf::from);
+
+    let marionette_host = matches.value_of("marionette_host").unwrap().to_string();
+    let marionette_port = match matches.value_of("marionette_port") {
+        Some(x) => match u16::from_str(x) {
+            Ok(x) => Some(x),
+            Err(_) => usage!("invalid Marionette port"),
+        },
+        None => None,
+    };
+
+    let log_level = if matches.is_present("log_level") {
+        Level::from_str(matches.value_of("log_level").unwrap()).ok()
+    } else {
+        Some(match matches.occurrences_of("verbosity") {
+            0 => Level::Info,
+            1 => Level::Debug,
+            _ => Level::Trace,
+        })
+    };
+    if let Some(ref level) = log_level {
+        logging::init_with_level(*level).unwrap();
+    } else {
+        logging::init().unwrap();
+    }
+
+    let settings = MarionetteSettings {
+        host: marionette_host,
+        port: marionette_port,
+        binary,
+        connect_existing: matches.is_present("connect_existing"),
+        jsdebugger: matches.is_present("jsdebugger"),
+    };
+    let handler = MarionetteHandler::new(settings);
+    let listening = webdriver::server::start(addr, handler, &extension_routes()[..])?;
+    debug!("Listening on {}", listening.socket);
+
+    Ok(())
+}
+
+fn main() {
+    use std::process::exit;
+
+    let mut app = make_app();
+
+    exit(match run(&mut app) {
+        Ok(_) => EXIT_SUCCESS,
+
+        Err(e) => {
+            eprintln!("{}: {}", get_program_name(), e);
+            if !e.help_included() {
+                print_help(&mut app);
+            }
+
+            e.exit_code()
+        }
+    });
+}
+
+fn make_app<'a, 'b>() -> App<'a, 'b> {
     App::new(format!("geckodriver {}", build::BuildInfo))
         .about("WebDriver implementation for Firefox")
         .arg(
             Arg::with_name("webdriver_host")
                 .long("host")
                 .takes_value(true)
                 .value_name("HOST")
                 .default_value("127.0.0.1")
@@ -182,90 +266,23 @@ fn app<'a, 'b>() -> App<'a, 'b> {
         .arg(
             Arg::with_name("version")
                 .short("V")
                 .long("version")
                 .help("Prints version and copying information"),
         )
 }
 
-fn run() -> ProgramResult<()> {
-    let matches = app().get_matches();
-
-    if matches.is_present("version") {
-        print_version();
-        return Ok(());
-    }
-
-    let host = matches.value_of("webdriver_host").unwrap();
-    let port = match u16::from_str(matches.value_of("webdriver_port").unwrap()) {
-        Ok(x) => x,
-        Err(_) => usage!("invalid WebDriver port"),
-    };
-    let addr = match IpAddr::from_str(host) {
-        Ok(addr) => SocketAddr::new(addr, port),
-        Err(_) => usage!("invalid host address"),
-    };
-
-    let binary = matches.value_of("binary").map(PathBuf::from);
-
-    let marionette_host = matches.value_of("marionette_host").unwrap().to_string();
-    let marionette_port = match matches.value_of("marionette_port") {
-        Some(x) => match u16::from_str(x) {
-            Ok(x) => Some(x),
-            Err(_) => usage!("invalid Marionette port"),
-        },
-        None => None,
-    };
-
-    let log_level = if matches.is_present("log_level") {
-        Level::from_str(matches.value_of("log_level").unwrap()).ok()
-    } else {
-        Some(match matches.occurrences_of("verbosity") {
-            0 => Level::Info,
-            1 => Level::Debug,
-            _ => Level::Trace,
-        })
-    };
-    if let Some(ref level) = log_level {
-        logging::init_with_level(*level).unwrap();
-    } else {
-        logging::init().unwrap();
-    }
-
-    let settings = MarionetteSettings {
-        host: marionette_host,
-        port: marionette_port,
-        binary,
-        connect_existing: matches.is_present("connect_existing"),
-        jsdebugger: matches.is_present("jsdebugger"),
-    };
-    let handler = MarionetteHandler::new(settings);
-    let listening = webdriver::server::start(addr, handler, &extension_routes()[..])?;
-    debug!("Listening on {}", listening.socket);
-
-    Ok(())
+fn get_program_name() -> String {
+    env::args().next().unwrap()
 }
 
-fn main() {
-    let exit_code = match run() {
-        Ok(_) => EXIT_SUCCESS,
-
-        Err(e) => {
-            eprintln!("{}: {}", get_program_name(), e);
-            e.exit_code()
-        }
-    };
-
-    std::io::stdout().flush().unwrap();
-    std::process::exit(exit_code);
-}
-
-fn get_program_name() -> String {
-    env::args().next().unwrap()
+fn print_help(app: &mut App) {
+    app.print_help().ok();
+    println!();
 }
 
 fn print_version() {
     println!("geckodriver {}", build::BuildInfo);
     println!("");
     println!("The source code of this program is available from");
     println!("testing/geckodriver in https://hg.mozilla.org/mozilla-central.");
     println!("");