mirror of
https://github.com/zed-industries/zed.git
synced 2026-07-10 00:13:29 +00:00
Fix blame hover popover not showing on first trigger when inline blame is disabled (#50769)
● Closes #50285 ## Summary When `inline_blame` is disabled in settings and `editor::BlameHover` is triggered, the blame popover fails to appear on the first invocation because `start_git_blame` kicks off an async task to fetch blame data, but `blame_hover` immediately tries to read that data synchronously before it's available. This fix registers a one-shot observation on the blame entity when it's freshly created, so the popover is shown once blame data has finished generating. Before you mark this PR as ready for review, make sure that you have: - [x] Added a solid test coverage and/or screenshots from doing manual testing - [x] Done a self-review taking into account security and performance aspects - [ ] ~Aligned any UI changes with the [UI checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)~ (No UI changes) ## Videos: ### Before: https://github.com/user-attachments/assets/d2ca4f8e-8186-49af-9908-5a82bfca0de2 Ps: pressed the keybind two times ### After: https://github.com/user-attachments/assets/08575cd9-2bbc-462d-92fa-f0e7ef23d8d6 ## Release Notes: - Fixed blame hover popover not appearing on first trigger when inline blame is disabled (#50285) --------- Co-authored-by: Chris Biscardi <chris@christopherbiscardi.com>
This commit is contained in:
parent
69664ab9d3
commit
c4a9b1aa4b
3 changed files with 122 additions and 3 deletions
|
|
@ -1079,6 +1079,7 @@ pub struct Editor {
|
|||
show_selection_menu: Option<bool>,
|
||||
blame: Option<Entity<GitBlame>>,
|
||||
blame_subscription: Option<Subscription>,
|
||||
pending_blame_hover_observation: Option<Subscription>,
|
||||
custom_context_menu: Option<
|
||||
Box<
|
||||
dyn 'static
|
||||
|
|
@ -2399,6 +2400,7 @@ impl Editor {
|
|||
}),
|
||||
blame: None,
|
||||
blame_subscription: None,
|
||||
pending_blame_hover_observation: None,
|
||||
|
||||
bookmark_store,
|
||||
breakpoint_store,
|
||||
|
|
|
|||
|
|
@ -638,6 +638,29 @@ impl Editor {
|
|||
window: &mut Window,
|
||||
cx: &mut Context<Self>,
|
||||
) {
|
||||
let just_started = self.blame.is_none();
|
||||
if just_started {
|
||||
self.start_git_blame(true, window, cx);
|
||||
}
|
||||
let Some(blame) = self.blame.as_ref() else {
|
||||
return;
|
||||
};
|
||||
|
||||
if just_started && !blame.read(cx).has_generated_entries() {
|
||||
let subscription = cx.observe_in(blame, window, |editor, blame, window, cx| {
|
||||
if blame.read(cx).has_generated_entries() {
|
||||
editor.pending_blame_hover_observation.take();
|
||||
editor.show_blame_hover_popover(window, cx);
|
||||
}
|
||||
});
|
||||
self.pending_blame_hover_observation = Some(subscription);
|
||||
return;
|
||||
}
|
||||
|
||||
self.show_blame_hover_popover(window, cx);
|
||||
}
|
||||
|
||||
fn show_blame_hover_popover(&mut self, window: &mut Window, cx: &mut Context<Self>) {
|
||||
let snapshot = self.snapshot(window, cx);
|
||||
let cursor = self
|
||||
.selections
|
||||
|
|
@ -647,9 +670,6 @@ impl Editor {
|
|||
return;
|
||||
};
|
||||
|
||||
if self.blame.is_none() {
|
||||
self.start_git_blame(true, window, cx);
|
||||
}
|
||||
let Some(blame) = self.blame.as_ref() else {
|
||||
return;
|
||||
};
|
||||
|
|
|
|||
|
|
@ -1295,4 +1295,101 @@ mod tests {
|
|||
filename: String::new(),
|
||||
}
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_blame_hover_shows_popover_on_first_trigger(cx: &mut gpui::TestAppContext) {
|
||||
init_test(cx);
|
||||
|
||||
cx.update(|cx| {
|
||||
use gpui::UpdateGlobal;
|
||||
settings::SettingsStore::update_global(
|
||||
cx,
|
||||
|store: &mut settings::SettingsStore, cx| {
|
||||
store
|
||||
.set_user_settings(r#"{"git": {"inline_blame": {"enabled": false}}}"#, cx)
|
||||
.expect("failed to set user settings");
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
let fs = FakeFs::new(cx.executor());
|
||||
fs.insert_tree(
|
||||
"/my-repo",
|
||||
json!({
|
||||
".git": {},
|
||||
"file.txt": "line 1\nline 2\nline 3\n"
|
||||
}),
|
||||
)
|
||||
.await;
|
||||
|
||||
fs.set_blame_for_repo(
|
||||
Path::new("/my-repo/.git"),
|
||||
vec![(
|
||||
repo_path("file.txt"),
|
||||
Blame {
|
||||
entries: vec![
|
||||
blame_entry("1b1b1b", 0..1),
|
||||
blame_entry("2c2c2c", 1..2),
|
||||
blame_entry("3d3d3d", 2..3),
|
||||
],
|
||||
..Default::default()
|
||||
},
|
||||
)],
|
||||
);
|
||||
|
||||
let project = project::Project::test(fs, ["/my-repo".as_ref()], cx).await;
|
||||
let buffer = project
|
||||
.update(cx, |project, cx| {
|
||||
project.open_local_buffer("/my-repo/file.txt", cx)
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
let multi_buffer = cx.new(|cx| MultiBuffer::singleton(buffer, cx));
|
||||
|
||||
let (editor, cx) = cx.add_window_view(|window, cx| {
|
||||
crate::test::build_editor_with_project(project, multi_buffer, window, cx)
|
||||
});
|
||||
|
||||
// Verify blame is not loaded yet
|
||||
editor.update(cx, |editor, _cx| {
|
||||
assert!(
|
||||
editor.blame().is_none(),
|
||||
"blame should not be loaded initially"
|
||||
);
|
||||
});
|
||||
|
||||
// Focus the editor so that blame generation proceeds
|
||||
editor.update_in(cx, |editor, window, cx| {
|
||||
editor.focus_handle.focus(window, cx);
|
||||
});
|
||||
|
||||
// Trigger BlameHover — this should start blame loading and defer showing the popover
|
||||
editor.update_in(cx, |editor, window, cx| {
|
||||
assert!(editor.blame().is_none());
|
||||
editor.blame_hover(&crate::BlameHover, window, cx);
|
||||
assert!(
|
||||
editor.blame().is_some(),
|
||||
"blame entity should be created after blame_hover"
|
||||
);
|
||||
assert!(
|
||||
editor.pending_blame_hover_observation.is_some(),
|
||||
"should have registered an observation to wait for blame data"
|
||||
);
|
||||
});
|
||||
|
||||
// Let the async blame generation complete
|
||||
cx.run_until_parked();
|
||||
|
||||
// The observation should have fired and cleaned itself up
|
||||
editor.update(cx, |editor, cx| {
|
||||
assert!(
|
||||
editor.pending_blame_hover_observation.is_none(),
|
||||
"observation should be consumed after blame data is generated"
|
||||
);
|
||||
assert!(
|
||||
editor.blame().unwrap().read(cx).has_generated_entries(),
|
||||
"blame should have generated entries"
|
||||
);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue