Repository: aome510/spotify-player (language: rust)

Issue:
Expose `clap`'s shell completion generation methods via CLI
## Who, What, and Why
As a user,
I want **shell completion**,
so that I do not need to memorise the CLI or rely on the CLI's help utility.
--> This allows a smoother CLI usage experience and potentially further improves CLI discoverability.

Specific scenarios:
+ _subcommand_ completion
    - there are a lot of subcommands, will be great to be able to search through them and list them all out temporarily with shell completions
+ _option_ completion
    - same reasoning as above
+ _argument_ completion 
    - subcommands such as `spotify_player get key <key>` have arguments with specific values.  Since they are nicely named but long, completion can speed this up by reducing risk of typo.

**shell completion**: custom TAB-completion available for **some shells** that requires custom script provided by the CLI program
**some shells**: `clap_complete 4.4.4` provides `bash`, `elvish`, `fish`, `powershell` and `zsh`

## How (roughly)
### new dependencies
+ `clap_complete`: https://docs.rs/clap_complete/latest/clap_complete/index.html
    - `clap = "4.4.10"` requires `clap_complete`, our master branch is at `clap = "4.4.10"` as of [2023-12-04]

### implementation directions
1. provide subcommand that prints to stdout
    - The [documentation above](https://docs.rs/clap_complete/latest/clap_complete/index.html) included an example implementation that prints completion script to stdout.
2. generate at build time
    - The example for `generate_to` included an implementation that generates completions within the `build.rs` script during build time: https://docs.rs/clap_complete/latest/clap_complete/generator/fn.generate_to.html

## When
NOT urgent.  
The CLI is already very much usable at the current state.

## Other informations
+ `spotify-tui` uses implementation direction 1, but they are using `clap = "2.33.3"`:
https://github.com/Rigellute/spotify-tui/blob/c4dcf6b9fd8318017acbdd7ec005955e26cf2794/src/main.rs#L171-L185
+ There was an issue with the said feature for `clap = "2.33.3"` BUT it _seems to be fixed_ since (`clap = "4.3.11"`, `clap_complete = "4.3.2"`): https://github.com/clap-rs/clap/issues/1822



Discussion / hints:

Fix patch:
diff --git a/Cargo.lock b/Cargo.lock
index 3185e56e..72fac2c1 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -568,6 +568,15 @@ dependencies = [
  "strsim",
 ]
 
+[[package]]
+name = "clap_complete"
+version = "4.4.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "bffe91f06a11b4b9420f62103854e90867812cd5d01557f853c5ee8e791b12ae"
+dependencies = [
+ "clap",
+]
+
 [[package]]
 name = "clap_derive"
 version = "4.4.7"
@@ -4120,6 +4129,7 @@ dependencies = [
  "backtrace",
  "chrono",
  "clap",
+ "clap_complete",
  "config_parser2",
  "copypasta",
  "crossterm",
diff --git a/spotify_player/Cargo.toml b/spotify_player/Cargo.toml
index 86b15bb0..eac4d52c 100644
--- a/spotify_player/Cargo.toml
+++ b/spotify_player/Cargo.toml
@@ -47,6 +47,7 @@ regex = "1.10.2"
 daemonize = { version = "0.5.0", optional = true }
 ttl_cache = "0.5.1"
 copypasta = { version = "0.10.0", optional = true }
+clap_complete = "4.4.4"
 
 [target.'cfg(target_os = "windows")'.dependencies.windows]
 version = "0.44.0"
diff --git a/spotify_player/src/cli/client.rs b/spotify_player/src/cli/client.rs
index 3692fbf9..2b3dd175 100644
--- a/spotify_player/src/cli/client.rs
+++ b/spotify_player/src/cli/client.rs
@@ -389,6 +389,8 @@ async fn handle_playback_request(
             PlayerRequest::StartPlayback(Playback::Context(context_id, None), Some(shuffle))
         }
         Command::PlayPause => PlayerRequest::ResumePause,
+        Command::Play => PlayerRequest::Resume,
+        Command::Pause => PlayerRequest::Pause,
         Command::Next => PlayerRequest::NextTrack,
         Command::Previous => PlayerRequest::PreviousTrack,
         Command::Shuffle => PlayerRequest::Shuffle,
diff --git a/spotify_player/src/cli/commands.rs b/spotify_player/src/cli/commands.rs
index be618107..8a35289e 100644
--- a/spotify_player/src/cli/commands.rs
+++ b/spotify_player/src/cli/commands.rs
@@ -1,4 +1,5 @@
 use clap::{builder::EnumValueParser, value_parser, Arg, ArgAction, ArgGroup, Command};
+use clap_complete::Shell;
 
 use super::{ContextType, ItemType, Key};
 
@@ -90,6 +91,8 @@ pub fn init_playback_subcommand() -> Command {
         .subcommand_required(true)
         .subcommand(init_playback_start_subcommand())
         .subcommand(Command::new("play-pause").about("Toggle between play and pause"))
+        .subcommand(Command::new("play").about("Resume the current playback if stopped"))
+        .subcommand(Command::new("pause").about("Pause the current playback if playing"))
         .subcommand(Command::new("next").about("Skip to the next track"))
         .subcommand(Command::new("previous").about("Skip to the previous track"))
         .subcommand(Command::new("shuffle").about("Toggle the shuffle mode"))
@@ -136,6 +139,17 @@ pub fn init_authenticate_command() -> Command {
     Command::new("authenticate").about("Authenticate the application")
 }
 
+pub fn init_generate_command() -> Command {
+    Command::new("generate")
+        .about("Generate shell completion for the application CLI")
+        .arg(
+            Arg::new("shell")
+                .action(ArgAction::Set)
+                .value_parser(value_parser!(Shell))
+                .required(true),
+        )
+}
+
 pub fn init_playlist_subcommand() -> Command {
     Command::new("playlist")
         .about("Playlist editing")
diff --git a/spotify_player/src/cli/handlers.rs b/spotify_player/src/cli/handlers.rs
index 81c800ec..eb32ce5d 100644
--- a/spotify_player/src/cli/handlers.rs
+++ b/spotify_player/src/cli/handlers.rs
@@ -6,6 +6,7 @@ use crate::{
 use super::*;
 use anyhow::{Context, Result};
 use clap::{ArgMatches, Id};
+use clap_complete::{generate, Shell};
 use std::net::UdpSocket;
 
 fn receive_response(socket: &UdpSocket) -> Result<Response> {
@@ -108,6 +109,8 @@ fn handle_playback_subcommand(args: &ArgMatches) -> Result<Request> {
             }
         },
         "play-pause" => Command::PlayPause,
+        "play" => Command::Play,
+        "pause" => Command::Pause,
         "next" => Command::Next,
         "previous" => Command::Previous,
         "shuffle" => Command::Shuffle,
@@ -180,12 +183,24 @@ fn try_connect_to_client(socket: &UdpSocket, configs: &state::Configs) -> Result
 pub fn handle_cli_subcommand(cmd: &str, args: &ArgMatches, configs: &state::Configs) -> Result<()> {
     let socket = UdpSocket::bind("127.0.0.1:0")?;
 
-    // handle `authenticate` command separately as `authenticate` doesn't require a client
-    if cmd == "authenticate" {
-        let auth_config = AuthConfig::new(configs)?;
-        let rt = tokio::runtime::Runtime::new()?;
-        rt.block_on(new_session_with_new_creds(&auth_config))?;
-        std::process::exit(0);
+    // handle commands that don't require a client separately
+    match cmd {
+        "authenticate" => {
+            let auth_config = AuthConfig::new(configs)?;
+            let rt = tokio::runtime::Runtime::new()?;
+            rt.block_on(new_session_with_new_creds(&auth_config))?;
+            std::process::exit(0);
+        }
+        "generate" => {
+            let gen = *args
+                .get_one::<Shell>("shell")
+                .expect("shell argument is required");
+            let mut cmd = init_cli()?;
+            let name = cmd.get_name().to_string();
+            generate(gen, &mut cmd, name, &mut std::io::stdout());
+            std::process::exit(0);
+        }
+        _ => {}
     }
 
     try_connect_to_client(&socket, configs).context("try to connect to a client")?;
diff --git a/spotify_player/src/cli/mod.rs b/spotify_player/src/cli/mod.rs
index 80241841..13877a26 100644
--- a/spotify_player/src/cli/mod.rs
+++ b/spotify_player/src/cli/mod.rs
@@ -2,13 +2,13 @@ mod client;
 mod commands;
 mod handlers;
 
+use crate::config;
 use rspotify::model::*;
 use serde::{Deserialize, Serialize};
 
 const MAX_REQUEST_SIZE: usize = 4096;
 
 pub use client::start_socket;
-pub use commands::*;
 pub use handlers::handle_cli_subcommand;
 
 #[derive(Debug, Serialize, Deserialize, clap::ValueEnum, Clone)]
@@ -97,6 +97,8 @@ pub enum Command {
     },
     StartRadio(ItemType, IdOrName),
     PlayPause,
+    Play,
+    Pause,
     Next,
     Previous,
     Shuffle,
@@ -143,3 +145,54 @@ impl ItemId {
         }
     }
 }
+
+pub fn init_cli() -> anyhow::Result<clap::Command> {
+    let default_cache_folder = config::get_cache_folder_path()?;
+    let default_config_folder = config::get_config_folder_path()?;
+
+    let cmd = clap::Command::new(env!("CARGO_PKG_NAME"))
+        .version(env!("CARGO_PKG_VERSION"))
+        .about(env!("CARGO_PKG_DESCRIPTION"))
+        .author(env!("CARGO_PKG_AUTHORS"))
+        .subcommand(commands::init_get_subcommand())
+        .subcommand(commands::init_playback_subcommand())
+        .subcommand(commands::init_connect_subcommand())
+        .subcommand(commands::init_like_command())
+        .subcommand(commands::init_authenticate_command())
+        .subcommand(commands::init_playlist_subcommand())
+        .subcommand(commands::init_generate_command())
+        .arg(
+            clap::Arg::new("theme")
+                .short('t')
+                .long("theme")
+                .value_name("THEME")
+                .help("Application theme"),
+        )
+        .arg(
+            clap::Arg::new("config-folder")
+                .short('c')
+                .long("config-folder")
+                .value_name("FOLDER")
+                .default_value(default_config_folder.into_os_string())
+                .help("Path to the application's config folder"),
+        )
+        .arg(
+            clap::Arg::new("cache-folder")
+                .short('C')
+                .long("cache-folder")
+                .value_name("FOLDER")
+                .default_value(default_cache_folder.into_os_string())
+                .help("Path to the application's cache folder"),
+        );
+
+    #[cfg(feature = "daemon")]
+    let cmd = cmd.arg(
+        clap::Arg::new("daemon")
+            .short('d')
+            .long("daemon")
+            .action(clap::ArgAction::SetTrue)
+            .help("Running the application as a daemon"),
+    );
+
+    Ok(cmd)
+}
diff --git a/spotify_player/src/client/mod.rs b/spotify_player/src/client/mod.rs
index efe33ed9..2e93d9db 100644
--- a/spotify_player/src/client/mod.rs
+++ b/spotify_player/src/client/mod.rs
@@ -129,7 +129,6 @@ impl Client {
         match request {
             PlayerRequest::NextTrack => self.spotify.next_track(device_id).await?,
             PlayerRequest::PreviousTrack => self.spotify.previous_track(device_id).await?,
-            #[cfg(feature = "media-control")]
             PlayerRequest::Resume => {
                 if !playback.is_playing {
                     self.spotify.resume_playback(device_id, None).await?;
@@ -137,7 +136,6 @@ impl Client {
                 }
             }
 
-            #[cfg(feature = "media-control")]
             PlayerRequest::Pause => {
                 if playback.is_playing {
                     self.spotify.pause_playback(device_id).await?;
@@ -1340,11 +1338,13 @@ impl Client {
             None => return Ok(()),
         };
 
-        let path = state.configs.cache_folder.join("image").join(format!(
+        let path = (format!(
             "{}-{}-cover.jpg",
             track.album.name,
             crate::utils::map_join(&track.album.artists, |a| &a.name, ", ")
-        ));
+        ))
+        .replace('/', ""); // don't want '/' character in the file's name
+        let path = state.configs.cache_folder.join("image").join(path);
 
         // Retrieve and save the new track's cover image into the cache folder.
         // The notify feature still requires the cover images to be stored inside the cache folder.
diff --git a/spotify_player/src/event/mod.rs b/spotify_player/src/event/mod.rs
index 4b229d7d..087bfb5a 100644
--- a/spotify_player/src/event/mod.rs
+++ b/spotify_player/src/event/mod.rs
@@ -22,9 +22,7 @@ mod window;
 pub enum PlayerRequest {
     NextTrack,
     PreviousTrack,
-    #[cfg(feature = "media-control")]
     Resume,
-    #[cfg(feature = "media-control")]
     Pause,
     ResumePause,
     SeekTrack(chrono::Duration),
diff --git a/spotify_player/src/main.rs b/spotify_player/src/main.rs
index e51a76bb..1d9fb718 100644
--- a/spotify_player/src/main.rs
+++ b/spotify_player/src/main.rs
@@ -17,56 +17,6 @@ mod utils;
 use anyhow::{Context, Result};
 use std::io::Write;
 
-fn init_app_cli_arguments() -> Result<clap::ArgMatches> {
-    let default_cache_folder = config::get_cache_folder_path()?;
-    let default_config_folder = config::get_config_folder_path()?;
-
-    let cmd = clap::Command::new(env!("CARGO_PKG_NAME"))
-        .version(env!("CARGO_PKG_VERSION"))
-        .about(env!("CARGO_PKG_DESCRIPTION"))
-        .author(env!("CARGO_PKG_AUTHORS"))
-        .subcommand(cli::init_get_subcommand())
-        .subcommand(cli::init_playback_subcommand())
-        .subcommand(cli::init_connect_subcommand())
-        .subcommand(cli::init_like_command())
-        .subcommand(cli::init_authenticate_command())
-        .subcommand(cli::init_playlist_subcommand())
-        .arg(
-            clap::Arg::new("theme")
-                .short('t')
-                .long("theme")
-                .value_name("THEME")
-                .help("Application theme"),
-        )
-        .arg(
-            clap::Arg::new("config-folder")
-                .short('c')
-                .long("config-folder")
-                .value_name("FOLDER")
-                .default_value(default_config_folder.into_os_string())
-                .help("Path to the application's config folder"),
-        )
-        .arg(
-            clap::Arg::new("cache-folder")
-                .short('C')
-                .long("cache-folder")
-                .value_name("FOLDER")
-                .default_value(default_cache_folder.into_os_string())
-                .help("Path to the application's cache folder"),
-        );
-
-    #[cfg(feature = "daemon")]
-    let cmd = cmd.arg(
-        clap::Arg::new("daemon")
-            .short('d')
-            .long("daemon")
-            .action(clap::ArgAction::SetTrue)
-            .help("Running the application as a daemon"),
-    );
-
-    Ok(cmd.get_matches())
-}
-
 // unused variables:
 // - `is_daemon` when the `streaming` feature is not enabled
 #[allow(unused_variables)]
@@ -289,7 +239,7 @@ async fn start_app(state: state::SharedState, is_daemon: bool) -> Result<()> {
 
 fn main() -> Result<()> {
     // parse command line arguments
-    let args = init_app_cli_arguments()?;
+    let args = cli::init_cli()?.get_matches();
 
     // initialize the application's cache and config folders
     let config_folder: std::path::PathBuf = args


Test patch: