Cole Miller created
Bug introduced in #35926
Release Notes:
- N/A.github/actions/run_tests_windows/action.yml | 1
crates/acp_thread/src/acp_thread.rs | 14 ++
crates/acp_thread/src/mention.rs | 83 ++++++++----------
crates/agent_ui/src/acp/message_editor.rs | 93 ++++++++-------------
crates/fs/src/fake_git_repo.rs | 6
crates/fs/src/fs.rs | 4
6 files changed, 87 insertions(+), 114 deletions(-)
@@ -56,7 +56,6 @@ runs:
$env:COMPlus_CreateDumpDiagnostics = "1"
cargo nextest run --workspace --no-fail-fast
- continue-on-error: true
- name: Analyze crash dumps
if: always()
@@ -2155,7 +2155,7 @@ mod tests {
"}
);
});
- assert_eq!(fs.files(), vec![Path::new("/test/file-0")]);
+ assert_eq!(fs.files(), vec![Path::new(path!("/test/file-0"))]);
cx.update(|cx| thread.update(cx, |thread, cx| thread.send(vec!["ipsum".into()], cx)))
.await
@@ -2185,7 +2185,10 @@ mod tests {
});
assert_eq!(
fs.files(),
- vec![Path::new("/test/file-0"), Path::new("/test/file-1")]
+ vec![
+ Path::new(path!("/test/file-0")),
+ Path::new(path!("/test/file-1"))
+ ]
);
// Checkpoint isn't stored when there are no changes.
@@ -2226,7 +2229,10 @@ mod tests {
});
assert_eq!(
fs.files(),
- vec![Path::new("/test/file-0"), Path::new("/test/file-1")]
+ vec![
+ Path::new(path!("/test/file-0")),
+ Path::new(path!("/test/file-1"))
+ ]
);
// Rewinding the conversation truncates the history and restores the checkpoint.
@@ -2254,7 +2260,7 @@ mod tests {
"}
);
});
- assert_eq!(fs.files(), vec![Path::new("/test/file-0")]);
+ assert_eq!(fs.files(), vec![Path::new(path!("/test/file-0"))]);
}
async fn run_until_first_tool_call(
@@ -52,6 +52,7 @@ impl MentionUri {
let path = url.path();
match url.scheme() {
"file" => {
+ let path = url.to_file_path().ok().context("Extracting file path")?;
if let Some(fragment) = url.fragment() {
let range = fragment
.strip_prefix("L")
@@ -72,23 +73,17 @@ impl MentionUri {
if let Some(name) = single_query_param(&url, "symbol")? {
Ok(Self::Symbol {
name,
- path: path.into(),
+ path,
line_range,
})
} else {
- Ok(Self::Selection {
- path: path.into(),
- line_range,
- })
+ Ok(Self::Selection { path, line_range })
}
} else {
- let abs_path =
- PathBuf::from(format!("{}{}", url.host_str().unwrap_or(""), path));
-
if input.ends_with("/") {
- Ok(Self::Directory { abs_path })
+ Ok(Self::Directory { abs_path: path })
} else {
- Ok(Self::File { abs_path })
+ Ok(Self::File { abs_path: path })
}
}
}
@@ -162,27 +157,17 @@ impl MentionUri {
pub fn to_uri(&self) -> Url {
match self {
MentionUri::File { abs_path } => {
- let mut url = Url::parse("file:///").unwrap();
- let path = abs_path.to_string_lossy();
- url.set_path(&path);
- url
+ Url::from_file_path(abs_path).expect("mention path should be absolute")
}
MentionUri::Directory { abs_path } => {
- let mut url = Url::parse("file:///").unwrap();
- let mut path = abs_path.to_string_lossy().to_string();
- if !path.ends_with("/") {
- path.push_str("/");
- }
- url.set_path(&path);
- url
+ Url::from_directory_path(abs_path).expect("mention path should be absolute")
}
MentionUri::Symbol {
path,
name,
line_range,
} => {
- let mut url = Url::parse("file:///").unwrap();
- url.set_path(&path.to_string_lossy());
+ let mut url = Url::from_file_path(path).expect("mention path should be absolute");
url.query_pairs_mut().append_pair("symbol", name);
url.set_fragment(Some(&format!(
"L{}:{}",
@@ -192,8 +177,7 @@ impl MentionUri {
url
}
MentionUri::Selection { path, line_range } => {
- let mut url = Url::parse("file:///").unwrap();
- url.set_path(&path.to_string_lossy());
+ let mut url = Url::from_file_path(path).expect("mention path should be absolute");
url.set_fragment(Some(&format!(
"L{}:{}",
line_range.start + 1,
@@ -266,15 +250,17 @@ pub fn selection_name(path: &Path, line_range: &Range<u32>) -> String {
#[cfg(test)]
mod tests {
+ use util::{path, uri};
+
use super::*;
#[test]
fn test_parse_file_uri() {
- let file_uri = "file:///path/to/file.rs";
+ let file_uri = uri!("file:///path/to/file.rs");
let parsed = MentionUri::parse(file_uri).unwrap();
match &parsed {
MentionUri::File { abs_path } => {
- assert_eq!(abs_path.to_str().unwrap(), "/path/to/file.rs");
+ assert_eq!(abs_path.to_str().unwrap(), path!("/path/to/file.rs"));
}
_ => panic!("Expected File variant"),
}
@@ -283,11 +269,11 @@ mod tests {
#[test]
fn test_parse_directory_uri() {
- let file_uri = "file:///path/to/dir/";
+ let file_uri = uri!("file:///path/to/dir/");
let parsed = MentionUri::parse(file_uri).unwrap();
match &parsed {
MentionUri::Directory { abs_path } => {
- assert_eq!(abs_path.to_str().unwrap(), "/path/to/dir/");
+ assert_eq!(abs_path.to_str().unwrap(), path!("/path/to/dir/"));
}
_ => panic!("Expected Directory variant"),
}
@@ -297,22 +283,24 @@ mod tests {
#[test]
fn test_to_directory_uri_with_slash() {
let uri = MentionUri::Directory {
- abs_path: PathBuf::from("/path/to/dir/"),
+ abs_path: PathBuf::from(path!("/path/to/dir/")),
};
- assert_eq!(uri.to_uri().to_string(), "file:///path/to/dir/");
+ let expected = uri!("file:///path/to/dir/");
+ assert_eq!(uri.to_uri().to_string(), expected);
}
#[test]
fn test_to_directory_uri_without_slash() {
let uri = MentionUri::Directory {
- abs_path: PathBuf::from("/path/to/dir"),
+ abs_path: PathBuf::from(path!("/path/to/dir")),
};
- assert_eq!(uri.to_uri().to_string(), "file:///path/to/dir/");
+ let expected = uri!("file:///path/to/dir/");
+ assert_eq!(uri.to_uri().to_string(), expected);
}
#[test]
fn test_parse_symbol_uri() {
- let symbol_uri = "file:///path/to/file.rs?symbol=MySymbol#L10:20";
+ let symbol_uri = uri!("file:///path/to/file.rs?symbol=MySymbol#L10:20");
let parsed = MentionUri::parse(symbol_uri).unwrap();
match &parsed {
MentionUri::Symbol {
@@ -320,7 +308,7 @@ mod tests {
name,
line_range,
} => {
- assert_eq!(path.to_str().unwrap(), "/path/to/file.rs");
+ assert_eq!(path.to_str().unwrap(), path!("/path/to/file.rs"));
assert_eq!(name, "MySymbol");
assert_eq!(line_range.start, 9);
assert_eq!(line_range.end, 19);
@@ -332,11 +320,11 @@ mod tests {
#[test]
fn test_parse_selection_uri() {
- let selection_uri = "file:///path/to/file.rs#L5:15";
+ let selection_uri = uri!("file:///path/to/file.rs#L5:15");
let parsed = MentionUri::parse(selection_uri).unwrap();
match &parsed {
MentionUri::Selection { path, line_range } => {
- assert_eq!(path.to_str().unwrap(), "/path/to/file.rs");
+ assert_eq!(path.to_str().unwrap(), path!("/path/to/file.rs"));
assert_eq!(line_range.start, 4);
assert_eq!(line_range.end, 14);
}
@@ -418,32 +406,35 @@ mod tests {
#[test]
fn test_invalid_line_range_format() {
// Missing L prefix
- assert!(MentionUri::parse("file:///path/to/file.rs#10:20").is_err());
+ assert!(MentionUri::parse(uri!("file:///path/to/file.rs#10:20")).is_err());
// Missing colon separator
- assert!(MentionUri::parse("file:///path/to/file.rs#L1020").is_err());
+ assert!(MentionUri::parse(uri!("file:///path/to/file.rs#L1020")).is_err());
// Invalid numbers
- assert!(MentionUri::parse("file:///path/to/file.rs#L10:abc").is_err());
- assert!(MentionUri::parse("file:///path/to/file.rs#Labc:20").is_err());
+ assert!(MentionUri::parse(uri!("file:///path/to/file.rs#L10:abc")).is_err());
+ assert!(MentionUri::parse(uri!("file:///path/to/file.rs#Labc:20")).is_err());
}
#[test]
fn test_invalid_query_parameters() {
// Invalid query parameter name
- assert!(MentionUri::parse("file:///path/to/file.rs#L10:20?invalid=test").is_err());
+ assert!(MentionUri::parse(uri!("file:///path/to/file.rs#L10:20?invalid=test")).is_err());
// Too many query parameters
assert!(
- MentionUri::parse("file:///path/to/file.rs#L10:20?symbol=test&another=param").is_err()
+ MentionUri::parse(uri!(
+ "file:///path/to/file.rs#L10:20?symbol=test&another=param"
+ ))
+ .is_err()
);
}
#[test]
fn test_zero_based_line_numbers() {
// Test that 0-based line numbers are rejected (should be 1-based)
- assert!(MentionUri::parse("file:///path/to/file.rs#L0:10").is_err());
- assert!(MentionUri::parse("file:///path/to/file.rs#L1:0").is_err());
- assert!(MentionUri::parse("file:///path/to/file.rs#L0:0").is_err());
+ assert!(MentionUri::parse(uri!("file:///path/to/file.rs#L0:10")).is_err());
+ assert!(MentionUri::parse(uri!("file:///path/to/file.rs#L1:0")).is_err());
+ assert!(MentionUri::parse(uri!("file:///path/to/file.rs#L0:0")).is_err());
}
}
@@ -1640,7 +1640,7 @@ mod tests {
use serde_json::json;
use text::Point;
use ui::{App, Context, IntoElement, Render, SharedString, Window};
- use util::path;
+ use util::{path, uri};
use workspace::{AppState, Item, Workspace};
use crate::acp::{
@@ -1950,13 +1950,12 @@ mod tests {
editor.confirm_completion(&editor::actions::ConfirmCompletion::default(), window, cx);
});
+ let url_one = uri!("file:///dir/a/one.txt");
editor.update(&mut cx, |editor, cx| {
- assert_eq!(editor.text(cx), "Lorem [@one.txt](file:///dir/a/one.txt) ");
+ let text = editor.text(cx);
+ assert_eq!(text, format!("Lorem [@one.txt]({url_one}) "));
assert!(!editor.has_visible_completions_menu());
- assert_eq!(
- fold_ranges(editor, cx),
- vec![Point::new(0, 6)..Point::new(0, 39)]
- );
+ assert_eq!(fold_ranges(editor, cx).len(), 1);
});
let contents = message_editor
@@ -1977,47 +1976,35 @@ mod tests {
contents,
[Mention::Text {
content: "1".into(),
- uri: "file:///dir/a/one.txt".parse().unwrap()
+ uri: url_one.parse().unwrap()
}]
);
cx.simulate_input(" ");
editor.update(&mut cx, |editor, cx| {
- assert_eq!(editor.text(cx), "Lorem [@one.txt](file:///dir/a/one.txt) ");
+ let text = editor.text(cx);
+ assert_eq!(text, format!("Lorem [@one.txt]({url_one}) "));
assert!(!editor.has_visible_completions_menu());
- assert_eq!(
- fold_ranges(editor, cx),
- vec![Point::new(0, 6)..Point::new(0, 39)]
- );
+ assert_eq!(fold_ranges(editor, cx).len(), 1);
});
cx.simulate_input("Ipsum ");
editor.update(&mut cx, |editor, cx| {
- assert_eq!(
- editor.text(cx),
- "Lorem [@one.txt](file:///dir/a/one.txt) Ipsum ",
- );
+ let text = editor.text(cx);
+ assert_eq!(text, format!("Lorem [@one.txt]({url_one}) Ipsum "),);
assert!(!editor.has_visible_completions_menu());
- assert_eq!(
- fold_ranges(editor, cx),
- vec![Point::new(0, 6)..Point::new(0, 39)]
- );
+ assert_eq!(fold_ranges(editor, cx).len(), 1);
});
cx.simulate_input("@file ");
editor.update(&mut cx, |editor, cx| {
- assert_eq!(
- editor.text(cx),
- "Lorem [@one.txt](file:///dir/a/one.txt) Ipsum @file ",
- );
+ let text = editor.text(cx);
+ assert_eq!(text, format!("Lorem [@one.txt]({url_one}) Ipsum @file "),);
assert!(editor.has_visible_completions_menu());
- assert_eq!(
- fold_ranges(editor, cx),
- vec![Point::new(0, 6)..Point::new(0, 39)]
- );
+ assert_eq!(fold_ranges(editor, cx).len(), 1);
});
editor.update_in(&mut cx, |editor, window, cx| {
@@ -2041,28 +2028,23 @@ mod tests {
.collect::<Vec<_>>();
assert_eq!(contents.len(), 2);
+ let url_eight = uri!("file:///dir/b/eight.txt");
pretty_assertions::assert_eq!(
contents[1],
Mention::Text {
content: "8".to_string(),
- uri: "file:///dir/b/eight.txt".parse().unwrap(),
+ uri: url_eight.parse().unwrap(),
}
);
editor.update(&mut cx, |editor, cx| {
- assert_eq!(
- editor.text(cx),
- "Lorem [@one.txt](file:///dir/a/one.txt) Ipsum [@eight.txt](file:///dir/b/eight.txt) "
- );
- assert!(!editor.has_visible_completions_menu());
- assert_eq!(
- fold_ranges(editor, cx),
- vec![
- Point::new(0, 6)..Point::new(0, 39),
- Point::new(0, 47)..Point::new(0, 84)
- ]
- );
- });
+ assert_eq!(
+ editor.text(cx),
+ format!("Lorem [@one.txt]({url_one}) Ipsum [@eight.txt]({url_eight}) ")
+ );
+ assert!(!editor.has_visible_completions_menu());
+ assert_eq!(fold_ranges(editor, cx).len(), 2);
+ });
let plain_text_language = Arc::new(language::Language::new(
language::LanguageConfig {
@@ -2108,7 +2090,7 @@ mod tests {
let fake_language_server = fake_language_servers.next().await.unwrap();
fake_language_server.set_request_handler::<lsp::WorkspaceSymbolRequest, _, _>(
- |_, _| async move {
+ move |_, _| async move {
Ok(Some(lsp::WorkspaceSymbolResponse::Flat(vec![
#[allow(deprecated)]
lsp::SymbolInformation {
@@ -2132,18 +2114,13 @@ mod tests {
cx.simulate_input("@symbol ");
editor.update(&mut cx, |editor, cx| {
- assert_eq!(
- editor.text(cx),
- "Lorem [@one.txt](file:///dir/a/one.txt) Ipsum [@eight.txt](file:///dir/b/eight.txt) @symbol "
- );
- assert!(editor.has_visible_completions_menu());
- assert_eq!(
- current_completion_labels(editor),
- &[
- "MySymbol",
- ]
- );
- });
+ assert_eq!(
+ editor.text(cx),
+ format!("Lorem [@one.txt]({url_one}) Ipsum [@eight.txt]({url_eight}) @symbol ")
+ );
+ assert!(editor.has_visible_completions_menu());
+ assert_eq!(current_completion_labels(editor), &["MySymbol"]);
+ });
editor.update_in(&mut cx, |editor, window, cx| {
editor.confirm_completion(&editor::actions::ConfirmCompletion::default(), window, cx);
@@ -2165,9 +2142,7 @@ mod tests {
contents[2],
Mention::Text {
content: "1".into(),
- uri: "file:///dir/a/one.txt?symbol=MySymbol#L1:1"
- .parse()
- .unwrap(),
+ uri: format!("{url_one}?symbol=MySymbol#L1:1").parse().unwrap(),
}
);
@@ -2176,7 +2151,7 @@ mod tests {
editor.read_with(&cx, |editor, cx| {
assert_eq!(
editor.text(cx),
- "Lorem [@one.txt](file:///dir/a/one.txt) Ipsum [@eight.txt](file:///dir/b/eight.txt) [@MySymbol](file:///dir/a/one.txt?symbol=MySymbol#L1:1) "
+ format!("Lorem [@one.txt]({url_one}) Ipsum [@eight.txt]({url_eight}) [@MySymbol]({url_one}?symbol=MySymbol#L1:1) ")
);
});
}
@@ -590,9 +590,9 @@ mod tests {
assert_eq!(
fs.files_with_contents(Path::new("")),
[
- (Path::new("/bar/baz").into(), b"qux".into()),
- (Path::new("/foo/a").into(), b"lorem".into()),
- (Path::new("/foo/b").into(), b"ipsum".into())
+ (Path::new(path!("/bar/baz")).into(), b"qux".into()),
+ (Path::new(path!("/foo/a")).into(), b"lorem".into()),
+ (Path::new(path!("/foo/b")).into(), b"ipsum".into())
]
);
}
@@ -1101,7 +1101,9 @@ impl FakeFsState {
) -> Option<(&mut FakeFsEntry, PathBuf)> {
let canonical_path = self.canonicalize(target, follow_symlink)?;
- let mut components = canonical_path.components();
+ let mut components = canonical_path
+ .components()
+ .skip_while(|component| matches!(component, Component::Prefix(_)));
let Some(Component::RootDir) = components.next() else {
panic!(
"the path {:?} was not canonicalized properly {:?}",