Force code editor for global search matches so line jump always works - #15277
Force code editor for global search matches so line jump always works#15277arpansahu wants to merge 1 commit into
Conversation
Fixes warpdotdev#9657 Global search results carry a specific matched line/column, but when a matched file is a markdown file and the user prefers the Markdown Viewer, the match was opened via FileTarget::MarkdownViewer, which has no line_col parameter at all - it's structurally unable to jump to a line. This made search jump-to-line silently fail for markdown files, opening them at the top instead of at the match. Force FileTarget::CodeEditor for search-match opens (both local and remote paths), bypassing the Markdown Viewer preference specifically for this flow, since a match inherently needs a specific line shown and only the raw code editor supports jumping to it today. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
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 @arpansahu 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 |
github-actions
Bot
added
the
external-contributor
|
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 Powered by Oz |
![warp-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 changes global search result handling so matches that would open in the Markdown Viewer instead open in the code editor, preserving the requested line jump. No approved spec context was available, and the security pass did not identify security-specific findings.
Concerns
- This is a user-facing behavior change, but the PR does not include screenshots or a screen recording demonstrating a global search match in a Markdown file jumping to the matched line end to end. Per the repo guidance, author environment limitations do not exempt the PR from visual evidence; capture it from a local desktop run or an agent environment with computer use and attach it to the PR.
- One added remote-path comment now contradicts the code by saying it mirrors file tree Markdown Viewer handling even though this path intentionally no longer does so.
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
| // Local-fs-based target resolution can't inspect remote | ||
| // files; mirror the file tree's remote handling (code | ||
| // editor, or markdown viewer by extension + preference). | ||
| // | ||
| // Unlike the file tree, matches always carry a specific line, and the | ||
| // Markdown Viewer doesn't support jumping to it, so always use the code | ||
| // editor here rather than respecting `prefer_markdown_viewer`. |
There was a problem hiding this comment.
| // Local-fs-based target resolution can't inspect remote | |
| // files; mirror the file tree's remote handling (code | |
| // editor, or markdown viewer by extension + preference). | |
| // | |
| // Unlike the file tree, matches always carry a specific line, and the | |
| // Markdown Viewer doesn't support jumping to it, so always use the code | |
| // editor here rather than respecting `prefer_markdown_viewer`. | |
| // Local-fs-based target resolution can't inspect remote files, and matches | |
| // always carry a specific line. The Markdown Viewer doesn't support jumping to | |
| // it, so always use the code editor rather than respecting `prefer_markdown_viewer`. |
1 participant
Description
Fixes #9657.
Clicking a global search result opens the matching file, but when the file is markdown and the user has the "prefer Markdown Viewer" setting enabled, the match opened at the top of the file instead of jumping to the matched line.
Root cause
Tracing
GlobalSearchViewEvent::OpenMatch->LeftPanelEvent::OpenFileWithTarget->open_file_with_target:FileTarget::CodeEditor,line_colis correctly threaded throughopen_or_focus_existing->jump_to_line_col_in_tab->ScrollPosition::LineAndColumn, and it works.FileTarget::MarkdownViewer, the file is opened viaopen_file_notebook->FilePane::new, neither of which accepts aline_colparameter at all. It's structurally impossible for the Markdown Viewer to jump to a line today.So any search match in a markdown file, when the Markdown Viewer preference is on, silently loses the requested line and opens at the top.
Fix
In
LeftPanelView::handle_global_search_event(app/src/workspace/view/left_panel.rs), forceFileTarget::CodeEditorfor search-match opens, for both local and remote paths, overriding the Markdown Viewer preference specifically for this flow. A search match always carries a specific line to jump to, and only the code editor supports that today, so this is the minimal, targeted fix (the underlying Markdown Viewer limitation is a separate, larger gap that would need aline_colparameter threaded throughopen_file_notebook/FilePane, out of scope for this issue).Linked Issue
Type of Change
Testing
I don't have access to a macOS/Linux GUI environment capable of running the full Warp app locally in this sandbox (no Metal toolchain license acceptance possible), so I was not able to capture a screen recording per the template. To compensate, I verified correctness through:
OpenMatch->OpenFileWithTarget->open_file_with_target->open_code/open_file_notebook) confirmingline_colis dropped only in the Markdown Viewer path, and thatFileTarget::CodeEditor/FileTarget::MarkdownViewervariant signatures (EditorLayout) match my usage exactly.warppackage (cargo check -p warp --target x86_64-unknown-linux-gnu) successfully with zero errors, using a Linux cross-toolchain to route around the local Xcode/Metal licensing blocker (this repo's macOS-only Metal shader build is skipped for non-macOS targets).cargo clippy -p warp --target x86_64-unknown-linux-gnu -- -D warnings- zero warnings/errors.cargo fmt --check -p warp- clean.I'm happy to add a video/screenshot if a maintainer can point me to a way to test this locally, or if CI produces build artifacts I can run.