fix(warpify): preserve auto-warpify snippet path separators on Windows#10595
fix(warpify): preserve auto-warpify snippet path separators on Windows#10595cesaryuan wants to merge 2 commits intowarpdotdev:masterfrom
Conversation
Co-Authored-By: Warp <agent@warp.dev>
github-actions
Bot
added
the
external-contributor
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I reviewed this pull request and requested human review from: Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR changes Warpify RC-file path generation from host-formatted PathBufs to snippet-safe strings, updates the success-block rendering to use those strings directly, and adds a regression test for target-OS path separators.
Concerns
No blocking correctness or security concerns found in the annotated diff.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
There was a problem hiding this comment.
Pull request overview
Fixes the auto-warpify “append to RC file” snippet rendering when Warp is running on Windows by preventing host-OS PathBuf formatting from introducing Windows-style separators into POSIX shell snippets.
Changes:
- Changed
ShellType::rc_file_pathsto return snippet-readyStringpaths instead ofPathBufs. - Updated Warpify success-block snippet generation to use the returned strings directly.
- Added a regression test asserting forward-slash paths for snippet display.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| crates/warp_terminal/src/shell/mod.rs | Switches RC “paths for snippet display” from PathBuf joins to preformatted strings to avoid host-OS separators. |
| crates/warp_terminal/src/shell/mod_tests.rs | Adds a unit test covering snippet-display path formatting (esp. when targeting Linux on Windows). |
| app/src/terminal/warpify/mod.rs | Removes PathBuf→UTF-8 conversion/placeholder logic and uses the new String paths directly when templating the snippet. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// Returns RC file paths for shell snippets using separators that match the target OS. | ||
| pub fn rc_file_paths(&self, os: TargetOS) -> Vec<String> { | ||
| let home_dir = match os { | ||
| TargetOS::Windows => "$HOME", | ||
| _ => "~", | ||
| }); | ||
| }; | ||
| let relative_paths = match (self, os) { | ||
| (ShellType::PowerShell, TargetOS::Windows) => { | ||
| vec![Path::new( | ||
| ".config/powershell/Microsoft.PowerShell_profile.ps1", | ||
| )] | ||
| vec![".config/powershell/Microsoft.PowerShell_profile.ps1"] | ||
| } | ||
| // We need to make sure this works for either editor of PowerShell (PowerShell Core or | ||
| // Windows PowerShell) so just write the file to both. | ||
| (ShellType::PowerShell, _) => vec![ | ||
| Path::new("Documents/PowerShell/Microsoft.PowerShell_profile.ps1"), | ||
| Path::new("Documents/WindowsPowerShell/Microsoft.PowerShell_profile.ps1"), | ||
| "Documents/PowerShell/Microsoft.PowerShell_profile.ps1", | ||
| "Documents/WindowsPowerShell/Microsoft.PowerShell_profile.ps1", | ||
| ], | ||
| (_, TargetOS::Windows) => vec![], | ||
| (ShellType::Bash, _) => vec![Path::new(".bashrc")], | ||
| (ShellType::Zsh, _) => vec![Path::new(".zshrc")], | ||
| (ShellType::Fish, _) => vec![Path::new(".config/fish/config.fish")], | ||
| (ShellType::Bash, _) => vec![".bashrc"], | ||
| (ShellType::Zsh, _) => vec![".zshrc"], | ||
| (ShellType::Fish, _) => vec![".config/fish/config.fish"], | ||
| }; | ||
|
|
||
| relative_paths | ||
| .iter() | ||
| .map(|relative_path| home_dir.join(relative_path)) | ||
| .into_iter() | ||
| .map(|relative_path| format!("{home_dir}/{relative_path}")) | ||
| .collect() |
There was a problem hiding this comment.
Updated in 75aba30. The implementation is intentionally keeping forward slashes for snippet output, so I changed the docstring to describe the real invariant instead of suggesting it varies by target OS separator style.
| #[test] | ||
| fn test_rc_file_paths_use_target_os_separators_for_display() { | ||
| assert_eq!( | ||
| ShellType::Bash.rc_file_paths(TargetOS::Linux), | ||
| vec!["~/.bashrc".to_string()] | ||
| ); | ||
| assert_eq!( | ||
| ShellType::Fish.rc_file_paths(TargetOS::Linux), | ||
| vec!["~/.config/fish/config.fish".to_string()] | ||
| ); | ||
| assert_eq!( | ||
| ShellType::PowerShell.rc_file_paths(TargetOS::Windows), | ||
| vec!["$HOME/.config/powershell/Microsoft.PowerShell_profile.ps1".to_string()] | ||
| ); | ||
| } |
There was a problem hiding this comment.
Updated in 75aba30. I renamed the regression test to make the intent explicit: we are asserting stable forward-slash paths for shell snippets, not generic target-OS path semantics.
Co-Authored-By: Warp <agent@warp.dev>
|
Addressed the Copilot review points in 75aba30. I clarified that |
Description
This fixes the auto-warpify command snippet shown after Warpify on Windows.
Previously, the snippet built RC file paths through host-platform
PathBufformatting. When Warp was running on Windows, that caused the displayed RC file path in the generated command to pick up Windows-style separators instead of the separator style expected by the target shell snippet. In practice, the reminder command could show the wrong path format for the shell snippet we were asking the user to copy.This change makes
rc_file_paths()return snippet-safe strings using target-OS separators, updates the Warpify success block to use those strings directly, and adds a regression test covering the displayed path format.Linked Issue
ready-to-specorready-to-implement.Testing
./script/runcargo fmt --allcargo clippy -p warp_terminal --all-targets -- -D warningscargo test -p warp_terminal rc_file_paths_use_target_os_separators_for_displayNo integration test was added because this change is isolated to path-string generation for the displayed shell snippet and is covered with a targeted regression test.
Agent Mode
Co-Authored-By: Warp agent@warp.dev