diff --git a/src/commands/configure_shell.rs b/src/commands/configure_shell.rs index d07ce0ce91..87b4ab54df 100644 --- a/src/commands/configure_shell.rs +++ b/src/commands/configure_shell.rs @@ -1,11 +1,11 @@ use std::collections::HashSet; use std::fs::{self, OpenOptions}; -use std::io::{self, BufRead, BufReader, Write}; +use std::io::{self, Write}; use std::path::{Path, PathBuf}; use anstyle::Style; use worktrunk::path::format_path_for_display; -use worktrunk::shell::{self, Shell}; +use worktrunk::shell::{self, Shell, find_exact_line_in_file_tree}; use worktrunk::styling::{ INFO_SYMBOL, SUCCESS_SYMBOL, eprint, eprintln, format_bash_with_gutter, format_toml, format_with_gutter, hint_message, prompt_message, warning_message, @@ -466,31 +466,21 @@ fn configure_shell_file( // For other shells, check if file exists if path.exists() { - // Read the file and check if our integration already exists - let file = fs::File::open(path) - .map_err(|e| format!("Failed to read {}: {}", format_path_for_display(path), e))?; - - let reader = BufReader::new(file); - - // Check for the exact conditional wrapper we would write - for line in reader.lines() { - let line = line.map_err(|e| { + if let Some(configured_path) = + find_exact_line_in_file_tree(path, &config_line).map_err(|e| { format!( - "Failed to read line from {}: {}", + "Failed to scan {} for existing shell integration: {}", format_path_for_display(path), e ) - })?; - - // Canonical detection: check if the line matches exactly what we write - if line.trim() == config_line { - return Ok(Some(ConfigureResult { - shell, - path: path.to_path_buf(), - action: ConfigAction::AlreadyExists, - config_line: config_line.clone(), - })); - } + })? + { + return Ok(Some(ConfigureResult { + shell, + path: configured_path, + action: ConfigAction::AlreadyExists, + config_line: config_line.clone(), + })); } // Line doesn't exist, add it diff --git a/src/shell/detection.rs b/src/shell/detection.rs index 1375de3d08..1b1aab837d 100644 --- a/src/shell/detection.rs +++ b/src/shell/detection.rs @@ -7,7 +7,7 @@ use std::collections::HashSet; use std::fs; use std::io::{BufRead, BufReader}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use super::paths::{home_dir_required, powershell_profile_paths}; @@ -305,54 +305,241 @@ pub struct BypassAlias { pub content: String, } -/// Scan a single file for shell integration lines and potential false negatives. -fn scan_file(path: &std::path::Path, cmd: &str) -> Option { - let file = fs::File::open(path).ok()?; +/// Check whether a file or any shell config sourced from it contains integration. +pub fn file_tree_has_integration(path: &Path, cmd: &str) -> Result { + let mut seen = HashSet::new(); + Ok(file_tree_find_match(path, &mut seen, &|line| { + is_shell_integration_line(line, cmd) + })? + .is_some()) +} + +/// Find the first file in a sourced config tree containing an exact line. +pub fn find_exact_line_in_file_tree( + path: &Path, + expected_line: &str, +) -> Result, std::io::Error> { + let expected_line = expected_line.trim().to_string(); + let mut seen = HashSet::new(); + file_tree_find_match(path, &mut seen, &|line| line.trim() == expected_line) +} + +fn file_tree_find_match( + path: &Path, + seen: &mut HashSet, + predicate: &F, +) -> Result, std::io::Error> +where + F: Fn(&str) -> bool, +{ + if !mark_path_seen(path, seen) { + return Ok(None); + } + + let file = match fs::File::open(path) { + Ok(file) => file, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(None), + Err(err) => return Err(err), + }; + + for line in BufReader::new(file).lines() { + let line = line?; + if predicate(&line) { + return Ok(Some(path.to_path_buf())); + } + + for sourced_path in collect_sourced_paths(&line, path) { + if let Some(found_path) = file_tree_find_match(&sourced_path, seen, predicate)? { + return Ok(Some(found_path)); + } + } + } + + Ok(None) +} + +fn scan_file_recursive( + path: &Path, + cmd: &str, + seen: &mut HashSet, + results: &mut Vec, +) -> Result<(), std::io::Error> { + if !mark_path_seen(path, seen) { + return Ok(()); + } + + let file = match fs::File::open(path) { + Ok(file) => file, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(()), + Err(err) => return Err(err), + }; + let reader = BufReader::new(file); let mut matched_lines = Vec::new(); let mut unmatched_candidates = Vec::new(); let mut bypass_aliases = Vec::new(); + let mut sourced_paths = Vec::new(); - for (line_number, line) in reader.lines().map_while(Result::ok).enumerate() { + for (line_number, line) in reader.lines().enumerate() { + let line = line?; let line_number = line_number + 1; // 1-based let trimmed = line.trim(); - // Skip empty lines and comments - if trimmed.is_empty() || trimmed.starts_with('#') { - continue; - } - if is_shell_integration_line(&line, cmd) { - matched_lines.push(DetectedLine { - line_number, - content: line.clone(), - }); - } else if contains_cmd_at_word_boundary(&line, cmd) { - unmatched_candidates.push(DetectedLine { - line_number, - content: line.clone(), - }); - } + if !trimmed.is_empty() && !trimmed.starts_with('#') { + if is_shell_integration_line(&line, cmd) { + matched_lines.push(DetectedLine { + line_number, + content: line.clone(), + }); + } else if contains_cmd_at_word_boundary(&line, cmd) { + unmatched_candidates.push(DetectedLine { + line_number, + content: line.clone(), + }); + } - // Check for aliases that bypass shell integration - if let Some(alias) = detect_bypass_alias(trimmed, cmd, line_number) { - bypass_aliases.push(BypassAlias { - content: line.clone(), - ..alias - }); + if let Some(alias) = detect_bypass_alias(trimmed, cmd, line_number) { + bypass_aliases.push(BypassAlias { + content: line.clone(), + ..alias + }); + } } + + sourced_paths.extend(collect_sourced_paths(&line, path)); + } + + if !matched_lines.is_empty() || !unmatched_candidates.is_empty() || !bypass_aliases.is_empty() { + results.push(FileDetectionResult { + path: path.to_path_buf(), + matched_lines, + unmatched_candidates, + bypass_aliases, + }); } - // Only return if we found something interesting - if matched_lines.is_empty() && unmatched_candidates.is_empty() && bypass_aliases.is_empty() { + for sourced_path in sourced_paths { + scan_file_recursive(&sourced_path, cmd, seen, results)?; + } + + Ok(()) +} + +fn mark_path_seen(path: &Path, seen: &mut HashSet) -> bool { + let visited = fs::canonicalize(path).unwrap_or_else(|_| path.to_path_buf()); + seen.insert(visited) +} + +fn collect_sourced_paths(line: &str, current_file: &Path) -> Vec { + split_shell_segments(line) + .into_iter() + .filter_map(|segment| parse_source_segment(segment.trim(), current_file)) + .collect() +} + +fn split_shell_segments(line: &str) -> Vec<&str> { + let mut segments = vec![line]; + for separator in ["&&", "||", ";"] { + segments = segments + .into_iter() + .flat_map(|segment| segment.split(separator)) + .collect(); + } + segments +} + +fn parse_source_segment(segment: &str, current_file: &Path) -> Option { + let rest = segment + .strip_prefix("source ") + .or_else(|| segment.strip_prefix(". "))? + .trim_start(); + + let target = extract_shell_word(rest)?; + if target.starts_with("<(") + || target.starts_with("=(") + || target.contains("$(") + || target.contains('`') + || target.contains('*') + || target.contains('?') + { return None; } - Some(FileDetectionResult { - path: path.to_path_buf(), - matched_lines, - unmatched_candidates, - bypass_aliases, - }) + resolve_source_path(target, current_file) +} + +fn extract_shell_word(input: &str) -> Option<&str> { + let input = input.trim_start(); + if input.is_empty() { + return None; + } + + if let Some(rest) = input.strip_prefix('"') { + let end = rest.find('"')?; + return Some(&rest[..end]); + } + + if let Some(rest) = input.strip_prefix('\'') { + let end = rest.find('\'')?; + return Some(&rest[..end]); + } + + input.split_whitespace().next() +} + +fn resolve_source_path(target: &str, current_file: &Path) -> Option { + let resolved = expand_shell_path(target)?; + if resolved.is_absolute() { + return Some(resolved); + } + + Some( + current_file + .parent() + .unwrap_or_else(|| Path::new(".")) + .join(resolved), + ) +} + +fn expand_shell_path(target: &str) -> Option { + if target == "~" { + return home_dir_required().ok(); + } + + if let Some(rest) = target.strip_prefix("~/") { + return Some(home_dir_required().ok()?.join(rest)); + } + + if let Some(rest) = target.strip_prefix("${") { + let end = rest.find('}')?; + let var_name = &rest[..end]; + let suffix = &rest[end + 1..]; + return env_var_path(var_name, suffix); + } + + if let Some(rest) = target.strip_prefix('$') { + let var_len = rest + .chars() + .take_while(|ch| ch.is_ascii_alphanumeric() || *ch == '_') + .count(); + if var_len > 0 { + let var_name = &rest[..var_len]; + let suffix = &rest[var_len..]; + return env_var_path(var_name, suffix); + } + } + + Some(PathBuf::from(target)) +} + +fn env_var_path(var_name: &str, suffix: &str) -> Option { + let base = std::env::var_os(var_name).map(PathBuf::from)?; + let suffix = suffix.trim_start_matches(['/', '\\']); + if suffix.is_empty() { + Some(base) + } else { + Some(base.join(suffix)) + } } /// Detect if a line defines an alias that bypasses shell integration. @@ -466,14 +653,13 @@ pub fn scan_for_detection_details(cmd: &str) -> Result, config_files.extend(powershell_profile_paths(&home)); // Deduplicate and scan + let mut root_paths = HashSet::new(); let mut seen = HashSet::new(); for path in config_files { - if !seen.insert(path.clone()) || !path.exists() { + if !root_paths.insert(path.clone()) || !path.exists() { continue; } - if let Some(result) = scan_file(&path, cmd) { - results.push(result); - } + scan_file_recursive(&path, cmd, &mut seen, &mut results)?; } Ok(results) @@ -482,6 +668,7 @@ pub fn scan_for_detection_details(cmd: &str) -> Result, #[cfg(test)] mod tests { use super::*; + use crate::shell::Shell; use rstest::rstest; // ========================================================================== @@ -1079,4 +1266,119 @@ mod tests { let result = detect_bypass_alias(r#"alias vim="nvim""#, "wt", 1); assert!(result.is_none()); } + + #[test] + fn test_find_exact_line_in_file_tree_in_sourced_file() { + let temp_dir = tempfile::tempdir().unwrap(); + let zshrc = temp_dir.path().join(".zshrc"); + let sourced_dir = temp_dir.path().join(".zsh").join("config.d"); + fs::create_dir_all(&sourced_dir).unwrap(); + let sourced_file = sourced_dir.join("init.zsh"); + let config_line = Shell::Zsh.config_line("wt"); + + fs::write(&zshrc, "source .zsh/config.d/init.zsh\n").unwrap(); + fs::write(&sourced_file, format!("{config_line}\n")).unwrap(); + + assert_eq!( + find_exact_line_in_file_tree(&zshrc, &config_line).unwrap(), + Some(sourced_file) + ); + } + + #[test] + fn test_split_shell_segments_handles_multi_statement_line() { + let line = r#"[[ -f x ]] && source "dir file/init.zsh"; echo ok || source other.zsh"#; + let segments: Vec<_> = split_shell_segments(line) + .into_iter() + .map(str::trim) + .collect(); + + assert_eq!( + segments, + vec![ + "[[ -f x ]]", + r#"source "dir file/init.zsh""#, + "echo ok", + "source other.zsh" + ] + ); + } + + #[test] + fn test_extract_shell_word_handles_quotes_and_plain_paths() { + assert_eq!( + extract_shell_word(r#""dir file/init.zsh" trailing"#), + Some("dir file/init.zsh") + ); + assert_eq!( + extract_shell_word("'dir file/init.zsh' trailing"), + Some("dir file/init.zsh") + ); + assert_eq!( + extract_shell_word("plain/path/init.zsh trailing"), + Some("plain/path/init.zsh") + ); + } + + #[test] + fn test_expand_shell_path_expands_home_forms() { + let home = home_dir_required().unwrap(); + + assert_eq!(expand_shell_path("~/init.zsh"), Some(home.join("init.zsh"))); + assert_eq!( + expand_shell_path("$HOME/init.zsh"), + Some(home.join("init.zsh")) + ); + assert_eq!( + expand_shell_path("${HOME}/init.zsh"), + Some(home.join("init.zsh")) + ); + } + + #[test] + fn test_parse_source_segment_resolves_quoted_relative_path() { + let temp_dir = tempfile::tempdir().unwrap(); + let zshrc = temp_dir.path().join(".zshrc"); + + assert_eq!( + parse_source_segment(r#"source "dir file/init.zsh""#, &zshrc), + Some(temp_dir.path().join("dir file").join("init.zsh")) + ); + } + + #[test] + fn test_parse_source_segment_skips_dynamic_sources() { + let current_file = Path::new("/tmp/.zshrc"); + + for segment in [ + "source <(wt config shell init zsh)", + "source =(wt config shell init zsh)", + "source $(printf init.zsh)", + "source `printf init.zsh`", + "source *.zsh", + "source init?.zsh", + ] { + assert_eq!( + parse_source_segment(segment, current_file), + None, + "Expected dynamic source to be skipped: {segment}" + ); + } + } + + #[test] + fn test_collect_sourced_paths_finds_source_in_multi_statement_line() { + let temp_dir = tempfile::tempdir().unwrap(); + let zshrc = temp_dir.path().join(".zshrc"); + + let paths = collect_sourced_paths( + r#"[[ -f x ]] && source "dir file/init.zsh" && echo ok"#, + &zshrc, + ); + + assert_eq!( + paths, + vec![temp_dir.path().join("dir file").join("init.zsh")] + ); + } } diff --git a/src/shell/mod.rs b/src/shell/mod.rs index 5e3bfb9b7d..3ce228c5de 100644 --- a/src/shell/mod.rs +++ b/src/shell/mod.rs @@ -9,14 +9,12 @@ mod detection; mod paths; mod utils; -use std::io::{BufRead, BufReader}; - use askama::Template; // Re-export public types and functions pub use detection::{ - BypassAlias, DetectedLine, FileDetectionResult, is_shell_integration_line, - is_shell_integration_line_for_uninstall, scan_for_detection_details, + BypassAlias, DetectedLine, FileDetectionResult, find_exact_line_in_file_tree, + is_shell_integration_line, is_shell_integration_line_for_uninstall, scan_for_detection_details, }; pub use paths::{completion_path, config_paths, legacy_fish_conf_d_path}; pub use utils::{current_shell, detect_zsh_compinit, extract_filename_from_path}; @@ -137,13 +135,7 @@ impl Shell { /// Check if a file contains shell integration lines for the given command. fn file_has_integration(path: &std::path::Path, cmd: &str) -> Result { - let file = std::fs::File::open(path)?; - for line in BufReader::new(file).lines() { - if is_shell_integration_line(&line?, cmd) { - return Ok(true); - } - } - Ok(false) + detection::file_tree_has_integration(path, cmd) } } @@ -494,6 +486,24 @@ mod tests { assert!(!Shell::file_has_integration(&empty_file, "wt").unwrap()); } + #[test] + fn test_file_has_integration_in_sourced_file() { + let temp_dir = tempfile::tempdir().unwrap(); + let zshrc = temp_dir.path().join(".zshrc"); + let sourced_dir = temp_dir.path().join(".zsh").join("config.d"); + std::fs::create_dir_all(&sourced_dir).unwrap(); + let sourced_file = sourced_dir.join("init.zsh"); + + std::fs::write(&zshrc, "source .zsh/config.d/init.zsh\n").unwrap(); + std::fs::write( + &sourced_file, + r#"if command -v wt >/dev/null 2>&1; then eval "$(command wt config shell init zsh)"; fi"#, + ) + .unwrap(); + + assert!(Shell::file_has_integration(&zshrc, "wt").unwrap()); + } + // Note: is_shell_configured() is not unit-tested because it requires // mutating HOME env var (unsafe). It's tested indirectly via integration // tests that exercise the shell integration warning paths. diff --git a/tests/integration_tests/config_show.rs b/tests/integration_tests/config_show.rs index f601b025f3..64aacbec8c 100644 --- a/tests/integration_tests/config_show.rs +++ b/tests/integration_tests/config_show.rs @@ -2,6 +2,7 @@ use crate::common::{ TestRepo, repo, set_temp_home_env, set_xdg_config_path, setup_snapshot_settings, setup_snapshot_settings_with_home, temp_home, wt_command, }; +use ansi_str::AnsiStr; use insta_cmd::assert_cmd_snapshot; use rstest::rstest; use std::fs; @@ -663,6 +664,56 @@ if command -v wt >/dev/null 2>&1; then eval "$(command wt config shell init zsh) }); } +#[rstest] +fn test_config_show_detects_zsh_integration_in_sourced_file( + mut repo: TestRepo, + temp_home: TempDir, +) { + repo.setup_mock_ci_tools_unauthenticated(); + + let global_config_dir = temp_home.path().join(".config").join("worktrunk"); + fs::create_dir_all(&global_config_dir).unwrap(); + fs::write(global_config_dir.join("config.toml"), "").unwrap(); + + let sourced_dir = temp_home.path().join(".zsh").join("config.d"); + fs::create_dir_all(&sourced_dir).unwrap(); + fs::write( + temp_home.path().join(".zshrc"), + "source $HOME/.zsh/config.d/init.zsh\n", + ) + .unwrap(); + fs::write( + sourced_dir.join("init.zsh"), + r#"if command -v wt >/dev/null 2>&1; then eval "$(command wt config shell init zsh)"; fi +"#, + ) + .unwrap(); + + let mut cmd = wt_command(); + repo.configure_wt_cmd(&mut cmd); + repo.configure_mock_commands(&mut cmd); + cmd.arg("config").arg("show").current_dir(repo.root_path()); + set_temp_home_env(&mut cmd, temp_home.path()); + set_xdg_config_path(&mut cmd, temp_home.path()); + cmd.env("WORKTRUNK_TEST_COMPINIT_CONFIGURED", "1"); + + let output = cmd.output().unwrap(); + assert!(output.status.success()); + + let stderr = String::from_utf8_lossy(&output.stderr); + let clean = stderr.ansi_strip().into_owned(); + assert!( + clean.contains( + "zsh: Already configured shell extension & completions @ ~/.zsh/config.d/init.zsh:1" + ), + "Expected sourced zsh config to be detected, got:\n{clean}" + ); + assert!( + !clean.contains("zsh: Not configured shell extension & completions"), + "Expected zsh to be configured, got:\n{clean}" + ); +} + /// Test that config show displays fish shell with completions configured #[rstest] fn test_config_show_fish_with_completions(mut repo: TestRepo, temp_home: TempDir) {