diff --git a/crates/auto_update/src/auto_update.rs b/crates/auto_update/src/auto_update.rs index 9786aa84d1..126e75ae83 100644 --- a/crates/auto_update/src/auto_update.rs +++ b/crates/auto_update/src/auto_update.rs @@ -13,7 +13,10 @@ use semver::Version; use serde::{Deserialize, Serialize}; use settings::{RegisterSetting, Settings, SettingsStore}; use smol::fs::File; -use smol::{fs, io::AsyncReadExt}; +use smol::{ + fs, + io::{AsyncReadExt, AsyncWriteExt}, +}; use std::mem; use std::{ env::{ @@ -126,20 +129,33 @@ pub struct AssetQuery<'a> { pub enum AutoUpdateStatus { Idle, Checking, - Downloading { version: VersionCheckType }, - Installing { version: VersionCheckType }, - Updated { version: VersionCheckType }, - Errored { error: Arc }, + Downloading { + version: VersionCheckType, + /// Download progress as a fraction in the range `0.0..=1.0`, or `None` + /// when the total download size is not yet known. + progress: Option, + }, + Installing { + version: VersionCheckType, + }, + Updated { + version: VersionCheckType, + }, + Errored { + error: Arc, + }, } impl PartialEq for AutoUpdateStatus { + // `progress` is deliberately not compared: two `Downloading` statuses for + // the same version are equal regardless of how far the download is. fn eq(&self, other: &Self) -> bool { match (self, other) { (AutoUpdateStatus::Idle, AutoUpdateStatus::Idle) => true, (AutoUpdateStatus::Checking, AutoUpdateStatus::Checking) => true, ( - AutoUpdateStatus::Downloading { version: v1 }, - AutoUpdateStatus::Downloading { version: v2 }, + AutoUpdateStatus::Downloading { version: v1, .. }, + AutoUpdateStatus::Downloading { version: v2, .. }, ) => v1 == v2, ( AutoUpdateStatus::Installing { version: v1 }, @@ -700,6 +716,7 @@ impl AutoUpdater { this.update(cx, |this, cx| { this.status = AutoUpdateStatus::Downloading { version: newer_version.clone(), + progress: None, }; cx.notify(); }); @@ -708,9 +725,27 @@ impl AutoUpdater { .await .context("Failed to create installer dir")?; let target_path = Self::target_path(&installer_dir).await?; - download_release(&target_path, fetched_release_data, client) - .await - .with_context(|| format!("Failed to download update to {}", target_path.display()))?; + let progress_entity = this.clone(); + let mut progress_cx = cx.clone(); + download_release( + &target_path, + fetched_release_data, + client, + move |progress| { + progress_entity.update(&mut progress_cx, |this, cx| { + if let AutoUpdateStatus::Downloading { + progress: current_progress, + .. + } = &mut this.status + { + *current_progress = progress; + cx.notify(); + } + }); + }, + ) + .await + .with_context(|| format!("Failed to download update to {}", target_path.display()))?; this.update(cx, |this, cx| { this.status = AutoUpdateStatus::Installing { @@ -989,6 +1024,7 @@ async fn download_release( target_path: &Path, release: ReleaseAsset, client: Arc, + mut on_progress: impl FnMut(Option), ) -> Result<()> { let mut target_file = File::create(&target_path).await?; @@ -998,7 +1034,40 @@ async fn download_release( "failed to download update: {:?}", response.status() ); - smol::io::copy(response.body_mut(), &mut target_file).await?; + + let total_bytes = response + .headers() + .get(http_client::http::header::CONTENT_LENGTH) + .and_then(|value| value.to_str().ok()) + .and_then(|value| value.parse::().ok()) + .filter(|total_bytes| *total_bytes > 0); + + let mut downloaded_bytes: u64 = 0; + let mut last_reported_percent: Option = None; + let mut buffer = [0u8; 8192]; + let body = response.body_mut(); + loop { + let bytes_read = body.read(&mut buffer).await?; + if bytes_read == 0 { + break; + } + target_file.write_all(&buffer[..bytes_read]).await?; + downloaded_bytes += bytes_read as u64; + + if let Some(total_bytes) = total_bytes { + let fraction = (downloaded_bytes as f32 / total_bytes as f32).clamp(0.0, 1.0); + // Only report when the whole-number percentage changes to avoid notifying the UI on every chunk. + let percent = (fraction * 100.0) as u8; + if last_reported_percent != Some(percent) { + last_reported_percent = Some(percent); + on_progress(Some(fraction)); + } + } + } + target_file.flush().await?; + if total_bytes.is_some() && last_reported_percent != Some(100) { + on_progress(Some(1.0)); + } log::info!("downloaded update. path:{:?}", target_path); Ok(()) @@ -1297,7 +1366,8 @@ mod tests { assert_eq!( status, AutoUpdateStatus::Downloading { - version: VersionCheckType::Semantic(semver::Version::new(0, 100, 1)) + version: VersionCheckType::Semantic(semver::Version::new(0, 100, 1)), + progress: None, } ); @@ -1337,6 +1407,114 @@ mod tests { assert_eq!(std::fs::read_to_string(path).unwrap(), ""); } + #[gpui::test] + async fn test_download_release_reports_progress(cx: &mut TestAppContext) { + cx.background_executor.allow_parking(); + + let body = vec![0u8; 20_000]; + let content_length = body.len(); + + let client = FakeHttpClient::create(move |_req| { + let body = body.clone(); + async move { + Ok(Response::builder() + .status(200) + .header( + http_client::http::header::CONTENT_LENGTH, + body.len().to_string(), + ) + .body(body.into()) + .unwrap()) + } + }); + + let temp_dir = tempdir().unwrap(); + let target_path = temp_dir.path().join("zed-download"); + let release = ReleaseAsset { + version: "1.0.0".to_string(), + url: "https://test.example/download".to_string(), + }; + + let reported = Rc::new(std::cell::RefCell::new(Vec::::new())); + download_release(&target_path, release, client, { + let reported = reported.clone(); + move |fraction| { + if let Some(fraction) = fraction { + reported.borrow_mut().push(fraction); + } + } + }) + .await + .unwrap(); + + let reported = reported.borrow(); + assert!( + reported.len() >= 2, + "expected progress to be reported across multiple reads, got {reported:?}" + ); + assert_eq!( + reported.last().copied(), + Some(1.0), + "download should finish at 100%" + ); + for fraction in reported.iter() { + assert!( + (0.0..=1.0).contains(fraction), + "progress {fraction} out of range" + ); + } + for pair in reported.windows(2) { + assert!( + pair[0] <= pair[1], + "progress must not decrease: {reported:?}" + ); + } + + let downloaded_len = std::fs::metadata(&target_path).unwrap().len(); + assert_eq!(downloaded_len, content_length as u64); + } + + #[gpui::test] + async fn test_download_release_without_content_length_reports_no_progress( + cx: &mut TestAppContext, + ) { + cx.background_executor.allow_parking(); + + let body = vec![0u8; 20_000]; + let content_length = body.len(); + + let client = FakeHttpClient::create(move |_req| { + let body = body.clone(); + async move { Ok(Response::builder().status(200).body(body.into()).unwrap()) } + }); + + let temp_dir = tempdir().unwrap(); + let target_path = temp_dir.path().join("zed-download"); + let release = ReleaseAsset { + version: "1.0.0".to_string(), + url: "https://test.example/download".to_string(), + }; + + let reported = Rc::new(std::cell::RefCell::new(Vec::>::new())); + download_release(&target_path, release, client, { + let reported = reported.clone(); + move |fraction| { + reported.borrow_mut().push(fraction); + } + }) + .await + .unwrap(); + + assert!( + reported.borrow().is_empty(), + "progress should not be reported when the total size is unknown, got {:?}", + reported.borrow() + ); + + let downloaded_len = std::fs::metadata(&target_path).unwrap().len(); + assert_eq!(downloaded_len, content_length as u64); + } + #[test] fn test_stable_does_not_update_when_fetched_version_is_not_higher() { let release_channel = ReleaseChannel::Stable; diff --git a/crates/title_bar/src/update_version.rs b/crates/title_bar/src/update_version.rs index 2ca96b5ac0..0c7db0d165 100644 --- a/crates/title_bar/src/update_version.rs +++ b/crates/title_bar/src/update_version.rs @@ -43,6 +43,7 @@ impl UpdateVersion { AutoUpdateStatus::Idle => AutoUpdateStatus::Checking, AutoUpdateStatus::Checking => AutoUpdateStatus::Downloading { version: VersionCheckType::Semantic(Version::new(1, 99, 0)), + progress: Some(0.5), }, AutoUpdateStatus::Downloading { .. } => AutoUpdateStatus::Installing { version: VersionCheckType::Semantic(Version::new(1, 99, 0)), @@ -85,9 +86,9 @@ impl Render for UpdateVersion { AutoUpdateStatus::Checking if self.update_check_type.is_manual() => { UpdateButton::checking().into_any_element() } - AutoUpdateStatus::Downloading { version } => { + AutoUpdateStatus::Downloading { version, progress } => { let version = Self::version_tooltip_message(&version); - UpdateButton::downloading(version).into_any_element() + UpdateButton::downloading(version, *progress).into_any_element() } AutoUpdateStatus::Installing { version } => { let version = Self::version_tooltip_message(&version); diff --git a/crates/ui/src/components/collab/update_button.rs b/crates/ui/src/components/collab/update_button.rs index c0e74867cc..38d6a16934 100644 --- a/crates/ui/src/components/collab/update_button.rs +++ b/crates/ui/src/components/collab/update_button.rs @@ -1,6 +1,10 @@ use gpui::{AnyElement, ClickEvent, prelude::*}; -use crate::{ButtonLike, CommonAnimationExt, Tooltip, prelude::*}; +use crate::{ButtonLike, CircularProgress, CommonAnimationExt, Tooltip, prelude::*}; + +const LOAD_CIRCLE_GLYPH_VIEWBOX: f32 = 16.0; +const LOAD_CIRCLE_GLYPH_STROKE_WIDTH: f32 = 1.2; +const LOAD_CIRCLE_GLYPH_RADIUS: f32 = 5.0; /// A button component displayed in the title bar to show auto-update status. #[derive(IntoElement, RegisterComponent)] @@ -12,6 +16,7 @@ pub struct UpdateButton { tooltip: Option, disabled: bool, show_dismiss: bool, + progress: Option, on_click: Option>, on_dismiss: Option>, } @@ -26,6 +31,7 @@ impl UpdateButton { tooltip: None, disabled: false, show_dismiss: false, + progress: None, on_click: None, on_dismiss: None, } @@ -78,15 +84,21 @@ impl UpdateButton { self } + pub fn progress(mut self, progress: impl Into>) -> Self { + self.progress = progress.into(); + self + } + pub fn checking() -> Self { Self::new(IconName::LoadCircle, "Checking for Zed Updates…") .icon_animate(true) .disabled(true) } - pub fn downloading(version: impl Into) -> Self { + pub fn downloading(version: impl Into, progress: Option) -> Self { Self::new(IconName::Download, "Downloading Zed Update…") .tooltip(version) + .progress(progress) .disabled(true) } @@ -112,24 +124,44 @@ impl UpdateButton { } impl RenderOnce for UpdateButton { - fn render(self, _window: &mut Window, cx: &mut App) -> impl IntoElement { + fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement { let border_color = if self.disabled { cx.theme().colors().border } else { cx.theme().colors().text.opacity(0.15) }; - let icon = Icon::new(self.icon) - .size(IconSize::XSmall) - .when_some(self.icon_color, |this, color| this.color(color)); - let icon_element = if self.icon_animate { - icon.with_rotate_animation(2).into_any_element() + let icon_element = if let Some(progress) = self.progress { + let progress = progress.clamp(0.0, 1.0); + let icon_box = IconSize::XSmall.rems().to_pixels(window.rem_size()); + let progress_color = Color::Default.color(cx); + CircularProgress::new(progress, 1.0, icon_box, cx) + .stroke_width( + icon_box * (LOAD_CIRCLE_GLYPH_STROKE_WIDTH / LOAD_CIRCLE_GLYPH_VIEWBOX), + ) + .radius(icon_box * (LOAD_CIRCLE_GLYPH_RADIUS / LOAD_CIRCLE_GLYPH_VIEWBOX)) + .bg_color(progress_color.opacity(0.2)) + .progress_color(progress_color) + .into_any_element() } else { - icon.into_any_element() + let icon = Icon::new(self.icon) + .size(IconSize::XSmall) + .when_some(self.icon_color, |this, color| this.color(color)); + if self.icon_animate { + icon.with_rotate_animation(2).into_any_element() + } else { + icon.into_any_element() + } }; let tooltip = self.tooltip.clone(); + let label_row = h_flex() + .h_full() + .gap_1() + .child(icon_element) + .child(Label::new(self.message).size(LabelSize::Small)); + h_flex() .mr_2() .rounded_sm() @@ -137,13 +169,7 @@ impl RenderOnce for UpdateButton { .border_color(border_color) .child( ButtonLike::new("update-button") - .child( - h_flex() - .h_full() - .gap_1() - .child(icon_element) - .child(Label::new(self.message).size(LabelSize::Small)), - ) + .child(label_row) .when_some(tooltip, |this, tooltip| { this.tooltip(Tooltip::text(tooltip)) }) @@ -189,7 +215,7 @@ impl Component for UpdateButton { single_example("Checking", UpdateButton::checking().into_any_element()), single_example( "Downloading", - UpdateButton::downloading(version).into_any_element(), + UpdateButton::downloading(version, Some(0.45)).into_any_element(), ), single_example( "Installing", diff --git a/crates/ui/src/components/progress/circular_progress.rs b/crates/ui/src/components/progress/circular_progress.rs index 69f93b740c..26e38258f9 100644 --- a/crates/ui/src/components/progress/circular_progress.rs +++ b/crates/ui/src/components/progress/circular_progress.rs @@ -11,6 +11,7 @@ pub struct CircularProgress { max_value: f32, size: Pixels, stroke_width: Pixels, + radius: Option, bg_color: Hsla, progress_color: Hsla, } @@ -22,6 +23,7 @@ impl CircularProgress { max_value, size, stroke_width: px(4.0), + radius: None, bg_color: cx.theme().colors().border_variant, progress_color: cx.theme().status().info, } @@ -51,6 +53,16 @@ impl CircularProgress { self } + /// Sets the ring radius explicitly, decoupling it from the layout box. + /// + /// By default the radius is derived from the size and stroke width so + /// the ring fills the box. Set it explicitly to draw a smaller ring + /// inside the box, e.g. to match the padding of an icon glyph. + pub fn radius(mut self, radius: Pixels) -> Self { + self.radius = Some(radius); + self + } + /// Sets the background circle color. pub fn bg_color(mut self, color: Hsla) -> Self { self.bg_color = color; @@ -69,6 +81,8 @@ impl RenderOnce for CircularProgress { let value = self.value; let max_value = self.max_value; let size = self.size; + let stroke_width = self.stroke_width; + let radius = self.radius.unwrap_or_else(|| (size / 2.0) - stroke_width); let bg_color = self.bg_color; let progress_color = self.progress_color; @@ -80,9 +94,6 @@ impl RenderOnce for CircularProgress { let center_x = bounds.origin.x + bounds.size.width / 2.0; let center_y = bounds.origin.y + bounds.size.height / 2.0; - let stroke_width = self.stroke_width; - let radius = (size / 2.0) - stroke_width; - // Draw background circle (full 360 degrees) let mut bg_builder = PathBuilder::stroke(stroke_width);