From e9e6f59c8b4df9337350df671b4db36726ab127f Mon Sep 17 00:00:00 2001 From: Narfinger Date: Wed, 28 May 2025 10:59:18 +0200 Subject: [PATCH 1/4] Servoshell: Replace getopts with bpaf for parsing arguments. This includes some small refactoring and some small breaking changes as listed below. Other than these I tried to keep the functionality exactly the same but because in the old code the parsing and settings of preferences was intermingled it was difficult to figure out. Small Breaking: - Size and resources-path were unused but appeared in the help. - soft-fail and hard-fail: Soft-fail flag got removed because it is too difficult to keep both. The default is now soft-fail and hard-fail can be enabled. Signed-off-by: Narfinger --- Cargo.lock | 31 +- Cargo.toml | 1 - ports/servoshell/Cargo.toml | 2 +- ports/servoshell/desktop/cli.rs | 3 + ports/servoshell/egl/android/simpleservo.rs | 3 + ports/servoshell/egl/ohos/simpleservo.rs | 3 + ports/servoshell/prefs.rs | 776 ++++++++------------ 7 files changed, 352 insertions(+), 467 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cfc9a0a0abb..bdae3a0b57e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -667,6 +667,26 @@ dependencies = [ "hex", ] +[[package]] +name = "bpaf" +version = "0.9.20" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "473976d7a8620bb1e06dcdd184407c2363fe4fec8e983ee03ed9197222634a31" +dependencies = [ + "bpaf_derive", +] + +[[package]] +name = "bpaf_derive" +version = "0.5.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fefb4feeec9a091705938922f26081aad77c64cd2e76cd1c4a9ece8e42e1618a" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "brotli" version = "8.0.1" @@ -2497,15 +2517,6 @@ dependencies = [ "windows-targets 0.48.5", ] -[[package]] -name = "getopts" -version = "0.2.21" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "14dbbfd5c71d70241ecf9e6f13737f7b5ce823821063188d7e46c41d371eebd5" -dependencies = [ - "unicode-width", -] - [[package]] name = "getrandom" version = "0.2.16" @@ -7014,6 +7025,7 @@ version = "0.0.1" dependencies = [ "android_logger", "backtrace", + "bpaf", "cc", "cfg-if", "dirs", @@ -7024,7 +7036,6 @@ dependencies = [ "egui_glow", "env_filter", "euclid", - "getopts", "gilrs", "glow", "headers 0.4.0", diff --git a/Cargo.toml b/Cargo.toml index c290a21f1fc..9ed5e1d90c3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -58,7 +58,6 @@ fnv = "1.0" fonts_traits = { path = "components/shared/fonts" } freetype-sys = "0.20" fxhash = "0.2" -getopts = "0.2.11" gleam = "0.15" glib = "0.19" glow = "0.16" diff --git a/ports/servoshell/Cargo.toml b/ports/servoshell/Cargo.toml index edeaef5408f..4e770875383 100644 --- a/ports/servoshell/Cargo.toml +++ b/ports/servoshell/Cargo.toml @@ -54,10 +54,10 @@ webgpu = ["libservo/webgpu"] webxr = ["libservo/webxr"] [dependencies] +bpaf = { version = "0.9.20", features = ["derive"] } cfg-if = { workspace = true } dpi = { workspace = true } euclid = { workspace = true } -getopts = { workspace = true } hitrace = { workspace = true, optional = true } image = { workspace = true } keyboard-types = { workspace = true } diff --git a/ports/servoshell/desktop/cli.rs b/ports/servoshell/desktop/cli.rs index 88de5086504..f8f0b079a97 100644 --- a/ports/servoshell/desktop/cli.rs +++ b/ports/servoshell/desktop/cli.rs @@ -24,6 +24,9 @@ pub fn main() { ArgumentParsingResult::ChromeProcess(opts, preferences, servoshell_preferences) => { (opts, preferences, servoshell_preferences) }, + ArgumentParsingResult::Exit => { + std::process::exit(0); + }, }; crate::init_tracing(servoshell_preferences.tracing_filter.as_deref()); diff --git a/ports/servoshell/egl/android/simpleservo.rs b/ports/servoshell/egl/android/simpleservo.rs index 4f7e35d3923..9adc43ad5f4 100644 --- a/ports/servoshell/egl/android/simpleservo.rs +++ b/ports/servoshell/egl/android/simpleservo.rs @@ -55,6 +55,9 @@ pub fn init( ArgumentParsingResult::ChromeProcess(opts, preferences, servoshell_preferences) => { (opts, preferences, servoshell_preferences) }, + ArgumentParsingResult::Exit => { + std::process::exit(0); + }, }; crate::init_tracing(servoshell_preferences.tracing_filter.as_deref()); diff --git a/ports/servoshell/egl/ohos/simpleservo.rs b/ports/servoshell/egl/ohos/simpleservo.rs index c867c7a5330..55da9ba1932 100644 --- a/ports/servoshell/egl/ohos/simpleservo.rs +++ b/ports/servoshell/egl/ohos/simpleservo.rs @@ -88,6 +88,9 @@ pub fn init( ArgumentParsingResult::ChromeProcess(opts, preferences, servoshell_preferences) => { (opts, preferences, servoshell_preferences) }, + ArgumentParsingResult::Exit => { + std::process::exit(0); + }, }; crate::init_tracing(servoshell_preferences.tracing_filter.as_deref()); diff --git a/ports/servoshell/prefs.rs b/ports/servoshell/prefs.rs index a602939cb65..cf9d26e837d 100644 --- a/ports/servoshell/prefs.rs +++ b/ports/servoshell/prefs.rs @@ -2,17 +2,18 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ +use core::panic; use std::collections::HashMap; -use std::fs::{File, read_to_string}; +use std::fs::{self, File, read_to_string}; use std::io::Read; use std::path::{Path, PathBuf}; #[cfg(any(target_os = "android", target_env = "ohos"))] use std::sync::OnceLock; -use std::{env, fs, process}; +use std::{env, process}; +use bpaf::*; use euclid::Size2D; -use getopts::{Matches, Options}; -use log::{error, warn}; +use log::warn; use serde_json::Value; use servo::config::opts::{DebugOptions, Opts, OutputOptions}; use servo::config::prefs::{PrefValue, Preferences}; @@ -127,7 +128,7 @@ pub fn default_config_dir() -> Option { /// Get a Servo [`Preferences`] to use when initializing Servo by first reading the user /// preferences file and then overriding these preferences with the ones from the `--prefs-file` /// command-line argument, if given. -fn get_preferences(opts_matches: &Matches, config_dir: &Option) -> Preferences { +fn get_preferences(prefs_files: &[String], config_dir: &Option) -> Preferences { // Do not read any preferences files from the disk when testing as we do not want it // to throw off test results. if cfg!(test) { @@ -149,7 +150,7 @@ fn get_preferences(opts_matches: &Matches, config_dir: &Option) -> Pref let mut preferences = Preferences::default(); apply_preferences(&mut preferences, user_prefs_hash); - for pref_file_path in opts_matches.opt_strs("prefs-file").iter() { + for pref_file_path in prefs_files.iter() { apply_preferences(&mut preferences, read_prefs_file(pref_file_path)) } @@ -181,393 +182,228 @@ pub fn read_prefs_map(txt: &str) -> HashMap { pub(crate) enum ArgumentParsingResult { ChromeProcess(Opts, Preferences, ServoShellPreferences), ContentProcess(String), + Exit, } -pub(crate) fn parse_command_line_arguments(args: Vec) -> ArgumentParsingResult { - let (app_name, args) = args.split_first().unwrap(); - - let mut opts = Options::new(); - opts.optopt( - "o", - "output", - "Path to an output image. The format of the image is determined by the extension. \ - Supports all formats that `rust-image` does.", - "output.png", - ); - opts.optopt("s", "size", "Size of tiles", "512"); - opts.optflagopt( - "p", - "profile", - "Time profiler flag and either a TSV output filename \ - OR an interval for output to Stdout (blank for Stdout with interval of 5s)", - "10 \ - OR time.tsv", - ); - opts.optflagopt( - "", - "profiler-trace-path", - "Path to dump a self-contained HTML timeline of profiler traces", - "", - ); - opts.optflag( - "x", - "exit", - "Exit after Servo has loaded the page and detected a stable output image", - ); - opts.optopt( - "y", - "layout-threads", - "Number of threads to use for layout", - "1", - ); - opts.optflag( - "i", - "nonincremental-layout", - "Enable to turn off incremental layout.", - ); - opts.optflagopt( - "", - "userscripts", - "Uses userscripts in resources/user-agent-js, or a specified full path", - "", - ); - opts.optmulti( - "", - "user-stylesheet", - "A user stylesheet to be added to every document", - "file.css", - ); - opts.optopt( - "", - "shaders", - "Shaders will be loaded from the specified directory instead of using the builtin ones.", - "", - ); - opts.optflag("z", "headless", "Headless mode"); - opts.optflag( - "f", - "hard-fail", - "Exit on thread failure instead of displaying about:failure", - ); - opts.optflag( - "F", - "soft-fail", - "Display about:failure on thread failure instead of exiting", - ); - opts.optflagopt("", "devtools", "Start remote devtools server on port", "0"); - opts.optflagopt( - "", - "webdriver", - "Start remote WebDriver server on port", - "7000", - ); - opts.optopt( - "", - "window-size", - "Set the initial window size in logical (device independenrt) pixels", - "1024x740", - ); - opts.optopt( - "", - "screen-size", - "Override the screen resolution in logical (device independent) pixels", - "1024x768", - ); - opts.optflag("M", "multiprocess", "Run in multiprocess mode"); - opts.optflag("B", "bhm", "Background Hang Monitor enabled"); - opts.optflag("S", "sandbox", "Run in a sandbox if multiprocess"); - opts.optopt( - "", - "random-pipeline-closure-probability", - "Probability of randomly closing a pipeline (for testing constellation hardening).", - "0.0", - ); - opts.optopt( - "", - "random-pipeline-closure-seed", - "A fixed seed for repeatbility of random pipeline closure.", - "", - ); - opts.optmulti( - "Z", - "debug", - "A comma-separated string of debug options. Pass help to show available options.", - "", - ); - opts.optflag("h", "help", "Print this message"); - opts.optopt( - "", - "resources-path", - "Path to find static resources", - "/home/servo/resources", - ); - opts.optopt( - "", - "certificate-path", - "Path to find SSL certificates", - "/home/servo/resources/certs", - ); - opts.optflag( - "", - "ignore-certificate-errors", - "Whether or not to completely ignore certificate errors", - ); - opts.optopt( - "", - "content-process", - "Run as a content process and connect to the given pipe", - "servo-ipc-channel.abcdefg", - ); - opts.optopt( - "", - "config-dir", - "config directory following xdg spec on linux platform", - "", - ); - opts.optflag("v", "version", "Display servo version information"); - opts.optflag("", "unminify-js", "Unminify Javascript"); - opts.optflag("", "print-pwm", "Print Progressive Web Metrics"); - opts.optopt( - "", - "local-script-source", - "Directory root with unminified scripts", - "", - ); - opts.optflag("", "unminify-css", "Unminify Css"); - - opts.optflag( - "", - "clean-shutdown", - "Do not shutdown until all threads have finished (macos only)", - ); - opts.optflag("b", "no-native-titlebar", "Do not use native titlebar"); - opts.optopt("", "device-pixel-ratio", "Device pixels per px", ""); - opts.optopt( - "u", - "user-agent", - "Set custom user agent string (or ios / android / desktop for platform default)", - "NCSA Mosaic/1.0 (X11;SunOS 4.1.4 sun4m)", - ); - opts.optmulti( - "", - "tracing-filter", - "Define a custom filter for traces. Overrides `SERVO_TRACING` if set.", - "FILTER", - ); - - #[cfg(target_env = "ohos")] - opts.optmulti( - "", - "log-filter", - "Define a custom filter for logging.", - "FILTER", - ); - - opts.optflag( - "", - "enable-experimental-web-platform-features", - "Whether or not to enable experimental web platform features.", - ); - opts.optmulti( - "", - "pref", - "A preference to set to enable", - "dom_bluetooth_enabled", - ); - opts.optmulti( - "", - "pref", - "A preference to set to disable", - "dom_webgpu_enabled=false", - ); - opts.optmulti( - "", - "prefs-file", - "Load in additional prefs from a file.", - "/path/to/prefs.json", - ); - - let opt_match = match opts.parse(args) { - Ok(m) => m, - Err(f) => args_fail(&f.to_string()), - }; - - if opt_match.opt_present("v") || opt_match.opt_present("version") { - println!("{}", crate::servo_version()); - process::exit(0); - } - - if opt_match.opt_present("h") || opt_match.opt_present("help") { - print_usage(app_name, &opts); - process::exit(0); - }; - - let config_dir = opt_match - .opt_str("config-dir") - .map(Into::into) - .or_else(default_config_dir) - .inspect(|path| { - if !path.exists() { - fs::create_dir_all(path).unwrap_or_else(|e| { - error!("Failed to create config directory at {:?}: {:?}", path, e) - }) - } - }); - - let mut preferences = get_preferences(&opt_match, &config_dir); - - // If this is the content process, we'll receive the real options over IPC. So just fill in - // some dummy options for now. - if let Some(content_process) = opt_match.opt_str("content-process") { - return ArgumentParsingResult::ContentProcess(content_process); - } - // Env-Filter directives are comma seperated. - let filters = opt_match.opt_strs("tracing-filter").join(","); - let tracing_filter = (!filters.is_empty()).then_some(filters); - - #[cfg(target_env = "ohos")] - let log_filter = { - let filters = opt_match.opt_strs("log-filter").join(","); - let log_filter = (!filters.is_empty()).then_some(filters).or_else(|| { - (!preferences.log_filter.is_empty()).then_some(preferences.log_filter.clone()) - }); - log::debug!("Set log_filter to: {:?}", log_filter); - log_filter - }; - - let mut debug_options = DebugOptions::default(); - for debug_string in opt_match.opt_strs("Z") { - if let Err(e) = debug_options.extend(debug_string) { - args_fail(&format!("error: unrecognized debug option: {}", e)); - } - } - - if debug_options.help { - print_debug_options_usage(app_name); - } - - // If only the flag is present, default to a 5 second period for both profilers - let time_profiling = if opt_match.opt_present("p") { - match opt_match.opt_str("p") { - Some(argument) => match argument.parse::() { - Ok(interval) => Some(OutputOptions::Stdout(interval)), - Err(_) => match ServoUrl::parse(&argument) { - Ok(_) => panic!("influxDB isn't supported anymore"), - Err(_) => Some(OutputOptions::FileName(argument)), - }, - }, - None => Some(OutputOptions::Stdout(5.0)), - } +/// Parse a resolution string into a Size2D +fn parse_resolution_string( + string: String, +) -> Result>, std::num::ParseIntError> { + if string.is_empty() { + Ok(None) } else { - // if the p option doesn't exist: - None - }; - - if let Some(ref time_profiler_trace_path) = opt_match.opt_str("profiler-trace-path") { - let mut path = PathBuf::from(time_profiler_trace_path); - path.pop(); - if let Err(why) = fs::create_dir_all(&path) { - error!( - "Couldn't create/open {:?}: {:?}", - Path::new(time_profiler_trace_path).to_string_lossy(), - why - ); - } - } - - let layout_threads: Option = opt_match.opt_str("y").map(|layout_threads_str| { - layout_threads_str - .parse() - .unwrap_or_else(|err| args_fail(&format!("Error parsing option: -y ({})", err))) - }); - - let nonincremental_layout = opt_match.opt_present("i"); - - let random_pipeline_closure_probability = opt_match - .opt_str("random-pipeline-closure-probability") - .map(|prob| { - prob.parse().unwrap_or_else(|err| { - args_fail(&format!( - "Error parsing option: --random-pipeline-closure-probability ({})", - err - )) - }) - }); - - let random_pipeline_closure_seed = - opt_match - .opt_str("random-pipeline-closure-seed") - .map(|seed| { - seed.parse().unwrap_or_else(|err| { - args_fail(&format!( - "Error parsing option: --random-pipeline-closure-seed ({})", - err - )) - }) - }); - - if opt_match.opt_present("devtools") { - let port = opt_match - .opt_str("devtools") - .map(|port| { - port.parse().unwrap_or_else(|err| { - args_fail(&format!("Error parsing option: --devtools ({})", err)) - }) - }) - .unwrap_or(preferences.devtools_server_port); - preferences.devtools_server_enabled = true; - preferences.devtools_server_port = port; - } - - let webdriver_port = opt_match.opt_default("webdriver", "7000").map(|port| { - port.parse().unwrap_or_else(|err| { - args_fail(&format!("Error parsing option: --webdriver ({})", err)) - }) - }); - - let parse_resolution_string = |string: String| { - let components: Vec = string + let components = string .split('x') - .map(|component| { - component.parse().unwrap_or_else(|error| { - args_fail(&format!("Error parsing resolution '{string}': {error}")); - }) - }) - .collect(); - Size2D::new(components[0], components[1]) - }; + .map(|component| component.parse::()) + .collect::, std::num::ParseIntError>>()?; + Ok(Some(Size2D::new(components[0], components[1]))) + } +} - let screen_size_override = opt_match - .opt_str("screen-size") - .map(parse_resolution_string); - - // Make sure the default window size is not larger than any provided screen size. - let default_window_size = Size2D::new(1024, 740); - let default_window_size = screen_size_override - .map_or(default_window_size, |screen_size_override| { - default_window_size.min(screen_size_override) - }); - let initial_window_size = opt_match - .opt_str("window-size") - .map_or(default_window_size, parse_resolution_string); - - let user_stylesheets = opt_match - .opt_strs("user-stylesheet") - .iter() +/// Parse stylesheets into the byte stream. +fn parse_user_stylesheets(string: String) -> Result, ServoUrl)>, std::io::Error> { + Ok(string + .split_whitespace() .map(|filename| { let cwd = env::current_dir().unwrap(); let path = cwd.join(filename); let url = ServoUrl::from_url(Url::from_file_path(&path).unwrap()); let mut contents = Vec::new(); File::open(path) - .unwrap_or_else(|err| args_fail(&format!("Couldn't open {}: {}", filename, err))) + .unwrap() .read_to_end(&mut contents) - .unwrap_or_else(|err| args_fail(&format!("Couldn't read {}: {}", filename, err))); + .unwrap(); (contents, url) }) - .collect(); + .collect()) +} - if opt_match.opt_present("enable-experimental-web-platform-features") { +/// parses the profiling string +fn parse_profiling(arg: String) -> Result, std::io::Error> { + if arg.is_empty() { + Ok(Some(OutputOptions::Stdout(5.0))) + } else { + match arg.parse::() { + Ok(interval) => Ok(Some(OutputOptions::Stdout(interval))), + Err(_) => match ServoUrl::parse(&arg) { + Ok(_) => panic!("influxDB isn't supported anymore"), + Err(_) => Ok(Some(OutputOptions::FileName(arg))), + }, + } + } +} + +#[derive(Bpaf, Clone, Debug)] +#[bpaf(options)] +struct CmdArgs { + /// Background Hang Monitor enabled + #[bpaf(long("bhm"), fallback(false))] + background_hang_monitor: bool, + + /// Path to find SSL certificates + #[bpaf(argument("/home/servo/resources/certs"))] + certificate_path: Option, + + /// Do not shutdown until all threads have finished (macos only) + #[bpaf(long, fallback(false))] + clean_shutdown: bool, + + /// config directory following xdg spec on linux platform + config_dir: Option, + + /// Run as a content process and connect to the given pipe + #[bpaf(argument("servo-ipc-channel.abcdefg"))] + content_process: Option, + + /// A comma-separated string of debug options. Pass help to show available options. + #[bpaf(short('Z'), long, fallback(vec![]), help("d"))] + debug: Vec, + + #[bpaf(argument("1.0"))] + device_pixel_ratio_override: Option, + + /// Start remote devtools using server on port + #[bpaf(argument("0"))] + devtools: Option, + + /// Wether or not to enable experimental web platform features. + #[bpaf(long, fallback(false))] + enable_experimental_web_platform_features: bool, + + // Exit after Servo has loaded the page and detected stable output image + #[bpaf(short('x'), long, fallback(false))] + exit: bool, + + /// Exit on thread failure instead of displaying about:failure + #[bpaf(short('f'), long, fallback(false))] + hard_fail: bool, + + /// Headless mode + #[bpaf(short('z'), long, fallback(false))] + headless: bool, + + /// Wether or not to completely ignore certificate errors + #[bpaf(long, fallback(false))] + ignore_certificate_errors: bool, + + /// Number of threads to use for layout + #[bpaf(short('y'), argument("1"))] + layout_threads: Option, + + /// Directory root with unminified scripts + local_script_source: Option, + + #[cfg(target_env = "ohos")] + /// Define a custom filter for logging + #[bpaf(argument("FILTER"))] + log_filter: Option, + + /// Run in multiprocess mode + #[bpaf(short('M'), long, fallback(true))] + multiprocess: bool, + + /// Enable to turn off incremental layout + #[bpaf(short('i'), long, flag(false, true))] + nonincremental_layout: bool, + + /// Path to an output image. The format of the image is determined + /// by the extension. Supports all formats that rust-image does. + #[bpaf(short('o'), long)] + output_image: Option, + + /// Time profiler flag and either a TSV output filename OR and interval + /// for output to Stdout (blank for Stdout with interval of 5s) + #[bpaf(short('p'), argument::("10 OR time.tsv"), parse(parse_profiling), fallback(None))] + profile: Option, + + /// Path to dump a self-contained HTML timeline of profiler traces + #[bpaf(long("profiler-trace-path"))] + time_profiler_trace_path: Option, + + /// A preference to set + #[bpaf(argument("dom_bluetooth_enabled"),fallback(vec![]))] + pref: Vec, + + /// Load an additional prefs from a file. + #[bpaf(long, argument("/path/to/prefs.json"), fallback(vec![]))] + prefs_files: Vec, + + print_pwm: bool, + + /// Probability of randomly closing a pipeline (for testing constellation hardening) + #[bpaf(argument("0.25"))] + random_pipeline_closure_probability: Option, + + /// A fixed seed for repeatability of random pipeline closure. + random_pipeline_closure_seed: Option, + + /// Run in sandbox if multiprocess + #[bpaf(short('S'), long, fallback(true))] + sandbox: bool, + + /// Shaders will be loaded from the specified directory instead of using the builtin ones. + shaders: Option, + + /// Override the screen resolution in logical (device independent) pixels + #[bpaf(short('x'), long("screen-size"), argument::("1024x768"), + parse(parse_resolution_string), fallback(None))] + screen_size_override: Option>, + + /// Define a custom filter for traces. Overridees `SERVO_TRACING` if set. + #[bpaf(long("tracing-filter"), argument("FILTER"))] + tracing_filter: Option, + + #[bpaf(long, fallback(false))] + no_native_titlebar: bool, + + /// Unminify Javascript + #[bpaf(long, fallback(false))] + unminify_js: bool, + + /// Unminify Css + #[bpaf(long, fallback(false))] + unminify_css: bool, + + /// Set custom user agent string (or ios/ android / desktop for platform default)"" + #[bpaf(argument::("NCSA mosaic/1.0 (X11;SunOS 4.1.4 sun4m"))] + user_agent: Option, + + /// Uses userscripts in resources/user-agent-js or a specified full path + #[bpaf(long, fallback(Some(PathBuf::from("resources/user-agent-js"))))] + userscripts_directory: Option, + + /// A user stylesheet ot be added to every document + #[bpaf(fallback(String::from("")), argument::("path.css"), parse(parse_user_stylesheets))] + user_stylesheets: Vec<(Vec, ServoUrl)>, + + /// Prints the version information + #[bpaf(short, long)] + version: bool, + + /// Start remote WebDriver server on port + #[bpaf(argument("7000"))] + webdriver_port: Option, + + /// Set the initial window size in logical (device independent) pixels" + #[bpaf(argument::("1024x740"), parse(parse_resolution_string), fallback(None))] + window_size: Option>, + + /// The url we should load + #[bpaf( + positional("URL"), + fallback(Some(String::from("https://www.servo.org"))) + )] + url: Option, +} + +/// In here we set preferences depending on args +fn set_prefs_from_cfg(preferences: &mut Preferences, cmd_args: &CmdArgs) { + if let Some(port) = cmd_args.devtools { + preferences.devtools_server_enabled = true; + preferences.devtools_server_port = port; + } + + if cmd_args.enable_experimental_web_platform_features { vec![ "dom_async_clipboard_enabled", "dom_fontface_enabled", @@ -591,37 +427,68 @@ pub(crate) fn parse_command_line_arguments(args: Vec) -> ArgumentParsing .for_each(|pref| preferences.set_value(pref, PrefValue::Bool(true))); } - // Handle all command-line preferences overrides. - for pref in opt_match.opt_strs("pref") { + for pref in cmd_args.pref.clone() { let split: Vec<&str> = pref.splitn(2, '=').collect(); let pref_name = split[0]; let pref_value = PrefValue::from_booleanish_str(split.get(1).copied().unwrap_or("true")); preferences.set_value(pref_name, pref_value); } - if let Some(layout_threads) = layout_threads { - preferences.layout_threads = layout_threads as i64; + if let Some(layout_threads) = cmd_args.layout_threads { + preferences.layout_threads = layout_threads; } - let no_native_titlebar = opt_match.opt_present("no-native-titlebar"); - let mut device_pixel_ratio_override = opt_match.opt_str("device-pixel-ratio").map(|dppx_str| { - dppx_str.parse().unwrap_or_else(|err| { - error!("Error parsing option: --device-pixel-ratio ({})", err); - process::exit(1); - }) - }); - - // If an output file is specified the device pixel ratio is always 1. - let output_image_path = opt_match.opt_str("o"); - if output_image_path.is_some() { - device_pixel_ratio_override = Some(1.0); + if cmd_args.headless && preferences.media_glvideo_enabled { + warn!("GL video rendering is not supported on headless windows."); + preferences.media_glvideo_enabled = false; } - let url = if !opt_match.free.is_empty() { - Some(opt_match.free[0][..].into()) - } else { - None - }; + if let Some(user_agent) = cmd_args.user_agent.clone() { + preferences.user_agent = user_agent; + } +} + +pub(crate) fn parse_command_line_arguments(args: Vec) -> ArgumentParsingResult { + let cmd_args = cmd_args().run_inner(&*args); + if let Err(e) = cmd_args { + e.print_message(80); + return ArgumentParsingResult::Exit; + } + + let cmd_args = cmd_args.unwrap(); + if cmd_args.version { + println!("{}", crate::servo_version()); + return ArgumentParsingResult::Exit; + } + + if cmd_args.debug.iter().any(|d| d.contains("help")) { + print_debug_options_usage("servo"); + return ArgumentParsingResult::Exit; + } + + // If this is the content process, we'll receive the real options over IPC. So fill in some dummy options for now + if let Some(content_process) = cmd_args.content_process { + return ArgumentParsingResult::ContentProcess(content_process); + } + + let config_dir = cmd_args + .config_dir + .clone() + .or_else(default_config_dir) + .inspect(|config_dir| { + if !config_dir.exists() { + fs::create_dir_all(config_dir).expect("Could not create config_dir"); + } + }); + if let Some(ref time_profiler_trace_path) = cmd_args.time_profiler_trace_path { + let mut path = PathBuf::from(time_profiler_trace_path); + path.pop(); + fs::create_dir_all(&path).expect("Error in creating profiler trace path"); + } + + let mut preferences = get_preferences(&cmd_args.prefs_files, &config_dir); + + set_prefs_from_cfg(&mut preferences, &cmd_args); // FIXME: enable JIT compilation on 32-bit Android after the startup crash issue (#31134) is fixed. if cfg!(target_os = "android") && cfg!(target_pointer_width = "32") { @@ -630,76 +497,74 @@ pub(crate) fn parse_command_line_arguments(args: Vec) -> ArgumentParsing preferences.js_ion_enabled = false; } - let exit_after_load = opt_match.opt_present("x") || output_image_path.is_some(); - let wait_for_stable_image = exit_after_load; + let device_pixel_ratio_override = if cmd_args.output_image.is_some() { + Some(1.0) + } else { + cmd_args.device_pixel_ratio_override + }; + + let default_window_size = Size2D::new(1024, 740); + let default_window_size = cmd_args + .screen_size_override + .map_or(default_window_size, |screen_size_override| { + default_window_size.min(screen_size_override) + }); + let servoshell_preferences = ServoShellPreferences { - url, - no_native_titlebar, + url: cmd_args.url, + no_native_titlebar: cmd_args.no_native_titlebar, device_pixel_ratio_override, - clean_shutdown: opt_match.opt_present("clean-shutdown"), - headless: opt_match.opt_present("z"), - tracing_filter, - initial_window_size, - screen_size_override, - output_image_path, - exit_after_stable_image: exit_after_load, - userscripts_directory: opt_match - .opt_default("userscripts", "resources/user-agent-js") - .map(PathBuf::from), + clean_shutdown: cmd_args.clean_shutdown, + headless: cmd_args.headless, + tracing_filter: cmd_args.tracing_filter, + initial_window_size: cmd_args.window_size.unwrap_or(default_window_size), + screen_size_override: cmd_args.screen_size_override, + output_image_path: cmd_args.output_image, + exit_after_stable_image: cmd_args.exit, + userscripts_directory: cmd_args.userscripts_directory, #[cfg(target_env = "ohos")] - log_filter, + log_filter: Some( + cmd_args + .log_filter + .unwrap_or(preferences.log_filter.clone()), + ), ..Default::default() }; - if servoshell_preferences.headless && preferences.media_glvideo_enabled { - warn!("GL video rendering is not supported on headless windows."); - preferences.media_glvideo_enabled = false; - } - - if let Some(user_agent) = opt_match.opt_str("user-agent") { - preferences.user_agent = user_agent; + let mut debug_options = DebugOptions::default(); + for i in cmd_args.debug { + debug_options + .extend(i) + .expect("Could not extend with debug option"); } let opts = Opts { - debug: debug_options.clone(), - wait_for_stable_image, - time_profiling, - time_profiler_trace_path: opt_match.opt_str("profiler-trace-path"), - nonincremental_layout, - user_stylesheets, - hard_fail: opt_match.opt_present("f") && !opt_match.opt_present("F"), - webdriver_port, - multiprocess: opt_match.opt_present("M"), - background_hang_monitor: opt_match.opt_present("B"), - sandbox: opt_match.opt_present("S"), - random_pipeline_closure_probability, - random_pipeline_closure_seed, - config_dir, - shaders_dir: opt_match.opt_str("shaders").map(Into::into), - certificate_path: opt_match.opt_str("certificate-path"), - ignore_certificate_errors: opt_match.opt_present("ignore-certificate-errors"), - unminify_js: opt_match.opt_present("unminify-js"), - local_script_source: opt_match.opt_str("local-script-source"), - unminify_css: opt_match.opt_present("unminify-css"), - print_pwm: opt_match.opt_present("print-pwm"), + debug: debug_options, + wait_for_stable_image: cmd_args.exit, + time_profiling: cmd_args.profile, + time_profiler_trace_path: cmd_args.time_profiler_trace_path, + nonincremental_layout: cmd_args.nonincremental_layout, + user_stylesheets: cmd_args.user_stylesheets, + hard_fail: cmd_args.hard_fail, + webdriver_port: cmd_args.webdriver_port, + multiprocess: cmd_args.multiprocess, + background_hang_monitor: cmd_args.background_hang_monitor, + sandbox: cmd_args.sandbox, + random_pipeline_closure_probability: cmd_args.random_pipeline_closure_probability, + random_pipeline_closure_seed: cmd_args.random_pipeline_closure_seed, + config_dir: config_dir.clone(), + shaders_dir: cmd_args.shaders, + certificate_path: cmd_args.certificate_path, + ignore_certificate_errors: cmd_args.ignore_certificate_errors, + unminify_js: cmd_args.unminify_js, + local_script_source: cmd_args.local_script_source, + unminify_css: cmd_args.unminify_css, + print_pwm: cmd_args.print_pwm, }; ArgumentParsingResult::ChromeProcess(opts, preferences, servoshell_preferences) } -fn args_fail(msg: &str) -> ! { - eprintln!("{}", msg); - process::exit(1) -} - -fn print_usage(app: &str, opts: &Options) { - let message = format!( - "Usage: {} [ options ... ] [URL]\n\twhere options include", - app - ); - println!("{}", opts.usage(&message)); -} - fn print_debug_options_usage(app: &str) { fn print_option(name: &str, description: &str) { println!("\t{:<35} {}", name, description); @@ -791,6 +656,7 @@ fn test_parse_pref(arg: &str) -> Preferences { unreachable!("No preferences for content process") }, ArgumentParsingResult::ChromeProcess(_, preferences, _) => preferences, + ArgumentParsingResult::Exit => panic!("should not happen"), } } From 488a395370813e3d6baaf99d53aaaa5e134748ac Mon Sep 17 00:00:00 2001 From: Narfinger Date: Fri, 30 May 2025 13:15:17 +0200 Subject: [PATCH 2/4] This adds some more tests to the parsing of preferences, especially some non-obvious ones. Signed-off-by: Narfinger --- components/config/opts.rs | 2 +- ports/servoshell/prefs.rs | 56 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/components/config/opts.rs b/components/config/opts.rs index 3db866a7443..878c99822a9 100644 --- a/components/config/opts.rs +++ b/components/config/opts.rs @@ -172,7 +172,7 @@ impl DebugOptions { } } -#[derive(Clone, Debug, Deserialize, Serialize)] +#[derive(Clone, Debug, Deserialize, Serialize, PartialEq)] pub enum OutputOptions { /// Database connection config (hostname, name, user, pass) FileName(String), diff --git a/ports/servoshell/prefs.rs b/ports/servoshell/prefs.rs index cf9d26e837d..f96abdf000c 100644 --- a/ports/servoshell/prefs.rs +++ b/ports/servoshell/prefs.rs @@ -705,3 +705,59 @@ fn test_create_prefs_map() { }"; assert_eq!(read_prefs_map(json_str).len(), 3); } + +#[cfg(test)] +fn test_parse(arg: &str) -> (Opts, Preferences, ServoShellPreferences) { + let args = vec!["servo".to_string(), arg.to_string()]; + match parse_command_line_arguments(args) { + ArgumentParsingResult::ContentProcess(..) => { + unreachable!("No preferences for content process") + }, + ArgumentParsingResult::ChromeProcess(opts, preferences, servoshell_preferences) => { + (opts, preferences, servoshell_preferences) + }, + ArgumentParsingResult::Exit => panic!("should not happen"), + } +} + +#[test] +fn test_profiling_args() { + assert_eq!( + test_parse("-p 10").0.time_profiling.unwrap(), + OutputOptions::Stdout(10_f64) + ); + + assert_eq!( + test_parse("-p").0.time_profiling.unwrap(), + OutputOptions::Stdout(5_f64) + ); + + assert_eq!( + test_parse("-p foo.txt").0.time_profiling.unwrap(), + OutputOptions::FileName(String::from("foo.txt")) + ); +} + +#[test] +fn test_servoshell_cmd() { + assert_eq!( + test_parse("-o foo.png").2.device_pixel_ratio_override, + Some(1.0) + ); + + assert_eq!( + test_parse("--screen-size=1000x1000") + .2 + .screen_size_override + .unwrap(), + Size2D::new(1000, 1000) + ); + + assert_eq!( + test_parse("--certificate-path=/tmp/test") + .0 + .certificate_path + .unwrap(), + String::from("/tmp/test") + ); +} From c084d64bde3bc2485a8a6a1d080096fc85a87928 Mon Sep 17 00:00:00 2001 From: Narfinger Date: Fri, 30 May 2025 13:59:47 +0200 Subject: [PATCH 3/4] address some comments Signed-off-by: Narfinger --- ports/servoshell/desktop/cli.rs | 3 + ports/servoshell/egl/android/simpleservo.rs | 1 + ports/servoshell/egl/ohos/simpleservo.rs | 3 +- ports/servoshell/prefs.rs | 64 +++++++++++++-------- 4 files changed, 45 insertions(+), 26 deletions(-) diff --git a/ports/servoshell/desktop/cli.rs b/ports/servoshell/desktop/cli.rs index f8f0b079a97..e10cecbe996 100644 --- a/ports/servoshell/desktop/cli.rs +++ b/ports/servoshell/desktop/cli.rs @@ -27,6 +27,9 @@ pub fn main() { ArgumentParsingResult::Exit => { std::process::exit(0); }, + ArgumentParsingResult::ErrorParsing => { + std::process::exit(1); + }, }; crate::init_tracing(servoshell_preferences.tracing_filter.as_deref()); diff --git a/ports/servoshell/egl/android/simpleservo.rs b/ports/servoshell/egl/android/simpleservo.rs index 9adc43ad5f4..e425b54c3d3 100644 --- a/ports/servoshell/egl/android/simpleservo.rs +++ b/ports/servoshell/egl/android/simpleservo.rs @@ -58,6 +58,7 @@ pub fn init( ArgumentParsingResult::Exit => { std::process::exit(0); }, + ArgumentParsingResult::ErrorParsing => std::process::exit(1), }; crate::init_tracing(servoshell_preferences.tracing_filter.as_deref()); diff --git a/ports/servoshell/egl/ohos/simpleservo.rs b/ports/servoshell/egl/ohos/simpleservo.rs index 55da9ba1932..c08c1d745d5 100644 --- a/ports/servoshell/egl/ohos/simpleservo.rs +++ b/ports/servoshell/egl/ohos/simpleservo.rs @@ -88,9 +88,10 @@ pub fn init( ArgumentParsingResult::ChromeProcess(opts, preferences, servoshell_preferences) => { (opts, preferences, servoshell_preferences) }, - ArgumentParsingResult::Exit => { + ArgumentParsingResult::Exit{ std::process::exit(0); }, + ArgumentParsingResult::ErrorParsing => std::process::exit(1); }; crate::init_tracing(servoshell_preferences.tracing_filter.as_deref()); diff --git a/ports/servoshell/prefs.rs b/ports/servoshell/prefs.rs index f96abdf000c..c55a6fff7ef 100644 --- a/ports/servoshell/prefs.rs +++ b/ports/servoshell/prefs.rs @@ -183,6 +183,7 @@ pub(crate) enum ArgumentParsingResult { ChromeProcess(Opts, Preferences, ServoShellPreferences), ContentProcess(String), Exit, + ErrorParsing, } /// Parse a resolution string into a Size2D @@ -287,7 +288,7 @@ struct CmdArgs { ignore_certificate_errors: bool, /// Number of threads to use for layout - #[bpaf(short('y'), argument("1"))] + #[bpaf(short('y'), long, argument("1"))] layout_threads: Option, /// Directory root with unminified scripts @@ -312,8 +313,8 @@ struct CmdArgs { output_image: Option, /// Time profiler flag and either a TSV output filename OR and interval - /// for output to Stdout (blank for Stdout with interval of 5s) - #[bpaf(short('p'), argument::("10 OR time.tsv"), parse(parse_profiling), fallback(None))] + /// for output to Stdout + #[bpaf(short('p'), long, argument::("10 OR time.tsv"), parse(parse_profiling), fallback(None))] profile: Option, /// Path to dump a self-contained HTML timeline of profiler traces @@ -389,15 +390,14 @@ struct CmdArgs { window_size: Option>, /// The url we should load - #[bpaf( - positional("URL"), - fallback(Some(String::from("https://www.servo.org"))) - )] - url: Option, + #[bpaf(positional("URL"))] + url: String, } -/// In here we set preferences depending on args -fn set_prefs_from_cfg(preferences: &mut Preferences, cmd_args: &CmdArgs) { +fn update_preferences_from_command_line_arguemnts( + preferences: &mut Preferences, + cmd_args: &CmdArgs, +) { if let Some(port) = cmd_args.devtools { preferences.devtools_server_enabled = true; preferences.devtools_server_port = port; @@ -427,7 +427,7 @@ fn set_prefs_from_cfg(preferences: &mut Preferences, cmd_args: &CmdArgs) { .for_each(|pref| preferences.set_value(pref, PrefValue::Bool(true))); } - for pref in cmd_args.pref.clone() { + for pref in &cmd_args.pref { let split: Vec<&str> = pref.splitn(2, '=').collect(); let pref_name = split[0]; let pref_value = PrefValue::from_booleanish_str(split.get(1).copied().unwrap_or("true")); @@ -450,9 +450,9 @@ fn set_prefs_from_cfg(preferences: &mut Preferences, cmd_args: &CmdArgs) { pub(crate) fn parse_command_line_arguments(args: Vec) -> ArgumentParsingResult { let cmd_args = cmd_args().run_inner(&*args); - if let Err(e) = cmd_args { - e.print_message(80); - return ArgumentParsingResult::Exit; + if let Err(error) = cmd_args { + error.print_message(80); + return ArgumentParsingResult::ErrorParsing; } let cmd_args = cmd_args.unwrap(); @@ -461,7 +461,11 @@ pub(crate) fn parse_command_line_arguments(args: Vec) -> ArgumentParsing return ArgumentParsingResult::Exit; } - if cmd_args.debug.iter().any(|d| d.contains("help")) { + if cmd_args + .debug + .iter() + .any(|debug_option| debug_option.contains("help")) + { print_debug_options_usage("servo"); return ArgumentParsingResult::Exit; } @@ -488,7 +492,7 @@ pub(crate) fn parse_command_line_arguments(args: Vec) -> ArgumentParsing let mut preferences = get_preferences(&cmd_args.prefs_files, &config_dir); - set_prefs_from_cfg(&mut preferences, &cmd_args); + update_preferences_from_command_line_arguemnts(&mut preferences, &cmd_args); // FIXME: enable JIT compilation on 32-bit Android after the startup crash issue (#31134) is fixed. if cfg!(target_os = "android") && cfg!(target_pointer_width = "32") { @@ -503,6 +507,7 @@ pub(crate) fn parse_command_line_arguments(args: Vec) -> ArgumentParsing cmd_args.device_pixel_ratio_override }; + // Make sure the default window size is not larger than any provided screen size. let default_window_size = Size2D::new(1024, 740); let default_window_size = cmd_args .screen_size_override @@ -511,7 +516,7 @@ pub(crate) fn parse_command_line_arguments(args: Vec) -> ArgumentParsing }); let servoshell_preferences = ServoShellPreferences { - url: cmd_args.url, + url: Some(cmd_args.url), no_native_titlebar: cmd_args.no_native_titlebar, device_pixel_ratio_override, clean_shutdown: cmd_args.clean_shutdown, @@ -532,10 +537,12 @@ pub(crate) fn parse_command_line_arguments(args: Vec) -> ArgumentParsing }; let mut debug_options = DebugOptions::default(); - for i in cmd_args.debug { - debug_options - .extend(i) - .expect("Could not extend with debug option"); + for debug_string in cmd_args.debug { + let result = debug_options.extend(debug_string); + if let Err(error) = result { + println!("error: urnecognized debug option: {}", error); + return ArgumentParsingResult::ErrorParsing; + } } let opts = Opts { @@ -656,7 +663,12 @@ fn test_parse_pref(arg: &str) -> Preferences { unreachable!("No preferences for content process") }, ArgumentParsingResult::ChromeProcess(_, preferences, _) => preferences, - ArgumentParsingResult::Exit => panic!("should not happen"), + ArgumentParsingResult::Exit => { + panic!("we supplied a --pref argument above which should be parsed") + }, + ArgumentParsingResult::ErrorParsing => { + unreachable!("we supplied a --pref argument above which should be parsed") + }, } } @@ -716,7 +728,9 @@ fn test_parse(arg: &str) -> (Opts, Preferences, ServoShellPreferences) { ArgumentParsingResult::ChromeProcess(opts, preferences, servoshell_preferences) => { (opts, preferences, servoshell_preferences) }, - ArgumentParsingResult::Exit => panic!("should not happen"), + ArgumentParsingResult::Exit | ArgumentParsingResult::ErrorParsing => { + unreachable!("We always valid preference in our test cases") + }, } } @@ -728,8 +742,8 @@ fn test_profiling_args() { ); assert_eq!( - test_parse("-p").0.time_profiling.unwrap(), - OutputOptions::Stdout(5_f64) + test_parse("-p 10.0").0.time_profiling.unwrap(), + OutputOptions::Stdout(10_f64) ); assert_eq!( From ed0597b6c46aa8f2b139f8002385ac772ce232e8 Mon Sep 17 00:00:00 2001 From: Narfinger Date: Mon, 2 Jun 2025 09:54:16 +0200 Subject: [PATCH 4/4] Some improvements. Thanks to @pacak for all the help! Signed-off-by: Narfinger --- components/config/opts.rs | 2 +- ports/servoshell/egl/ohos/simpleservo.rs | 6 +- ports/servoshell/lib.rs | 4 +- ports/servoshell/prefs.rs | 164 ++++++++++++++--------- 4 files changed, 108 insertions(+), 68 deletions(-) diff --git a/components/config/opts.rs b/components/config/opts.rs index 878c99822a9..5cc44ba9f1f 100644 --- a/components/config/opts.rs +++ b/components/config/opts.rs @@ -172,7 +172,7 @@ impl DebugOptions { } } -#[derive(Clone, Debug, Deserialize, Serialize, PartialEq)] +#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] pub enum OutputOptions { /// Database connection config (hostname, name, user, pass) FileName(String), diff --git a/ports/servoshell/egl/ohos/simpleservo.rs b/ports/servoshell/egl/ohos/simpleservo.rs index c08c1d745d5..2f144c3c714 100644 --- a/ports/servoshell/egl/ohos/simpleservo.rs +++ b/ports/servoshell/egl/ohos/simpleservo.rs @@ -88,10 +88,8 @@ pub fn init( ArgumentParsingResult::ChromeProcess(opts, preferences, servoshell_preferences) => { (opts, preferences, servoshell_preferences) }, - ArgumentParsingResult::Exit{ - std::process::exit(0); - }, - ArgumentParsingResult::ErrorParsing => std::process::exit(1); + ArgumentParsingResult::Exit => std::process::exit(0), + ArgumentParsingResult::ErrorParsing => std::process::exit(1), }; crate::init_tracing(servoshell_preferences.tracing_filter.as_deref()); diff --git a/ports/servoshell/lib.rs b/ports/servoshell/lib.rs index 54b4d2d54a0..57e1047a92d 100644 --- a/ports/servoshell/lib.rs +++ b/ports/servoshell/lib.rs @@ -96,9 +96,7 @@ pub fn init_tracing(filter_directives: Option<&str>) { } } -pub fn servo_version() -> String { - format!("Servo {}-{}", env!("CARGO_PKG_VERSION"), env!("GIT_SHA")) -} +pub const VERSION: &str = concat!("Servo ", env!("CARGO_PKG_VERSION"), "-", env!("GIT_SHA")); /// Plumbs tracing spans into HiTrace, with the following caveats: /// diff --git a/ports/servoshell/prefs.rs b/ports/servoshell/prefs.rs index c55a6fff7ef..71202a48a54 100644 --- a/ports/servoshell/prefs.rs +++ b/ports/servoshell/prefs.rs @@ -21,6 +21,8 @@ use servo::servo_geometry::DeviceIndependentPixel; use servo::servo_url::ServoUrl; use url::Url; +use crate::VERSION; + #[cfg_attr(any(target_os = "android", target_env = "ohos"), allow(dead_code))] #[derive(Clone)] pub(crate) struct ServoShellPreferences { @@ -128,7 +130,7 @@ pub fn default_config_dir() -> Option { /// Get a Servo [`Preferences`] to use when initializing Servo by first reading the user /// preferences file and then overriding these preferences with the ones from the `--prefs-file` /// command-line argument, if given. -fn get_preferences(prefs_files: &[String], config_dir: &Option) -> Preferences { +fn get_preferences(prefs_files: &[PathBuf], config_dir: &Option) -> Preferences { // Do not read any preferences files from the disk when testing as we do not want it // to throw off test results. if cfg!(test) { @@ -219,37 +221,55 @@ fn parse_user_stylesheets(string: String) -> Result, ServoUrl)>, st .collect()) } -/// parses the profiling string -fn parse_profiling(arg: String) -> Result, std::io::Error> { - if arg.is_empty() { - Ok(Some(OutputOptions::Stdout(5.0))) - } else { - match arg.parse::() { - Ok(interval) => Ok(Some(OutputOptions::Stdout(interval))), - Err(_) => match ServoUrl::parse(&arg) { - Ok(_) => panic!("influxDB isn't supported anymore"), - Err(_) => Ok(Some(OutputOptions::FileName(arg))), - }, - } - } +/// parses the profiling string and returns the correct value +fn profile() -> impl Parser> { + let profile_flag = short('p') + .long("profile") + .help("uses 5.0 output as standard if no argument supplied") + .req_flag(None); + let profile_arg = short('p') + .long("profile") + .argument::("10 or time.tsv") + .map(Some); + let profile = construct!([profile_arg, profile_flag]); + + // This implements Parser> + let opt_opt_string_parser = profile.optional(); + + opt_opt_string_parser.map(|val| match val { + Some(Some(parsed_string)) => { + if let Ok(float) = parsed_string.parse::() { + Some(OutputOptions::Stdout(float)) + } else { + Some(OutputOptions::FileName(parsed_string)) + } + }, + Some(None) => Some(OutputOptions::Stdout(5.0)), + _ => None, + }) +} + +fn map_debug_options(arg: String) -> Vec { + arg.split(',').map(|s| s.to_owned()).collect() } #[derive(Bpaf, Clone, Debug)] -#[bpaf(options)] +#[bpaf(options, version(VERSION))] struct CmdArgs { /// Background Hang Monitor enabled - #[bpaf(long("bhm"), fallback(false))] + #[bpaf(long("bhm"))] background_hang_monitor: bool, /// Path to find SSL certificates #[bpaf(argument("/home/servo/resources/certs"))] - certificate_path: Option, + certificate_path: Option, /// Do not shutdown until all threads have finished (macos only) - #[bpaf(long, fallback(false))] + #[bpaf(long)] clean_shutdown: bool, /// config directory following xdg spec on linux platform + #[bpaf(argument("~/.config/servo"))] config_dir: Option, /// Run as a content process and connect to the given pipe @@ -257,7 +277,13 @@ struct CmdArgs { content_process: Option, /// A comma-separated string of debug options. Pass help to show available options. - #[bpaf(short('Z'), long, fallback(vec![]), help("d"))] + #[bpaf( + short('Z'), + argument("layout_grid_enabled=true,dom_async_clipboard_enabled"), + long, + map(map_debug_options), + fallback(vec![]) + )] debug: Vec, #[bpaf(argument("1.0"))] @@ -265,26 +291,26 @@ struct CmdArgs { /// Start remote devtools using server on port #[bpaf(argument("0"))] - devtools: Option, + devtools: Option, /// Wether or not to enable experimental web platform features. - #[bpaf(long, fallback(false))] + #[bpaf(long)] enable_experimental_web_platform_features: bool, // Exit after Servo has loaded the page and detected stable output image - #[bpaf(short('x'), long, fallback(false))] + #[bpaf(short('x'), long)] exit: bool, /// Exit on thread failure instead of displaying about:failure - #[bpaf(short('f'), long, fallback(false))] + #[bpaf(short('f'), long)] hard_fail: bool, /// Headless mode - #[bpaf(short('z'), long, fallback(false))] + #[bpaf(short('z'), long)] headless: bool, /// Wether or not to completely ignore certificate errors - #[bpaf(long, fallback(false))] + #[bpaf(long)] ignore_certificate_errors: bool, /// Number of threads to use for layout @@ -292,7 +318,8 @@ struct CmdArgs { layout_threads: Option, /// Directory root with unminified scripts - local_script_source: Option, + #[bpaf(argument("~/.local/share/servo"))] + local_script_source: Option, #[cfg(target_env = "ohos")] /// Define a custom filter for logging @@ -300,7 +327,7 @@ struct CmdArgs { log_filter: Option, /// Run in multiprocess mode - #[bpaf(short('M'), long, fallback(true))] + #[bpaf(short('M'), long, flag(true, false))] multiprocess: bool, /// Enable to turn off incremental layout @@ -309,25 +336,25 @@ struct CmdArgs { /// Path to an output image. The format of the image is determined /// by the extension. Supports all formats that rust-image does. - #[bpaf(short('o'), long)] - output_image: Option, + #[bpaf(short('o'), argument("test.png"), long)] + output_image: Option, /// Time profiler flag and either a TSV output filename OR and interval /// for output to Stdout - #[bpaf(short('p'), long, argument::("10 OR time.tsv"), parse(parse_profiling), fallback(None))] + #[bpaf(external)] profile: Option, /// Path to dump a self-contained HTML timeline of profiler traces - #[bpaf(long("profiler-trace-path"))] - time_profiler_trace_path: Option, + #[bpaf(argument("trace.html"), long("profiler-trace-path"))] + time_profiler_trace_path: Option, /// A preference to set - #[bpaf(argument("dom_bluetooth_enabled"),fallback(vec![]))] + #[bpaf(argument("dom_bluetooth_enabled"), many)] pref: Vec, /// Load an additional prefs from a file. - #[bpaf(long, argument("/path/to/prefs.json"), fallback(vec![]))] - prefs_files: Vec, + #[bpaf(long, argument("/path/to/prefs.json"), many)] + prefs_files: Vec, print_pwm: bool, @@ -339,7 +366,7 @@ struct CmdArgs { random_pipeline_closure_seed: Option, /// Run in sandbox if multiprocess - #[bpaf(short('S'), long, fallback(true))] + #[bpaf(short('S'), long, flag(true, false))] sandbox: bool, /// Shaders will be loaded from the specified directory instead of using the builtin ones. @@ -354,15 +381,15 @@ struct CmdArgs { #[bpaf(long("tracing-filter"), argument("FILTER"))] tracing_filter: Option, - #[bpaf(long, fallback(false))] + #[bpaf(long)] no_native_titlebar: bool, /// Unminify Javascript - #[bpaf(long, fallback(false))] + #[bpaf(long)] unminify_js: bool, /// Unminify Css - #[bpaf(long, fallback(false))] + #[bpaf(long)] unminify_css: bool, /// Set custom user agent string (or ios/ android / desktop for platform default)"" @@ -370,17 +397,17 @@ struct CmdArgs { user_agent: Option, /// Uses userscripts in resources/user-agent-js or a specified full path - #[bpaf(long, fallback(Some(PathBuf::from("resources/user-agent-js"))))] - userscripts_directory: Option, + #[bpaf( + long, + argument("resources/user-agent-js"), + fallback(PathBuf::from("resources/user-agent-js")) + )] + userscripts_directory: PathBuf, /// A user stylesheet ot be added to every document - #[bpaf(fallback(String::from("")), argument::("path.css"), parse(parse_user_stylesheets))] + #[bpaf(argument::("path.css"), parse(parse_user_stylesheets), fallback(vec![]))] user_stylesheets: Vec<(Vec, ServoUrl)>, - /// Prints the version information - #[bpaf(short, long)] - version: bool, - /// Start remote WebDriver server on port #[bpaf(argument("7000"))] webdriver_port: Option, @@ -390,7 +417,7 @@ struct CmdArgs { window_size: Option>, /// The url we should load - #[bpaf(positional("URL"))] + #[bpaf(positional("URL"), fallback(String::from("https://www.servo.org")))] url: String, } @@ -400,7 +427,7 @@ fn update_preferences_from_command_line_arguemnts( ) { if let Some(port) = cmd_args.devtools { preferences.devtools_server_enabled = true; - preferences.devtools_server_port = port; + preferences.devtools_server_port = port as i64; } if cmd_args.enable_experimental_web_platform_features { @@ -449,17 +476,18 @@ fn update_preferences_from_command_line_arguemnts( } pub(crate) fn parse_command_line_arguments(args: Vec) -> ArgumentParsingResult { - let cmd_args = cmd_args().run_inner(&*args); + // we do not want the binary name in the arguments + let args_without_binary = args + .split_first() + .expect("Expectd executable name and and arguments") + .1; + let cmd_args = cmd_args().run_inner(Args::from(args_without_binary)); if let Err(error) = cmd_args { error.print_message(80); return ArgumentParsingResult::ErrorParsing; } let cmd_args = cmd_args.unwrap(); - if cmd_args.version { - println!("{}", crate::servo_version()); - return ArgumentParsingResult::Exit; - } if cmd_args .debug @@ -524,9 +552,11 @@ pub(crate) fn parse_command_line_arguments(args: Vec) -> ArgumentParsing tracing_filter: cmd_args.tracing_filter, initial_window_size: cmd_args.window_size.unwrap_or(default_window_size), screen_size_override: cmd_args.screen_size_override, - output_image_path: cmd_args.output_image, + output_image_path: cmd_args + .output_image + .map(|p| p.to_string_lossy().into_owned()), exit_after_stable_image: cmd_args.exit, - userscripts_directory: cmd_args.userscripts_directory, + userscripts_directory: Some(cmd_args.userscripts_directory), #[cfg(target_env = "ohos")] log_filter: Some( cmd_args @@ -549,7 +579,9 @@ pub(crate) fn parse_command_line_arguments(args: Vec) -> ArgumentParsing debug: debug_options, wait_for_stable_image: cmd_args.exit, time_profiling: cmd_args.profile, - time_profiler_trace_path: cmd_args.time_profiler_trace_path, + time_profiler_trace_path: cmd_args + .time_profiler_trace_path + .map(|p| p.to_string_lossy().into_owned()), nonincremental_layout: cmd_args.nonincremental_layout, user_stylesheets: cmd_args.user_stylesheets, hard_fail: cmd_args.hard_fail, @@ -561,10 +593,14 @@ pub(crate) fn parse_command_line_arguments(args: Vec) -> ArgumentParsing random_pipeline_closure_seed: cmd_args.random_pipeline_closure_seed, config_dir: config_dir.clone(), shaders_dir: cmd_args.shaders, - certificate_path: cmd_args.certificate_path, + certificate_path: cmd_args + .certificate_path + .map(|p| p.to_string_lossy().into_owned()), ignore_certificate_errors: cmd_args.ignore_certificate_errors, unminify_js: cmd_args.unminify_js, - local_script_source: cmd_args.local_script_source, + local_script_source: cmd_args + .local_script_source + .map(|p| p.to_string_lossy().into_owned()), unminify_css: cmd_args.unminify_css, print_pwm: cmd_args.print_pwm, }; @@ -720,7 +756,10 @@ fn test_create_prefs_map() { #[cfg(test)] fn test_parse(arg: &str) -> (Opts, Preferences, ServoShellPreferences) { - let args = vec!["servo".to_string(), arg.to_string()]; + let mut args = vec!["servo".to_string()]; + // bpaf requires the arguments that are separated by whitespace to be different elements of the vector. + let mut args_split = arg.split_whitespace().map(|s| s.to_owned()).collect(); + args.append(&mut args_split); match parse_command_line_arguments(args) { ArgumentParsingResult::ContentProcess(..) => { unreachable!("No preferences for content process") @@ -729,13 +768,18 @@ fn test_parse(arg: &str) -> (Opts, Preferences, ServoShellPreferences) { (opts, preferences, servoshell_preferences) }, ArgumentParsingResult::Exit | ArgumentParsingResult::ErrorParsing => { - unreachable!("We always valid preference in our test cases") + unreachable!("We always have valid preference in our test cases") }, } } #[test] fn test_profiling_args() { + assert_eq!( + test_parse("-p").0.time_profiling.unwrap(), + OutputOptions::Stdout(5_f64) + ); + assert_eq!( test_parse("-p 10").0.time_profiling.unwrap(), OutputOptions::Stdout(10_f64)