From 00169e0ae21dbc4f6626f3aa03fdf5991a1ac4dc Mon Sep 17 00:00:00 2001
From: Anthony Eid <56899983+Anthony-Eid@users.noreply.github.com>
Date: Sun, 14 Dec 2025 07:55:19 -0500
Subject: [PATCH 001/257] git: Fix create remote branch (#44805)
Fix a bug where the branch picker would be dismissed before completing
the add remote flow, thus making Zed unable to add remote repositories
through the branch picker.
This bug was caused by the picker always being dismissed on the confirm
action, so the fix was stopping the branch modal from being dismissed
too early.
I also cleaned up the UI a bit and code.
1. Removed the loading field from the Branch delegate because it was
never used and the activity indicator will show remote add command if it
takes a while.
2. I replaced some async task spawning with the use of `cx.defer`.
3. Added a `add remote name` fake entry when the picker is in the name
remote state. I did this so the UI would be consistent with the other
states.
4. Added two regression tests.
4.1 One to prevent this bug from occurring again:
https://github.com/zed-industries/zed/pull/44742
4.2 Another to prevent the early dismissal bug from occurring
5. Made `init_branch_list_test` param order consistent with Zed's code
base
###### Updated UI
Release Notes:
- N/A
---
Cargo.lock | 1 +
crates/git_ui/Cargo.toml | 1 +
crates/git_ui/src/branch_picker.rs | 379 +++++++++++++++++------------
3 files changed, 232 insertions(+), 149 deletions(-)
diff --git a/Cargo.lock b/Cargo.lock
index 834a072a92ff7334338b018eaecbdf7d71c48cdc..e465ff483bcb6cc9528403bbb8f3bd883a6af871 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -7045,6 +7045,7 @@ dependencies = [
"picker",
"pretty_assertions",
"project",
+ "rand 0.9.2",
"recent_projects",
"remote",
"schemars",
diff --git a/crates/git_ui/Cargo.toml b/crates/git_ui/Cargo.toml
index beaf192b0ef538fb524ff4986710255040b89f27..6747daa09d2801ad8c05c17fb04cb3ab235cdbff 100644
--- a/crates/git_ui/Cargo.toml
+++ b/crates/git_ui/Cargo.toml
@@ -74,6 +74,7 @@ gpui = { workspace = true, features = ["test-support"] }
indoc.workspace = true
pretty_assertions.workspace = true
project = { workspace = true, features = ["test-support"] }
+rand.workspace = true
settings = { workspace = true, features = ["test-support"] }
unindent.workspace = true
workspace = { workspace = true, features = ["test-support"] }
diff --git a/crates/git_ui/src/branch_picker.rs b/crates/git_ui/src/branch_picker.rs
index 8a08736d8bace6a77963c4325406d340903f1b73..79cd89d1485f6d99349b43d92c17261cf8a644e2 100644
--- a/crates/git_ui/src/branch_picker.rs
+++ b/crates/git_ui/src/branch_picker.rs
@@ -6,7 +6,7 @@ use collections::HashSet;
use git::repository::Branch;
use gpui::http_client::Url;
use gpui::{
- Action, App, AsyncApp, Context, DismissEvent, Entity, EventEmitter, FocusHandle, Focusable,
+ Action, App, Context, DismissEvent, Entity, EventEmitter, FocusHandle, Focusable,
InteractiveElement, IntoElement, Modifiers, ModifiersChangedEvent, ParentElement, Render,
SharedString, Styled, Subscription, Task, WeakEntity, Window, actions, rems,
};
@@ -17,8 +17,8 @@ use settings::Settings;
use std::sync::Arc;
use time::OffsetDateTime;
use ui::{
- CommonAnimationExt, Divider, HighlightedLabel, KeyBinding, ListHeader, ListItem,
- ListItemSpacing, Tooltip, prelude::*,
+ Divider, HighlightedLabel, KeyBinding, ListHeader, ListItem, ListItemSpacing, Tooltip,
+ prelude::*,
};
use util::ResultExt;
use workspace::notifications::DetachAndPromptErr;
@@ -232,21 +232,12 @@ impl BranchList {
window: &mut Window,
cx: &mut Context,
) {
- self.picker.update(cx, |this, cx| {
- this.delegate.display_remotes = !this.delegate.display_remotes;
- cx.spawn_in(window, async move |this, cx| {
- this.update_in(cx, |picker, window, cx| {
- let last_query = picker.delegate.last_query.clone();
- picker.delegate.update_matches(last_query, window, cx)
- })?
- .await;
-
- Result::Ok::<_, anyhow::Error>(())
- })
- .detach_and_log_err(cx);
+ self.picker.update(cx, |picker, cx| {
+ picker.delegate.branch_filter = picker.delegate.branch_filter.invert();
+ picker.update_matches(picker.query(cx), window, cx);
+ picker.refresh_placeholder(window, cx);
+ cx.notify();
});
-
- cx.notify();
}
}
impl ModalView for BranchList {}
@@ -289,6 +280,10 @@ enum Entry {
NewBranch {
name: String,
},
+ NewRemoteName {
+ name: String,
+ url: SharedString,
+ },
}
impl Entry {
@@ -304,6 +299,7 @@ impl Entry {
Entry::Branch { branch, .. } => branch.name(),
Entry::NewUrl { url, .. } => url.as_str(),
Entry::NewBranch { name, .. } => name.as_str(),
+ Entry::NewRemoteName { name, .. } => name.as_str(),
}
}
@@ -318,6 +314,23 @@ impl Entry {
}
}
+#[derive(Clone, Copy, PartialEq)]
+enum BranchFilter {
+ /// Only show local branches
+ Local,
+ /// Only show remote branches
+ Remote,
+}
+
+impl BranchFilter {
+ fn invert(&self) -> Self {
+ match self {
+ BranchFilter::Local => BranchFilter::Remote,
+ BranchFilter::Remote => BranchFilter::Local,
+ }
+ }
+}
+
pub struct BranchListDelegate {
workspace: Option>,
matches: Vec,
@@ -328,9 +341,8 @@ pub struct BranchListDelegate {
selected_index: usize,
last_query: String,
modifiers: Modifiers,
- display_remotes: bool,
+ branch_filter: BranchFilter,
state: PickerState,
- loading: bool,
focus_handle: FocusHandle,
}
@@ -363,9 +375,8 @@ impl BranchListDelegate {
selected_index: 0,
last_query: Default::default(),
modifiers: Default::default(),
- display_remotes: false,
+ branch_filter: BranchFilter::Local,
state: PickerState::List,
- loading: false,
focus_handle: cx.focus_handle(),
}
}
@@ -406,37 +417,13 @@ impl BranchListDelegate {
let Some(repo) = self.repo.clone() else {
return;
};
- cx.spawn(async move |this, cx| {
- this.update(cx, |picker, cx| {
- picker.delegate.loading = true;
- cx.notify();
- })
- .log_err();
- let stop_loader = |this: &WeakEntity>, cx: &mut AsyncApp| {
- this.update(cx, |picker, cx| {
- picker.delegate.loading = false;
- cx.notify();
- })
- .log_err();
- };
- repo.update(cx, |repo, _| repo.create_remote(remote_name, remote_url))
- .inspect_err(|_err| {
- stop_loader(&this, cx);
- })?
- .await
- .inspect_err(|_err| {
- stop_loader(&this, cx);
- })?
- .inspect_err(|_err| {
- stop_loader(&this, cx);
- })?;
- stop_loader(&this, cx);
- Ok(())
- })
- .detach_and_prompt_err("Failed to create remote", window, cx, |e, _, _cx| {
- Some(e.to_string())
- });
+ let receiver = repo.update(cx, |repo, _| repo.create_remote(remote_name, remote_url));
+
+ cx.background_spawn(async move { receiver.await? })
+ .detach_and_prompt_err("Failed to create remote", window, cx, |e, _, _cx| {
+ Some(e.to_string())
+ });
cx.emit(DismissEvent);
}
@@ -528,29 +515,33 @@ impl PickerDelegate for BranchListDelegate {
type ListItem = ListItem;
fn placeholder_text(&self, _window: &mut Window, _cx: &mut App) -> Arc {
- "Select branch…".into()
+ match self.state {
+ PickerState::List | PickerState::NewRemote | PickerState::NewBranch => {
+ match self.branch_filter {
+ BranchFilter::Local => "Select branch…",
+ BranchFilter::Remote => "Select remote…",
+ }
+ }
+ PickerState::CreateRemote(_) => "Enter a name for this remote…",
+ }
+ .into()
+ }
+
+ fn no_matches_text(&self, _window: &mut Window, _cx: &mut App) -> Option {
+ match self.state {
+ PickerState::CreateRemote(_) => {
+ Some(SharedString::new_static("Remote name can't be empty"))
+ }
+ _ => None,
+ }
}
fn render_editor(
&self,
editor: &Entity,
- window: &mut Window,
- cx: &mut Context>,
+ _window: &mut Window,
+ _cx: &mut Context>,
) -> Div {
- cx.update_entity(editor, move |editor, cx| {
- let placeholder = match self.state {
- PickerState::List | PickerState::NewRemote | PickerState::NewBranch => {
- if self.display_remotes {
- "Select remote…"
- } else {
- "Select branch…"
- }
- }
- PickerState::CreateRemote(_) => "Choose a name…",
- };
- editor.set_placeholder_text(placeholder, window, cx);
- });
-
let focus_handle = self.focus_handle.clone();
v_flex()
@@ -568,16 +559,14 @@ impl PickerDelegate for BranchListDelegate {
.when(
self.editor_position() == PickerEditorPosition::End,
|this| {
- let tooltip_label = if self.display_remotes {
- "Turn Off Remote Filter"
- } else {
- "Filter Remote Branches"
+ let tooltip_label = match self.branch_filter {
+ BranchFilter::Local => "Turn Off Remote Filter",
+ BranchFilter::Remote => "Filter Remote Branches",
};
this.gap_1().justify_between().child({
IconButton::new("filter-remotes", IconName::Filter)
- .disabled(self.loading)
- .toggle_state(self.display_remotes)
+ .toggle_state(self.branch_filter == BranchFilter::Remote)
.tooltip(move |_, cx| {
Tooltip::for_action_in(
tooltip_label,
@@ -636,13 +625,13 @@ impl PickerDelegate for BranchListDelegate {
return Task::ready(());
};
- let display_remotes = self.display_remotes;
+ let display_remotes = self.branch_filter;
cx.spawn_in(window, async move |picker, cx| {
let mut matches: Vec = if query.is_empty() {
all_branches
.into_iter()
.filter(|branch| {
- if display_remotes {
+ if display_remotes == BranchFilter::Remote {
branch.is_remote()
} else {
!branch.is_remote()
@@ -657,7 +646,7 @@ impl PickerDelegate for BranchListDelegate {
let branches = all_branches
.iter()
.filter(|branch| {
- if display_remotes {
+ if display_remotes == BranchFilter::Remote {
branch.is_remote()
} else {
!branch.is_remote()
@@ -688,11 +677,19 @@ impl PickerDelegate for BranchListDelegate {
};
picker
.update(cx, |picker, _| {
- if matches!(picker.delegate.state, PickerState::CreateRemote(_)) {
+ if let PickerState::CreateRemote(url) = &picker.delegate.state {
+ let query = query.replace(' ', "-");
+ if !query.is_empty() {
+ picker.delegate.matches = vec![Entry::NewRemoteName {
+ name: query.clone(),
+ url: url.clone(),
+ }];
+ picker.delegate.selected_index = 0;
+ } else {
+ picker.delegate.matches = Vec::new();
+ picker.delegate.selected_index = 0;
+ }
picker.delegate.last_query = query;
- picker.delegate.matches = Vec::new();
- picker.delegate.selected_index = 0;
-
return;
}
@@ -736,13 +733,6 @@ impl PickerDelegate for BranchListDelegate {
}
fn confirm(&mut self, secondary: bool, window: &mut Window, cx: &mut Context>) {
- if let PickerState::CreateRemote(remote_url) = &self.state {
- self.create_remote(self.last_query.clone(), remote_url.to_string(), window, cx);
- self.state = PickerState::List;
- cx.notify();
- return;
- }
-
let Some(entry) = self.matches.get(self.selected_index()) else {
return;
};
@@ -785,13 +775,19 @@ impl PickerDelegate for BranchListDelegate {
self.state = PickerState::CreateRemote(url.clone().into());
self.matches = Vec::new();
self.selected_index = 0;
- cx.spawn_in(window, async move |this, cx| {
- this.update_in(cx, |picker, window, cx| {
- picker.set_query("", window, cx);
- })
- })
- .detach_and_log_err(cx);
- cx.notify();
+
+ cx.defer_in(window, |picker, window, cx| {
+ picker.refresh_placeholder(window, cx);
+ picker.set_query("", window, cx);
+ cx.notify();
+ });
+
+ // returning early to prevent dismissing the modal, so a user can enter
+ // a remote name first.
+ return;
+ }
+ Entry::NewRemoteName { name, url } => {
+ self.create_remote(name.clone(), url.to_string(), window, cx);
}
Entry::NewBranch { name } => {
let from_branch = if secondary {
@@ -842,17 +838,13 @@ impl PickerDelegate for BranchListDelegate {
.unwrap_or_else(|| (None, None, None));
let entry_icon = match entry {
- Entry::NewUrl { .. } | Entry::NewBranch { .. } => {
+ Entry::NewUrl { .. } | Entry::NewBranch { .. } | Entry::NewRemoteName { .. } => {
Icon::new(IconName::Plus).color(Color::Muted)
}
-
- Entry::Branch { .. } => {
- if self.display_remotes {
- Icon::new(IconName::Screen).color(Color::Muted)
- } else {
- Icon::new(IconName::GitBranchAlt).color(Color::Muted)
- }
- }
+ Entry::Branch { .. } => match self.branch_filter {
+ BranchFilter::Local => Icon::new(IconName::GitBranchAlt).color(Color::Muted),
+ BranchFilter::Remote => Icon::new(IconName::Screen).color(Color::Muted),
+ },
};
let entry_title = match entry {
@@ -864,6 +856,10 @@ impl PickerDelegate for BranchListDelegate {
.single_line()
.truncate()
.into_any_element(),
+ Entry::NewRemoteName { name, .. } => Label::new(format!("Create Remote: \"{name}\""))
+ .single_line()
+ .truncate()
+ .into_any_element(),
Entry::Branch { branch, positions } => {
HighlightedLabel::new(branch.name().to_string(), positions.clone())
.single_line()
@@ -873,7 +869,10 @@ impl PickerDelegate for BranchListDelegate {
};
let focus_handle = self.focus_handle.clone();
- let is_new_items = matches!(entry, Entry::NewUrl { .. } | Entry::NewBranch { .. });
+ let is_new_items = matches!(
+ entry,
+ Entry::NewUrl { .. } | Entry::NewBranch { .. } | Entry::NewRemoteName { .. }
+ );
let delete_branch_button = IconButton::new("delete", IconName::Trash)
.tooltip(move |_, cx| {
@@ -935,6 +934,9 @@ impl PickerDelegate for BranchListDelegate {
Entry::NewUrl { url } => {
format!("Based off {url}")
}
+ Entry::NewRemoteName { url, .. } => {
+ format!("Based off {url}")
+ }
Entry::NewBranch { .. } => {
if let Some(current_branch) =
self.repo.as_ref().and_then(|repo| {
@@ -1033,10 +1035,9 @@ impl PickerDelegate for BranchListDelegate {
_cx: &mut Context>,
) -> Option {
matches!(self.state, PickerState::List).then(|| {
- let label = if self.display_remotes {
- "Remote"
- } else {
- "Local"
+ let label = match self.branch_filter {
+ BranchFilter::Local => "Local",
+ BranchFilter::Remote => "Remote",
};
ListHeader::new(label).inset(true).into_any_element()
@@ -1047,11 +1048,7 @@ impl PickerDelegate for BranchListDelegate {
if self.editor_position() == PickerEditorPosition::End {
return None;
}
-
let focus_handle = self.focus_handle.clone();
- let loading_icon = Icon::new(IconName::LoadCircle)
- .size(IconSize::Small)
- .with_rotate_animation(3);
let footer_container = || {
h_flex()
@@ -1090,7 +1087,6 @@ impl PickerDelegate for BranchListDelegate {
.gap_1()
.child(
Button::new("delete-branch", "Delete")
- .disabled(self.loading)
.key_binding(
KeyBinding::for_action_in(
&branch_picker::DeleteBranch,
@@ -1138,17 +1134,15 @@ impl PickerDelegate for BranchListDelegate {
)
},
)
- } else if self.loading {
- this.justify_between()
- .child(loading_icon)
- .child(delete_and_select_btns)
} else {
this.justify_between()
.child({
let focus_handle = focus_handle.clone();
Button::new("filter-remotes", "Filter Remotes")
- .disabled(self.loading)
- .toggle_state(self.display_remotes)
+ .toggle_state(matches!(
+ self.branch_filter,
+ BranchFilter::Remote
+ ))
.key_binding(
KeyBinding::for_action_in(
&branch_picker::FilterRemotes,
@@ -1213,14 +1207,15 @@ impl PickerDelegate for BranchListDelegate {
footer_container()
.justify_end()
.child(
- Label::new("Choose a name for this remote repository")
- .size(LabelSize::Small)
- .color(Color::Muted),
- )
- .child(
- Label::new("Save")
- .size(LabelSize::Small)
- .color(Color::Muted),
+ Button::new("branch-from-default", "Confirm")
+ .key_binding(
+ KeyBinding::for_action_in(&menu::Confirm, &focus_handle, cx)
+ .map(|kb| kb.size(rems_from_px(12.))),
+ )
+ .on_click(cx.listener(|this, _, window, cx| {
+ this.delegate.confirm(false, window, cx);
+ }))
+ .disabled(self.last_query.is_empty()),
)
.into_any_element(),
),
@@ -1237,6 +1232,7 @@ mod tests {
use git::repository::{CommitSummary, Remote};
use gpui::{TestAppContext, VisualTestContext};
use project::{FakeFs, Project};
+ use rand::{Rng, rngs::StdRng};
use serde_json::json;
use settings::SettingsStore;
use util::path;
@@ -1284,10 +1280,10 @@ mod tests {
}
fn init_branch_list_test(
- cx: &mut TestAppContext,
repository: Option>,
branches: Vec,
- ) -> (VisualTestContext, Entity) {
+ cx: &mut TestAppContext,
+ ) -> (Entity, VisualTestContext) {
let window = cx.add_window(|window, cx| {
let mut delegate =
BranchListDelegate::new(None, repository, BranchListStyle::Modal, cx);
@@ -1313,7 +1309,7 @@ mod tests {
let branch_list = window.root(cx).unwrap();
let cx = VisualTestContext::from_window(*window, cx);
- (cx, branch_list)
+ (branch_list, cx)
}
async fn init_fake_repository(cx: &mut TestAppContext) -> Entity {
@@ -1347,7 +1343,7 @@ mod tests {
init_test(cx);
let branches = create_test_branches();
- let (mut ctx, branch_list) = init_branch_list_test(cx, None, branches);
+ let (branch_list, mut ctx) = init_branch_list_test(None, branches, cx);
let cx = &mut ctx;
branch_list
@@ -1423,7 +1419,7 @@ mod tests {
.await;
cx.run_until_parked();
- let (mut ctx, branch_list) = init_branch_list_test(cx, repository.into(), branches);
+ let (branch_list, mut ctx) = init_branch_list_test(repository.into(), branches, cx);
let cx = &mut ctx;
update_branch_list_matches_with_empty_query(&branch_list, cx).await;
@@ -1488,12 +1484,12 @@ mod tests {
.await;
cx.run_until_parked();
- let (mut ctx, branch_list) = init_branch_list_test(cx, repository.into(), branches);
+ let (branch_list, mut ctx) = init_branch_list_test(repository.into(), branches, cx);
let cx = &mut ctx;
// Enable remote filter
branch_list.update(cx, |branch_list, cx| {
branch_list.picker.update(cx, |picker, _cx| {
- picker.delegate.display_remotes = true;
+ picker.delegate.branch_filter = BranchFilter::Remote;
});
});
update_branch_list_matches_with_empty_query(&branch_list, cx).await;
@@ -1546,7 +1542,7 @@ mod tests {
create_test_branch("develop", false, None, Some(700)),
];
- let (mut ctx, branch_list) = init_branch_list_test(cx, None, branches);
+ let (branch_list, mut ctx) = init_branch_list_test(None, branches, cx);
let cx = &mut ctx;
update_branch_list_matches_with_empty_query(&branch_list, cx).await;
@@ -1573,7 +1569,7 @@ mod tests {
let last_match = picker.delegate.matches.last().unwrap();
assert!(!last_match.is_new_branch());
assert!(!last_match.is_new_url());
- picker.delegate.display_remotes = true;
+ picker.delegate.branch_filter = BranchFilter::Remote;
picker.delegate.update_matches(String::new(), window, cx)
})
})
@@ -1600,7 +1596,7 @@ mod tests {
// Verify the last entry is NOT the "create new branch" option
let last_match = picker.delegate.matches.last().unwrap();
assert!(!last_match.is_new_url());
- picker.delegate.display_remotes = true;
+ picker.delegate.branch_filter = BranchFilter::Remote;
picker
.delegate
.update_matches(String::from("fork"), window, cx)
@@ -1629,22 +1625,27 @@ mod tests {
#[gpui::test]
async fn test_new_branch_creation_with_query(test_cx: &mut TestAppContext) {
+ const MAIN_BRANCH: &str = "main";
+ const FEATURE_BRANCH: &str = "feature";
+ const NEW_BRANCH: &str = "new-feature-branch";
+
init_test(test_cx);
let repository = init_fake_repository(test_cx).await;
let branches = vec![
- create_test_branch("main", true, None, Some(1000)),
- create_test_branch("feature", false, None, Some(900)),
+ create_test_branch(MAIN_BRANCH, true, None, Some(1000)),
+ create_test_branch(FEATURE_BRANCH, false, None, Some(900)),
];
- let (mut ctx, branch_list) = init_branch_list_test(test_cx, repository.into(), branches);
+ let (branch_list, mut ctx) = init_branch_list_test(repository.into(), branches, test_cx);
let cx = &mut ctx;
branch_list
.update_in(cx, |branch_list, window, cx| {
branch_list.picker.update(cx, |picker, cx| {
- let query = "new-feature-branch".to_string();
- picker.delegate.update_matches(query, window, cx)
+ picker
+ .delegate
+ .update_matches(NEW_BRANCH.to_string(), window, cx)
})
})
.await;
@@ -1655,7 +1656,7 @@ mod tests {
branch_list.picker.update(cx, |picker, cx| {
let last_match = picker.delegate.matches.last().unwrap();
assert!(last_match.is_new_branch());
- assert_eq!(last_match.name(), "new-feature-branch");
+ assert_eq!(last_match.name(), NEW_BRANCH);
// State is NewBranch because no existing branches fuzzy-match the query
assert!(matches!(picker.delegate.state, PickerState::NewBranch));
picker.delegate.confirm(false, window, cx);
@@ -1680,11 +1681,11 @@ mod tests {
let new_branch = branches
.into_iter()
- .find(|branch| branch.name() == "new-feature-branch")
+ .find(|branch| branch.name() == NEW_BRANCH)
.expect("new-feature-branch should exist");
assert_eq!(
new_branch.ref_name.as_ref(),
- "refs/heads/new-feature-branch",
+ &format!("refs/heads/{NEW_BRANCH}"),
"branch ref_name should not have duplicate refs/heads/ prefix"
);
}
@@ -1695,7 +1696,7 @@ mod tests {
let repository = init_fake_repository(cx).await;
let branches = vec![create_test_branch("main", true, None, Some(1000))];
- let (mut ctx, branch_list) = init_branch_list_test(cx, repository.into(), branches);
+ let (branch_list, mut ctx) = init_branch_list_test(repository.into(), branches, cx);
let cx = &mut ctx;
branch_list
@@ -1734,8 +1735,13 @@ mod tests {
branch_list.update_in(cx, |branch_list, window, cx| {
branch_list.picker.update(cx, |picker, cx| {
+ assert_eq!(picker.delegate.matches.len(), 1);
+ assert!(matches!(
+ picker.delegate.matches.first(),
+ Some(Entry::NewRemoteName { name, url })
+ if name == "my_new_remote" && url.as_ref() == "https://github.com/user/repo.git"
+ ));
picker.delegate.confirm(false, window, cx);
- assert_eq!(picker.delegate.matches.len(), 0);
})
});
cx.run_until_parked();
@@ -1768,7 +1774,7 @@ mod tests {
init_test(cx);
let branches = vec![create_test_branch("main_branch", true, None, Some(1000))];
- let (mut ctx, branch_list) = init_branch_list_test(cx, None, branches);
+ let (branch_list, mut ctx) = init_branch_list_test(None, branches, cx);
let cx = &mut ctx;
branch_list
@@ -1823,4 +1829,79 @@ mod tests {
})
});
}
+
+ #[gpui::test]
+ async fn test_confirm_remote_url_does_not_dismiss(cx: &mut TestAppContext) {
+ const REMOTE_URL: &str = "https://github.com/user/repo.git";
+
+ init_test(cx);
+ let branches = vec![create_test_branch("main", true, None, Some(1000))];
+
+ let (branch_list, mut ctx) = init_branch_list_test(None, branches, cx);
+ let cx = &mut ctx;
+
+ let subscription = cx.update(|_, cx| {
+ cx.subscribe(&branch_list, |_, _: &DismissEvent, _| {
+ panic!("DismissEvent should not be emitted when confirming a remote URL");
+ })
+ });
+
+ branch_list
+ .update_in(cx, |branch_list, window, cx| {
+ window.focus(&branch_list.picker_focus_handle);
+ branch_list.picker.update(cx, |picker, cx| {
+ picker
+ .delegate
+ .update_matches(REMOTE_URL.to_string(), window, cx)
+ })
+ })
+ .await;
+
+ cx.run_until_parked();
+
+ branch_list.update_in(cx, |branch_list, window, cx| {
+ branch_list.picker.update(cx, |picker, cx| {
+ let last_match = picker.delegate.matches.last().unwrap();
+ assert!(last_match.is_new_url());
+ assert!(matches!(picker.delegate.state, PickerState::NewRemote));
+
+ picker.delegate.confirm(false, window, cx);
+
+ assert!(
+ matches!(picker.delegate.state, PickerState::CreateRemote(ref url) if url.as_ref() == REMOTE_URL),
+ "State should transition to CreateRemote with the URL"
+ );
+ });
+
+ assert!(
+ branch_list.picker_focus_handle.is_focused(window),
+ "Branch list picker should still be focused after confirming remote URL"
+ );
+ });
+
+ cx.run_until_parked();
+
+ drop(subscription);
+ }
+
+ #[gpui::test(iterations = 10)]
+ async fn test_empty_query_displays_all_branches(mut rng: StdRng, cx: &mut TestAppContext) {
+ init_test(cx);
+ let branch_count = rng.random_range(13..540);
+
+ let branches: Vec = (0..branch_count)
+ .map(|i| create_test_branch(&format!("branch-{:02}", i), i == 0, None, Some(i * 100)))
+ .collect();
+
+ let (branch_list, mut ctx) = init_branch_list_test(None, branches, cx);
+ let cx = &mut ctx;
+
+ update_branch_list_matches_with_empty_query(&branch_list, cx).await;
+
+ branch_list.update(cx, |branch_list, cx| {
+ branch_list.picker.update(cx, |picker, _cx| {
+ assert_eq!(picker.delegate.matches.len(), branch_count as usize);
+ })
+ });
+ }
}
From e9073eceeba8e1a71e966582566179517591ec6b Mon Sep 17 00:00:00 2001
From: Danilo Leal <67129314+danilo-leal@users.noreply.github.com>
Date: Sun, 14 Dec 2025 10:48:23 -0300
Subject: [PATCH 002/257] agent_ui: Fix fallback icon used for external agents
(#44777)
When an external agent doesn't provide an icon, we were using different
fallback icons in all the places we display icons (settings view, thread
new menu, and the thread view toolbar itself).
Release Notes:
- N/A
---
crates/agent_ui/src/agent_configuration.rs | 3 ++-
crates/agent_ui/src/agent_panel.rs | 21 +++++++++++----------
2 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/crates/agent_ui/src/agent_configuration.rs b/crates/agent_ui/src/agent_configuration.rs
index 8619b085c00268d6d157dee37411ff36ba4d5680..24f019c605d1b167e62a6e68dfc1f3ed07c73f1c 100644
--- a/crates/agent_ui/src/agent_configuration.rs
+++ b/crates/agent_ui/src/agent_configuration.rs
@@ -975,7 +975,7 @@ impl AgentConfiguration {
let icon = if let Some(icon_path) = agent_server_store.agent_icon(&name) {
AgentIcon::Path(icon_path)
} else {
- AgentIcon::Name(IconName::Ai)
+ AgentIcon::Name(IconName::Sparkle)
};
let display_name = agent_server_store
.agent_display_name(&name)
@@ -1137,6 +1137,7 @@ impl AgentConfiguration {
) -> impl IntoElement {
let id = id.into();
let display_name = display_name.into();
+
let icon = match icon {
AgentIcon::Name(icon_name) => Icon::new(icon_name)
.size(IconSize::Small)
diff --git a/crates/agent_ui/src/agent_panel.rs b/crates/agent_ui/src/agent_panel.rs
index 2f6a722b471a189eafbc7aadbddb927476e4b3b9..97c7aecb8e34563db0adfa6bdbeda31140fd6cdd 100644
--- a/crates/agent_ui/src/agent_panel.rs
+++ b/crates/agent_ui/src/agent_panel.rs
@@ -259,7 +259,7 @@ impl AgentType {
Self::Gemini => Some(IconName::AiGemini),
Self::ClaudeCode => Some(IconName::AiClaude),
Self::Codex => Some(IconName::AiOpenAi),
- Self::Custom { .. } => Some(IconName::Terminal),
+ Self::Custom { .. } => Some(IconName::Sparkle),
}
}
}
@@ -1851,14 +1851,17 @@ impl AgentPanel {
let agent_server_store = self.project.read(cx).agent_server_store().clone();
let focus_handle = self.focus_handle(cx);
- // Get custom icon path for selected agent before building menu (to avoid borrow issues)
- let selected_agent_custom_icon =
+ let (selected_agent_custom_icon, selected_agent_label) =
if let AgentType::Custom { name, .. } = &self.selected_agent {
- agent_server_store
- .read(cx)
- .agent_icon(&ExternalAgentServerName(name.clone()))
+ let store = agent_server_store.read(cx);
+ let icon = store.agent_icon(&ExternalAgentServerName(name.clone()));
+
+ let label = store
+ .agent_display_name(&ExternalAgentServerName(name.clone()))
+ .unwrap_or_else(|| self.selected_agent.label());
+ (icon, label)
} else {
- None
+ (None, self.selected_agent.label())
};
let active_thread = match &self.active_view {
@@ -2090,7 +2093,7 @@ impl AgentPanel {
if let Some(icon_path) = icon_path {
entry = entry.custom_icon_svg(icon_path);
} else {
- entry = entry.icon(IconName::Terminal);
+ entry = entry.icon(IconName::Sparkle);
}
entry = entry
.when(
@@ -2154,8 +2157,6 @@ impl AgentPanel {
}
});
- let selected_agent_label = self.selected_agent.label();
-
let is_thread_loading = self
.active_thread_view()
.map(|thread| thread.read(cx).is_loading())
From 13594bd97ec1250d49d99c3587575607837bad97 Mon Sep 17 00:00:00 2001
From: Lukas Wirth
Date: Sun, 14 Dec 2025 19:01:22 +0100
Subject: [PATCH 003/257] keymap: More default keymap fixes for windows/linux
(#44821)
Release Notes:
- N/A *or* Added/Fixed/Improved ...
---
assets/keymaps/default-linux.json | 2 +-
assets/keymaps/default-windows.json | 4 +++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/assets/keymaps/default-linux.json b/assets/keymaps/default-linux.json
index 0bcbb455b502642237347cf9fc36b91eab83f20b..872544cdff0bc03bbafd6b711fa7adb2f5e2d008 100644
--- a/assets/keymaps/default-linux.json
+++ b/assets/keymaps/default-linux.json
@@ -63,7 +63,6 @@
"delete": "editor::Delete",
"tab": "editor::Tab",
"shift-tab": "editor::Backtab",
- "ctrl-k": "editor::CutToEndOfLine",
"ctrl-k ctrl-q": "editor::Rewrap",
"ctrl-k q": "editor::Rewrap",
"ctrl-backspace": ["editor::DeleteToPreviousWordStart", { "ignore_newlines": false, "ignore_brackets": false }],
@@ -501,6 +500,7 @@
"ctrl-k ctrl-i": "editor::Hover",
"ctrl-k ctrl-b": "editor::BlameHover",
"ctrl-/": ["editor::ToggleComments", { "advance_downwards": false }],
+ "ctrl-k ctrl-c": ["editor::ToggleComments", { "advance_downwards": false }],
"f8": ["editor::GoToDiagnostic", { "severity": { "min": "hint", "max": "error" } }],
"shift-f8": ["editor::GoToPreviousDiagnostic", { "severity": { "min": "hint", "max": "error" } }],
"f2": "editor::Rename",
diff --git a/assets/keymaps/default-windows.json b/assets/keymaps/default-windows.json
index 51943ab35587e633a25eb9420c45dff21048330a..ae051f233e344cc6b961612c690ae1b5107fb2c0 100644
--- a/assets/keymaps/default-windows.json
+++ b/assets/keymaps/default-windows.json
@@ -63,7 +63,6 @@
"delete": "editor::Delete",
"tab": "editor::Tab",
"shift-tab": "editor::Backtab",
- "ctrl-k": "editor::CutToEndOfLine",
"ctrl-k ctrl-q": "editor::Rewrap",
"ctrl-k q": "editor::Rewrap",
"ctrl-backspace": ["editor::DeleteToPreviousWordStart", { "ignore_newlines": false, "ignore_brackets": false }],
@@ -465,8 +464,10 @@
"ctrl-k ctrl-w": "workspace::CloseAllItemsAndPanes",
"back": "pane::GoBack",
"alt--": "pane::GoBack",
+ "alt-left": "pane::GoBack",
"forward": "pane::GoForward",
"alt-=": "pane::GoForward",
+ "alt-right": "pane::GoForward",
"f3": "search::SelectNextMatch",
"shift-f3": "search::SelectPreviousMatch",
"ctrl-shift-f": "project_search::ToggleFocus",
@@ -508,6 +509,7 @@
"ctrl-k ctrl-b": "editor::BlameHover",
"ctrl-k ctrl-f": "editor::FormatSelections",
"ctrl-/": ["editor::ToggleComments", { "advance_downwards": false }],
+ "ctrl-k ctrl-c": ["editor::ToggleComments", { "advance_downwards": false }],
"f8": ["editor::GoToDiagnostic", { "severity": { "min": "hint", "max": "error" } }],
"shift-f8": ["editor::GoToPreviousDiagnostic", { "severity": { "min": "hint", "max": "error" } }],
"f2": "editor::Rename",
From f80ef9a3c52a0d07bc3db536f853b6e0083dfdd3 Mon Sep 17 00:00:00 2001
From: Lukas Wirth
Date: Sun, 14 Dec 2025 19:21:50 +0100
Subject: [PATCH 004/257] editor: Fix inlay hovers blinking in sync with
cursors (#44822)
This change matches how normal hovers are handled (which early return
with `None` in this branch)
Release Notes:
- Fixed hover boxes for inlays blinking in and out without movement when
cursor blinking was enabled
---
crates/editor/src/hover_popover.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/crates/editor/src/hover_popover.rs b/crates/editor/src/hover_popover.rs
index edf10671b9e4c63e2918f6e144ba1b553e44daca..7c3e41e8c2edf721fbcae729069eecb640e2246c 100644
--- a/crates/editor/src/hover_popover.rs
+++ b/crates/editor/src/hover_popover.rs
@@ -151,7 +151,7 @@ pub fn hover_at_inlay(
false
})
{
- hide_hover(editor, cx);
+ return;
}
let hover_popover_delay = EditorSettings::get_global(cx).hover_popover_delay.0;
From 26b261a33645f20993fd8f819109b37cabcdc67c Mon Sep 17 00:00:00 2001
From: Nathan Sobo
Date: Sun, 14 Dec 2025 11:47:15 -0700
Subject: [PATCH 005/257] Implement Sum trait for Pixels (#44809)
This adds implementations of `std::iter::Sum` for `Pixels`, allowing the
use of `.sum()` on iterators of `Pixels` values.
### Changes
- Implement `Sum` for `Pixels` (owned values)
- Implement `Sum<&Pixels>` for `Pixels` (references)
This enables ergonomic patterns like:
```rust
let total: Pixels = pixel_values.iter().sum();
```
---
crates/gpui/src/geometry.rs | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/crates/gpui/src/geometry.rs b/crates/gpui/src/geometry.rs
index f466624dfb91af9b4a33421ea15827ebe2559665..fc735ba5e0e7e719ed12b6b1b168ec3ee49e22bb 100644
--- a/crates/gpui/src/geometry.rs
+++ b/crates/gpui/src/geometry.rs
@@ -2648,6 +2648,18 @@ impl Debug for Pixels {
}
}
+impl std::iter::Sum for Pixels {
+ fn sum>(iter: I) -> Self {
+ iter.fold(Self::ZERO, |a, b| a + b)
+ }
+}
+
+impl<'a> std::iter::Sum<&'a Pixels> for Pixels {
+ fn sum>(iter: I) -> Self {
+ iter.fold(Self::ZERO, |a, b| a + *b)
+ }
+}
+
impl TryFrom<&'_ str> for Pixels {
type Error = anyhow::Error;
From a51585d2daaa975409a835f43574c8bb5bcc9d5b Mon Sep 17 00:00:00 2001
From: Nathan Sobo
Date: Sun, 14 Dec 2025 12:58:26 -0700
Subject: [PATCH 006/257] Fix race condition in
test_collaborating_with_completion (#44806)
The test `test_collaborating_with_completion` has a latent race
condition that hasn't manifested on CI yet but could cause hangs with
certain task orderings.
## The Bug
Commit `fd1494c31a` set up LSP request handlers AFTER typing the trigger
character:
```rust
// Type trigger first - spawns async tasks to send completion request
editor_b.update_in(cx_b, |editor, window, cx| {
editor.handle_input(".", window, cx);
});
// THEN set up handlers (race condition!)
fake_language_server
.set_request_handler::(...)
.next().await.unwrap(); // Waits for handler to receive a request
```
Whether this works depends on task scheduling order, which varies by
seed. If the completion request is processed before the handler is
registered, the request goes to `on_unhandled_notification` which claims
to handle it but sends no response, causing a hang.
## Changes
- Move handler setup BEFORE typing the trigger character
- Make `TestDispatcher::spawn_realtime` panic to prevent future
non-determinism from real OS threads
- Add `execution_hash()` and `execution_count()` to TestDispatcher for
debugging
- Add `DEBUG_SCHEDULER=1` logging for task execution tracing
- Document the investigation in `situation.md`
cc @localcc @SomeoneToIgnore (authors of related commits)
Release Notes:
- N/A
---------
Co-authored-by: Kirill Bulatov
---
crates/collab/src/tests/editor_tests.rs | 109 ++++++++++++------------
1 file changed, 54 insertions(+), 55 deletions(-)
diff --git a/crates/collab/src/tests/editor_tests.rs b/crates/collab/src/tests/editor_tests.rs
index ba92e868126c7f27fb5051021fce44fe43c8d5e7..4e6cdb0e79aba494bd01137cc262a097a084217e 100644
--- a/crates/collab/src/tests/editor_tests.rs
+++ b/crates/collab/src/tests/editor_tests.rs
@@ -312,6 +312,49 @@ async fn test_collaborating_with_completion(cx_a: &mut TestAppContext, cx_b: &mu
"Rust",
FakeLspAdapter {
capabilities: capabilities.clone(),
+ initializer: Some(Box::new(|fake_server| {
+ fake_server.set_request_handler::(
+ |params, _| async move {
+ assert_eq!(
+ params.text_document_position.text_document.uri,
+ lsp::Uri::from_file_path(path!("/a/main.rs")).unwrap(),
+ );
+ assert_eq!(
+ params.text_document_position.position,
+ lsp::Position::new(0, 14),
+ );
+
+ Ok(Some(lsp::CompletionResponse::Array(vec![
+ lsp::CompletionItem {
+ label: "first_method(…)".into(),
+ detail: Some("fn(&mut self, B) -> C".into()),
+ text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit {
+ new_text: "first_method($1)".to_string(),
+ range: lsp::Range::new(
+ lsp::Position::new(0, 14),
+ lsp::Position::new(0, 14),
+ ),
+ })),
+ insert_text_format: Some(lsp::InsertTextFormat::SNIPPET),
+ ..Default::default()
+ },
+ lsp::CompletionItem {
+ label: "second_method(…)".into(),
+ detail: Some("fn(&mut self, C) -> D".into()),
+ text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit {
+ new_text: "second_method()".to_string(),
+ range: lsp::Range::new(
+ lsp::Position::new(0, 14),
+ lsp::Position::new(0, 14),
+ ),
+ })),
+ insert_text_format: Some(lsp::InsertTextFormat::SNIPPET),
+ ..Default::default()
+ },
+ ])))
+ },
+ );
+ })),
..FakeLspAdapter::default()
},
),
@@ -320,6 +363,11 @@ async fn test_collaborating_with_completion(cx_a: &mut TestAppContext, cx_b: &mu
FakeLspAdapter {
name: "fake-analyzer",
capabilities: capabilities.clone(),
+ initializer: Some(Box::new(|fake_server| {
+ fake_server.set_request_handler::(
+ |_, _| async move { Ok(None) },
+ );
+ })),
..FakeLspAdapter::default()
},
),
@@ -373,6 +421,7 @@ async fn test_collaborating_with_completion(cx_a: &mut TestAppContext, cx_b: &mu
let fake_language_server = fake_language_servers[0].next().await.unwrap();
let second_fake_language_server = fake_language_servers[1].next().await.unwrap();
cx_a.background_executor.run_until_parked();
+ cx_b.background_executor.run_until_parked();
buffer_b.read_with(cx_b, |buffer, _| {
assert!(!buffer.completion_triggers().is_empty())
@@ -387,58 +436,9 @@ async fn test_collaborating_with_completion(cx_a: &mut TestAppContext, cx_b: &mu
});
cx_b.focus(&editor_b);
- // Receive a completion request as the host's language server.
- // Return some completions from the host's language server.
- cx_a.executor().start_waiting();
- fake_language_server
- .set_request_handler::(|params, _| async move {
- assert_eq!(
- params.text_document_position.text_document.uri,
- lsp::Uri::from_file_path(path!("/a/main.rs")).unwrap(),
- );
- assert_eq!(
- params.text_document_position.position,
- lsp::Position::new(0, 14),
- );
-
- Ok(Some(lsp::CompletionResponse::Array(vec![
- lsp::CompletionItem {
- label: "first_method(…)".into(),
- detail: Some("fn(&mut self, B) -> C".into()),
- text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit {
- new_text: "first_method($1)".to_string(),
- range: lsp::Range::new(
- lsp::Position::new(0, 14),
- lsp::Position::new(0, 14),
- ),
- })),
- insert_text_format: Some(lsp::InsertTextFormat::SNIPPET),
- ..Default::default()
- },
- lsp::CompletionItem {
- label: "second_method(…)".into(),
- detail: Some("fn(&mut self, C) -> D".into()),
- text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit {
- new_text: "second_method()".to_string(),
- range: lsp::Range::new(
- lsp::Position::new(0, 14),
- lsp::Position::new(0, 14),
- ),
- })),
- insert_text_format: Some(lsp::InsertTextFormat::SNIPPET),
- ..Default::default()
- },
- ])))
- })
- .next()
- .await
- .unwrap();
- second_fake_language_server
- .set_request_handler::(|_, _| async move { Ok(None) })
- .next()
- .await
- .unwrap();
- cx_a.executor().finish_waiting();
+ // Allow the completion request to propagate from guest to host to LSP.
+ cx_b.background_executor.run_until_parked();
+ cx_a.background_executor.run_until_parked();
// Open the buffer on the host.
let buffer_a = project_a
@@ -484,6 +484,7 @@ async fn test_collaborating_with_completion(cx_a: &mut TestAppContext, cx_b: &mu
// The additional edit is applied.
cx_a.executor().run_until_parked();
+ cx_b.executor().run_until_parked();
buffer_a.read_with(cx_a, |buffer, _| {
assert_eq!(
@@ -641,13 +642,11 @@ async fn test_collaborating_with_completion(cx_a: &mut TestAppContext, cx_b: &mu
),
})),
insert_text_format: Some(lsp::InsertTextFormat::SNIPPET),
- ..Default::default()
+ ..lsp::CompletionItem::default()
},
])))
});
- cx_b.executor().run_until_parked();
-
// Await both language server responses
first_lsp_completion.next().await.unwrap();
second_lsp_completion.next().await.unwrap();
From 86aa9abc9036fa94cf2a98aefbefb1d7c71ad699 Mon Sep 17 00:00:00 2001
From: Cole Miller
Date: Sun, 14 Dec 2025 21:48:15 -0500
Subject: [PATCH 007/257] git: Avoid removing project excerpts for dirty
buffers (#44312)
Imitating the approach of #41829. Prevents e.g. reverting a hunk and
having that excerpt yanked out from under the cursor.
Release Notes:
- git: Improved stability of excerpts when editing in the project diff.
---
crates/acp_thread/src/diff.rs | 2 +-
crates/agent_ui/src/agent_diff.rs | 7 +-
crates/editor/src/editor.rs | 40 +---------
crates/editor/src/items.rs | 1 -
crates/git_ui/src/project_diff.rs | 114 +++++++++++++++++++++-------
crates/multi_buffer/src/path_key.rs | 15 ++--
6 files changed, 104 insertions(+), 75 deletions(-)
diff --git a/crates/acp_thread/src/diff.rs b/crates/acp_thread/src/diff.rs
index f17e9d0fce404483ae99efc95bf666586c1f644b..cae1aad90810c217324659d29c065af443494933 100644
--- a/crates/acp_thread/src/diff.rs
+++ b/crates/acp_thread/src/diff.rs
@@ -166,7 +166,7 @@ impl Diff {
}
pub fn has_revealed_range(&self, cx: &App) -> bool {
- self.multibuffer().read(cx).excerpt_paths().next().is_some()
+ self.multibuffer().read(cx).paths().next().is_some()
}
pub fn needs_update(&self, old_text: &str, new_text: &str, cx: &App) -> bool {
diff --git a/crates/agent_ui/src/agent_diff.rs b/crates/agent_ui/src/agent_diff.rs
index 11acd649ef9df500edf99926e754228e4c41e7bc..06fce64819d3ce66b9e39f2b83cbebefb6ba9698 100644
--- a/crates/agent_ui/src/agent_diff.rs
+++ b/crates/agent_ui/src/agent_diff.rs
@@ -130,7 +130,12 @@ impl AgentDiffPane {
.action_log()
.read(cx)
.changed_buffers(cx);
- let mut paths_to_delete = self.multibuffer.read(cx).paths().collect::>();
+ let mut paths_to_delete = self
+ .multibuffer
+ .read(cx)
+ .paths()
+ .cloned()
+ .collect::>();
for (buffer, diff_handle) in changed_buffers {
if buffer.read(cx).file().is_none() {
diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs
index cddb20d83e0b9066fcfd882aa5325624cbadf92e..923b5dc1540d93bd849f5a50a8d51052f79f93a0 100644
--- a/crates/editor/src/editor.rs
+++ b/crates/editor/src/editor.rs
@@ -22956,10 +22956,7 @@ impl Editor {
window: &mut Window,
cx: &mut Context,
) {
- let workspace = self.workspace();
- let project = self.project();
- let save_tasks = self.buffer().update(cx, |multi_buffer, cx| {
- let mut tasks = Vec::new();
+ self.buffer().update(cx, |multi_buffer, cx| {
for (buffer_id, changes) in revert_changes {
if let Some(buffer) = multi_buffer.buffer(buffer_id) {
buffer.update(cx, |buffer, cx| {
@@ -22971,44 +22968,9 @@ impl Editor {
cx,
);
});
-
- if let Some(project) =
- project.filter(|_| multi_buffer.all_diff_hunks_expanded())
- {
- project.update(cx, |project, cx| {
- tasks.push((buffer.clone(), project.save_buffer(buffer, cx)));
- })
- }
}
}
- tasks
});
- cx.spawn_in(window, async move |_, cx| {
- for (buffer, task) in save_tasks {
- let result = task.await;
- if result.is_err() {
- let Some(path) = buffer
- .read_with(cx, |buffer, cx| buffer.project_path(cx))
- .ok()
- else {
- continue;
- };
- if let Some((workspace, path)) = workspace.as_ref().zip(path) {
- let Some(task) = cx
- .update_window_entity(workspace, |workspace, window, cx| {
- workspace
- .open_path_preview(path, None, false, false, false, window, cx)
- })
- .ok()
- else {
- continue;
- };
- task.await.log_err();
- }
- }
- }
- })
- .detach();
self.change_selections(SelectionEffects::no_scroll(), window, cx, |selections| {
selections.refresh()
});
diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs
index 3b9c17f80f10116f2302bab203966922cbf0bcb2..cfbb7c975c844f08d76a5568f1e02dfe3d7d74f1 100644
--- a/crates/editor/src/items.rs
+++ b/crates/editor/src/items.rs
@@ -842,7 +842,6 @@ impl Item for Editor {
.map(|handle| handle.read(cx).base_buffer().unwrap_or(handle.clone()))
.collect::>();
- // let mut buffers_to_save =
let buffers_to_save = if self.buffer.read(cx).is_singleton() && !options.autosave {
buffers
} else {
diff --git a/crates/git_ui/src/project_diff.rs b/crates/git_ui/src/project_diff.rs
index f40d70da6494cf8491c1d3d7909a288e5f99023c..3f689567327e280f7e9911699e10159340ddb8d5 100644
--- a/crates/git_ui/src/project_diff.rs
+++ b/crates/git_ui/src/project_diff.rs
@@ -74,6 +74,13 @@ pub struct ProjectDiff {
_subscription: Subscription,
}
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub enum RefreshReason {
+ DiffChanged,
+ StatusesChanged,
+ EditorSaved,
+}
+
const CONFLICT_SORT_PREFIX: u64 = 1;
const TRACKED_SORT_PREFIX: u64 = 2;
const NEW_SORT_PREFIX: u64 = 3;
@@ -278,7 +285,7 @@ impl ProjectDiff {
BranchDiffEvent::FileListChanged => {
this._task = window.spawn(cx, {
let this = cx.weak_entity();
- async |cx| Self::refresh(this, cx).await
+ async |cx| Self::refresh(this, RefreshReason::StatusesChanged, cx).await
})
}
},
@@ -297,7 +304,7 @@ impl ProjectDiff {
this._task = {
window.spawn(cx, {
let this = cx.weak_entity();
- async |cx| Self::refresh(this, cx).await
+ async |cx| Self::refresh(this, RefreshReason::StatusesChanged, cx).await
})
}
}
@@ -308,7 +315,7 @@ impl ProjectDiff {
let task = window.spawn(cx, {
let this = cx.weak_entity();
- async |cx| Self::refresh(this, cx).await
+ async |cx| Self::refresh(this, RefreshReason::StatusesChanged, cx).await
});
Self {
@@ -448,19 +455,27 @@ impl ProjectDiff {
window: &mut Window,
cx: &mut Context,
) {
- if let EditorEvent::SelectionsChanged { local: true } = event {
- let Some(project_path) = self.active_path(cx) else {
- return;
- };
- self.workspace
- .update(cx, |workspace, cx| {
- if let Some(git_panel) = workspace.panel::(cx) {
- git_panel.update(cx, |git_panel, cx| {
- git_panel.select_entry_by_path(project_path, window, cx)
- })
- }
- })
- .ok();
+ match event {
+ EditorEvent::SelectionsChanged { local: true } => {
+ let Some(project_path) = self.active_path(cx) else {
+ return;
+ };
+ self.workspace
+ .update(cx, |workspace, cx| {
+ if let Some(git_panel) = workspace.panel::(cx) {
+ git_panel.update(cx, |git_panel, cx| {
+ git_panel.select_entry_by_path(project_path, window, cx)
+ })
+ }
+ })
+ .ok();
+ }
+ EditorEvent::Saved => {
+ self._task = cx.spawn_in(window, async move |this, cx| {
+ Self::refresh(this, RefreshReason::EditorSaved, cx).await
+ });
+ }
+ _ => {}
}
if editor.focus_handle(cx).contains_focused(window, cx)
&& self.multibuffer.read(cx).is_empty()
@@ -482,7 +497,7 @@ impl ProjectDiff {
let subscription = cx.subscribe_in(&diff, window, move |this, _, _, window, cx| {
this._task = window.spawn(cx, {
let this = cx.weak_entity();
- async |cx| Self::refresh(this, cx).await
+ async |cx| Self::refresh(this, RefreshReason::DiffChanged, cx).await
})
});
self.buffer_diff_subscriptions
@@ -581,14 +596,23 @@ impl ProjectDiff {
}
}
- pub async fn refresh(this: WeakEntity, cx: &mut AsyncWindowContext) -> Result<()> {
+ pub async fn refresh(
+ this: WeakEntity,
+ reason: RefreshReason,
+ cx: &mut AsyncWindowContext,
+ ) -> Result<()> {
let mut path_keys = Vec::new();
let buffers_to_load = this.update(cx, |this, cx| {
let (repo, buffers_to_load) = this.branch_diff.update(cx, |branch_diff, cx| {
let load_buffers = branch_diff.load_buffers(cx);
(branch_diff.repo().cloned(), load_buffers)
});
- let mut previous_paths = this.multibuffer.read(cx).paths().collect::>();
+ let mut previous_paths = this
+ .multibuffer
+ .read(cx)
+ .paths()
+ .cloned()
+ .collect::>();
if let Some(repo) = repo {
let repo = repo.read(cx);
@@ -605,8 +629,20 @@ impl ProjectDiff {
this.multibuffer.update(cx, |multibuffer, cx| {
for path in previous_paths {
+ if let Some(buffer) = multibuffer.buffer_for_path(&path, cx) {
+ let skip = match reason {
+ RefreshReason::DiffChanged | RefreshReason::EditorSaved => {
+ buffer.read(cx).is_dirty()
+ }
+ RefreshReason::StatusesChanged => false,
+ };
+ if skip {
+ continue;
+ }
+ }
+
this.buffer_diff_subscriptions.remove(&path.path);
- multibuffer.remove_excerpts_for_path(path, cx);
+ multibuffer.remove_excerpts_for_path(path.clone(), cx);
}
});
buffers_to_load
@@ -619,7 +655,27 @@ impl ProjectDiff {
yield_now().await;
cx.update(|window, cx| {
this.update(cx, |this, cx| {
- this.register_buffer(path_key, entry.file_status, buffer, diff, window, cx)
+ let multibuffer = this.multibuffer.read(cx);
+ let skip = multibuffer.buffer(buffer.read(cx).remote_id()).is_some()
+ && multibuffer
+ .diff_for(buffer.read(cx).remote_id())
+ .is_some_and(|prev_diff| prev_diff.entity_id() == diff.entity_id())
+ && match reason {
+ RefreshReason::DiffChanged | RefreshReason::EditorSaved => {
+ buffer.read(cx).is_dirty()
+ }
+ RefreshReason::StatusesChanged => false,
+ };
+ if !skip {
+ this.register_buffer(
+ path_key,
+ entry.file_status,
+ buffer,
+ diff,
+ window,
+ cx,
+ )
+ }
})
.ok();
})?;
@@ -637,7 +693,7 @@ impl ProjectDiff {
pub fn excerpt_paths(&self, cx: &App) -> Vec> {
self.multibuffer
.read(cx)
- .excerpt_paths()
+ .paths()
.map(|key| key.path.clone())
.collect()
}
@@ -1650,9 +1706,13 @@ mod tests {
.unindent(),
);
- editor.update_in(cx, |editor, window, cx| {
- editor.git_restore(&Default::default(), window, cx);
- });
+ editor
+ .update_in(cx, |editor, window, cx| {
+ editor.git_restore(&Default::default(), window, cx);
+ editor.save(SaveOptions::default(), project.clone(), window, cx)
+ })
+ .await
+ .unwrap();
cx.run_until_parked();
assert_state_with_diff(&editor, cx, &"ˇ".unindent());
@@ -1841,8 +1901,8 @@ mod tests {
cx,
&"
- original
- + ˇdifferent
- "
+ + different
+ ˇ"
.unindent(),
);
}
diff --git a/crates/multi_buffer/src/path_key.rs b/crates/multi_buffer/src/path_key.rs
index 119194d088c946941b13ffab3f6f2b3ea126cd09..10d4088fd4bc28449c8a4ee74095ad31a45fbcf3 100644
--- a/crates/multi_buffer/src/path_key.rs
+++ b/crates/multi_buffer/src/path_key.rs
@@ -43,8 +43,8 @@ impl PathKey {
}
impl MultiBuffer {
- pub fn paths(&self) -> impl Iterator- + '_ {
- self.excerpts_by_path.keys().cloned()
+ pub fn paths(&self) -> impl Iterator
- + '_ {
+ self.excerpts_by_path.keys()
}
pub fn remove_excerpts_for_path(&mut self, path: PathKey, cx: &mut Context) {
@@ -58,15 +58,18 @@ impl MultiBuffer {
}
}
- pub fn location_for_path(&self, path: &PathKey, cx: &App) -> Option {
+ pub fn buffer_for_path(&self, path: &PathKey, cx: &App) -> Option> {
let excerpt_id = self.excerpts_by_path.get(path)?.first()?;
let snapshot = self.read(cx);
let excerpt = snapshot.excerpt(*excerpt_id)?;
- Some(Anchor::in_buffer(excerpt.id, excerpt.range.context.start))
+ self.buffer(excerpt.buffer_id)
}
- pub fn excerpt_paths(&self) -> impl Iterator
- {
- self.excerpts_by_path.keys()
+ pub fn location_for_path(&self, path: &PathKey, cx: &App) -> Option {
+ let excerpt_id = self.excerpts_by_path.get(path)?.first()?;
+ let snapshot = self.read(cx);
+ let excerpt = snapshot.excerpt(*excerpt_id)?;
+ Some(Anchor::in_buffer(excerpt.id, excerpt.range.context.start))
}
/// Sets excerpts, returns `true` if at least one new excerpt was added.
From d7da5d3efdd2283cb70035e2e6c0a40d8cf02a0b Mon Sep 17 00:00:00 2001
From: Mikayla Maki
Date: Sun, 14 Dec 2025 20:07:44 -0800
Subject: [PATCH 008/257] Finish inline telemetry changes (#44842)
Closes #ISSUE
Release Notes:
- N/A
---
Cargo.lock | 4 +-
crates/agent_ui/Cargo.toml | 1 -
crates/agent_ui/src/agent_ui.rs | 11 +-
crates/agent_ui/src/buffer_codegen.rs | 148 +++++-----
crates/agent_ui/src/inline_assistant.rs | 125 ++++----
crates/agent_ui/src/inline_prompt_editor.rs | 276 ++++++++++--------
crates/agent_ui/src/terminal_codegen.rs | 66 +++--
.../agent_ui/src/terminal_inline_assistant.rs | 83 +++---
crates/agent_ui/src/text_thread_editor.rs | 1 -
crates/assistant_text_thread/Cargo.toml | 2 +-
.../src/assistant_text_thread_tests.rs | 9 -
.../assistant_text_thread/src/text_thread.rs | 52 ++--
.../src/text_thread_store.rs | 14 +-
crates/language_model/Cargo.toml | 1 -
crates/language_model/src/telemetry.rs | 124 +++++---
crates/settings_ui/src/settings_ui.rs | 7 -
16 files changed, 499 insertions(+), 425 deletions(-)
diff --git a/Cargo.lock b/Cargo.lock
index e465ff483bcb6cc9528403bbb8f3bd883a6af871..436da4aef8c0849a61336a9645639c17da731029 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -388,7 +388,6 @@ dependencies = [
"streaming_diff",
"task",
"telemetry",
- "telemetry_events",
"terminal",
"terminal_view",
"text",
@@ -894,7 +893,7 @@ dependencies = [
"settings",
"smallvec",
"smol",
- "telemetry_events",
+ "telemetry",
"text",
"ui",
"unindent",
@@ -8817,7 +8816,6 @@ dependencies = [
"serde_json",
"settings",
"smol",
- "telemetry_events",
"thiserror 2.0.17",
"util",
"zed_env_vars",
diff --git a/crates/agent_ui/Cargo.toml b/crates/agent_ui/Cargo.toml
index 2af0ce6fbd2b636d19d9cb8e544851514800313c..b235799635ce81b02fd6fcd5d4d7a53a6957eb77 100644
--- a/crates/agent_ui/Cargo.toml
+++ b/crates/agent_ui/Cargo.toml
@@ -84,7 +84,6 @@ smol.workspace = true
streaming_diff.workspace = true
task.workspace = true
telemetry.workspace = true
-telemetry_events.workspace = true
terminal.workspace = true
terminal_view.workspace = true
text.workspace = true
diff --git a/crates/agent_ui/src/agent_ui.rs b/crates/agent_ui/src/agent_ui.rs
index eb7785fad59894012251c84319af7fca306f2882..cd6113bfa6c611c8d2a6b9d43294e77737b7a9ae 100644
--- a/crates/agent_ui/src/agent_ui.rs
+++ b/crates/agent_ui/src/agent_ui.rs
@@ -216,7 +216,7 @@ pub fn init(
is_eval: bool,
cx: &mut App,
) {
- assistant_text_thread::init(client.clone(), cx);
+ assistant_text_thread::init(client, cx);
rules_library::init(cx);
if !is_eval {
// Initializing the language model from the user settings messes with the eval, so we only initialize them when
@@ -229,13 +229,8 @@ pub fn init(
TextThreadEditor::init(cx);
register_slash_commands(cx);
- inline_assistant::init(
- fs.clone(),
- prompt_builder.clone(),
- client.telemetry().clone(),
- cx,
- );
- terminal_inline_assistant::init(fs.clone(), prompt_builder, client.telemetry().clone(), cx);
+ inline_assistant::init(fs.clone(), prompt_builder.clone(), cx);
+ terminal_inline_assistant::init(fs.clone(), prompt_builder, cx);
cx.observe_new(move |workspace, window, cx| {
ConfigureContextServerModal::register(workspace, language_registry.clone(), window, cx)
})
diff --git a/crates/agent_ui/src/buffer_codegen.rs b/crates/agent_ui/src/buffer_codegen.rs
index e2c67a04167d7080a6f94b9ee2a8fae516d487d7..bb05d5e04deb06f82dfc8e5dae0d871648f1d11e 100644
--- a/crates/agent_ui/src/buffer_codegen.rs
+++ b/crates/agent_ui/src/buffer_codegen.rs
@@ -1,8 +1,8 @@
use crate::{context::LoadedContext, inline_prompt_editor::CodegenStatus};
use agent_settings::AgentSettings;
use anyhow::{Context as _, Result};
+use uuid::Uuid;
-use client::telemetry::Telemetry;
use cloud_llm_client::CompletionIntent;
use collections::HashSet;
use editor::{Anchor, AnchorRangeExt, MultiBuffer, MultiBufferSnapshot, ToOffset as _, ToPoint};
@@ -15,12 +15,12 @@ use futures::{
stream::BoxStream,
};
use gpui::{App, AppContext as _, AsyncApp, Context, Entity, EventEmitter, Subscription, Task};
-use language::{Buffer, IndentKind, Point, TransactionId, line_diff};
+use language::{Buffer, IndentKind, LanguageName, Point, TransactionId, line_diff};
use language_model::{
LanguageModel, LanguageModelCompletionError, LanguageModelCompletionEvent,
LanguageModelRegistry, LanguageModelRequest, LanguageModelRequestMessage,
LanguageModelRequestTool, LanguageModelTextStream, LanguageModelToolChoice,
- LanguageModelToolUse, Role, TokenUsage, report_assistant_event,
+ LanguageModelToolUse, Role, TokenUsage,
};
use multi_buffer::MultiBufferRow;
use parking_lot::Mutex;
@@ -41,7 +41,6 @@ use std::{
time::Instant,
};
use streaming_diff::{CharOperation, LineDiff, LineOperation, StreamingDiff};
-use telemetry_events::{AssistantEventData, AssistantKind, AssistantPhase};
use ui::SharedString;
/// Use this tool to provide a message to the user when you're unable to complete a task.
@@ -77,9 +76,9 @@ pub struct BufferCodegen {
buffer: Entity,
range: Range,
initial_transaction_id: Option,
- telemetry: Arc,
builder: Arc,
pub is_insertion: bool,
+ session_id: Uuid,
}
impl BufferCodegen {
@@ -87,7 +86,7 @@ impl BufferCodegen {
buffer: Entity,
range: Range,
initial_transaction_id: Option,
- telemetry: Arc,
+ session_id: Uuid,
builder: Arc,
cx: &mut Context,
) -> Self {
@@ -96,8 +95,8 @@ impl BufferCodegen {
buffer.clone(),
range.clone(),
false,
- Some(telemetry.clone()),
builder.clone(),
+ session_id,
cx,
)
});
@@ -110,8 +109,8 @@ impl BufferCodegen {
buffer,
range,
initial_transaction_id,
- telemetry,
builder,
+ session_id,
};
this.activate(0, cx);
this
@@ -134,6 +133,10 @@ impl BufferCodegen {
&self.alternatives[self.active_alternative]
}
+ pub fn language_name(&self, cx: &App) -> Option {
+ self.active_alternative().read(cx).language_name(cx)
+ }
+
pub fn status<'a>(&self, cx: &'a App) -> &'a CodegenStatus {
&self.active_alternative().read(cx).status
}
@@ -192,8 +195,8 @@ impl BufferCodegen {
self.buffer.clone(),
self.range.clone(),
false,
- Some(self.telemetry.clone()),
self.builder.clone(),
+ self.session_id,
cx,
)
}));
@@ -256,6 +259,10 @@ impl BufferCodegen {
pub fn selected_text<'a>(&self, cx: &'a App) -> Option<&'a str> {
self.active_alternative().read(cx).selected_text()
}
+
+ pub fn session_id(&self) -> Uuid {
+ self.session_id
+ }
}
impl EventEmitter for BufferCodegen {}
@@ -271,7 +278,6 @@ pub struct CodegenAlternative {
status: CodegenStatus,
generation: Task<()>,
diff: Diff,
- telemetry: Option>,
_subscription: gpui::Subscription,
builder: Arc,
active: bool,
@@ -282,6 +288,7 @@ pub struct CodegenAlternative {
selected_text: Option,
pub message_id: Option,
pub model_explanation: Option,
+ session_id: Uuid,
}
impl EventEmitter for CodegenAlternative {}
@@ -291,8 +298,8 @@ impl CodegenAlternative {
buffer: Entity,
range: Range,
active: bool,
- telemetry: Option>,
builder: Arc,
+ session_id: Uuid,
cx: &mut Context,
) -> Self {
let snapshot = buffer.read(cx).snapshot(cx);
@@ -331,7 +338,6 @@ impl CodegenAlternative {
status: CodegenStatus::Idle,
generation: Task::ready(()),
diff: Diff::default(),
- telemetry,
builder,
active: active,
edits: Vec::new(),
@@ -341,10 +347,18 @@ impl CodegenAlternative {
completion: None,
selected_text: None,
model_explanation: None,
+ session_id,
_subscription: cx.subscribe(&buffer, Self::handle_buffer_event),
}
}
+ pub fn language_name(&self, cx: &App) -> Option {
+ self.old_buffer
+ .read(cx)
+ .language()
+ .map(|language| language.name())
+ }
+
pub fn set_active(&mut self, active: bool, cx: &mut Context) {
if active != self.active {
self.active = active;
@@ -407,34 +421,28 @@ impl CodegenAlternative {
self.edit_position = Some(self.range.start.bias_right(&self.snapshot));
- let api_key = model.api_key(cx);
- let telemetry_id = model.telemetry_id();
- let provider_id = model.provider_id();
-
if Self::use_streaming_tools(model.as_ref(), cx) {
let request = self.build_request(&model, user_prompt, context_task, cx)?;
- let completion_events =
- cx.spawn(async move |_, cx| model.stream_completion(request.await, cx).await);
- self.generation = self.handle_completion(
- telemetry_id,
- provider_id.to_string(),
- api_key,
- completion_events,
- cx,
- );
+ let completion_events = cx.spawn({
+ let model = model.clone();
+ async move |_, cx| model.stream_completion(request.await, cx).await
+ });
+ self.generation = self.handle_completion(model, completion_events, cx);
} else {
let stream: LocalBoxFuture> =
if user_prompt.trim().to_lowercase() == "delete" {
async { Ok(LanguageModelTextStream::default()) }.boxed_local()
} else {
let request = self.build_request(&model, user_prompt, context_task, cx)?;
- cx.spawn(async move |_, cx| {
- Ok(model.stream_completion_text(request.await, cx).await?)
+ cx.spawn({
+ let model = model.clone();
+ async move |_, cx| {
+ Ok(model.stream_completion_text(request.await, cx).await?)
+ }
})
.boxed_local()
};
- self.generation =
- self.handle_stream(telemetry_id, provider_id.to_string(), api_key, stream, cx);
+ self.generation = self.handle_stream(model, stream, cx);
}
Ok(())
@@ -621,12 +629,14 @@ impl CodegenAlternative {
pub fn handle_stream(
&mut self,
- model_telemetry_id: String,
- model_provider_id: String,
- model_api_key: Option,
+ model: Arc,
stream: impl 'static + Future