From 7370ae14b9770980285fda29a818ec3ccb306966 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Thu, 7 May 2026 21:56:09 -0400 Subject: [PATCH] Add download file actions --- Cargo.lock | 1 + crates/ely_app/Cargo.toml | 1 + crates/ely_app/src/main.rs | 1 + crates/ely_app/src/services/download_files.rs | 74 +++++++++++++++++++ crates/ely_app/src/services/mod.rs | 1 + .../src/shell/internal_pages/downloads.rs | 57 +++++++++++++- crates/ely_app/src/shell/mod.rs | 30 +++++++- crates/ely_browser_core/src/error.rs | 3 + .../ely_browser_core/src/state/downloads.rs | 24 +++++- crates/ely_browser_core/tests/downloads.rs | 44 ++++++++++- crates/ely_domain/src/download.rs | 16 ++++ 11 files changed, 248 insertions(+), 4 deletions(-) create mode 100644 crates/ely_app/src/services/download_files.rs create mode 100644 crates/ely_app/src/services/mod.rs diff --git a/Cargo.lock b/Cargo.lock index 6db83ab..5518fd3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2202,6 +2202,7 @@ dependencies = [ "gpui", "gpui-component", "gpui-component-assets", + "thiserror 2.0.18", ] [[package]] diff --git a/crates/ely_app/Cargo.toml b/crates/ely_app/Cargo.toml index c224a29..102abcd 100644 --- a/crates/ely_app/Cargo.toml +++ b/crates/ely_app/Cargo.toml @@ -12,6 +12,7 @@ ely_domain = { path = "../ely_domain" } gpui.workspace = true gpui-component.workspace = true gpui-component-assets.workspace = true +thiserror.workspace = true [lints] workspace = true diff --git a/crates/ely_app/src/main.rs b/crates/ely_app/src/main.rs index e90f5f7..8e971a6 100644 --- a/crates/ely_app/src/main.rs +++ b/crates/ely_app/src/main.rs @@ -1,3 +1,4 @@ +mod services; mod shell; use gpui::{ diff --git a/crates/ely_app/src/services/download_files.rs b/crates/ely_app/src/services/download_files.rs new file mode 100644 index 0000000..9e8630e --- /dev/null +++ b/crates/ely_app/src/services/download_files.rs @@ -0,0 +1,74 @@ +use std::{ + fs, io, + path::{Path, PathBuf}, + process::{Command, ExitStatus}, +}; + +use thiserror::Error; + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum DownloadFileAction { + Open, + Reveal, +} + +#[derive(Debug, Error)] +pub enum DownloadFileError { + #[error("download file is unavailable: {path:?}")] + FileUnavailable { path: PathBuf }, + + #[error("failed to launch {action} for download file: {path:?}: {source}")] + LaunchFailed { action: &'static str, path: PathBuf, source: io::Error }, + + #[error("{action} failed for download file: {path:?} ({status})")] + CommandFailed { action: &'static str, path: PathBuf, status: ExitStatus }, +} + +impl DownloadFileAction { + pub fn run(self, path: &Path) -> Result<(), DownloadFileError> { + ensure_regular_file(path)?; + let status = + self.command(path).status().map_err(|source| DownloadFileError::LaunchFailed { + action: self.label(), + path: path.to_path_buf(), + source, + })?; + + if status.success() { + return Ok(()); + } + + Err(DownloadFileError::CommandFailed { + action: self.label(), + path: path.to_path_buf(), + status, + }) + } + + fn command(self, path: &Path) -> Command { + let mut command = Command::new("/usr/bin/open"); + match self { + Self::Open => { + command.arg(path); + } + Self::Reveal => { + command.arg("-R").arg(path); + } + } + command + } + + fn label(self) -> &'static str { + match self { + Self::Open => "open", + Self::Reveal => "reveal", + } + } +} + +fn ensure_regular_file(path: &Path) -> Result<(), DownloadFileError> { + match fs::metadata(path) { + Ok(metadata) if metadata.is_file() => Ok(()), + Ok(_) | Err(_) => Err(DownloadFileError::FileUnavailable { path: path.to_path_buf() }), + } +} diff --git a/crates/ely_app/src/services/mod.rs b/crates/ely_app/src/services/mod.rs new file mode 100644 index 0000000..36af1df --- /dev/null +++ b/crates/ely_app/src/services/mod.rs @@ -0,0 +1 @@ +pub mod download_files; diff --git a/crates/ely_app/src/shell/internal_pages/downloads.rs b/crates/ely_app/src/shell/internal_pages/downloads.rs index 1567481..af5f92b 100644 --- a/crates/ely_app/src/shell/internal_pages/downloads.rs +++ b/crates/ely_app/src/shell/internal_pages/downloads.rs @@ -75,6 +75,9 @@ impl ElyShell { ), ), ) + .when_some(self.download_file_error.clone(), |this, message| { + this.child(render_download_file_error(message)) + }) .child(self.render_downloads_list(snapshot, cx)), ) } @@ -168,7 +171,7 @@ impl ElyShell { div() .min_w_0() .truncate() - .child(download_destination_label(entry.destination())), + .child(download_entry_location_label(entry)), ) .when(entry.security().requires_prompt(), |this| { this.child(render_security_prompt(entry.security())) @@ -277,6 +280,34 @@ impl ElyShell { ) }, ) + .when( + matches!(entry.state(), DownloadState::Completed) + && entry.target_file_path().is_some(), + |this| { + let open_id = entry.id().clone(); + let reveal_id = entry.id().clone(); + + this.child( + download_action_button("open", index, IconName::ExternalLink, "Open File") + .on_click(cx.listener(move |shell, _, _, cx| { + shell.open_download_file(&open_id, cx); + })) + .into_any_element(), + ) + .child( + download_action_button( + "reveal", + index, + IconName::FolderOpen, + "Reveal in Finder", + ) + .on_click(cx.listener(move |shell, _, _, cx| { + shell.reveal_download_file(&reveal_id, cx); + })) + .into_any_element(), + ) + }, + ) .into_any_element() } } @@ -320,6 +351,30 @@ fn download_destination_label(destination: &DownloadDestination) -> String { } } +fn download_entry_location_label(entry: &DownloadEntry) -> String { + match entry.target_file_path() { + Some(path) => path.display().to_string(), + None => download_destination_label(entry.destination()), + } +} + +fn render_download_file_error(message: String) -> AnyElement { + div() + .rounded_md() + .border_1() + .border_color(rgb(colors::ERROR)) + .px_3() + .py_2() + .flex() + .items_center() + .gap_2() + .text_xs() + .text_color(rgb(colors::ERROR)) + .child(IconName::TriangleAlert) + .child(message) + .into_any_element() +} + fn render_security_prompt(security: &DownloadSecurity) -> AnyElement { div() .flex() diff --git a/crates/ely_app/src/shell/mod.rs b/crates/ely_app/src/shell/mod.rs index 885d118..f6c26bf 100644 --- a/crates/ely_app/src/shell/mod.rs +++ b/crates/ely_app/src/shell/mod.rs @@ -9,7 +9,7 @@ use gpui_component::input::{InputEvent, InputState, SelectAll}; use crate::{ CloseCurrentTab, FocusAddressBar, FocusCommandMode, OpenDownloads, OpenHistory, OpenNewTab, OpenSettings, RestoreClosedTab, SelectNextTab, SelectPreviousTab, ToggleFavoriteTab, - TogglePinnedTab, + TogglePinnedTab, services::download_files::DownloadFileAction, }; enum ShellState { @@ -22,6 +22,7 @@ pub struct ElyShell { focus_handle: FocusHandle, command_input: Entity, last_intent: Option, + download_file_error: Option, _command_subscription: Subscription, } @@ -77,6 +78,7 @@ impl ElyShell { focus_handle: cx.focus_handle(), command_input, last_intent: None, + download_file_error: None, _command_subscription: command_subscription, } } @@ -247,6 +249,32 @@ impl ElyShell { } } + fn open_download_file(&mut self, download_id: &DownloadId, cx: &mut Context) { + self.run_download_file_action(download_id, DownloadFileAction::Open, cx); + } + + fn reveal_download_file(&mut self, download_id: &DownloadId, cx: &mut Context) { + self.run_download_file_action(download_id, DownloadFileAction::Reveal, cx); + } + + fn run_download_file_action( + &mut self, + download_id: &DownloadId, + action: DownloadFileAction, + cx: &mut Context, + ) { + let result = match &self.state { + ShellState::Ready(core) => core + .download_target_file_path(download_id) + .map_err(|error| error.to_string()) + .and_then(|path| action.run(&path).map_err(|error| error.to_string())), + ShellState::StartupError(message) => Err(message.clone()), + }; + + self.download_file_error = result.err(); + cx.notify(); + } + fn on_close_current_tab( &mut self, _: &CloseCurrentTab, diff --git a/crates/ely_browser_core/src/error.rs b/crates/ely_browser_core/src/error.rs index f8b6ee4..ea24e57 100644 --- a/crates/ely_browser_core/src/error.rs +++ b/crates/ely_browser_core/src/error.rs @@ -18,6 +18,9 @@ pub enum CoreError { #[error("download not found: {id}")] DownloadNotFound { id: DownloadId }, + #[error("download target path is unavailable: {id}")] + DownloadTargetPathUnavailable { id: DownloadId }, + #[error("favorite limit reached: {limit}")] FavoriteLimitReached { limit: usize }, diff --git a/crates/ely_browser_core/src/state/downloads.rs b/crates/ely_browser_core/src/state/downloads.rs index 2ff5898..8023370 100644 --- a/crates/ely_browser_core/src/state/downloads.rs +++ b/crates/ely_browser_core/src/state/downloads.rs @@ -1,4 +1,4 @@ -use std::time::SystemTime; +use std::{path::PathBuf, time::SystemTime}; use ely_domain::{DownloadEntry, DownloadId, UrlText}; @@ -70,6 +70,17 @@ impl BrowserCore { Ok(()) } + pub fn download_target_file_path( + &self, + download_id: &DownloadId, + ) -> Result { + let entry = self.visible_download_entry(download_id)?; + entry + .target_file_path() + .map(PathBuf::from) + .ok_or_else(|| CoreError::DownloadTargetPathUnavailable { id: download_id.clone() }) + } + pub(super) fn visible_downloads(&self) -> Vec { self.download_entries .iter() @@ -78,6 +89,17 @@ impl BrowserCore { .collect() } + fn visible_download_entry( + &self, + download_id: &DownloadId, + ) -> Result<&DownloadEntry, CoreError> { + self.download_entries + .iter() + .filter(|entry| entry.profile_id() == &self.active_profile_id) + .find(|entry| entry.id() == download_id) + .ok_or_else(|| CoreError::DownloadNotFound { id: download_id.clone() }) + } + fn download_entry_mut( &mut self, download_id: &DownloadId, diff --git a/crates/ely_browser_core/tests/downloads.rs b/crates/ely_browser_core/tests/downloads.rs index 9af9599..15c727e 100644 --- a/crates/ely_browser_core/tests/downloads.rs +++ b/crates/ely_browser_core/tests/downloads.rs @@ -1,4 +1,4 @@ -use std::error::Error; +use std::{error::Error, path::Path}; use ely_browser_core::{BrowserCore, CoreError, InitialBrowserConfig}; use ely_domain::{ @@ -21,6 +21,7 @@ fn download_entries_stay_with_active_profile() -> Result<(), Box> { assert_eq!(default_snapshot.download_entries.len(), 1); assert_eq!(default_snapshot.download_entries[0].file_name(), "report.pdf"); assert_eq!(default_snapshot.download_entries[0].profile_id(), &default_profile_id); + assert_eq!(default_snapshot.download_entries[0].target_file_path(), None); core.create_profile("Personal", 0xf54e00, ProfileKind::Standard)?; let personal_snapshot = core.snapshot()?; @@ -82,10 +83,51 @@ fn records_active_profile_download_policy_on_started_entry() -> Result<(), Box Result<(), Box> { + let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?; + let profile_id = core.active_tab()?.profile_id().clone(); + core.set_profile_download_policy( + &profile_id, + DownloadPolicy::fixed_directory("/tmp/ely-work-downloads")?, + )?; + + let download_id = core.record_download_started( + UrlText::parse("https://example.com/report.pdf")?, + "report.pdf", + Some(2048), + )?; + + assert_eq!( + core.download_target_file_path(&download_id)?, + Path::new("/tmp/ely-work-downloads/report.pdf") + ); + Ok(()) +} + +#[test] +fn rejects_target_path_for_ask_every_time_download() -> Result<(), Box> { + let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?; + let download_id = core.record_download_started( + UrlText::parse("https://example.com/report.pdf")?, + "report.pdf", + Some(2048), + )?; + + let error = match core.download_target_file_path(&download_id) { + Ok(_) => return Err("download target path should require a fixed destination".into()), + Err(error) => error, + }; + + assert_eq!(error, CoreError::DownloadTargetPathUnavailable { id: download_id }); + Ok(()) +} + #[test] fn download_policies_stay_with_profile() -> Result<(), Box> { let mut core = BrowserCore::new(InitialBrowserConfig::ely_defaults()?)?; diff --git a/crates/ely_domain/src/download.rs b/crates/ely_domain/src/download.rs index 2f5f895..866ac1a 100644 --- a/crates/ely_domain/src/download.rs +++ b/crates/ely_domain/src/download.rs @@ -38,6 +38,7 @@ pub struct DownloadEntry { source_url: UrlText, file_name: String, destination: DownloadDestination, + target_file_path: Option, security: DownloadSecurity, state: DownloadState, received_bytes: u64, @@ -95,6 +96,14 @@ impl DownloadDestination { Self::FixedDirectory(path) => Some(path.as_path()), } } + + pub fn target_file_path(&self, file_name: &str) -> Result, DomainError> { + validate_file_name(file_name)?; + Ok(match self { + Self::AskEveryTime => None, + Self::FixedDirectory(path) => Some(path.join(file_name)), + }) + } } impl DownloadPolicy { @@ -143,6 +152,7 @@ impl DownloadEntry { return Err(DomainError::EmptyField { field: "file_name" }); } validate_file_name(file_name)?; + let target_file_path = destination.target_file_path(file_name)?; Ok(Self { id: DownloadId::new(), @@ -150,6 +160,7 @@ impl DownloadEntry { source_url, file_name: file_name.to_string(), destination, + target_file_path, security: DownloadSecurity::for_file_name(file_name), state: DownloadState::InProgress, received_bytes: 0, @@ -229,6 +240,11 @@ impl DownloadEntry { &self.destination } + #[must_use] + pub fn target_file_path(&self) -> Option<&Path> { + self.target_file_path.as_deref() + } + #[must_use] pub fn security(&self) -> &DownloadSecurity { &self.security