Skip to content

Fix file path detection across wrapped lines#9356

Open
Obed0101 wants to merge 1 commit intowarpdotdev:masterfrom
Obed0101:obed/fix-long-wrapped-file-paths
Open

Fix file path detection across wrapped lines#9356
Obed0101 wants to merge 1 commit intowarpdotdev:masterfrom
Obed0101:obed/fix-long-wrapped-file-paths

Conversation

@Obed0101
Copy link
Copy Markdown

Description

Fixes #9193.

This updates terminal file path detection so fragments split only by a soft-wrapped terminal line are treated as one continuous path. Fragments are still kept separate when either side contains a file-link separator.

A regression test covers the long wrapped path from the issue and verifies detection from both sides of the wrap boundary.

Testing

  • cargo fmt --check
  • cargo test -p warp terminal::model::grid::grid_handler::tests::test_possible_file_paths_across_wrapped_lines --no-default-features

Server API dependencies

None.

Changelog Entries for Stable

CHANGELOG-BUG-FIX: Fixed long file paths split across wrapped terminal lines not being detected as file links.

@cla-bot
Copy link
Copy Markdown

cla-bot Bot commented Apr 29, 2026

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 @Obed0101 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 @cla-bot check to trigger another check.

@oz-for-oss
Copy link
Copy Markdown
Contributor

oz-for-oss Bot commented Apr 29, 2026

@Obed0101

I'm starting a first review of this pull request.

You can follow along in the session on Warp.

I approved this pull request and requested human review from: @vorporeal, @alokedesai, @zachbai.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

I approved this pull request. No matching stakeholder was found for the changed files, so no human reviewers were requested.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

Copy link
Copy Markdown
Contributor

@oz-for-oss oz-for-oss Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overview

This PR fixes terminal file path detection across soft-wrapped lines by merging adjacent path fragments only when neither side contains a file-link separator, and adds a regression test for hovering on both sides of the wrap boundary.

Concerns

  • No blocking correctness, security, error-handling, or performance 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

Copy link
Copy Markdown
Contributor

@oz-for-oss oz-for-oss Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overview

This PR fixes file path candidate generation across soft-wrapped terminal rows by concatenating adjacent wrapped fragments only when neither side contains a file-link separator. It also adds a regression test covering hover detection on both sides of the wrap boundary.

Concerns

  • No blocking correctness or security 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

@Obed0101
Copy link
Copy Markdown
Author

@cla-bot check

@cla-bot cla-bot Bot added the cla-signed label Apr 29, 2026
@cla-bot
Copy link
Copy Markdown

cla-bot Bot commented Apr 29, 2026

The cla-bot has been summoned, and re-checked this pull request!

@alokedesai alokedesai requested review from kevinyang372 and removed request for alokedesai, vorporeal and zachbai April 29, 2026 16:27
@alokedesai
Copy link
Copy Markdown
Member

Thanks for the PR! @kevinyang372 could you take a look?

@kevinyang372
Copy link
Copy Markdown
Member

Hi @Obed0101 I am not able to repro the original issue. I also confirmed the regression test you added is in fact passing on current master too.

I don't think the bug is in the grid handler code and it might be platform related. Are you able to repro this?

@Obed0101
Copy link
Copy Markdown
Author

Obed0101 commented Apr 29, 2026

Thanks for checking. You're right — if the regression test passes on current master, it doesn't prove the issue.

I based the patch on the soft-wrap fragment merge logic and the contradictory comment there, but I agree we shouldn't merge this unless it reproduces the reported behavior or has a failing regression on master.

I'll take another pass to reproduce the original issue and trace whether the bug is actually in link validation/platform behavior instead of GridHandler. If I can't find a failing repro, I'm happy to close this PR to avoid merging an unnecessary change.

@kevinyang372
Copy link
Copy Markdown
Member

That sounds good. Thanks @Obed0101!

@captainsafia captainsafia added the external-contributor Indicates that a PR has been opened by someone outside the Warp team. label Apr 30, 2026 — with Warp Dev Github Integration
@Obed0101 Obed0101 force-pushed the obed/fix-long-wrapped-file-paths branch from fb6b469 to 6471374 Compare May 1, 2026 15:38
@Obed0101
Copy link
Copy Markdown
Author

Obed0101 commented May 1, 2026

I was able to reproduce the issue in the local app with a narrow terminal. The failure mode I saw was that Cmd-click sometimes resolved a valid prefix directory, for example .../wrappers/adapters, instead of the full existing README.md path when the path wrapped across several rows.

I updated the patch to handle paths spanning more than two soft-wrapped rows. The previous regression only covered a two-row wrap, which is why it did not prove the bug. The new regression covers a file path split across four wrapped rows and verifies that the full path candidate/range is produced from hover points on each wrapped row.

Validation:

cargo test -p warp terminal::model::grid::grid_handler::tests::test_possible_file_paths_across_multiple_wrapped_lines --no-default-features
cargo test -p warp terminal::model::grid::grid_handler::tests::test_possible_file_paths --no-default-features
cargo build --bin warp-oss --features gui,skip_login

I also verified manually in the local app that the narrow wrapped path now opens the full README.md instead of the prefix directory.

Copy link
Copy Markdown
Member

@kevinyang372 kevinyang372 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice find! This is definitely something we should fix

My high-level feedback here is this change is trying to do too many things:

  1. [Not controversial] For a link within max scan length, highlighting stops working when we have >2 softwrapped rows
  2. [More controversial] Also including some changes to bump overall scan length, this will have significant performance implications since now we are doing a lot more scanning when user moves their mouse

Let's focus on 1) and make sure we are scanning forward and backward in the grid until we fully fill up the max scan length limit.

Also I think the logic could be simplified a bit. Essentially what we want is:

  1. Iterate backwards until we either 1) hit a hard-wrapped line 2) fill up max scan limit. Push line range content into fragments
  2. Iterate forwards until we either 1) hit a hard-wrapped line 2) fill up max scan limit. Push line range content into fragments (some of the logic could definitely be shared with above)

@Obed0101 Obed0101 force-pushed the obed/fix-long-wrapped-file-paths branch from 6471374 to 3ce3596 Compare May 2, 2026 05:16
@Obed0101
Copy link
Copy Markdown
Author

Obed0101 commented May 2, 2026

Thanks, agreed. I narrowed the patch to preserve the existing file-link scan budget instead of increasing it.

The updated version still scans across multiple soft-wrapped rows, but only until the existing LINK_NUM_CHARACTER_SCAN budget is filled in each direction. That keeps the mouse-hover work bounded the same way as before, while fixing the >2 soft-wrapped row case when it fits inside that budget.

I also adjusted the regression coverage so it verifies the multi-row behavior within the existing scan limit.

Validation:

cargo test -p warp terminal::model::grid::grid_handler::tests::test_possible_file_paths --no-default-features

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed external-contributor Indicates that a PR has been opened by someone outside the Warp team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Warp doesn't parse long path

4 participants