From b239c3ec4a3e6fa959cd63cea4ec14914b6da5e2 Mon Sep 17 00:00:00 2001 From: JonCholas <125853206+JonCholas@users.noreply.github.com> Date: Sun, 9 Mar 2025 01:45:06 +1100 Subject: [PATCH] Avoid consider PATH updated when the export is commented in the shellrc (#12043) ## Summary The way the `tool update-shell` checks if the command to export the PATH exists or not in the RC files is a blind search, and therefore if finds the command inside comments. example with .zshenv This content ``` # uv # export PATH="/Users/cholas/.local/bin:$PATH" ``` Generates the following msg ``` error: The executable directory /Users/cholas/.local/bin is not in PATH, but the Zsh configuration files are already up-to-date ``` With this change, that content won't be considered as configured and the following will be added ``` # uv export PATH="/Users/cholas/.local/bin:$PATH" ``` This will make the `update-shell` more reliable ## Test Plan I tested with and without the change with commented export in zsh in mac. Tested running `cargo run -- tool update-shell` --------- Co-authored-by: Charlie Marsh --- crates/uv/src/commands/tool/update_shell.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/uv/src/commands/tool/update_shell.rs b/crates/uv/src/commands/tool/update_shell.rs index 36cf7c91cdcd..f306d80fea23 100644 --- a/crates/uv/src/commands/tool/update_shell.rs +++ b/crates/uv/src/commands/tool/update_shell.rs @@ -73,7 +73,12 @@ pub(crate) async fn update_shell(printer: Printer) -> Result { // Search for the command in the file, to avoid redundant updates. match fs_err::tokio::read_to_string(&file).await { Ok(contents) => { - if contents.contains(&command) { + if contents + .lines() + .map(str::trim) + .filter(|line| !line.starts_with('#')) + .any(|line| line.contains(&command)) + { debug!( "Skipping already-updated configuration file: {}", file.simplified_display()