diff --git a/cli/src/commands.rs b/cli/src/commands.rs index 073385d..b1211b8 100644 --- a/cli/src/commands.rs +++ b/cli/src/commands.rs @@ -449,10 +449,22 @@ pub fn parse_command(args: &[String], flags: &Flags) -> Result { - // screenshot [selector] [path] + // screenshot [selector] [path] [--full/-f] // selector: @ref or CSS selector // path: file path (contains / or . or ends with known extension) - let (selector, path) = match (rest.first(), rest.get(1)) { + let mut full_page = false; + let positional: Vec<&str> = rest + .iter() + .filter(|arg| match **arg { + "--full" | "-f" => { + full_page = true; + false + } + _ => true, + }) + .copied() + .collect(); + let (selector, path) = match (positional.first(), positional.get(1)) { (Some(first), Some(second)) => { // Two args: first is selector, second is path (Some(*first), Some(*second)) @@ -480,7 +492,7 @@ pub fn parse_command(args: &[String], flags: &Flags) -> Result Result parse_diff(&rest, &id, flags), + "diff" => parse_diff(&rest, &id), // === Batch === "batch" => { @@ -1329,7 +1341,7 @@ pub fn parse_command(args: &[String], flags: &Flags) -> Result Result { +fn parse_diff(rest: &[&str], id: &str) -> Result { const VALID: &[&str] = &["snapshot", "screenshot", "url"]; match rest.first().copied() { @@ -1474,27 +1486,24 @@ fn parse_diff(rest: &[&str], id: &str, flags: &Flags) -> Result { + "--full" | "-f" => { obj.insert("fullPage".to_string(), json!(true)); } other if other.starts_with('-') => { return Err(ParseError::InvalidValue { message: format!("Unknown flag: {}", other), - usage: "diff screenshot --baseline [--output ] [--threshold <0-1>] [--selector ] [--full]", + usage: "diff screenshot --baseline [--output ] [--threshold <0-1>] [--selector ] [--full/-f]", }); } other => { return Err(ParseError::InvalidValue { message: format!("Unexpected argument: {}", other), - usage: "diff screenshot --baseline [--output ] [--threshold <0-1>] [--selector ] [--full]", + usage: "diff screenshot --baseline [--output ] [--threshold <0-1>] [--selector ] [--full/-f]", }); } } i += 1; } - if flags.full { - obj.insert("fullPage".to_string(), json!(true)); - } if !obj.contains_key("baseline") { return Err(ParseError::MissingArguments { context: "diff screenshot".to_string(), @@ -1525,7 +1534,7 @@ fn parse_diff(rest: &[&str], id: &str, flags: &Flags) -> Result { obj.insert("screenshot".to_string(), json!(true)); } - "--full" => { + "--full" | "-f" => { obj.insert("fullPage".to_string(), json!(true)); } "--wait-until" => { @@ -1580,21 +1589,18 @@ fn parse_diff(rest: &[&str], id: &str, flags: &Flags) -> Result { return Err(ParseError::InvalidValue { message: format!("Unknown flag: {}", other), - usage: "diff url [--screenshot] [--full] [--wait-until ] [--selector ] [--compact] [--depth ]", + usage: "diff url [--screenshot] [--full/-f] [--wait-until ] [--selector ] [--compact] [--depth ]", }); } other => { return Err(ParseError::InvalidValue { message: format!("Unexpected argument: {}", other), - usage: "diff url [--screenshot] [--full] [--wait-until ] [--selector ] [--compact] [--depth ]", + usage: "diff url [--screenshot] [--full/-f] [--wait-until ] [--selector ] [--compact] [--depth ]", }); } } i += 1; } - if flags.full { - obj.insert("fullPage".to_string(), json!(true)); - } Ok(cmd) } Some(sub) => Err(ParseError::UnknownSubcommand { @@ -2165,7 +2171,6 @@ mod tests { Flags { session: "test".to_string(), json: false, - full: false, headed: false, debug: false, headers: None, @@ -2728,9 +2733,14 @@ mod tests { #[test] fn test_screenshot_full_page() { - let mut flags = default_flags(); - flags.full = true; - let cmd = parse_command(&args("screenshot"), &flags).unwrap(); + let cmd = parse_command(&args("screenshot --full"), &default_flags()).unwrap(); + assert_eq!(cmd["action"], "screenshot"); + assert_eq!(cmd["fullPage"], true); + } + + #[test] + fn test_screenshot_full_page_shorthand() { + let cmd = parse_command(&args("screenshot -f"), &default_flags()).unwrap(); assert_eq!(cmd["action"], "screenshot"); assert_eq!(cmd["fullPage"], true); } @@ -3598,10 +3608,23 @@ mod tests { } #[test] - fn test_diff_screenshot_global_full_flag() { - let mut flags = default_flags(); - flags.full = true; - let cmd = parse_command(&args("diff screenshot --baseline b.png"), &flags).unwrap(); + fn test_diff_screenshot_command_full_flag() { + let cmd = parse_command( + &args("diff screenshot --baseline b.png --full"), + &default_flags(), + ) + .unwrap(); + assert_eq!(cmd["action"], "diff_screenshot"); + assert_eq!(cmd["fullPage"], true); + } + + #[test] + fn test_diff_screenshot_command_full_flag_shorthand() { + let cmd = parse_command( + &args("diff screenshot --baseline b.png -f"), + &default_flags(), + ) + .unwrap(); assert_eq!(cmd["action"], "diff_screenshot"); assert_eq!(cmd["fullPage"], true); } @@ -3642,10 +3665,12 @@ mod tests { } #[test] - fn test_diff_url_global_full_flag() { - let mut flags = default_flags(); - flags.full = true; - let cmd = parse_command(&args("diff url https://a.com https://b.com"), &flags).unwrap(); + fn test_diff_url_command_full_flag() { + let cmd = parse_command( + &args("diff url https://a.com https://b.com --full"), + &default_flags(), + ) + .unwrap(); assert_eq!(cmd["fullPage"], true); } diff --git a/cli/src/flags.rs b/cli/src/flags.rs index 83896f7..8eae1be 100644 --- a/cli/src/flags.rs +++ b/cli/src/flags.rs @@ -55,7 +55,6 @@ fn parse_idle_timeout_value(value: Option, source: &str) -> Option, pub json: Option, - pub full: Option, pub debug: Option, pub session: Option, pub session_name: Option, @@ -95,7 +94,6 @@ impl Config { Config { headed: other.headed.or(self.headed), json: other.json.or(self.json), - full: other.full.or(self.full), debug: other.debug.or(self.debug), session: other.session.or(self.session), session_name: other.session_name.or(self.session_name), @@ -267,7 +265,6 @@ pub fn load_config(args: &[String]) -> Result { pub struct Flags { pub json: bool, - pub full: bool, pub headed: bool, pub debug: bool, pub session: String, @@ -342,7 +339,6 @@ pub fn parse_flags(args: &[String]) -> Flags { let mut flags = Flags { json: env_var_is_truthy("AGENT_BROWSER_JSON") || config.json.unwrap_or(false), - full: env_var_is_truthy("AGENT_BROWSER_FULL") || config.full.unwrap_or(false), headed: env_var_is_truthy("AGENT_BROWSER_HEADED") || config.headed.unwrap_or(false), debug: env_var_is_truthy("AGENT_BROWSER_DEBUG") || config.debug.unwrap_or(false), session: env::var("AGENT_BROWSER_SESSION") @@ -447,13 +443,6 @@ pub fn parse_flags(args: &[String]) -> Flags { i += 1; } } - "--full" | "-f" => { - let (val, consumed) = parse_bool_arg(args, i); - flags.full = val; - if consumed { - i += 1; - } - } "--headed" => { let (val, consumed) = parse_bool_arg(args, i); flags.headed = val; @@ -722,7 +711,6 @@ pub fn clean_args(args: &[String]) -> Vec { // Boolean flags that optionally take true/false const GLOBAL_BOOL_FLAGS: &[&str] = &[ "--json", - "--full", "--headed", "--debug", "--ignore-https-errors", @@ -776,7 +764,7 @@ pub fn clean_args(args: &[String]) -> Vec { i += 1; continue; } - if GLOBAL_BOOL_FLAGS.contains(&arg.as_str()) || arg == "-f" { + if GLOBAL_BOOL_FLAGS.contains(&arg.as_str()) { if let Some(v) = args.get(i + 1) { if matches!(v.as_str(), "true" | "false") { i += 1; @@ -1024,7 +1012,6 @@ mod tests { let json = r#"{ "headed": true, "json": true, - "full": true, "debug": true, "session": "test-session", "sessionName": "my-app", @@ -1047,7 +1034,6 @@ mod tests { let config: Config = serde_json::from_str(json).unwrap(); assert_eq!(config.headed, Some(true)); assert_eq!(config.json, Some(true)); - assert_eq!(config.full, Some(true)); assert_eq!(config.debug, Some(true)); assert_eq!(config.session.as_deref(), Some("test-session")); assert_eq!(config.session_name.as_deref(), Some("my-app")); @@ -1323,36 +1309,6 @@ mod tests { assert!(!flags.auto_connect); } - #[test] - fn test_full_bare_defaults_true() { - let flags = parse_flags(&args("--full open example.com")); - assert!(flags.full); - } - - #[test] - fn test_full_false() { - let flags = parse_flags(&args("--full false open example.com")); - assert!(!flags.full); - } - - #[test] - fn test_full_short_flag() { - let flags = parse_flags(&args("-f open example.com")); - assert!(flags.full); - } - - #[test] - fn test_clean_args_removes_full_with_value() { - let cleaned = clean_args(&args("--full false open example.com")); - assert_eq!(cleaned, vec!["open", "example.com"]); - } - - #[test] - fn test_clean_args_removes_short_full() { - let cleaned = clean_args(&args("-f open example.com")); - assert_eq!(cleaned, vec!["open", "example.com"]); - } - #[test] fn test_clean_args_removes_bool_flag_with_value() { let cleaned = clean_args(&args("--headed false --debug true open example.com")); diff --git a/cli/src/output.rs b/cli/src/output.rs index cd6f879..d32637a 100644 --- a/cli/src/output.rs +++ b/cli/src/output.rs @@ -2655,7 +2655,6 @@ Environment: AGENT_BROWSER_EXTENSIONS Comma-separated browser extension paths AGENT_BROWSER_HEADED Show browser window (not headless) AGENT_BROWSER_JSON JSON output - AGENT_BROWSER_FULL Full page screenshot AGENT_BROWSER_ANNOTATE Annotated screenshot with numbered labels and legend AGENT_BROWSER_DEBUG Debug output AGENT_BROWSER_IGNORE_HTTPS_ERRORS Ignore HTTPS certificate errors