diff --git a/Cargo.lock b/Cargo.lock index df968c662..577844b2e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -14878,6 +14878,8 @@ dependencies = [ "regex", "serde", "serde_json", + "shlex", + "tempfile", "thiserror 2.0.17", "typed-path 0.10.0", "warpui", diff --git a/app/src/terminal/view/link_detection.rs b/app/src/terminal/view/link_detection.rs index 916bf825a..533bbaa39 100644 --- a/app/src/terminal/view/link_detection.rs +++ b/app/src/terminal/view/link_detection.rs @@ -27,6 +27,9 @@ cfg_if::cfg_if! { util::openable_file_type::FileTarget, }; use std::path::PathBuf; + use warp_util::listing_command::{ + listing_command_argument_dir, DEFAULT_LISTING_COMMANDS, + }; use warp_util::path::CleanPathResult; use warp_util::path::LineAndColumnArg; } @@ -443,15 +446,49 @@ impl super::TerminalView { ) { // For AltScreen we scan for relative path with the current working directory. // For BlockList we scan for relative path with the pwd of the hovered block. - let pwd_to_scan_for = match position { - WithinModel::AltScreen(_) => self.pwd_if_local(ctx), - WithinModel::BlockList(inner) => self - .model - .lock() - .block_list() - .block_at(inner.block_index) - .filter(|block| !self.is_block_considered_remote(block.session_id(), None, ctx)) // Don't scan for file links if the block is on remote sessions - .and_then(|block| block.pwd().map(String::from)), + // + // For BlockList we also look at the block's command: if the block ran a + // directory-listing command (`ls DIR/`, `eza DIR/`, etc.), bare filenames in + // its output are rooted at DIR, not at the block's pwd. `listing_dir_to_scan` + // holds the resolved argument directory (`pwd.join(DIR)`) in that case, and is + // tried first during candidate resolution. If the block's command is not a + // listing command, or has no directory argument, this is `None` and the + // existing pwd-only resolution is used. + // + // `top_level_command` resolves shell aliases (e.g. `ll` → `ls`) via + // `Block::top_level_command`, which delegates to the active session's alias + // table. We pass the resolved name to `listing_command_argument_dir` so + // user-aliased listing commands trigger the fix without needing their alias + // name in the `DEFAULT_LISTING_COMMANDS` list. + let (pwd_to_scan_for, listing_dir_to_scan) = match position { + WithinModel::AltScreen(_) => (self.pwd_if_local(ctx), None), + WithinModel::BlockList(inner) => { + let sessions = self.sessions.as_ref(ctx); + let model_guard = self.model.lock(); + let (pwd, command, top_level) = model_guard + .block_list() + .block_at(inner.block_index) + .filter(|block| !self.is_block_considered_remote(block.session_id(), None, ctx)) + .map(|block| { + ( + block.pwd().map(String::from), + block.command_to_string(), + block.top_level_command(sessions), + ) + }) + .unwrap_or((None, String::new(), None)); + drop(model_guard); + + let listing_dir = pwd.as_deref().and_then(|pwd_str| { + listing_command_argument_dir( + &command, + top_level.as_deref(), + std::path::Path::new(pwd_str), + DEFAULT_LISTING_COMMANDS, + ) + }); + (pwd, listing_dir) + } }; match pwd_to_scan_for { @@ -473,6 +510,7 @@ impl super::TerminalView { .spawn(move || { let paths = Self::compute_valid_paths( &path, + listing_dir_to_scan.as_deref(), possible_paths, max_columns, shell_launch_data, @@ -499,20 +537,49 @@ impl super::TerminalView { fn compute_valid_paths( working_directory: &str, + listing_dir: Option<&std::path::Path>, possible_paths: impl Iterator>, max_columns: usize, shell_launch_data: Option, ) -> Option { + // Try to resolve `clean_path` against the block's listing-command argument + // directory first (if any), then against the block's working directory. The + // listing directory takes precedence so bare filenames from `ls DIR/` output + // resolve into DIR, not into CWD — fixing the silent-misresolution bug where + // a same-named file in CWD would otherwise win. + let try_resolve = |clean_path: &CleanPathResult| -> Option { + // Only attempt listing-dir resolution for bare entry names (no path + // separators). Candidates like `subdir/README.md`, `./foo`, or absolute + // paths already resolve correctly against CWD and must not be double-joined + // with the listing directory. + if let Some(listing_dir) = listing_dir { + let path_str = &clean_path.path; + let is_bare_name = !path_str.contains('/') + && !path_str.contains('\\') + && !path_str.starts_with('~'); + if is_bare_name { + if let Some(resolved) = absolute_path_if_valid( + clean_path, + ShellPathType::PlatformNative(listing_dir.to_path_buf()), + shell_launch_data.as_ref(), + ) { + return Some(resolved); + } + } + } + absolute_path_if_valid( + clean_path, + ShellPathType::ShellNative(working_directory.to_string()), + shell_launch_data.as_ref(), + ) + }; + let mut link = None; 'path_loop: for within_model_possible_path in possible_paths { let possible_path = within_model_possible_path.get_inner(); // We want to check if the clean path result is a valid path and get the canonical // absolute path back. - let absolute_path = absolute_path_if_valid( - &possible_path.path, - ShellPathType::ShellNative(working_directory.to_string()), - shell_launch_data.as_ref(), - ); + let absolute_path = try_resolve(&possible_path.path); if let Some(absolute_path) = absolute_path { link = Some(Self::create_valid_link( @@ -530,11 +597,7 @@ impl super::TerminalView { path: new_possible_path.into(), line_and_column_num: possible_path.path.line_and_column_num, }; - let absolute_path = absolute_path_if_valid( - &new_possible_cleaned_path, - ShellPathType::ShellNative(working_directory.to_string()), - shell_launch_data.as_ref(), - ); + let absolute_path = try_resolve(&new_possible_cleaned_path); // check if new_possible_path is valid if let Some(absolute_path) = absolute_path { @@ -562,11 +625,7 @@ impl super::TerminalView { path: new_possible_path.into(), line_and_column_num: possible_path.path.line_and_column_num, }; - let absolute_path = absolute_path_if_valid( - &new_possible_cleaned_path, - ShellPathType::ShellNative(working_directory.to_string()), - shell_launch_data.as_ref(), - ); + let absolute_path = try_resolve(&new_possible_cleaned_path); // check if new_possible_path is valid if let Some(absolute_path) = absolute_path { diff --git a/crates/warp_util/Cargo.toml b/crates/warp_util/Cargo.toml index 9996cff86..d54934161 100644 --- a/crates/warp_util/Cargo.toml +++ b/crates/warp_util/Cargo.toml @@ -18,6 +18,7 @@ hex.workspace = true lazy_static.workspace = true mime_guess.workspace = true regex.workspace = true +shlex = "1.3.0" thiserror.workspace = true typed-path.workspace = true dunce.workspace = true @@ -33,4 +34,5 @@ windows.workspace = true gloo.workspace = true [dev-dependencies] +tempfile.workspace = true warpui.workspace = true diff --git a/crates/warp_util/src/lib.rs b/crates/warp_util/src/lib.rs index 49fd688c6..b8c917bbe 100644 --- a/crates/warp_util/src/lib.rs +++ b/crates/warp_util/src/lib.rs @@ -7,6 +7,7 @@ pub mod assets; pub mod content_version; pub mod file; pub mod file_type; +pub mod listing_command; pub mod on_cancel; pub mod path; pub mod standardized_path; diff --git a/crates/warp_util/src/listing_command.rs b/crates/warp_util/src/listing_command.rs new file mode 100644 index 000000000..d8d35950b --- /dev/null +++ b/crates/warp_util/src/listing_command.rs @@ -0,0 +1,189 @@ +//! Helpers for understanding the structure of shell commands whose output contains +//! bare filenames rooted at a directory argument (e.g. `ls SUBDIR/`, `tree SUBDIR`). +//! +//! When the terminal detects a clickable filename in the output of such a command, +//! resolving the candidate against the block's CWD alone is incorrect: bare +//! filenames listed by `ls SUBDIR/` are rooted at `SUBDIR`, not at the CWD. If the +//! user happens to have a same-named file in the CWD (very common for `README.md`), +//! the detector would silently open the wrong file. See sibling module docs for the +//! full bug class. +//! +//! This module provides a narrow helper that parses a command line to extract the +//! first positional path argument, joins it to the block's CWD, and returns it only +//! if the resolved path is a directory on disk. + +use std::path::{Path, PathBuf}; + +/// Default set of directory-listing commands whose bare-filename output should be +/// resolved against their directory argument. +/// +/// Exposed as a constant so callers can start from this set and extend it from user +/// settings. Deliberately conservative: `ls` is the canonical case. Modern ls +/// replacements (`exa`, `eza`, `lsd`) behave identically for plain-arg output and +/// are included. +/// +/// Not included by default: +/// - `tree` produces output with box-drawing characters that the grid tokenizer +/// currently does not recognize as link separators (tracked separately). Adding +/// `tree` here has no effect until that tokenizer fix lands. +/// - Recursive listings (`ls -R`, `tree`) have multiple root directories within a +/// single block and need per-section resolution, which is out of scope here. +pub const DEFAULT_LISTING_COMMANDS: &[&str] = &["ls", "exa", "eza", "lsd"]; + +/// Given a shell command line and the block's CWD, return the directory argument +/// (joined to CWD) if the command is a directory-listing command from `listing_commands` +/// and the first positional argument resolves to an existing directory on disk. +/// +/// Returns `None` if: +/// - The command cannot be tokenized. +/// - The command name is not in `listing_commands`. +/// - There is no positional argument (e.g. plain `ls`). +/// - The positional argument does not resolve to an existing directory. +/// - The command uses `-R`/`--recursive` (per-section roots). +/// - The command uses `-d`/`--directory` (lists the operand itself, not entries). +/// - The command has multiple positional operands (mixed roots). +/// +/// `resolved_command_name` is an optional override for the command-name match. When +/// the caller has already resolved shell aliases (e.g. via `Block::top_level_command`), +/// pass the alias-resolved name here and we match against it instead of the raw first +/// token. Positional-argument extraction still uses the raw command tokens, which is +/// correct for the common case where aliases only add flags (`alias ll='ls -l'` → +/// `ll DIR/` still has `DIR/` as the first positional). Aliases that introduce their +/// own positional arguments (`alias lsd='ls /tmp'`) are a known limitation: we'd pick +/// the user's typed arg, missing the aliased one. Rare and documented. +/// +/// ## Examples +/// +/// ```text +/// "ls -la subdir/" + cwd=/a/b -> Some(/a/b/subdir) (if /a/b/subdir is a dir) +/// "ls --color=always subdir" + cwd=/a/b -> Some(/a/b/subdir) (if dir) +/// "ls /etc/" + cwd=/a/b -> Some(/etc) (if dir) +/// "ls" + cwd=/a/b -> None +/// "ls subdir1/ subdir2/" + cwd=/a/b -> None (multi-dir rejected) +/// "cat subdir/foo" + cwd=/a/b -> None (cat not in listing_commands) +/// ``` +/// +/// With `resolved_command_name`: +/// +/// ```text +/// "ll subdir/", resolved="ls" + cwd=/a/b -> Some(/a/b/subdir) +/// "ll subdir/", resolved=None + cwd=/a/b -> None (ll not in listing_commands) +/// ``` +/// +/// ## Shell parsing +/// +/// Uses `shlex` to split the command into tokens. This handles POSIX-style quoting +/// (`'...'`, `"..."`, and backslash escapes). It does NOT handle: +/// - Shell variable expansion (`$HOME`, `$VAR`). If a user runs `ls $HOME/foo/`, the +/// command as stored in the block may be either pre- or post-expansion depending on +/// how Warp captures it; this function sees whatever is in the string and will fail +/// gracefully if `$HOME/foo` is not a valid directory name. +/// - Aliases with baked-in positional arguments. `alias lsd='ls /tmp'; lsd DIR/` will +/// resolve to `DIR/` (the user-typed arg), not `/tmp` (the aliased arg). This is a +/// rare edge case; if it matters, callers can expand the full alias upstream and +/// pass the expanded command string. +/// - Environment variable prefixes (`FOO=bar ls DIR/`). Handled by skipping leading +/// `KEY=VALUE`-shaped tokens. +/// - Compound shells (`cd /x && ls DIR/`). We only inspect the first command. +/// +/// ## Flag heuristic +/// +/// After identifying a listing command, we skip all tokens that start with `-` as +/// flag tokens. This is a simplification: `ls` flags with values like +/// `--color=always` are a single token that starts with `-` (handled correctly); +/// `--color always` (two tokens) would incorrectly consume `always` as a flag, but no +/// `ls` flag actually takes a path-like separate value, so this is safe for `ls`. +pub fn listing_command_argument_dir( + command: &str, + resolved_command_name: Option<&str>, + pwd: &Path, + listing_commands: &[&str], +) -> Option { + let tokens = shlex::split(command)?; + let mut iter = tokens.iter(); + + // First non-empty token is the command name. Skip leading env-var assignments + // (e.g. `FOO=bar ls DIR/`) by stepping past any `KEY=VALUE`-shaped tokens. + let raw_command_name = loop { + let token = iter.next()?; + if token.contains('=') && !token.starts_with('-') { + continue; + } + break token.as_str(); + }; + + // Use the alias-resolved name for the listing-command match if provided; otherwise + // fall back to the raw first token. Positional extraction below always uses the + // raw tokens regardless. + let name_for_match = resolved_command_name.unwrap_or(raw_command_name); + if !listing_commands.contains(&name_for_match) { + return None; + } + + // Collect remaining tokens, rejecting modes whose output is not a single + // directory's entries. + let mut positionals = Vec::new(); + for token in iter { + if token.starts_with('-') { + // Reject recursive listings — output has per-section roots we can't resolve. + // Reject -d/--directory — output names the operand itself, not entries under it. + if token == "--recursive" + || token == "--directory" + || (!token.starts_with("--") && (token.contains('R') || token.contains('d'))) + { + return None; + } + continue; + } + positionals.push(token.as_str()); + } + + let first_positional = positionals.first()?; + + // Expand tilde if present. Without shell-level `$HOME`, `~` by itself or `~/foo` + // won't resolve otherwise. We reuse the minimal expansion here rather than + // depending on `shellexpand` to keep warp_util lean. + let expanded = expand_leading_tilde(first_positional); + + // Join with pwd if relative; leave absolute paths alone. + let candidate = if expanded.is_absolute() { + expanded + } else { + pwd.join(&expanded) + }; + + // Only return the path if it actually resolves to a directory on disk. This + // guards against the user typing a typo'd path, or a path that exists as a file + // (which wouldn't be a listing target anyway). + if !candidate.is_dir() { + return None; + } + + // Reject multi-operand commands — `ls DIR FILE` mixes roots (entries under DIR + // alongside bare file operands rooted at CWD), so picking a single listing root + // would misresolve some entries. + if positionals.len() > 1 { + return None; + } + + Some(candidate) +} + +/// Expands a leading `~` or `~/` in a path to `$HOME` / `$HOME/`. Returns the input +/// unchanged if it does not start with `~`, or if `$HOME` cannot be determined. +fn expand_leading_tilde(token: &str) -> PathBuf { + if let Some(rest) = token.strip_prefix("~/") { + if let Some(home) = dirs::home_dir() { + return home.join(rest); + } + } else if token == "~" { + if let Some(home) = dirs::home_dir() { + return home; + } + } + PathBuf::from(token) +} + +#[cfg(test)] +#[path = "listing_command_test.rs"] +mod tests; diff --git a/crates/warp_util/src/listing_command_test.rs b/crates/warp_util/src/listing_command_test.rs new file mode 100644 index 000000000..702fc5de7 --- /dev/null +++ b/crates/warp_util/src/listing_command_test.rs @@ -0,0 +1,423 @@ +use super::*; +use std::fs; +use std::path::{Path, PathBuf}; +use tempfile::tempdir; + +// Small convenience: set of listing commands for tests. Matches the default set +// unless a test specifically overrides it. +const LS_ONLY: &[&str] = &["ls"]; +const DEFAULTS: &[&str] = DEFAULT_LISTING_COMMANDS; + +/// Test-only wrapper matching the pre-alias signature of `listing_command_argument_dir`. +/// Most tests don't care about alias resolution, so pass `None` by default. Tests that +/// specifically exercise alias handling call `listing_command_argument_dir` directly. +fn listing_command_argument_dir( + command: &str, + pwd: &Path, + listing_commands: &[&str], +) -> Option { + super::listing_command_argument_dir(command, None, pwd, listing_commands) +} + +#[test] +fn returns_none_for_non_listing_command() { + let tmp = tempdir().unwrap(); + fs::create_dir(tmp.path().join("subdir")).unwrap(); + + assert_eq!( + listing_command_argument_dir("cat subdir/foo", tmp.path(), LS_ONLY), + None + ); + assert_eq!( + listing_command_argument_dir("git status", tmp.path(), LS_ONLY), + None + ); + assert_eq!( + listing_command_argument_dir("find . -name foo", tmp.path(), LS_ONLY), + None + ); +} + +#[test] +fn returns_none_for_ls_with_no_arg() { + let tmp = tempdir().unwrap(); + assert_eq!( + listing_command_argument_dir("ls", tmp.path(), LS_ONLY), + None + ); + assert_eq!( + listing_command_argument_dir("ls -la", tmp.path(), LS_ONLY), + None + ); + assert_eq!( + listing_command_argument_dir("ls --color=always", tmp.path(), LS_ONLY), + None + ); +} + +#[test] +fn returns_dir_for_ls_subdir() { + let tmp = tempdir().unwrap(); + fs::create_dir(tmp.path().join("subdir")).unwrap(); + + let got = listing_command_argument_dir("ls subdir", tmp.path(), LS_ONLY); + assert_eq!(got, Some(tmp.path().join("subdir"))); +} + +#[test] +fn returns_dir_for_ls_subdir_trailing_slash() { + let tmp = tempdir().unwrap(); + fs::create_dir(tmp.path().join("subdir")).unwrap(); + + let got = listing_command_argument_dir("ls subdir/", tmp.path(), LS_ONLY); + // PathBuf::join(Path::new("subdir/")) normalizes to "subdir", so compare canonically. + assert_eq!( + got.map(|p| p.canonicalize().unwrap()), + Some(tmp.path().join("subdir").canonicalize().unwrap()) + ); +} + +#[test] +fn skips_flags_before_positional() { + let tmp = tempdir().unwrap(); + fs::create_dir(tmp.path().join("subdir")).unwrap(); + + for cmd in &[ + "ls -la subdir", + "ls -la -A subdir", + "ls --color=always subdir", + "ls -l --color=always -A subdir", + ] { + let got = listing_command_argument_dir(cmd, tmp.path(), LS_ONLY); + assert_eq!( + got, + Some(tmp.path().join("subdir")), + "command {cmd:?} should resolve to subdir" + ); + } +} + +#[test] +fn returns_none_if_positional_does_not_exist() { + let tmp = tempdir().unwrap(); + // no subdir created + assert_eq!( + listing_command_argument_dir("ls nonexistent", tmp.path(), LS_ONLY), + None + ); +} + +#[test] +fn returns_none_if_positional_is_a_file_not_a_dir() { + let tmp = tempdir().unwrap(); + fs::write(tmp.path().join("somefile"), b"hi").unwrap(); + + // `ls somefile` is valid usage but it's listing a file, not a directory; a + // file argument gives no useful directory root for output, so we return None + // and let the default resolution (block.pwd) apply. + assert_eq!( + listing_command_argument_dir("ls somefile", tmp.path(), LS_ONLY), + None + ); +} + +#[test] +fn absolute_path_argument() { + let tmp = tempdir().unwrap(); + fs::create_dir(tmp.path().join("subdir")).unwrap(); + let abs = tmp.path().join("subdir"); + + // Pass an unrelated pwd; the absolute arg should win. + let unrelated = tempdir().unwrap(); + let got = + listing_command_argument_dir(&format!("ls {}", abs.display()), unrelated.path(), LS_ONLY); + assert_eq!(got, Some(abs)); +} + +#[test] +fn quoted_argument_with_spaces() { + let tmp = tempdir().unwrap(); + fs::create_dir(tmp.path().join("dir with spaces")).unwrap(); + + let got = listing_command_argument_dir(r#"ls "dir with spaces""#, tmp.path(), LS_ONLY); + assert_eq!(got, Some(tmp.path().join("dir with spaces"))); + + let got_single = listing_command_argument_dir(r#"ls 'dir with spaces'"#, tmp.path(), LS_ONLY); + assert_eq!(got_single, Some(tmp.path().join("dir with spaces"))); +} + +#[test] +fn rejects_multi_operand_commands() { + let tmp = tempdir().unwrap(); + fs::create_dir(tmp.path().join("a")).unwrap(); + fs::create_dir(tmp.path().join("b")).unwrap(); + fs::write(tmp.path().join("somefile"), b"hi").unwrap(); + + // Both positionals are directories → ambiguous, return None. + assert_eq!( + listing_command_argument_dir("ls a b", tmp.path(), LS_ONLY), + None + ); + assert_eq!( + listing_command_argument_dir("ls -la a b", tmp.path(), LS_ONLY), + None + ); + + // Dir + file → still multi-operand, output mixes roots, return None. + assert_eq!( + listing_command_argument_dir("ls a somefile", tmp.path(), LS_ONLY), + None + ); +} + +#[test] +fn returns_none_for_malformed_command() { + let tmp = tempdir().unwrap(); + // shlex::split returns None for unclosed quotes. + assert_eq!( + listing_command_argument_dir(r#"ls "unterminated"#, tmp.path(), LS_ONLY), + None + ); +} + +#[test] +fn returns_none_for_empty_command() { + let tmp = tempdir().unwrap(); + assert_eq!(listing_command_argument_dir("", tmp.path(), LS_ONLY), None); + assert_eq!( + listing_command_argument_dir(" ", tmp.path(), LS_ONLY), + None + ); +} + +#[test] +fn honors_custom_listing_command_set() { + let tmp = tempdir().unwrap(); + fs::create_dir(tmp.path().join("subdir")).unwrap(); + + // By default, `eza` is a listing command via DEFAULTS. + assert_eq!( + listing_command_argument_dir("eza subdir", tmp.path(), DEFAULTS), + Some(tmp.path().join("subdir")) + ); + + // But if the user narrows to just "ls", `eza` no longer triggers. + assert_eq!( + listing_command_argument_dir("eza subdir", tmp.path(), LS_ONLY), + None + ); + + // A user-defined alias like "ll" can be added to the set. + let user_set: &[&str] = &["ls", "ll"]; + assert_eq!( + listing_command_argument_dir("ll subdir", tmp.path(), user_set), + Some(tmp.path().join("subdir")) + ); +} + +#[test] +fn skips_leading_env_var_assignments() { + let tmp = tempdir().unwrap(); + fs::create_dir(tmp.path().join("subdir")).unwrap(); + + let got = listing_command_argument_dir("LS_COLORS=auto ls subdir", tmp.path(), LS_ONLY); + assert_eq!(got, Some(tmp.path().join("subdir"))); + + let got_multi = listing_command_argument_dir("FOO=1 BAR=2 ls subdir", tmp.path(), LS_ONLY); + assert_eq!(got_multi, Some(tmp.path().join("subdir"))); +} + +#[test] +fn tilde_expansion_of_argument() { + // Only run if HOME is set to a real directory that exists. + let Some(home) = dirs::home_dir() else { + return; + }; + if !home.is_dir() { + return; + } + + let got = listing_command_argument_dir("ls ~", home.parent().unwrap_or(&home), LS_ONLY); + assert_eq!(got, Some(home.clone())); + + // `~/` on its own expands to $HOME. + let got_slash = listing_command_argument_dir("ls ~/", home.parent().unwrap_or(&home), LS_ONLY); + assert_eq!(got_slash, Some(home)); +} + +#[test] +fn does_not_double_join_when_output_paths_are_already_rooted() { + // This test documents the intended contract: `listing_command_argument_dir` + // only extracts the command's literal directory argument. It does NOT and + // MUST NOT try to be clever about commands like `find DIR -name foo` where + // the output already contains `DIR/foo` — for those, the caller should not + // invoke this helper (or the command should not be in `listing_commands`). + // + // This is enforced by `listing_commands` being an allowlist: callers + // opt commands in explicitly. `find` is not in the default set. + let tmp = tempdir().unwrap(); + fs::create_dir(tmp.path().join("subdir")).unwrap(); + + assert_eq!( + listing_command_argument_dir("find subdir -name foo", tmp.path(), DEFAULTS), + None, + "find is not in DEFAULT_LISTING_COMMANDS — confirms allowlist semantics" + ); +} + +#[test] +fn realistic_ls_variants() { + let tmp = tempdir().unwrap(); + fs::create_dir(tmp.path().join("specs")).unwrap(); + + // The exact forms users type every day. + let variants = &[ + "ls specs", + "ls specs/", + "ls -la specs", + "ls -la specs/", + "ls -A specs", + "ls --color=always specs", + "ls -l --color=always specs", + ]; + for v in variants { + let got = listing_command_argument_dir(v, tmp.path(), LS_ONLY); + assert_eq!( + got.map(|p| p.canonicalize().unwrap()), + Some(tmp.path().join("specs").canonicalize().unwrap()), + "variant {v:?} should resolve to specs" + ); + } +} + +// -- Alias resolution tests -- +// +// These tests exercise the `resolved_command_name` override that callers use when +// they've already resolved shell aliases upstream (in Warp, via +// `Block::top_level_command(sessions)`). + +#[test] +fn alias_resolved_name_triggers_listing_lookup() { + let tmp = tempdir().unwrap(); + fs::create_dir(tmp.path().join("subdir")).unwrap(); + + // Raw command is `ll subdir` (the user's alias); resolved name is `ls`. + // `ll` is not in LS_ONLY, so without the resolved-name override this would + // return None. With the override, it resolves as if the command were `ls subdir`. + let got = super::listing_command_argument_dir("ll subdir", Some("ls"), tmp.path(), LS_ONLY); + assert_eq!(got, Some(tmp.path().join("subdir"))); +} + +#[test] +fn alias_resolved_name_respects_flags_in_raw_command() { + let tmp = tempdir().unwrap(); + fs::create_dir(tmp.path().join("subdir")).unwrap(); + + // Common alias: `alias la='ls -A'`. The user types `la subdir`; after alias + // expansion the effective command is `ls -A subdir`. We don't need to see the + // `-A` to parse correctly — our tokenizer still skips flags in the raw command + // and picks up `subdir` as the first positional. + let got = super::listing_command_argument_dir("la subdir", Some("ls"), tmp.path(), LS_ONLY); + assert_eq!(got, Some(tmp.path().join("subdir"))); +} + +#[test] +fn alias_resolved_name_overrides_raw_token_for_matching_only() { + let tmp = tempdir().unwrap(); + fs::create_dir(tmp.path().join("subdir")).unwrap(); + + // If the resolved name is NOT a listing command, we return None even if the + // raw first token (`ls`) would have matched. The resolved name is authoritative. + let got = super::listing_command_argument_dir("ls subdir", Some("bat"), tmp.path(), LS_ONLY); + assert_eq!(got, None); +} + +#[test] +fn alias_resolution_uses_raw_tokens_for_positional_extraction() { + let tmp = tempdir().unwrap(); + fs::create_dir(tmp.path().join("user_typed_dir")).unwrap(); + + // Known limitation: aliases with baked-in positional args are not expanded. + // `alias lsd='ls /tmp'; lsd user_typed_dir` → raw tokens are + // `["lsd", "user_typed_dir"]`, resolved name is `ls`. We pick + // `user_typed_dir` as the first positional (the user's typed arg), not + // `/tmp` (the aliased arg). This test documents that behavior. + let got = + super::listing_command_argument_dir("lsd user_typed_dir", Some("ls"), tmp.path(), LS_ONLY); + assert_eq!(got, Some(tmp.path().join("user_typed_dir"))); +} + +#[test] +fn alias_resolution_with_no_resolved_name_falls_back_to_raw_token() { + let tmp = tempdir().unwrap(); + fs::create_dir(tmp.path().join("subdir")).unwrap(); + + // When `resolved_command_name` is `None`, behavior matches the pre-alias + // contract (match against raw first token). + let got = super::listing_command_argument_dir("ls subdir", None, tmp.path(), LS_ONLY); + assert_eq!(got, Some(tmp.path().join("subdir"))); + + let got_miss = super::listing_command_argument_dir("ll subdir", None, tmp.path(), LS_ONLY); + assert_eq!(got_miss, None, "without resolved name, ll misses"); +} + +#[test] +fn rejects_recursive_flag_short() { + let tmp = tempdir().unwrap(); + fs::create_dir(tmp.path().join("subdir")).unwrap(); + + assert_eq!( + listing_command_argument_dir("ls -R subdir", tmp.path(), LS_ONLY), + None + ); + assert_eq!( + listing_command_argument_dir("ls -lR subdir", tmp.path(), LS_ONLY), + None + ); + assert_eq!( + listing_command_argument_dir("ls -laR subdir", tmp.path(), LS_ONLY), + None + ); +} + +#[test] +fn rejects_recursive_flag_long() { + let tmp = tempdir().unwrap(); + fs::create_dir(tmp.path().join("subdir")).unwrap(); + + assert_eq!( + listing_command_argument_dir("ls --recursive subdir", tmp.path(), LS_ONLY), + None + ); + assert_eq!( + listing_command_argument_dir("ls -la --recursive subdir", tmp.path(), LS_ONLY), + None + ); +} + +#[test] +fn rejects_directory_flag_short() { + let tmp = tempdir().unwrap(); + fs::create_dir(tmp.path().join("subdir")).unwrap(); + + // -d lists the directory operand itself, not entries under it. + assert_eq!( + listing_command_argument_dir("ls -d subdir", tmp.path(), LS_ONLY), + None + ); + assert_eq!( + listing_command_argument_dir("ls -ld subdir", tmp.path(), LS_ONLY), + None + ); +} + +#[test] +fn rejects_directory_flag_long() { + let tmp = tempdir().unwrap(); + fs::create_dir(tmp.path().join("subdir")).unwrap(); + + assert_eq!( + listing_command_argument_dir("ls --directory subdir", tmp.path(), LS_ONLY), + None + ); +}