terminal: Fix file paths links with URL escapes not being clickable
For #31827
URL Decoding Fix for Terminal File Path Clicking
Discussion
This change does not allow for paths that literally have %XX inside of them. If any such paths exist, they will fail to ctrl+click. A larger change would be needed to handle that.
Problem
In the terminal, you could ctrl+click file paths to open them in the editor, but this didn't work when the paths contained URL-encoded characters (percent-encoded sequences like %CE%BB for Greek letter λ).
Example Issue
- This worked:
dashboardλ.mts:3:8 - This didn't work:
dashboard%CE%BB.mts:3:8
The URL-encoded form %CE%BB represents the Greek letter λ (lambda), but the terminal wasn't decoding these sequences before trying to open the files.
Solution
Added URL decoding functionality to the terminal path detection system:
-
Added urlencoding dependency to
crates/terminal/Cargo.toml -
Created decode_file_path function in
crates/terminal/src/terminal.rsthat:- Attempts to decode URL-encoded paths using
urlencoding::decode() - Falls back to the original string if decoding fails
- Handles malformed encodings gracefully
- Attempts to decode URL-encoded paths using
-
Applied decoding to PathLikeTarget creation for both:
- Regular file paths detected by word regex
- File:// URLs that are treated as paths
Code Changes
New Function
/// Decodes URL-encoded file paths to handle cases where terminal output contains
/// percent-encoded characters (e.g., %CE%BB for λ).
/// Falls back to the original string if decoding fails.
fn decode_file_path(path: &str) -> String {
urlencoding::decode(path)
.map(|decoded| decoded.into_owned())
.unwrap_or_else(|_| path.to_string())
}
Modified PathLikeTarget Creation
The function is now called when creating PathLikeTarget instances:
- For file:// URLs:
decode_file_path(path) - For regular paths:
decode_file_path(&maybe_url_or_path)
Testing
Added comprehensive test coverage in test_decode_file_path() that verifies:
- Normal paths remain unchanged
- URL-encoded characters are properly decoded (λ, spaces, slashes)
- Paths with line numbers work correctly
- Invalid encodings fall back gracefully
- Mixed encoding scenarios work
Impact
This fix enables ctrl+click functionality for file paths containing non-ASCII characters that appear URL-encoded in terminal output, making the feature work consistently with tools that output percent-encoded file paths.
The change is backward compatible - all existing functionality continues to work unchanged, and the fix only activates when URL-encoded sequences are detected.
Release Notes:
- File paths printed in the terminal that have
%XXescape sequences will now be properly decoded so that ctrl+click will open them
| Warnings | |
|---|---|
| :warning: |
This PR is missing release notes. Please add a "Release Notes" section that describes the change:
If your change is not user-facing, you can use "N/A" for the entry:
|
Generated by :no_entry_sign: dangerJS against d49165ab949cf069c0c296dad9730ee6ea361d98
Updated PR with Release Notes.
Thanks for this! Out of interest, what tools output filenames with url-encoding?
Although it's unusual, dashboard%CE%BB.mts is a valid filename (and is distinct from dashboardλ.mts), so I'd prefer to fix the upstream tool to output valid filenames if possible. (For comparison, neither iTerm2 nor VSCodium do URL decoding like this).
Relatedly, in the case of file:// URLs, it seems like we should (but don't) support url-encoding.
Thanks for the response @ConradIrwin. The tool in question is dprint the rust library. It is specifically with file:// paths where encoding is used, not regular paths.
The behavior of vscode is to support file:// urls with encoding. Here is an actual error off of my terminal:
DprintError("Expected '{', got 'namespace' at file:///home/jason/code/acp7r/p-sprint-2025-q1/smart/df4l-admin/src/api/test2%CE%BB.mts:3:8
It would make sense to me to have this only apply to file://, and leave /regular/paths/alone
Do you agree with that assessment? I could revise the patch accordingly if you do.
Yes, that makes a lot more sense, thanks for clarifying!
Looks like maybe the issue is here: https://github.com/zed-industries/zed/blob/3b344532202d9ab3d723d310f0c62ba418963ae3/crates/terminal/src/terminal.rs#L1037, where we strip off the prefix but don't decode the result.
but don't decode the result
Later, MaybeNavigationTarget::PathLike is parsed as PathWithPosition::from_path.
Would not it be more beneficial to alter PathWithPosition::from_path to handle decodes?
I imagine, parsing such strings into file finder is also expected to be opened?
I don't think so. I have two files: %61.txt and a.txt, and I need to be able to open either with zed %61.txt or zed a.txt, so we can't treat those as the same.
On the other hand, in a file URL the former can only be represented as file:///blah/%2561.txt, so we do need to decode when parsing URLs.
@ConradIrwin I adjusted the implementation to keep it very focused on just decoding file:// urls, with a fallback to not decoding if there are unicode errors.
I have tested this on all my use cases, and it does not affect paths that are NOT file://
Please check it out!
I rebased on origin/main to resolve conflicts.
Hey @ConradIrwin can we get this merged in?
Sorry for dropping this!