feat(yt/comments/subs/set-status): Change order of `status` and `sub`

It just makes more sense to say:
`yt subs set-status active <sub name>`
instead of:
`yt subs set-status <sub name> active`

3 files changed, 51 insertions, 27 deletions

diff --git a/crates/yt/src/commands/mod.rs b/crates/yt/src/commands/mod.rs
index 493ddc6..234b5b3 100644
--- a/crates/yt/src/commands/mod.rs
+++ b/crates/yt/src/commands/mod.rs
@@ -23,7 +23,7 @@ use crate::{
         update::UpdateCommand, videos::VideosCommand, watch::WatchCommand,
     },
     config::Config,
-    storage::db::subscription::Subscriptions,
+    storage::db::subscription::{Subscription, Subscriptions},
 };
 
 pub(super) mod implm;
@@ -126,9 +126,18 @@ impl Default for Command {
         }
     }
 }
+fn complete_subscription_active(current: &OsStr) -> Vec<CompletionCandidate> {
+    complete_subscription_condition(current, |sub| sub.is_active)
+}
+fn complete_subscription_inactive(current: &OsStr) -> Vec<CompletionCandidate> {
+    complete_subscription_condition(current, |sub| !sub.is_active)
+}
 
-fn complete_subscription(current: &OsStr) -> Vec<CompletionCandidate> {
-    let mut output = vec![];
+fn complete_subscription_condition(
+    current: &OsStr,
+    condition: fn(&Subscription) -> bool,
+) -> Vec<CompletionCandidate> {
+    let output = vec![];
 
     let Some(current_prog) = current.to_str().map(ToOwned::to_owned) else {
         return output;
@@ -151,14 +160,21 @@ fn complete_subscription(current: &OsStr) -> Vec<CompletionCandidate> {
             return output;
         };
 
-        for sub in all.0.into_keys() {
-            if sub.starts_with(&current_prog) {
-                output.push(CompletionCandidate::new(sub));
+        let mut pre_output = vec![];
+        for (sub_name, sub) in all.0 {
+            if sub_name.starts_with(&current_prog) && condition(&sub) {
+                pre_output.push((sub_name.clone(), CompletionCandidate::new(sub_name)));
             }
         }
 
-        output
+        pre_output.sort_by_key(|(name, _)| name.clone());
+
+        pre_output.into_iter().map(|(_, comp)| comp).collect()
     });
 
     handle.join().unwrap_or_default()
 }
+
+fn complete_subscription(current: &OsStr) -> Vec<CompletionCandidate> {
+    complete_subscription_condition(current, |_| true)
+}
diff --git a/crates/yt/src/commands/subscriptions/implm.rs b/crates/yt/src/commands/subscriptions/implm.rs
index 1e2e545..a4d9d6d 100644
--- a/crates/yt/src/commands/subscriptions/implm.rs
+++ b/crates/yt/src/commands/subscriptions/implm.rs
@@ -12,7 +12,7 @@ use std::str::FromStr;
 
 use crate::{
     app::App,
-    commands::subscriptions::{SubscriptionCommand, SubscriptionStatus},
+    commands::subscriptions::SubscriptionCommand,
     storage::db::{
         insert::{Operations, subscription::Operation},
         subscription::{Subscription, Subscriptions, check_url},
@@ -55,21 +55,22 @@ impl SubscriptionCommand {
                     .await
                     .with_context(|| format!("Failed to unsubscribe from {name:?}"))?;
             }
-            SubscriptionCommand::SetStatus { name, new_status } => {
+            SubscriptionCommand::SetStatus { new_status } => {
                 let mut present_subscriptions = Subscriptions::get(app).await?;
 
                 let mut ops = Operations::new("Subscribe: Set Status");
+
+                let (new_status, name) = match new_status {
+                    super::NewStatus::Active { name } => (true, name),
+                    super::NewStatus::Inactive { name } => (false, name),
+                };
+
                 if let Some(subscription) = present_subscriptions.0.remove(&name) {
-                    subscription.set_is_active(
-                        match new_status {
-                            SubscriptionStatus::Active => true,
-                            SubscriptionStatus::Inactive => false,
-                        },
-                        &mut ops,
-                    );
+                    subscription.set_is_active(new_status, &mut ops);
                 } else {
                     bail!("Couldn't find subscription: '{}'", &name);
                 }
+
                 ops.commit(app)
                     .await
                     .with_context(|| format!("Failed to change status of {name:?}"))?;
diff --git a/crates/yt/src/commands/subscriptions/mod.rs b/crates/yt/src/commands/subscriptions/mod.rs
index 6a16a9a..2ffc309 100644
--- a/crates/yt/src/commands/subscriptions/mod.rs
+++ b/crates/yt/src/commands/subscriptions/mod.rs
@@ -10,11 +10,13 @@
 
 use std::path::PathBuf;
 
-use clap::{Subcommand, ValueEnum};
+use clap::Subcommand;
 use clap_complete::ArgValueCompleter;
 use url::Url;
 
-use crate::commands::complete_subscription;
+use crate::commands::{
+    complete_subscription, complete_subscription_active, complete_subscription_inactive,
+};
 
 mod implm;
 
@@ -45,12 +47,9 @@ pub(super) enum SubscriptionCommand {
     ///
     /// An active subscription will be updated in `yt update`, while an inactive one will not.
     SetStatus {
-        /// The human readable name of the subscription
-        #[arg(add = ArgValueCompleter::new(complete_subscription))]
-        name: String,
-
         /// What should this subscription be considered now?
-        new_status: SubscriptionStatus,
+        #[command(subcommand)]
+        new_status: NewStatus,
     },
 
     /// Import a bunch of URLs as subscriptions.
@@ -77,8 +76,16 @@ pub(super) enum SubscriptionCommand {
     },
 }
 
-#[derive(ValueEnum, Debug, Clone, Copy)]
-pub(in crate::commands) enum SubscriptionStatus {
-    Active,
-    Inactive,
+#[derive(Subcommand, Debug, Clone)]
+pub(in crate::commands) enum NewStatus {
+    Active {
+        /// The human readable name of the subscription
+        #[arg(add = ArgValueCompleter::new(complete_subscription_inactive))]
+        name: String,
+    },
+    Inactive {
+        /// The human readable name of the subscription
+        #[arg(add = ArgValueCompleter::new(complete_subscription_active))]
+        name: String,
+    },
 }