Add Shift+Click to extend terminal text selection (iTerm2-style)#11933
Add Shift+Click to extend terminal text selection (iTerm2-style)#11933cola-runner wants to merge 2 commits into
Conversation
|
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 @cola-runner 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 |
|
This PR is not linked to an issue that is marked with Issue-state enforcement details:
Readiness check:
To continue, link this PR to a same-repo issue such as Powered by Oz |
There was a problem hiding this comment.
This PR is not linked to an issue that is marked with ready-to-implement.
Issue-state enforcement details:
-
Associated same-repo issues checked: #11932
-
Required readiness label:
ready-to-implement
Readiness check:
- #11932: missing
ready-to-implement; readiness labels present: none
To continue, link this PR to a same-repo issue such as Closes #123 in the PR description, and make sure that issue has ready-to-implement.
Powered by Oz
|
@cla-bot check |
|
The cla-bot has been summoned, and re-checked this pull request! |
|
/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 adds a feature-flagged Shift+Click path that extends an existing block-list terminal text selection by updating the selection tail, plus the supporting selection action and feature-flag wiring. No approved spec context was available, and I did not find security concerns in the changed code.
Concerns
- For this user-facing terminal selection behavior, the PR does not include screenshots or a screen recording demonstrating the end-to-end interaction, and the description says the GUI has not been manually exercised. Please attach visual evidence from a desktop run or from a coding agent with computer use enabled.
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
|
Hi @cola-runner — a reviewer requested changes on this PR and it hasn't had activity from you in 7 days. When you get a chance, please push updates or reply to the review so a reviewer can take another look. Without activity, this PR will be automatically closed after 30 days of inactivity. |
7ac3967 to
23781d2
Compare
|
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 adds a feature-flagged Shift+Click path that extends an existing terminal text selection by dispatching a new SelectAction::Extend/BlockTextSelectAction::Extend variant and updating the block-list selection tail.
Concerns
- Shift+Click followed by dragging appears to lose the promised fine-tuning behavior because the existing drag handler treats held Shift as block-selection mode and suppresses text-selection updates.
- No security findings.
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
|
Updated the Testing section with macOS GUI verification for the cross-screen workflow, and linked the reviewed evidence: https://gist.github.com/cola-runner/37a80d8cf984abe661964e1b9d3fa9a9 /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: Comment Powered by Oz |
Oz no longer requests changes for this pull request after the latest automated review.
There was a problem hiding this comment.
Overview
This PR adds a feature-flagged Shift+Click path for extending existing block-list terminal text selections, adds the corresponding SelectAction::Extend handling, and wires the new compile-time feature flag into the app feature initialization. The PR description includes visual evidence for the user-facing behavior, and no approved spec context was provided for implementation drift checks.
Concerns
- No blocking correctness, security, or spec-alignment concerns found in the changed lines.
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
seemeroland
left a comment
There was a problem hiding this comment.
Thanks for taking a stab at this!
23781d2 to
3342f80
Compare
|
Updated this PR on top of latest
Validation on latest
|
|
Added another small test-only commit for edge coverage:
Validation after the test commit:
|
|
Hi @seemeroland, I addressed your review feedback in the latest updates:
Validation:
Would you mind taking another look when you have a chance? |
Move Shift+Click extension into shared selection-model logic that updates the nearest boundary, wire it through block-list and alt-screen selections, and keep Shift+Click+drag updating text selection. Add unit coverage for extending before/after selections and shrinking from inside the current range.
Cover same-row nearest-boundary shrink behavior, reversed block-list selections, and no-op extension when no selection exists.
3522604 to
8ccfbbe
Compare
|
Hi @seemeroland, following up here. I rebased this PR onto latest
All previous review threads are resolved, and the current head is cc @harryalbert @adrawbond-alto @BobbyWang0120 since you have recently touched nearby terminal view / mouse dispatch / terminal event paths. Would one of you be able to take a look or help route this to the right owner? |
Summary
Adds iTerm2-style Shift+Click to extend a terminal text selection. With a selection already present, Shift + left-click moves the nearest selection boundary to the clicked point:
A plain click still starts a new selection, and Shift+Click+drag can continue fine-tuning the extended text selection. This makes selecting long / multi-screen output much easier: pick a start, scroll, then Shift+Click the other side instead of drag-and-auto-scrolling the whole way.
Closes #3426
(Supersedes the duplicate reports #11932, #9963, and #4715 -- same iTerm2-style Shift+Click extend behavior.)
How it works
The shared selection model now exposes nearest-boundary extension logic, so block-list and alt-screen selections use the same behavior.
selection.rs): adds reusable nearest-boundary logic for moving start vs end when extending.block_list_element.rs,view.rs): Shift+Click dispatchesExtendwhen a simple text selection already exists and the click is not on a link / revealed secret. Shift+Click+drag continues dispatching text-selection updates instead of being misclassified as block selection.alt_screen_element.rs,alt_screen.rs,view.rs): Shift+Click extension is wired for full-screen apps as well.Scope / follow-ups
Testing
Verified after rebasing onto latest
upstream/master(f554cf76) with this PR applied:cargo test -p warp nearest_boundary --lib-- 10 passedcargo test -p warp test_alt_screen_extend_selection_without_existing_selection_noops --lib-- 1 passedcargo check -p warpEarlier macOS GUI evidence for the cross-screen block-list workflow is here: https://gist.github.com/cola-runner/37a80d8cf984abe661964e1b9d3fa9a9
Generated with Claude Code