From bfc89302f83641f39f4c9bf4cabd4a89efa95cc2 Mon Sep 17 00:00:00 2001
From: Juliette Cordor <me@cordor.dev>
Date: Wed, 27 Nov 2024 14:27:00 +1100
Subject: [PATCH] refactor match to not return reference

---
 CHANGELOG.md                 |  1 +
 src/commands/app/cleanup.rs  | 13 ++----
 src/commands/app/download.rs | 81 ++++++++++++++++++++----------------
 3 files changed, 49 insertions(+), 46 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 3bf059ad..5db5656b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Removed `json` flag from `app download` command
 - Download progress bars now show app name instead of url leaf
 - Download hash checks now report to a progress bar rather than a print message for each
+- Renamed `packages` parameter to `apps` in `app download` command (this should not affect usage at all)
 
 ## [1.15.0] - 2024-03-11
 
diff --git a/src/commands/app/cleanup.rs b/src/commands/app/cleanup.rs
index ed214a9f..d913139e 100644
--- a/src/commands/app/cleanup.rs
+++ b/src/commands/app/cleanup.rs
@@ -5,8 +5,8 @@ use sprinkles::{
 };
 
 use crate::{
+    abandon,
     handlers::{AppsDecider, ListApps},
-    output::colours::eprintln_yellow,
 };
 
 #[derive(Debug, Clone, Parser)]
@@ -30,14 +30,9 @@ pub struct Args {
 
 impl super::Command for Args {
     async fn runner(self, ctx: &impl ScoopContext) -> anyhow::Result<()> {
-        let cleanup_apps = match AppsDecider::new(ctx, self.list_apps(), self.apps)
-            .decide()?
-            .as_deref()
-        {
-            Some([]) | None => {
-                eprintln_yellow!("No apps selected. Exiting now.");
-                return Ok(());
-            }
+        let cleanup_apps = match AppsDecider::new(ctx, self.list_apps(), self.apps).decide()? {
+            Some(apps) if apps.is_empty() => abandon!("No apps selected"),
+            None => abandon!("No apps selected"),
             Some(apps) => apps,
         };
 
diff --git a/src/commands/app/download.rs b/src/commands/app/download.rs
index 7ee74dbc..623e2334 100644
--- a/src/commands/app/download.rs
+++ b/src/commands/app/download.rs
@@ -23,6 +23,7 @@ use sprinkles::{
 
 use crate::{
     abandon,
+    handlers::{AppsDecider, ListApps},
     models::status::Info,
     output::colours::{bright_red, eprintln_yellow},
 };
@@ -37,7 +38,7 @@ pub struct Args {
     no_hash_check: bool,
 
     #[clap(help = "The packages to download")]
-    packages: Vec<package::Reference>,
+    apps: Vec<package::Reference>,
 
     #[clap(long, help = "Download new versions of all outdated apps")]
     outdated: bool,
@@ -47,14 +48,10 @@ impl super::Command for Args {
     const BETA: bool = true;
 
     async fn runner(self, ctx: &impl ScoopContext) -> Result<(), anyhow::Error> {
-        let packages = if self.packages.is_empty() {
-            if self.outdated {
-                list_outdated(ctx)?
-            } else {
-                abandon!("No packages provided")
-            }
-        } else {
-            self.packages
+        let packages = match AppsDecider::new(ctx, self.list_apps(), self.apps).decide()? {
+            Some(apps) if apps.is_empty() => abandon!("No apps selected"),
+            None => abandon!("No apps selected"),
+            Some(apps) => apps,
         };
 
         if self.no_hash_check {
@@ -147,34 +144,44 @@ impl super::Command for Args {
     }
 }
 
-fn list_outdated(ctx: &impl ScoopContext) -> Result<Vec<package::Reference>, anyhow::Error> {
-    let apps = install::Manifest::list_all_unchecked(ctx)?;
-
-    Ok(apps
-        .par_iter()
-        .flat_map(|app| -> anyhow::Result<Info> {
-            if let Some(bucket) = &app.bucket {
-                let local_manifest = app.get_manifest(ctx)?;
-                // TODO: Add the option to check all buckets and find the highest version (will require semver to order versions)
-                let bucket = Bucket::from_name(ctx, bucket)?;
-
-                match Info::from_manifests(ctx, &local_manifest, &bucket) {
-                    Ok(info) => Ok(info),
-                    Err(err) => {
-                        error!(
-                            "Failed to get status for {}: {:?}",
-                            unsafe { app.name() },
-                            err
-                        );
-                        Err(err)?
-                    }
-                }
+impl Args {
+    fn list_apps<C: ScoopContext>(&self) -> impl ListApps<C> + use<C> {
+        let outdated = self.outdated;
+        move |ctx: &C| {
+            if outdated {
+                let apps = install::Manifest::list_all_unchecked(ctx)?;
+
+                Ok(Some(
+                    apps.par_iter()
+                        .flat_map(|app| -> anyhow::Result<Info> {
+                            if let Some(bucket) = &app.bucket {
+                                let local_manifest = app.get_manifest(ctx)?;
+                                // TODO: Add the option to check all buckets and find the highest version (will require semver to order versions)
+                                let bucket = Bucket::from_name(ctx, bucket)?;
+
+                                match Info::from_manifests(ctx, &local_manifest, &bucket) {
+                                    Ok(info) => Ok(info),
+                                    Err(err) => {
+                                        error!(
+                                            "Failed to get status for {}: {:?}",
+                                            unsafe { app.name() },
+                                            err
+                                        );
+                                        Err(err)?
+                                    }
+                                }
+                            } else {
+                                error!("no bucket specified");
+                                anyhow::bail!("no bucket specified")
+                            }
+                        })
+                        .filter(|app| app.current != app.available)
+                        .map(|app| manifest::Reference::Name(app.name).into_package_ref())
+                        .collect::<Vec<_>>(),
+                ))
             } else {
-                error!("no bucket specified");
-                anyhow::bail!("no bucket specified")
+                anyhow::Ok(None)
             }
-        })
-        .filter(|app| app.current != app.available)
-        .map(|app| manifest::Reference::Name(app.name).into_package_ref())
-        .collect::<Vec<_>>())
+        }
+    }
 }