Compare commits
3 Commits
fallback_p
...
re-enable-
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
6c15ff0862 | ||
|
|
f1fede44d6 | ||
|
|
3febcd953d |
@@ -14,9 +14,11 @@ use gpui::{
|
||||
WeakEntity, Window, percentage, prelude::*,
|
||||
};
|
||||
|
||||
use language::language_settings::{self, FormatOnSave};
|
||||
use language::{Buffer, Capability, DiskState, OffsetRangeExt, Point};
|
||||
use language_model::StopReason;
|
||||
use multi_buffer::PathKey;
|
||||
use project::lsp_store::{FormatTrigger, LspFormatTarget};
|
||||
use project::{Project, ProjectItem, ProjectPath};
|
||||
use settings::{Settings, SettingsStore};
|
||||
use std::{
|
||||
@@ -123,7 +125,11 @@ impl AgentDiffPane {
|
||||
fn update_excerpts(&mut self, window: &mut Window, cx: &mut Context<Self>) {
|
||||
let thread = self.thread.read(cx);
|
||||
let changed_buffers = thread.action_log().read(cx).changed_buffers(cx);
|
||||
let mut paths_to_delete = self.multibuffer.read(cx).paths().collect::<HashSet<_>>();
|
||||
let mut paths_to_delete = self
|
||||
.multibuffer
|
||||
.read(cx)
|
||||
.paths()
|
||||
.collect::<collections::HashSet<_>>();
|
||||
|
||||
for (buffer, diff_handle) in changed_buffers {
|
||||
if buffer.read(cx).file().is_none() {
|
||||
@@ -1353,6 +1359,7 @@ impl AgentDiff {
|
||||
| ThreadEvent::ShowError(_)
|
||||
| ThreadEvent::CompletionCanceled => {
|
||||
self.update_reviewing_editors(workspace, window, cx);
|
||||
self.format_changed_buffers(workspace, window, cx);
|
||||
}
|
||||
// intentionally being exhaustive in case we add a variant we should handle
|
||||
ThreadEvent::Stopped(Ok(StopReason::ToolUse))
|
||||
@@ -1455,6 +1462,75 @@ impl AgentDiff {
|
||||
self.update_reviewing_editors(&workspace, window, cx);
|
||||
}
|
||||
|
||||
fn format_changed_buffers(
|
||||
&mut self,
|
||||
workspace: &WeakEntity<Workspace>,
|
||||
_window: &mut Window,
|
||||
cx: &mut Context<Self>,
|
||||
) {
|
||||
let Some(workspace) = workspace.upgrade() else {
|
||||
return;
|
||||
};
|
||||
|
||||
let Some(thread) = self
|
||||
.workspace_threads
|
||||
.get(&workspace.downgrade())
|
||||
.and_then(|t| t.thread.upgrade())
|
||||
else {
|
||||
return;
|
||||
};
|
||||
|
||||
let action_log = thread.read(cx).action_log().clone();
|
||||
let all_changed_buffers = action_log.read(cx).changed_buffers(cx);
|
||||
|
||||
// Filter buffers that should be formatted based on format_on_save settings
|
||||
let buffers_to_format: HashSet<_> = all_changed_buffers
|
||||
.into_iter()
|
||||
.filter(|(buffer, _)| {
|
||||
buffer.read(cx).file().is_some() && {
|
||||
let settings = language_settings::language_settings(
|
||||
buffer.read(cx).language().map(|l| l.name()),
|
||||
buffer.read(cx).file(),
|
||||
cx,
|
||||
);
|
||||
// Format if FormatOnSave is On or List (not Off)
|
||||
!matches!(settings.format_on_save, FormatOnSave::Off)
|
||||
}
|
||||
})
|
||||
.map(|(buffer, _)| buffer)
|
||||
.collect();
|
||||
|
||||
if buffers_to_format.is_empty() {
|
||||
return;
|
||||
}
|
||||
let project = workspace.read(cx).project().clone();
|
||||
|
||||
let action_log_for_task = action_log.clone();
|
||||
let buffers_to_mark = buffers_to_format.clone();
|
||||
|
||||
let format_task = project.update(cx, |project, cx| {
|
||||
project.format(
|
||||
buffers_to_format,
|
||||
LspFormatTarget::Buffers,
|
||||
false, // Don't push to history since the agent already modified the buffers
|
||||
FormatTrigger::Save,
|
||||
cx,
|
||||
)
|
||||
});
|
||||
|
||||
cx.spawn(async move |_, cx| {
|
||||
format_task.await.ok();
|
||||
|
||||
// Mark these buffers as having formatting-only changes after formatting completes
|
||||
action_log_for_task
|
||||
.update(cx, |action_log, _| {
|
||||
action_log.mark_buffers_as_formatting_only(&buffers_to_mark);
|
||||
})
|
||||
.ok();
|
||||
})
|
||||
.detach();
|
||||
}
|
||||
|
||||
fn update_reviewing_editors(
|
||||
&mut self,
|
||||
workspace: &WeakEntity<Workspace>,
|
||||
@@ -1739,14 +1815,21 @@ mod tests {
|
||||
use agent_settings::AgentSettings;
|
||||
use assistant_tool::ToolWorkingSet;
|
||||
use editor::EditorSettings;
|
||||
use fs::Fs;
|
||||
use futures::StreamExt;
|
||||
use gpui::{TestAppContext, UpdateGlobal, VisualTestContext};
|
||||
use language::{FakeLspAdapter, Language, LanguageConfig, LanguageMatcher, Point};
|
||||
|
||||
use language_settings::{AllLanguageSettings, FormatOnSave};
|
||||
use lsp;
|
||||
use project::{FakeFs, Project};
|
||||
use prompt_store::PromptBuilder;
|
||||
use serde_json::json;
|
||||
use settings::{Settings, SettingsStore};
|
||||
use std::sync::Arc;
|
||||
use std::{collections::HashSet, sync::Arc};
|
||||
use theme::ThemeSettings;
|
||||
use util::path;
|
||||
use workspace::Workspace;
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_multibuffer_agent_diff(cx: &mut TestAppContext) {
|
||||
@@ -1795,10 +1878,16 @@ mod tests {
|
||||
|
||||
let (workspace, cx) =
|
||||
cx.add_window_view(|window, cx| Workspace::test_new(project.clone(), window, cx));
|
||||
let agent_diff = cx.new_window_entity(|window, cx| {
|
||||
|
||||
// Set up AgentDiff using set_active_thread
|
||||
cx.update(|window, cx| {
|
||||
AgentDiff::set_active_thread(&workspace.downgrade(), &thread, window, cx);
|
||||
});
|
||||
|
||||
let agent_diff_pane = cx.new_window_entity(|window, cx| {
|
||||
AgentDiffPane::new(thread.clone(), workspace.downgrade(), window, cx)
|
||||
});
|
||||
let editor = agent_diff.read_with(cx, |diff, _cx| diff.editor.clone());
|
||||
let editor = agent_diff_pane.read_with(cx, |diff, _cx| diff.editor.clone());
|
||||
|
||||
let buffer = project
|
||||
.update(cx, |project, cx| project.open_buffer(buffer_path, cx))
|
||||
@@ -1837,7 +1926,7 @@ mod tests {
|
||||
);
|
||||
|
||||
// After keeping a hunk, the cursor should be positioned on the second hunk.
|
||||
agent_diff.update_in(cx, |diff, window, cx| diff.keep(&Keep, window, cx));
|
||||
agent_diff_pane.update_in(cx, |diff, window, cx| diff.keep(&Keep, window, cx));
|
||||
cx.run_until_parked();
|
||||
assert_eq!(
|
||||
editor.read_with(cx, |editor, cx| editor.text(cx)),
|
||||
@@ -1856,7 +1945,7 @@ mod tests {
|
||||
selections.select_ranges([Point::new(10, 0)..Point::new(10, 0)])
|
||||
});
|
||||
});
|
||||
agent_diff.update_in(cx, |diff, window, cx| {
|
||||
agent_diff_pane.update_in(cx, |diff, window, cx| {
|
||||
diff.reject(&crate::Reject, window, cx)
|
||||
});
|
||||
cx.run_until_parked();
|
||||
@@ -1872,7 +1961,7 @@ mod tests {
|
||||
);
|
||||
|
||||
// Keeping a range that doesn't intersect the current selection doesn't move it.
|
||||
agent_diff.update_in(cx, |_diff, window, cx| {
|
||||
agent_diff_pane.update_in(cx, |_diff, window, cx| {
|
||||
let position = editor
|
||||
.read(cx)
|
||||
.buffer()
|
||||
@@ -1904,6 +1993,115 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_format_with_list_setting(cx: &mut TestAppContext) {
|
||||
cx.update(|cx| {
|
||||
let settings_store = SettingsStore::test(cx);
|
||||
cx.set_global(settings_store);
|
||||
language::init(cx);
|
||||
Project::init_settings(cx);
|
||||
|
||||
SettingsStore::update_global(cx, |store, cx| {
|
||||
store.update_user_settings::<AllLanguageSettings>(cx, |settings| {
|
||||
use language_settings::{Formatter, FormatterList};
|
||||
settings.defaults.format_on_save = Some(FormatOnSave::List(FormatterList(
|
||||
vec![Formatter::LanguageServer { name: None }].into(),
|
||||
)));
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
let fs = FakeFs::new(cx.executor());
|
||||
fs.insert_tree(
|
||||
path!("/test"),
|
||||
json!({"src": {"main.rs": "initial content"}}),
|
||||
)
|
||||
.await;
|
||||
|
||||
let project = Project::test(fs.clone(), [path!("/test").as_ref()], cx).await;
|
||||
|
||||
let rust_language = Arc::new(Language::new(
|
||||
LanguageConfig {
|
||||
name: "Rust".into(),
|
||||
matcher: LanguageMatcher {
|
||||
path_suffixes: vec!["rs".to_string()],
|
||||
..Default::default()
|
||||
},
|
||||
..Default::default()
|
||||
},
|
||||
None,
|
||||
));
|
||||
|
||||
let language_registry = project.read_with(cx, |project, _| project.languages().clone());
|
||||
language_registry.add(rust_language);
|
||||
|
||||
let mut fake_language_servers = language_registry.register_fake_lsp(
|
||||
"Rust",
|
||||
FakeLspAdapter {
|
||||
capabilities: lsp::ServerCapabilities {
|
||||
document_formatting_provider: Some(lsp::OneOf::Left(true)),
|
||||
..Default::default()
|
||||
},
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
|
||||
let buffer = project
|
||||
.update(cx, |project, cx| {
|
||||
project.open_local_buffer(path!("/test/src/main.rs"), cx)
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let _handle = project.update(cx, |project, cx| {
|
||||
project.register_buffer_with_language_servers(&buffer, cx)
|
||||
});
|
||||
|
||||
const FORMATTED_CONTENT: &str = "fn main() {\n println!(\"Hello!\");\n}\n";
|
||||
|
||||
let fake_language_server = fake_language_servers.next().await.unwrap();
|
||||
fake_language_server.set_request_handler::<lsp::request::Formatting, _, _>({
|
||||
|_, _| async move {
|
||||
Ok(Some(vec![lsp::TextEdit {
|
||||
range: lsp::Range::new(lsp::Position::new(0, 0), lsp::Position::new(1, 0)),
|
||||
new_text: FORMATTED_CONTENT.to_string(),
|
||||
}]))
|
||||
}
|
||||
});
|
||||
|
||||
buffer.update(cx, |buffer, cx| {
|
||||
buffer.edit(
|
||||
[(0..buffer.len(), "fn main() {println!(\"Hello!\");}\n")],
|
||||
None,
|
||||
cx,
|
||||
);
|
||||
});
|
||||
|
||||
let format_task = project.update(cx, |project, cx| {
|
||||
project.format(
|
||||
HashSet::from_iter([buffer.clone()]),
|
||||
LspFormatTarget::Buffers,
|
||||
false,
|
||||
FormatTrigger::Save,
|
||||
cx,
|
||||
)
|
||||
});
|
||||
|
||||
format_task.await.unwrap();
|
||||
|
||||
project
|
||||
.update(cx, |project, cx| project.save_buffer(buffer.clone(), cx))
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let content = fs.load(path!("/test/src/main.rs").as_ref()).await.unwrap();
|
||||
assert_eq!(
|
||||
content.replace("\r\n", "\n"),
|
||||
FORMATTED_CONTENT,
|
||||
"Code should be formatted when format_on_save is set to List with language server formatter"
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_singleton_agent_diff(cx: &mut TestAppContext) {
|
||||
cx.update(|cx| {
|
||||
@@ -2214,4 +2412,208 @@ mod tests {
|
||||
});
|
||||
cx.run_until_parked();
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_simple_format_on_save(cx: &mut TestAppContext) {
|
||||
cx.update(|cx| {
|
||||
let settings_store = SettingsStore::test(cx);
|
||||
cx.set_global(settings_store);
|
||||
language::init(cx);
|
||||
Project::init_settings(cx);
|
||||
|
||||
SettingsStore::update_global(cx, |store, cx| {
|
||||
store.update_user_settings::<AllLanguageSettings>(cx, |settings| {
|
||||
settings.defaults.format_on_save = Some(FormatOnSave::On);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
let fs = FakeFs::new(cx.executor());
|
||||
fs.insert_tree(
|
||||
path!("/test"),
|
||||
json!({"src": {"main.rs": "fn main() {println!(\"Hello!\");}\n"}}),
|
||||
)
|
||||
.await;
|
||||
|
||||
let project = Project::test(fs.clone(), [path!("/test").as_ref()], cx).await;
|
||||
|
||||
let rust_language = Arc::new(Language::new(
|
||||
LanguageConfig {
|
||||
name: "Rust".into(),
|
||||
matcher: LanguageMatcher {
|
||||
path_suffixes: vec!["rs".to_string()],
|
||||
..Default::default()
|
||||
},
|
||||
..Default::default()
|
||||
},
|
||||
None,
|
||||
));
|
||||
|
||||
let language_registry = project.read_with(cx, |project, _| project.languages().clone());
|
||||
language_registry.add(rust_language);
|
||||
|
||||
let mut fake_language_servers = language_registry.register_fake_lsp(
|
||||
"Rust",
|
||||
FakeLspAdapter {
|
||||
capabilities: lsp::ServerCapabilities {
|
||||
document_formatting_provider: Some(lsp::OneOf::Left(true)),
|
||||
..Default::default()
|
||||
},
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
|
||||
let buffer = project
|
||||
.update(cx, |project, cx| {
|
||||
project.open_local_buffer(path!("/test/src/main.rs"), cx)
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let _handle = project.update(cx, |project, cx| {
|
||||
project.register_buffer_with_language_servers(&buffer, cx)
|
||||
});
|
||||
|
||||
const FORMATTED_CONTENT: &str = "fn main() {\n println!(\"Hello!\");\n}\n";
|
||||
|
||||
let fake_language_server = fake_language_servers.next().await.unwrap();
|
||||
fake_language_server.set_request_handler::<lsp::request::Formatting, _, _>({
|
||||
|_, _| async move {
|
||||
Ok(Some(vec![lsp::TextEdit {
|
||||
range: lsp::Range::new(lsp::Position::new(0, 0), lsp::Position::new(1, 0)),
|
||||
new_text: FORMATTED_CONTENT.to_string(),
|
||||
}]))
|
||||
}
|
||||
});
|
||||
|
||||
let format_task = project.update(cx, |project, cx| {
|
||||
project.format(
|
||||
HashSet::from_iter([buffer.clone()]),
|
||||
LspFormatTarget::Buffers,
|
||||
false,
|
||||
FormatTrigger::Save,
|
||||
cx,
|
||||
)
|
||||
});
|
||||
|
||||
format_task.await.unwrap();
|
||||
|
||||
project
|
||||
.update(cx, |project, cx| project.save_buffer(buffer.clone(), cx))
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let content = fs.load(path!("/test/src/main.rs").as_ref()).await.unwrap();
|
||||
assert_eq!(
|
||||
content.replace("\r\n", "\n"),
|
||||
FORMATTED_CONTENT,
|
||||
"Code should be formatted"
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_format_on_save_after_agent_finishes(cx: &mut TestAppContext) {
|
||||
cx.update(|cx| {
|
||||
let settings_store = SettingsStore::test(cx);
|
||||
cx.set_global(settings_store);
|
||||
language::init(cx);
|
||||
Project::init_settings(cx);
|
||||
|
||||
SettingsStore::update_global(cx, |store, cx| {
|
||||
store.update_user_settings::<AllLanguageSettings>(cx, |settings| {
|
||||
settings.defaults.format_on_save = Some(FormatOnSave::On);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
let fs = FakeFs::new(cx.executor());
|
||||
fs.insert_tree(
|
||||
path!("/test"),
|
||||
json!({"src": {"main.rs": "initial content"}}),
|
||||
)
|
||||
.await;
|
||||
|
||||
let project = Project::test(fs.clone(), [path!("/test").as_ref()], cx).await;
|
||||
|
||||
let rust_language = Arc::new(Language::new(
|
||||
LanguageConfig {
|
||||
name: "Rust".into(),
|
||||
matcher: LanguageMatcher {
|
||||
path_suffixes: vec!["rs".to_string()],
|
||||
..Default::default()
|
||||
},
|
||||
..Default::default()
|
||||
},
|
||||
None,
|
||||
));
|
||||
|
||||
let language_registry = project.read_with(cx, |project, _| project.languages().clone());
|
||||
language_registry.add(rust_language);
|
||||
|
||||
let mut fake_language_servers = language_registry.register_fake_lsp(
|
||||
"Rust",
|
||||
FakeLspAdapter {
|
||||
capabilities: lsp::ServerCapabilities {
|
||||
document_formatting_provider: Some(lsp::OneOf::Left(true)),
|
||||
..Default::default()
|
||||
},
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
|
||||
let buffer = project
|
||||
.update(cx, |project, cx| {
|
||||
project.open_local_buffer(path!("/test/src/main.rs"), cx)
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let _handle = project.update(cx, |project, cx| {
|
||||
project.register_buffer_with_language_servers(&buffer, cx)
|
||||
});
|
||||
|
||||
const FORMATTED_CONTENT: &str = "fn main() {\n println!(\"Hello!\");\n}\n";
|
||||
|
||||
let fake_language_server = fake_language_servers.next().await.unwrap();
|
||||
fake_language_server.set_request_handler::<lsp::request::Formatting, _, _>({
|
||||
|_, _| async move {
|
||||
Ok(Some(vec![lsp::TextEdit {
|
||||
range: lsp::Range::new(lsp::Position::new(0, 0), lsp::Position::new(1, 0)),
|
||||
new_text: FORMATTED_CONTENT.to_string(),
|
||||
}]))
|
||||
}
|
||||
});
|
||||
|
||||
buffer.update(cx, |buffer, cx| {
|
||||
buffer.edit(
|
||||
[(0..buffer.len(), "fn main() {println!(\"Hello!\");}\n")],
|
||||
None,
|
||||
cx,
|
||||
);
|
||||
});
|
||||
|
||||
let format_task = project.update(cx, |project, cx| {
|
||||
project.format(
|
||||
HashSet::from_iter([buffer.clone()]),
|
||||
LspFormatTarget::Buffers,
|
||||
false,
|
||||
FormatTrigger::Save,
|
||||
cx,
|
||||
)
|
||||
});
|
||||
|
||||
format_task.await.unwrap();
|
||||
|
||||
project
|
||||
.update(cx, |project, cx| project.save_buffer(buffer.clone(), cx))
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let content = fs.load(path!("/test/src/main.rs").as_ref()).await.unwrap();
|
||||
assert_eq!(
|
||||
content.replace("\r\n", "\n"),
|
||||
FORMATTED_CONTENT,
|
||||
"Code should be formatted when format_on_save is enabled"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1405,21 +1405,29 @@ impl Thread {
|
||||
cx: &App,
|
||||
) {
|
||||
const STALE_FILES_HEADER: &str = "These files changed since last read:";
|
||||
const FORMATTED_FILES_HEADER: &str = "FYI, these files were reformatted using an automatic code formatter since you last read them (this means some parts of the file may have different line numbers now, but otherwise you should assume the semantics are the same as before. You MUST NOT reread the files, or mention anything about being aware that they were formatted, unless you actually need to do something involving line numbers):";
|
||||
|
||||
let mut stale_message = String::new();
|
||||
let mut formatted_message = String::new();
|
||||
|
||||
let action_log = self.action_log.read(cx);
|
||||
|
||||
for stale_file in action_log.stale_buffers(cx) {
|
||||
for (stale_file, formatting_only) in action_log.stale_buffers(cx) {
|
||||
let Some(file) = stale_file.read(cx).file() else {
|
||||
continue;
|
||||
};
|
||||
|
||||
if stale_message.is_empty() {
|
||||
write!(&mut stale_message, "{}\n", STALE_FILES_HEADER).ok();
|
||||
if formatting_only {
|
||||
if formatted_message.is_empty() {
|
||||
write!(&mut formatted_message, "{}\n", FORMATTED_FILES_HEADER).ok();
|
||||
}
|
||||
writeln!(&mut formatted_message, "- {}", file.path().display()).ok();
|
||||
} else {
|
||||
if stale_message.is_empty() {
|
||||
write!(&mut stale_message, "{}\n", STALE_FILES_HEADER).ok();
|
||||
}
|
||||
writeln!(&mut stale_message, "- {}", file.path().display()).ok();
|
||||
}
|
||||
|
||||
writeln!(&mut stale_message, "- {}", file.path().display()).ok();
|
||||
}
|
||||
|
||||
let mut content = Vec::with_capacity(2);
|
||||
@@ -1428,6 +1436,10 @@ impl Thread {
|
||||
content.push(stale_message.into());
|
||||
}
|
||||
|
||||
if !formatted_message.is_empty() {
|
||||
content.push(formatted_message.into());
|
||||
}
|
||||
|
||||
if !content.is_empty() {
|
||||
let context_message = LanguageModelRequestMessage {
|
||||
role: Role::User,
|
||||
@@ -2837,6 +2849,7 @@ mod tests {
|
||||
use crate::{ThreadStore, context::load_context, context_store::ContextStore, thread_store};
|
||||
use agent_settings::{AgentSettings, LanguageModelParameters};
|
||||
use assistant_tool::ToolRegistry;
|
||||
use collections;
|
||||
use editor::EditorSettings;
|
||||
use gpui::TestAppContext;
|
||||
use language_model::fake_provider::{FakeLanguageModel, FakeLanguageModelProvider};
|
||||
@@ -3160,6 +3173,231 @@ fn main() {{
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_formatting_only_changes_workflow(cx: &mut TestAppContext) {
|
||||
init_test_settings(cx);
|
||||
|
||||
let project = create_test_project(
|
||||
cx,
|
||||
json!({"code.rs": "fn main() {\n println!(\"Hello, world!\");\n}"}),
|
||||
)
|
||||
.await;
|
||||
|
||||
let (_workspace, _thread_store, thread, context_store, model) =
|
||||
setup_test_environment(cx, project.clone()).await;
|
||||
|
||||
// Open buffer and add it to context
|
||||
let buffer = add_file_to_context(&project, &context_store, "test/code.rs", cx)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let context =
|
||||
context_store.read_with(cx, |store, _| store.context().next().cloned().unwrap());
|
||||
let loaded_context = cx
|
||||
.update(|cx| load_context(vec![context], &project, &None, cx))
|
||||
.await;
|
||||
|
||||
// Insert user message with the buffer as context
|
||||
thread.update(cx, |thread, cx| {
|
||||
thread.insert_user_message("Look at this code", loaded_context, None, Vec::new(), cx)
|
||||
});
|
||||
|
||||
// The buffer is now tracked by the agent
|
||||
let action_log = thread.read_with(cx, |thread, _| thread.action_log().clone());
|
||||
|
||||
// Test the workflow where:
|
||||
// 1. Agent reads buffer
|
||||
// 2. Agent makes edits (poorly formatted)
|
||||
// 3. Agent completion triggers formatting
|
||||
// 4. Buffer is marked as formatting-only
|
||||
|
||||
// Simulate agent making an edit
|
||||
buffer.update(cx, |buffer, cx| {
|
||||
buffer.edit(
|
||||
[(0..buffer.len(), "fn main(){println!(\"Hello, world!\");}")],
|
||||
None,
|
||||
cx,
|
||||
);
|
||||
});
|
||||
|
||||
// Mark as agent edit - this sets formatting_only_changes = true
|
||||
action_log.update(cx, |action_log, cx| {
|
||||
action_log.buffer_edited(buffer.clone(), cx);
|
||||
});
|
||||
|
||||
cx.run_until_parked();
|
||||
|
||||
// Verify that the buffer is marked with formatting_only_changes = true
|
||||
// after agent edits
|
||||
let _is_formatting_only_after_agent_edit = action_log.read_with(cx, |log, cx| {
|
||||
// Check stale buffers to see if it's marked as formatting-only
|
||||
log.stale_buffers(cx)
|
||||
.find(|(b, _)| b.entity_id() == buffer.entity_id())
|
||||
.map(|(_, formatting_only)| formatting_only)
|
||||
.unwrap_or(false)
|
||||
});
|
||||
|
||||
// After agent edits, it should be marked as formatting-only
|
||||
// (no stale check will pass because tracked version matches buffer version)
|
||||
// But the internal state should be set correctly
|
||||
|
||||
// Now test that user edits clear the formatting-only flag
|
||||
buffer.update(cx, |buffer, cx| {
|
||||
buffer.edit(
|
||||
[(buffer.len()..buffer.len(), "\n// User comment")],
|
||||
None,
|
||||
cx,
|
||||
);
|
||||
});
|
||||
|
||||
// This triggers handle_buffer_edited which clears formatting_only_changes
|
||||
|
||||
// Insert another message to check the state
|
||||
thread.update(cx, |thread, cx| {
|
||||
thread.insert_user_message(
|
||||
"Check the state",
|
||||
ContextLoadResult::default(),
|
||||
None,
|
||||
Vec::new(),
|
||||
cx,
|
||||
)
|
||||
});
|
||||
|
||||
let request = thread.update(cx, |thread, cx| {
|
||||
thread.to_completion_request(model.clone(), cx)
|
||||
});
|
||||
|
||||
// Should show regular stale warning, not formatting-only
|
||||
let has_formatting_warning = request.messages.iter().any(|msg| {
|
||||
msg.string_contents()
|
||||
.contains("These files were reformatted (line numbers may have changed):")
|
||||
});
|
||||
let has_stale_warning = request.messages.iter().any(|msg| {
|
||||
msg.string_contents()
|
||||
.contains("These files changed since last read:")
|
||||
});
|
||||
|
||||
assert!(
|
||||
!has_formatting_warning,
|
||||
"Should not have formatting-only warning after user edit"
|
||||
);
|
||||
assert!(
|
||||
has_stale_warning,
|
||||
"Should have regular stale warning after user edit"
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_user_edits_followed_by_formatting_bug(cx: &mut TestAppContext) {
|
||||
init_test_settings(cx);
|
||||
|
||||
let project = create_test_project(
|
||||
cx,
|
||||
json!({"code.rs": "fn main(){println!(\"Hello, world!\");}"}),
|
||||
)
|
||||
.await;
|
||||
|
||||
let (_workspace, _thread_store, thread, context_store, model) =
|
||||
setup_test_environment(cx, project.clone()).await;
|
||||
|
||||
// Open buffer and add it to context
|
||||
let buffer = add_file_to_context(&project, &context_store, "test/code.rs", cx)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let context =
|
||||
context_store.read_with(cx, |store, _| store.context().next().cloned().unwrap());
|
||||
let loaded_context = cx
|
||||
.update(|cx| load_context(vec![context], &project, &None, cx))
|
||||
.await;
|
||||
|
||||
// Insert user message with the buffer as context
|
||||
thread.update(cx, |thread, cx| {
|
||||
thread.insert_user_message("Look at this code", loaded_context, None, Vec::new(), cx)
|
||||
});
|
||||
|
||||
// Create initial request
|
||||
let initial_request = thread.update(cx, |thread, cx| {
|
||||
thread.to_completion_request(model.clone(), cx)
|
||||
});
|
||||
|
||||
assert!(
|
||||
!initial_request.messages.iter().any(|msg| {
|
||||
msg.string_contents()
|
||||
.contains("These files changed since last read:")
|
||||
}),
|
||||
"Should not have stale buffer warning initially"
|
||||
);
|
||||
|
||||
// Get the action log
|
||||
let action_log = thread.read_with(cx, |thread, _| thread.action_log().clone());
|
||||
|
||||
// USER makes a manual edit (adding a comment)
|
||||
buffer.update(cx, |buffer, cx| {
|
||||
buffer.edit(
|
||||
[(
|
||||
buffer.len()..buffer.len(),
|
||||
"\n// User's important comment\n",
|
||||
)],
|
||||
None,
|
||||
cx,
|
||||
);
|
||||
});
|
||||
|
||||
// Then formatting happens afterwards (e.g., when thread completes)
|
||||
buffer.update(cx, |buffer, cx| {
|
||||
// Simulate formatting that also reformats the user's comment
|
||||
buffer.edit(
|
||||
[(0..buffer.len(), "fn main() {\n println!(\"Hello, world!\");\n}\n// User's important comment\n")],
|
||||
None,
|
||||
cx,
|
||||
);
|
||||
});
|
||||
|
||||
// Mark the buffer as having formatting-only changes (this is the bug!)
|
||||
action_log.update(cx, |action_log, _| {
|
||||
let buffers = collections::HashSet::from_iter([buffer.clone()]);
|
||||
action_log.mark_buffers_as_formatting_only(&buffers);
|
||||
});
|
||||
|
||||
// Insert another message
|
||||
thread.update(cx, |thread, cx| {
|
||||
thread.insert_user_message(
|
||||
"What about now?",
|
||||
ContextLoadResult::default(),
|
||||
None,
|
||||
Vec::new(),
|
||||
cx,
|
||||
)
|
||||
});
|
||||
|
||||
// Create a new request
|
||||
let new_request = thread.update(cx, |thread, cx| {
|
||||
thread.to_completion_request(model.clone(), cx)
|
||||
});
|
||||
|
||||
// Check what kind of warning we get
|
||||
let has_formatting_warning = new_request.messages.iter().any(|msg| {
|
||||
msg.string_contents()
|
||||
.contains("These files were reformatted (line numbers may have changed):")
|
||||
});
|
||||
let has_stale_warning = new_request.messages.iter().any(|msg| {
|
||||
msg.string_contents()
|
||||
.contains("These files changed since last read:")
|
||||
});
|
||||
|
||||
// This test should FAIL with the current implementation
|
||||
// because the user's edit will be hidden by marking it as formatting-only
|
||||
assert!(
|
||||
!has_formatting_warning,
|
||||
"Should NOT have formatting-only warning when user edits are present"
|
||||
);
|
||||
assert!(
|
||||
has_stale_warning,
|
||||
"Should have regular stale buffer warning because user made edits"
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_stale_buffer_notification(cx: &mut TestAppContext) {
|
||||
init_test_settings(cx);
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
use anyhow::{Context as _, Result};
|
||||
use buffer_diff::BufferDiff;
|
||||
use collections::BTreeMap;
|
||||
use collections::{BTreeMap, HashSet};
|
||||
use futures::{StreamExt, channel::mpsc};
|
||||
use gpui::{App, AppContext, AsyncApp, Context, Entity, Subscription, Task, WeakEntity};
|
||||
use language::{Anchor, Buffer, BufferEvent, DiskState, Point, ToPoint};
|
||||
@@ -110,6 +110,8 @@ impl ActionLog {
|
||||
snapshot: text_snapshot.clone(),
|
||||
status,
|
||||
version: buffer.read(cx).version(),
|
||||
formatting_only_changes: false,
|
||||
last_editor: ChangeAuthor::Agent,
|
||||
diff,
|
||||
diff_update: diff_update_tx,
|
||||
_open_lsp_handle: open_lsp_handle,
|
||||
@@ -147,6 +149,8 @@ impl ActionLog {
|
||||
let Some(tracked_buffer) = self.tracked_buffers.get_mut(&buffer) else {
|
||||
return;
|
||||
};
|
||||
tracked_buffer.formatting_only_changes = false;
|
||||
tracked_buffer.last_editor = ChangeAuthor::User;
|
||||
tracked_buffer.schedule_diff_update(ChangeAuthor::User, cx);
|
||||
}
|
||||
|
||||
@@ -309,6 +313,9 @@ impl ActionLog {
|
||||
if let TrackedBufferStatus::Deleted = tracked_buffer.status {
|
||||
tracked_buffer.status = TrackedBufferStatus::Modified;
|
||||
}
|
||||
tracked_buffer.last_editor = ChangeAuthor::Agent;
|
||||
// When agent edits, assume subsequent changes are formatting-only until user edits
|
||||
tracked_buffer.formatting_only_changes = true;
|
||||
tracked_buffer.schedule_diff_update(ChangeAuthor::Agent, cx);
|
||||
}
|
||||
|
||||
@@ -524,7 +531,10 @@ impl ActionLog {
|
||||
}
|
||||
|
||||
/// Iterate over buffers changed since last read or edited by the model
|
||||
pub fn stale_buffers<'a>(&'a self, cx: &'a App) -> impl Iterator<Item = &'a Entity<Buffer>> {
|
||||
pub fn stale_buffers<'a>(
|
||||
&'a self,
|
||||
cx: &'a App,
|
||||
) -> impl Iterator<Item = (&'a Entity<Buffer>, bool)> {
|
||||
self.tracked_buffers
|
||||
.iter()
|
||||
.filter(|(buffer, tracked)| {
|
||||
@@ -535,7 +545,20 @@ impl ActionLog {
|
||||
.file()
|
||||
.map_or(false, |file| file.disk_state() != DiskState::Deleted)
|
||||
})
|
||||
.map(|(buffer, _)| buffer)
|
||||
.map(|(buffer, tracked)| (buffer, tracked.formatting_only_changes))
|
||||
}
|
||||
|
||||
pub fn mark_buffers_as_formatting_only(&mut self, buffers: &HashSet<Entity<Buffer>>) {
|
||||
for buffer in buffers {
|
||||
if let Some(tracked) = self.tracked_buffers.get_mut(buffer) {
|
||||
// Only mark as formatting-only if the last editor was the agent
|
||||
// If a user has edited the buffer since the agent last saw it,
|
||||
// we should not mark it as formatting-only
|
||||
if tracked.last_editor == ChangeAuthor::Agent {
|
||||
tracked.formatting_only_changes = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -655,7 +678,7 @@ fn point_to_row_edit(edit: Edit<Point>, old_text: &Rope, new_text: &Rope) -> Edi
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Copy, Clone, Debug)]
|
||||
#[derive(Clone, Copy, Debug, PartialEq)]
|
||||
enum ChangeAuthor {
|
||||
User,
|
||||
Agent,
|
||||
@@ -673,6 +696,8 @@ struct TrackedBuffer {
|
||||
unreviewed_changes: Patch<u32>,
|
||||
status: TrackedBufferStatus,
|
||||
version: clock::Global,
|
||||
formatting_only_changes: bool,
|
||||
last_editor: ChangeAuthor,
|
||||
diff: Entity<BufferDiff>,
|
||||
snapshot: text::BufferSnapshot,
|
||||
diff_update: mpsc::UnboundedSender<(ChangeAuthor, text::BufferSnapshot)>,
|
||||
|
||||
Reference in New Issue
Block a user