fix(themes): Handle tilde in theme paths#9634
fix(themes): Handle tilde in theme paths#9634rrroyal wants to merge 6 commits intowarpdotdev:masterfrom
Conversation
|
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: @vorporeal. 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 adds tilde expansion and contraction for custom theme paths, including the settings-file path that uses SettingsValue and serde-based paths used for sync/serialization.
Concerns
- None.
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
captainsafia
added
the
external-contributor
Warp Dev Github Integration
vorporeal
requested review from
a team,
kevinchevalier and
vorporeal
and removed request for
a team and
vorporeal
|
Thank you for the submission! This works really well. Can you run |
done @kevinchevalier :) thanks for the quick review! |
There was a problem hiding this comment.
Thank you for the PR! Requesting changes because we should make sure this works on windows before merging it.
We should at least add tests to make sure tilde expansion and Windows paths (with backslashes) work too. I left some comments on the Windows portability aspect on a related PR here, in case the example there is useful.
| ); | ||
| } | ||
|
|
||
| #[cfg(not(windows))] |
There was a problem hiding this comment.
We should make sure this feature works on Windows, too. Could you add Windows-specific tests too?
4 participants
Description
When using settings sync between machines with different usernames, custom theme gets reset every time the settings sync. All settings reference mention
~/.warp/themes/my-theme.yaml, so figured it should support it. It didn't, so I (with some help from Copilot) have added it.Linked Issue
Mainly targeted at my own use case, but probably also #6678.
Screenshots / Videos
Testing
Verified in real-world with
./script/run/cargo runusing paths with and without~.Tested only on macOS - I don't have access to a Windows or Linux machine as of right now.
Also added tests for themes (
theme_test.rs) andwarp_core(theme_tests.rs). If they seem redundant, please let me know and I'll remove one of them.Agent Mode
CHANGELOG-IMPROVEMENT: Added support for tilde (
~) in custom theme paths.