Skip to content

fix: detect tree output filenames as file links#10004

Open
princepal9120 wants to merge 2 commits intowarpdotdev:masterfrom
princepal9120:prince/fix-tree-box-drawing-file-links
Open

fix: detect tree output filenames as file links#10004
princepal9120 wants to merge 2 commits intowarpdotdev:masterfrom
princepal9120:prince/fix-tree-box-drawing-file-links

Conversation

@princepal9120
Copy link
Copy Markdown
Contributor

@princepal9120 princepal9120 commented May 4, 2026

Summary

  • Treat common Unicode box-drawing glyphs as file-link separators
  • Add file-path candidate tests for tree-style output, including multibyte and absolute path leaves

Fixes #9909

Testing

  • Not run. Rust toolchain is not installed in this environment (cargo: command not found).

@cla-bot
Copy link
Copy Markdown

cla-bot Bot commented May 4, 2026

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Prince Pal.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email [email protected]
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@github-actions github-actions Bot added the external-contributor Indicates that a PR has been opened by someone outside the Warp team. label May 4, 2026
@oz-for-oss
Copy link
Copy Markdown
Contributor

oz-for-oss Bot commented May 4, 2026 •
edited
Loading

@princepal9120

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

You can view the conversation on Warp.

I completed the review and no human review was requested for this pull request.

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

Powered by Oz

oz-for-oss[bot]
oz-for-oss Bot previously requested changes May 4, 2026
View reviewed changes
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 extends file-link separator handling to common tree-output box-drawing glyphs and adds regression tests for tree-style path candidates.

Concerns

  • The added tree-output tests use whitespace before the filename, so they do not prove the new box-drawing separators are required; the existing space separator already isolates the filename.

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

Comment thread app/src/util/link_detection_test.rs Outdated
@princepal9120 princepal9120 force-pushed the prince/fix-tree-box-drawing-file-links branch from e6f9105 to 6515123 Compare May 4, 2026 13:24
@cla-bot cla-bot Bot added the cla-signed label May 4, 2026
@princepal9120
Copy link
Copy Markdown
Contributor Author

princepal9120 commented May 4, 2026

/oz-review

@oz-for-oss
Copy link
Copy Markdown
Contributor

oz-for-oss Bot commented May 4, 2026 •
edited
Loading

@princepal9120

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: @warpdotdev/oss-maintainers.

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

Powered by Oz

@oz-for-oss oz-for-oss Bot dismissed their stale review May 4, 2026 13:41

Oz no longer requests changes for this pull request after the latest automated review.

oz-for-oss[bot]
oz-for-oss Bot reviewed May 4, 2026
View reviewed changes
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 expands file-link separator handling to include common tree-style box-drawing glyphs and adds candidate-generation coverage for tree output, including multibyte filenames and absolute path leaves.

Concerns

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

@oz-for-oss oz-for-oss Bot requested review from a team and vorporeal and removed request for a team May 4, 2026 13:41
vorporeal
vorporeal approved these changes May 4, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@vorporeal vorporeal left a comment

Choose a reason for hiding this comment

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

thanks for the fix!

@vorporeal
Copy link
Copy Markdown
Contributor

vorporeal commented May 4, 2026

@princepal9120 you'll need to modify your commit so the author information matches your github account, and then sign our CLA - this is a requirement for us to accept contributions.

you can use the following to do so:

git commit --amend --author="New Name <[email protected]>" --no-edit

where [email protected] is an email address associated with your GitHub account.

@princepal9120
Copy link
Copy Markdown
Contributor Author

princepal9120 commented May 4, 2026

@princepal9120 you'll need to modify your commit so the author information matches your github account, and then sign our CLA - this is a requirement for us to accept contributions.

you can use the following to do so:

git commit --amend --author="New Name <[email protected]>" --no-edit

where [email protected] is an email address associated with your GitHub account.

Done

@vorporeal
Copy link
Copy Markdown
Contributor

vorporeal commented May 4, 2026

looks like some of the tests you added are failing, can you fix? the issue is a char/byte index mismatch issue - looks like you're trying to index using character indices, but rust's string indexing uses byte indices.

vorporeal
vorporeal reviewed May 5, 2026
View reviewed changes
Comment thread app/src/util/link_detection.rs
Comment on lines +230 to +235
#[cfg_attr(not(feature = "local_fs"), allow(dead_code))]
#[derive(Clone, Copy)]
struct SeparatorByteRange {
start: usize,
end: usize,
}
Copy link
Copy Markdown
Contributor

@vorporeal vorporeal May 5, 2026

Choose a reason for hiding this comment

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

we already have a string_offset::ByteOffset wrapper type that we've created specifically for semantically clarifying byte vs. char offsets like this - thoughts on using Range<ByteOffset> instead of the custom type?

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

Reviewers

@vorporeal vorporeal vorporeal approved these changes

@oz-for-oss oz-for-oss[bot] oz-for-oss[bot] left review comments

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Box-drawing characters not recognized as link separators; tree output never clickable

2 participants

@princepal9120 @vorporeal