git: Add notification to git clone#41712
Conversation
|
Thanks! What do you think about putting this with a spinner in the activity bar at the bottom instead (where it says 
|
|
Since git clone is implemented on the zed/crates/language/src/language_registry.rs Lines 1286 to 1290 in a112153 That is, using channels to send status for long running fs jobs and using |
837f8c3 to
c507c9c
Compare
cole-miller
left a comment
There was a problem hiding this comment.
Thanks! A follow-up comment about the code structure but this is looking good.
| pub type JobId = usize; | ||
|
|
||
| #[derive(Clone, Debug)] | ||
| pub struct JobInfo { | ||
| pub start: Instant, | ||
| pub message: SharedString, | ||
| pub id: JobId, | ||
| } | ||
|
|
||
| #[derive(Debug, Clone)] | ||
| pub enum JobEvent { | ||
| Started { info: JobInfo }, | ||
| Completed { id: JobId }, | ||
| } | ||
|
|
||
| pub type JobEventSender = futures::channel::mpsc::UnboundedSender<JobEvent>; | ||
| pub type JobEventReceiver = futures::channel::mpsc::UnboundedReceiver<JobEvent>; | ||
|
|
||
| type JobEventBroadcast = Vec<JobEventSender>; | ||
|
|
||
| static GLOBAL_JOB_EVENTS_SUBSCRIBERS: std::sync::Mutex<JobEventBroadcast> = | ||
| std::sync::Mutex::new(Vec::new()); | ||
|
|
||
| pub fn subscribe_to_job_events() -> JobEventReceiver { | ||
| let (sender, receiver) = futures::channel::mpsc::unbounded(); | ||
|
|
||
| if let Ok(mut subscribers) = GLOBAL_JOB_EVENTS_SUBSCRIBERS.lock() { | ||
| subscribers.push(sender); | ||
| } | ||
|
|
||
| receiver | ||
| } | ||
|
|
||
| fn send_job_event(event: JobEvent) { | ||
| if let Ok(mut subscribers) = GLOBAL_JOB_EVENTS_SUBSCRIBERS.lock() { | ||
| subscribers.retain(|sender| sender.unbounded_send(event.clone()).is_ok()); | ||
| } | ||
| } | ||
|
|
||
| struct JobTracker { | ||
| id: JobId, | ||
| } | ||
|
|
||
| impl JobTracker { | ||
| fn new(info: JobInfo) -> Self { | ||
| let id = info.id; | ||
| send_job_event(JobEvent::Started { info }); | ||
| Self { id } | ||
| } | ||
| } | ||
|
|
||
| impl Drop for JobTracker { | ||
| fn drop(&mut self) { | ||
| send_job_event(JobEvent::Completed { id: self.id }); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Wondering if we can find a way to structure this code without a global subscription. What if the ActivityIndicator held an Arc<dyn Fs>, and the Fs trait had a peek_current_job method that just returned an Option<String>? (And I guess a start instant.)
There was a problem hiding this comment.
This could be done, however peek_current_job would have to be called inside the ActivityIndicator::content_to_render function, which is called on every render. And because we have an Arc<dyn Fs> we will have to keep the jobs inside a Mutex hence locking on every render to get the current job, and sometimes when two or more windows are open, multiple ActivityIndicator instances will be calling on that function.
I could try to rework this and make a function on that trait that returns a JobEventReceiver like I'm currently doing only this is more integrated to the Fs trait and we loose that global subscription
There was a problem hiding this comment.
Thank you for the explanation! That makes sense, and I like the new in-between structure with a method on the Fs trait.
This reverts commit 19b9fe7.
1c3960e to
695dad3
Compare
Adds a simple notification when cloning a repo using the integrated git clone on Zed. Before this, the user had no feedback after starting the cloning action. Demo: https://github.com/user-attachments/assets/72fcdf1b-fc99-4fe5-8db2-7c30b170f12f Not sure about that icon I'm using for the animation, but that can be easily changed. Release Notes: - Added notification when cloning a repo from zed
Adds a simple notification when cloning a repo using the integrated git clone on Zed. Before this, the user had no feedback after starting the cloning action.
Demo:
2025-11-05.21-20-38.mp4
Not sure about that icon I'm using for the animation, but that can be easily changed.
Release Notes: