acp_thread: Support absolute file mentions with row fragments (#53229)
Sometimes the contents in the agent's thread are direct file links. Codex ACP almost every time uses absolute file paths instead of file://path/to/file. This is resulting in an error described in the #49978 . Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Closes #49978 Release Notes: - Fixed absolute path mentions in `acp_thread` so direct file references (including `:line` and `#Lline` formats) now open correctly instead of resolving to invalid/empty files. --------- Co-authored-by: Eric Holk <eric@zed.dev>
This commit is contained in:
RevantarkandEric Holk authored and GitHub committed
1 parent
5a6fadf854
commit
9f29a9ebb8
1 file changed
+189
-1
@@ -12,7 +12,10 @@ use std::{
|
||||
use ui::{App, IconName, SharedString};
|
||||
use url::Url;
|
||||
use urlencoding::decode;
|
||||
use util::{ResultExt, paths::PathStyle};
|
||||
use util::{
|
||||
ResultExt,
|
||||
paths::{PathStyle, PathWithPosition, is_absolute},
|
||||
};
|
||||
|
||||
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize, Hash)]
|
||||
pub enum MentionUri {
|
||||
@@ -65,6 +68,11 @@ pub enum MentionUri {
|
||||
|
||||
impl MentionUri {
|
||||
pub fn parse(input: &str, path_style: PathStyle) -> Result<Self> {
|
||||
let input = input
|
||||
.strip_prefix('`')
|
||||
.and_then(|input| input.strip_suffix('`'))
|
||||
.unwrap_or(input);
|
||||
|
||||
fn parse_line_range(fragment: &str) -> Result<RangeInclusive<u32>> {
|
||||
let range = fragment.strip_prefix("L").unwrap_or(fragment);
|
||||
|
||||
@@ -92,6 +100,39 @@ impl MentionUri {
|
||||
Ok(start_line..=end_line)
|
||||
}
|
||||
|
||||
let parse_absolute_path = |input: &str| -> Result<Self> {
|
||||
let (path_input, fragment) = input
|
||||
.split_once('#')
|
||||
.map_or((input, None), |(path, fragment)| (path, Some(fragment)));
|
||||
|
||||
if let Some(fragment) = fragment.and_then(|fragment| parse_line_range(fragment).ok()) {
|
||||
return Ok(MentionUri::Selection {
|
||||
abs_path: Some(path_input.into()),
|
||||
line_range: fragment,
|
||||
});
|
||||
}
|
||||
|
||||
let path_with_position = PathWithPosition::parse_str(path_input);
|
||||
let abs_path = path_with_position.path;
|
||||
if let Some(row) = path_with_position.row {
|
||||
let line = row
|
||||
.checked_sub(1)
|
||||
.context("Line numbers should be 1-based")?;
|
||||
// TODO: Preserve column info too.
|
||||
Ok(MentionUri::Selection {
|
||||
abs_path: Some(abs_path),
|
||||
line_range: line..=line,
|
||||
})
|
||||
} else {
|
||||
Ok(MentionUri::File { abs_path })
|
||||
}
|
||||
};
|
||||
|
||||
if is_absolute(input, path_style) && !input.contains("://") {
|
||||
return parse_absolute_path(input)
|
||||
.with_context(|| format!("Invalid absolute path mention URI: {input}"));
|
||||
}
|
||||
|
||||
let url = url::Url::parse(input)?;
|
||||
let path = url.path();
|
||||
match url.scheme() {
|
||||
@@ -737,6 +778,153 @@ mod tests {
|
||||
assert!(MentionUri::parse("zed:///agent/unknown/test", PathStyle::local()).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_parse_absolute_file_path() {
|
||||
let file_path = path!("/path/to/file.rs");
|
||||
let parsed = MentionUri::parse(file_path, PathStyle::local()).unwrap();
|
||||
match &parsed {
|
||||
MentionUri::File { abs_path } => {
|
||||
assert_eq!(abs_path, Path::new(file_path));
|
||||
}
|
||||
_ => panic!("Expected File variant"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_parse_absolute_file_path_with_row() {
|
||||
let file_path = "/path/to/file.rs:42";
|
||||
let parsed = MentionUri::parse(file_path, PathStyle::Posix).unwrap();
|
||||
match &parsed {
|
||||
MentionUri::Selection {
|
||||
abs_path: path,
|
||||
line_range,
|
||||
} => {
|
||||
assert_eq!(path.as_ref().unwrap(), Path::new("/path/to/file.rs"));
|
||||
assert_eq!(line_range.start(), &41);
|
||||
assert_eq!(line_range.end(), &41);
|
||||
}
|
||||
_ => panic!("Expected Selection variant"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_parse_absolute_file_path_with_fragment_line() {
|
||||
let file_path = "/path/to/file.rs#L42";
|
||||
let parsed = MentionUri::parse(file_path, PathStyle::Posix).unwrap();
|
||||
match &parsed {
|
||||
MentionUri::Selection {
|
||||
abs_path: path,
|
||||
line_range,
|
||||
} => {
|
||||
assert_eq!(path.as_ref().unwrap(), Path::new("/path/to/file.rs"));
|
||||
assert_eq!(line_range.start(), &41);
|
||||
assert_eq!(line_range.end(), &41);
|
||||
}
|
||||
_ => panic!("Expected Selection variant"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_parse_absolute_windows_path() {
|
||||
let file_path = "C:\\Users\\zed\\project\\main.rs";
|
||||
let parsed = MentionUri::parse(file_path, PathStyle::Windows).unwrap();
|
||||
match &parsed {
|
||||
MentionUri::File { abs_path } => {
|
||||
assert_eq!(abs_path, Path::new("C:\\Users\\zed\\project\\main.rs"));
|
||||
}
|
||||
_ => panic!("Expected File variant"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_parse_absolute_windows_file_path_with_row() {
|
||||
let file_path = "C:\\Users\\zed\\project\\main.rs:42";
|
||||
let parsed = MentionUri::parse(file_path, PathStyle::Windows).unwrap();
|
||||
match &parsed {
|
||||
MentionUri::Selection {
|
||||
abs_path: path,
|
||||
line_range,
|
||||
} => {
|
||||
assert_eq!(
|
||||
path.as_ref().unwrap(),
|
||||
Path::new("C:\\Users\\zed\\project\\main.rs")
|
||||
);
|
||||
assert_eq!(line_range.start(), &41);
|
||||
assert_eq!(line_range.end(), &41);
|
||||
}
|
||||
_ => panic!("Expected Selection variant"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_parse_absolute_windows_file_path_with_fragment_line() {
|
||||
let file_path = "C:\\Users\\zed\\project\\main.rs#L42";
|
||||
let parsed = MentionUri::parse(file_path, PathStyle::Windows).unwrap();
|
||||
match &parsed {
|
||||
MentionUri::Selection {
|
||||
abs_path: path,
|
||||
line_range,
|
||||
} => {
|
||||
assert_eq!(
|
||||
path.as_ref().unwrap(),
|
||||
Path::new("C:\\Users\\zed\\project\\main.rs")
|
||||
);
|
||||
assert_eq!(line_range.start(), &41);
|
||||
assert_eq!(line_range.end(), &41);
|
||||
}
|
||||
_ => panic!("Expected Selection variant"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_parse_backticked_absolute_file_path() {
|
||||
let file_path = "`/path/to/file.rs`";
|
||||
let parsed = MentionUri::parse(file_path, PathStyle::Posix).unwrap();
|
||||
match &parsed {
|
||||
MentionUri::File { abs_path } => {
|
||||
assert_eq!(abs_path, Path::new("/path/to/file.rs"));
|
||||
}
|
||||
_ => panic!("Expected File variant"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_parse_backticked_absolute_file_path_with_fragment_line() {
|
||||
let file_path = "`/path/to/file.rs#L42`";
|
||||
let parsed = MentionUri::parse(file_path, PathStyle::Posix).unwrap();
|
||||
match &parsed {
|
||||
MentionUri::Selection {
|
||||
abs_path: path,
|
||||
line_range,
|
||||
} => {
|
||||
assert_eq!(path.as_ref().unwrap(), Path::new("/path/to/file.rs"));
|
||||
assert_eq!(line_range.start(), &41);
|
||||
assert_eq!(line_range.end(), &41);
|
||||
}
|
||||
_ => panic!("Expected Selection variant"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_parse_backticked_absolute_windows_file_path_with_fragment_line() {
|
||||
let file_path = "`C:\\Users\\zed\\project\\main.rs#L42`";
|
||||
let parsed = MentionUri::parse(file_path, PathStyle::Windows).unwrap();
|
||||
match &parsed {
|
||||
MentionUri::Selection {
|
||||
abs_path: path,
|
||||
line_range,
|
||||
} => {
|
||||
assert_eq!(
|
||||
path.as_ref().unwrap(),
|
||||
Path::new("C:\\Users\\zed\\project\\main.rs")
|
||||
);
|
||||
assert_eq!(line_range.start(), &41);
|
||||
assert_eq!(line_range.end(), &41);
|
||||
}
|
||||
_ => panic!("Expected Selection variant"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_single_line_number() {
|
||||
// https://github.com/zed-industries/zed/issues/46114
|
||||
|
||||
Reference in new issue
Block a user