Improve repo picker readability for long saved-repo paths#9451
Improve repo picker readability for long saved-repo paths#9451saucy-tech wants to merge 3 commits intowarpdotdev:masterfrom
Conversation
Saved repos with long shared parent paths (e.g. iCloud, Google Drive) collapse into visually identical truncated rows in the tab-config type="repo" picker, making it impossible to tell them apart at the moment of selection. See warpdotdev#9439. Three coordinated display-only changes: * Display labels run through warp_util::path::user_friendly_path so the home directory is abbreviated to `~`. The action and the cached self.selected continue to carry the raw absolute path so launched sessions resolve correctly. Pattern mirrors the existing usage in tab_configs/action_sidecar.rs and tab_configs/session_config_rendering.rs. * DropdownItem gains optional tooltip and max_lines builder methods, threading through to MenuItemFields::with_tooltip and MenuItemFields::new_multiline at the menu primitive layer. Other FilterableDropdown call sites are unaffected (both fields default to None). * Repo picker rows opt in to a 2-line soft-wrap and a tooltip that surfaces the full untruncated absolute path, so even after `~` substitution the user can recover the original on hover. Co-Authored-By: Oz <oz-agent@warp.dev>
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @saucy-tech on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
|
I ran into an unexpected error while working on this. You can view the conversation on Warp. Powered by Oz |
|
@cla-bot check |
|
The cla-bot has been summoned, and re-checked this pull request! |
|
I'm starting a first review of this pull request. I reviewed this pull request and requested human review from: @alokedesai. I left feedback as a comment so a maintainer can approve. Comment I completed the review and posted feedback on this pull request. Comment I completed the review and posted feedback on this pull request. Comment I completed the review and posted feedback on this pull request. Comment I completed the review and posted feedback on this pull request. Comment I completed the review and posted feedback on this pull request. Comment You can view the conversation on Warp. I completed the review and posted feedback on this pull request. 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 updates the tab config repo picker display so saved repo paths use a home-directory ~ abbreviation, wrap long rows to two lines, and expose the raw absolute path in a tooltip while preserving raw paths in selection actions.
Concerns
- No blocking correctness, security, or error-handling concerns found in the changed 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
|
Hey @saucy-tech, thanks so much for the PR! I think this is a great start, but I think we should go with this solution instead: #9439 (comment) I think the main change should be that we don't soft wrap and instead clip from the left instead of clip from the right. Once that change is made, this PR should be good to go! |
@bholmesdev asked to use @moirahuang's suggestion (left-clip instead of soft-wrap) while keeping the `~` substitution and full-path tooltip. This commit makes that swap: * Repo picker rows now left-truncate with a leading `…` when the `~`-abbreviated path exceeds `MAX_REPO_DISPLAY_LEN`, so the trailing repo name — the most distinctive part of the path — stays visible. * `DropdownItem::with_max_lines` and the multiline plumbing through `MenuItemFields::new_multiline` are removed; rows render as a single clipped line again. * `RepoPicker::selected_value` no longer falls back to the dropdown's `selected_item_label` (which would now be the formatted display string). `refresh_items` already mirrors the raw path into `self.selected`, so the fallback is unnecessary. `~` substitution and the hover tooltip are unchanged. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Overview
This PR changes the tab config repo picker to display friendlier saved-repo paths and adds tooltip support to dropdown items.
Concerns
- The picker now truncates the label before constructing each
DropdownItem, butFilterableDropdownfilters only againstdisplay_text, so users can no longer search for path segments that were elided and the intended two-line soft-wrap behavior is not implemented. - Preselection now looks up rows by formatted display text, which can collide after
~substitution/truncation and select the wrong underlying repo action. - Supplemental security pass found no security-specific issues.
Verdict
Found: 0 critical, 2 important, 0 suggestions
Request changes
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.
Overview
This PR abbreviates and truncates saved repo paths in the tab config repo picker, adds tooltip/filter metadata to dropdown items, and switches filtering to use the richer match text.
Concerns
- The picker now selects preselected repos by the clipped display label. When two saved repos truncate to the same visible label, the highlighted/default menu item can point at a different raw path than
self.selected, and pressing Enter can switch to the wrong repo.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
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.
Overview
This PR abbreviates and truncates repo picker labels, adds tooltip/filter metadata to dropdown items, and filters against the expanded match text.
Concerns
- The repo picker now supplies lossy/truncated labels, but the filterable dropdown still preserves and restores selection by label, so colliding displayed labels can cause keyboard selection to jump to a different repo action.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| let item_matches_filter = item | ||
| .filter_match_text() | ||
| .to_lowercase() | ||
| .contains(&filter_query); |
There was a problem hiding this comment.
display_text and calls set_selected_by_name; because the repo picker now passes a truncated label, duplicate labels can make filtering select/submit the wrong repo action. Preserve the current row by action or another unique key instead of the rendered label.
|
Hey @alokedesai made the change you asked for in 69e19c0! Oz has flagged a few follow-ups since; how would you like me to handle them? |
captainsafia
added
the
external-contributor
Warp Dev Github Integration
|
Mind addressing the comments and rerunning oz-review? once it's approved i'll take a look |
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @claude on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
saucy-tech
force-pushed
the
gh9439-repo-picker-paths
branch
from
47d1ca4 to
9bc883f
Compare
There was a problem hiding this comment.
Overview
This PR changes the tab-config repo picker to display friendlier repo paths, adds tooltip and filter-text support to dropdown items, and switches filterable dropdown selection preservation to use actions instead of rendered labels. A supplemental security pass did not identify security issues.
Concerns
RepoPicker::selected_value()no longer reports the raw path for an initially auto-selected row when there is no explicit default or previous selection, so repo params can render a selected repo but submit as empty until clicked.- The repo display text is still destructively left-truncated to a fixed character count, so paths with identical suffixes remain ambiguous in the picker despite the readability goal.
Verdict
Found: 0 critical, 2 important, 0 suggestions
Request changes
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.
Overview
This PR changes the tab config repo picker display path, adds tooltip/filter-text support to dropdown items, and updates FilterableDropdown selection tracking to use actions so truncated labels can collide safely.
Concerns
RepoPicker::selected_valuenow returns onlyself.selected, so a repo picker opened without an explicit/default path can visibly auto-select the first row while still reporting no usable repo value to callers such as required repo params.- The repo picker display path is hard-truncated to 40 characters rather than rendered as a two-line soft-wrapped label, so the main readability change described by the PR is not actually implemented and same-suffix paths can still collide visually until hover.
- Supplemental security pass found no additional security-specific findings.
Verdict
Found: 0 critical, 2 important, 0 suggestions
Request changes
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.
Overview
This PR improves the tab config repo picker by displaying home-relative paths, adding multiline dropdown rows, and exposing tooltips through the shared dropdown item API.
Concerns
- Repo picker filtering regresses for absolute paths under the user's home directory because items now match only the
~-substituted display text. Users who paste or type the full absolute path can no longer find matching saved repos even though the raw path is still the backing value.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
|
/oz-review |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I reviewed this pull request and requested human review from: @moirahuang. Comment Powered by Oz |
oz-for-oss
Bot
dismissed stale reviews from themself
Oz no longer requests changes for this pull request after the latest automated review.
oz-for-oss
Bot
dismissed stale reviews from themself
Oz no longer requests changes for this pull request after the latest automated review.
There was a problem hiding this comment.
Overview
This PR improves the tab config repo picker display by abbreviating home-directory paths, allowing saved repo rows to wrap over two lines, preserving selection identity by action instead of label, and adding tooltips/filter text for the raw absolute path. It also extends shared dropdown primitives in an opt-in way and updates affected generic bounds.
Concerns
- No blocking correctness issues found in the changed diff lines.
- Supplemental security pass: no security concerns identified; the full path tooltip is a local UI disclosure consistent with the stated picker behavior.
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
Oz is passing. Thank you Warp for going OSS, and allowing me to submit an issue and contribute the PR! |
alokedesai
left a comment
alokedesai
left a comment
There was a problem hiding this comment.
Thank you so much for working on this! I left a more detailed review. Once those get addressed, I think this is good to go :D
saucy-tech
force-pushed
the
gh9439-repo-picker-paths
branch
from
2a1e975 to
69e19c0
Compare
|
/oz-review |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR changes the tab config repo picker to display user-friendly saved-repo paths and adds dropdown tooltip support. The security pass found no issues.
Concerns
- Pre-selection now looks up rows by the formatted display string, which can collide after path abbreviation/truncation and select the wrong repo.
- Repo labels are still pre-truncated to a fixed character count, so the picker does not get a full label to soft-wrap and still discards path context in the row itself.
Verdict
Found: 0 critical, 2 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| // The dropdown looks up by `display_text`, so format the raw path | ||
| // with the same formatter used to build the items above. | ||
| if let Some(ref raw) = raw_to_select { | ||
| let display = format_display_path(raw, home.as_deref()); | ||
| dropdown.set_selected_by_name(display.as_str(), ctx); |
There was a problem hiding this comment.
~/truncated display string; select by the raw action instead.
| // The dropdown looks up by `display_text`, so format the raw path | |
| // with the same formatter used to build the items above. | |
| if let Some(ref raw) = raw_to_select { | |
| let display = format_display_path(raw, home.as_deref()); | |
| dropdown.set_selected_by_name(display.as_str(), ctx); | |
| if let Some(ref raw) = raw_to_select { | |
| dropdown.set_selected_by_action(RepoPickerAction::Select(raw.clone()), ctx); |
| /// the result is longer than [`MAX_REPO_DISPLAY_LEN`]. | ||
| fn format_display_path(raw_path: &str, home: Option<&str>) -> String { | ||
| let abbreviated = user_friendly_path(raw_path, home); | ||
| truncate_from_beginning(&abbreviated, MAX_REPO_DISPLAY_LEN) |
There was a problem hiding this comment.
Defends against label collision when two paths share a 40-char trailing suffix and render identical after `truncate_from_beginning`. Uses the existing public `FilterableDropdown::set_selected_by_action` — no new plumbing.
|
Hey @alokedesai — sorry for the back-and-forth here. I force-pushed back to That removes the broader Oz-driven additions from the previous version:
I made one small follow-up commit on top ( Oz is still asking for soft-wrap/multiline rows, but I left that out because your earlier direction was to avoid soft-wrap and clip from the left instead. If you'd prefer the multiline rendering after all, I can restore only that piece; otherwise I think the PR is now back to the intended scope. |
4 participants
Description
Fixes #9439.
The tab config
type = "repo"picker renders saved repos as raw, single-line, fixed-width entries. When several saved repos live deep inside paths like~/Library/Mobile Documents/iCloud~md~obsidian/Documents/..., the visible portion of two different rows can be byte-for-byte identical, so the user can't reliably pick the right one without trial and error.Three coordinated display-only changes, addressing @captainsafia's two asks plus the tooltip path flagged in Oz's original triage:
~substitution for the home directory in the displayed label. Labels run throughwarp_util::path::user_friendly_path— the same helper that's already used intab_configs/action_sidecar.rsandtab_configs/session_config_rendering.rs— so the convention stays consistent across the module. The dropdown action and the cachedself.selectedcontinue to carry the raw absolute path so launched sessions resolve correctly. Saved-repo persistence and thecdtarget are unaffected.DropdownItemgains an optionalwith_max_lines(n)builder that routes through to the existingMenuItemFields::new_multiline(label, max_lines), which usesText::soft_wrap(true)capped atmax_lines * font_size * line_height_ratio. The repo picker opts in to two lines.MenuItemFields::with_tooltip, exposed through a newwith_tooltipbuilder onDropdownItem. Even after~substitution the user can recover the original full path on hover.Both new
DropdownItembuilders are opt-in (default toNone), so otherFilterableDropdownconsumers are unaffected.Out of scope (left for follow-ups)
Testing
cargo fmt --check -p warpis clean.I was not able to run
cargo check,cargo nextest, orcargo clippylocally: only Xcode Command Line Tools is installed on this machine, not full Xcode, socrates/warpui/build.rsfails at the Metal shader compile step (xcrun: error: unable to find utility "metal"). I'm relying on the CI build for compile / clippy / tests verification on this PR. Happy to address whatever CI surfaces.The change leans on already-tested primitives:
warp_util::path::user_friendly_pathhas unit tests incrates/warp_util/src/path_test.rs.MenuItemFields::new_multilineandMenuItemFields::with_tooltipare existing menu primitives used elsewhere in the app.If a maintainer would like additional unit coverage I'm happy to add it;
repo_picker.rsitself wires thePersistedWorkspacesingleton, which doesn't have an obvious lightweight harness for the kind of unit test that would be most informative, and I'd appreciate guidance on where you'd want it to live.Agent Mode
Changelog Entries for Stable
CHANGELOG-IMPROVEMENT: Tab config repo picker now abbreviates the home directory to `~`, soft-wraps long paths to two lines, and surfaces the full path on hover so deeply nested saved repos stay distinguishable.