From d9054c125cc8781931f6ac58cd672809311c8906 Mon Sep 17 00:00:00 2001 From: Julia Mertz Date: Fri, 9 Aug 2024 15:16:37 +0200 Subject: [PATCH 1/5] feat: client_id_cmd config option cleanup function doc --- docs/config.md | 1 + spotify_player/src/cli/handlers.rs | 4 ++-- spotify_player/src/config/mod.rs | 30 ++++++++++++++++++++++++++++++ spotify_player/src/main.rs | 3 ++- 4 files changed, 35 insertions(+), 3 deletions(-) diff --git a/docs/config.md b/docs/config.md index 300fbc17..484d4054 100644 --- a/docs/config.md +++ b/docs/config.md @@ -25,6 +25,7 @@ All configuration files should be placed inside the application's configuration | Option | Description | Default | | --------------------------------- | ---------------------------------------------------------------------------------------- | ------------------------------------------------------- | | `client_id` | the Spotify client's ID | `65b708073fc0480ea92a077233ca87bd` | +| `client_id_cmd` | a shell command that prints the Spotify client ID to stdout (overrides `client_id`) | `None` | | `client_port` | the port that the application's client is running on to handle CLI commands | `8080` | | `tracks_playback_limit` | the limit for the number of tracks played in a **tracks** playback | `50` | | `playback_format` | the format of the text in the playback's window | `{status} {track} • {artists}\n{album}\n{metadata}` | diff --git a/spotify_player/src/cli/handlers.rs b/spotify_player/src/cli/handlers.rs index 95a466fe..9f972995 100644 --- a/spotify_player/src/cli/handlers.rs +++ b/spotify_player/src/cli/handlers.rs @@ -158,8 +158,8 @@ fn try_connect_to_client(socket: &UdpSocket, configs: &config::Configs) -> Resul let session = rt.block_on(new_session(&auth_config, false))?; // create a Spotify API client - let client = - client::Client::new(session, auth_config, configs.app_config.client_id.clone()); + let client_id = configs.app_config.get_client_id()?; + let client = client::Client::new(session, auth_config, client_id); rt.block_on(client.refresh_token())?; // create a client socket for handling CLI commands diff --git a/spotify_player/src/config/mod.rs b/spotify_player/src/config/mod.rs index 36f91e4c..3b9ddb1e 100644 --- a/spotify_player/src/config/mod.rs +++ b/spotify_player/src/config/mod.rs @@ -48,6 +48,7 @@ impl Configs { pub struct AppConfig { pub theme: String, pub client_id: String, + pub client_id_cmd: Option, pub client_port: u16, @@ -229,6 +230,7 @@ impl Default for AppConfig { theme: "dracula".to_owned(), // official Spotify web app's client id client_id: "65b708073fc0480ea92a077233ca87bd".to_string(), + client_id_cmd: None, client_port: 8080, @@ -386,6 +388,34 @@ impl AppConfig { ..Default::default() } } + + /// Returns stdout of `client_id_cmd` if set, otherwise it returns the the value of `client_id` + pub fn get_client_id(&self) -> Result { + if let Some(cmd) = self.client_id_cmd.clone() { + let output = if cfg!(target_os = "windows") { + std::process::Command::new("cmd") + .args(["/C", &cmd]) + .output() + } else { + std::process::Command::new("sh") + .arg("-c") + .arg(&cmd) + .output() + }?; + + if !output.status.success() { + let stderr = String::from_utf8(output.stderr)?; + tracing::error!("Running command: {}, returned error: {}", cmd, stderr); + anyhow::bail!(stderr); + } + + let stdout = String::from_utf8(output.stdout)?; + tracing::info!("Running command: {}, returned client id: {}", cmd, stdout); + return Ok(stdout); + } + + Ok(self.client_id.clone()) + } } /// gets the application's configuration folder path diff --git a/spotify_player/src/main.rs b/spotify_player/src/main.rs index 8046da57..de305c50 100644 --- a/spotify_player/src/main.rs +++ b/spotify_player/src/main.rs @@ -117,7 +117,8 @@ async fn start_app(state: &state::SharedState) -> Result<()> { let session = auth::new_session(&auth_config, !state.is_daemon).await?; // create a Spotify API client - let client = client::Client::new(session, auth_config, configs.app_config.client_id.clone()); + let client_id = configs.app_config.get_client_id()?; + let client = client::Client::new(session, auth_config, client_id); client.refresh_token().await?; // initialize Spotify-related stuff From 177ce013f531ef379c13100382b73170b797aeec Mon Sep 17 00:00:00 2001 From: Julia Mertz Date: Sat, 17 Aug 2024 20:57:02 +0200 Subject: [PATCH 2/5] use command struct for client_id_command option --- docs/config.md | 2 +- spotify_player/src/config/mod.rs | 52 ++++++++++++++++---------------- spotify_player/src/streaming.rs | 14 ++------- 3 files changed, 29 insertions(+), 39 deletions(-) diff --git a/docs/config.md b/docs/config.md index 484d4054..d60b7c03 100644 --- a/docs/config.md +++ b/docs/config.md @@ -25,7 +25,7 @@ All configuration files should be placed inside the application's configuration | Option | Description | Default | | --------------------------------- | ---------------------------------------------------------------------------------------- | ------------------------------------------------------- | | `client_id` | the Spotify client's ID | `65b708073fc0480ea92a077233ca87bd` | -| `client_id_cmd` | a shell command that prints the Spotify client ID to stdout (overrides `client_id`) | `None` | +| `client_id_command` | a shell command that prints the Spotify client ID to stdout (overrides `client_id`) | `None` | | `client_port` | the port that the application's client is running on to handle CLI commands | `8080` | | `tracks_playback_limit` | the limit for the number of tracks played in a **tracks** playback | `50` | | `playback_format` | the format of the text in the playback's window | `{status} {track} • {artists}\n{album}\n{metadata}` | diff --git a/spotify_player/src/config/mod.rs b/spotify_player/src/config/mod.rs index 3b9ddb1e..7558d796 100644 --- a/spotify_player/src/config/mod.rs +++ b/spotify_player/src/config/mod.rs @@ -48,7 +48,7 @@ impl Configs { pub struct AppConfig { pub theme: String, pub client_id: String, - pub client_id_cmd: Option, + pub client_id_command: Option, pub client_port: u16, @@ -142,6 +142,26 @@ pub struct Command { pub args: Vec, } +impl Command { + pub fn execute(&self, extra_args: Option>) -> anyhow::Result { + let mut args = self.args.clone(); + extra_args.map_or_else(|| {}, |extra| args.extend(extra)); + + let output = std::process::Command::new(&self.command) + .args(&args) + .output()?; + + // running the command failed, report the command's stderr as an error + if !output.status.success() { + let stderr = std::str::from_utf8(&output.stderr)?.to_string(); + anyhow::bail!(stderr); + } + + let stdout = std::str::from_utf8(&output.stdout)?.to_string(); + Ok(stdout) + } +} + #[derive(Debug, Deserialize, Serialize, ConfigParse, Clone)] /// Application device configurations pub struct DeviceConfig { @@ -230,7 +250,7 @@ impl Default for AppConfig { theme: "dracula".to_owned(), // official Spotify web app's client id client_id: "65b708073fc0480ea92a077233ca87bd".to_string(), - client_id_cmd: None, + client_id_command: None, client_port: 8080, @@ -389,32 +409,12 @@ impl AppConfig { } } - /// Returns stdout of `client_id_cmd` if set, otherwise it returns the the value of `client_id` + /// Returns stdout of `client_id_command` if set, otherwise it returns the the value of `client_id` pub fn get_client_id(&self) -> Result { - if let Some(cmd) = self.client_id_cmd.clone() { - let output = if cfg!(target_os = "windows") { - std::process::Command::new("cmd") - .args(["/C", &cmd]) - .output() - } else { - std::process::Command::new("sh") - .arg("-c") - .arg(&cmd) - .output() - }?; - - if !output.status.success() { - let stderr = String::from_utf8(output.stderr)?; - tracing::error!("Running command: {}, returned error: {}", cmd, stderr); - anyhow::bail!(stderr); - } - - let stdout = String::from_utf8(output.stdout)?; - tracing::info!("Running command: {}, returned client id: {}", cmd, stdout); - return Ok(stdout); + match self.client_id_command.clone() { + Some(cmd) => cmd.execute(None), + None => Ok(self.client_id.clone()), } - - Ok(self.client_id.clone()) } } diff --git a/spotify_player/src/streaming.rs b/spotify_player/src/streaming.rs index 858f8d92..6b491fd8 100644 --- a/spotify_player/src/streaming.rs +++ b/spotify_player/src/streaming.rs @@ -143,18 +143,8 @@ fn execute_player_event_hook_command( cmd: &config::Command, event: PlayerEvent, ) -> anyhow::Result<()> { - let mut args = cmd.args.clone(); - args.extend(event.args()); - - let output = std::process::Command::new(&cmd.command) - .args(&args) - .output()?; - - // running the player hook command failed, report the command's stderr as an error - if !output.status.success() { - let stderr = std::str::from_utf8(&output.stderr)?.to_string(); - anyhow::bail!(stderr); - } + let args = Some(event.args()); + cmd.execute(args)?; Ok(()) } From e178ba5285eeb849d5147f972a4c968ee4f6ee2c Mon Sep 17 00:00:00 2001 From: jmertz Date: Sat, 17 Aug 2024 23:00:58 +0200 Subject: [PATCH 3/5] Update spotify_player/src/streaming.rs Co-authored-by: Thang Pham --- spotify_player/src/streaming.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spotify_player/src/streaming.rs b/spotify_player/src/streaming.rs index 6b491fd8..5d159ac5 100644 --- a/spotify_player/src/streaming.rs +++ b/spotify_player/src/streaming.rs @@ -143,8 +143,7 @@ fn execute_player_event_hook_command( cmd: &config::Command, event: PlayerEvent, ) -> anyhow::Result<()> { - let args = Some(event.args()); - cmd.execute(args)?; + cmd.execute(Some(event.args()))?; Ok(()) } From 9cfc8096ea81f029a6eed2fb937cd7ae4cf88bf6 Mon Sep 17 00:00:00 2001 From: jmertz Date: Sat, 17 Aug 2024 23:11:42 +0200 Subject: [PATCH 4/5] Apply suggestions from code review Co-authored-by: Thang Pham --- spotify_player/src/config/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spotify_player/src/config/mod.rs b/spotify_player/src/config/mod.rs index 7558d796..e69cf5c6 100644 --- a/spotify_player/src/config/mod.rs +++ b/spotify_player/src/config/mod.rs @@ -143,15 +143,15 @@ pub struct Command { } impl Command { + /// Execute a command, returning stdout if succeeded or stderr if failed pub fn execute(&self, extra_args: Option>) -> anyhow::Result { let mut args = self.args.clone(); - extra_args.map_or_else(|| {}, |extra| args.extend(extra)); + args.extend(extra.unwrap_or_default()); let output = std::process::Command::new(&self.command) .args(&args) .output()?; - // running the command failed, report the command's stderr as an error if !output.status.success() { let stderr = std::str::from_utf8(&output.stderr)?.to_string(); anyhow::bail!(stderr); @@ -411,8 +411,8 @@ impl AppConfig { /// Returns stdout of `client_id_command` if set, otherwise it returns the the value of `client_id` pub fn get_client_id(&self) -> Result { - match self.client_id_command.clone() { - Some(cmd) => cmd.execute(None), + match self.client_id_command { + Some(ref cmd) => cmd.execute(None), None => Ok(self.client_id.clone()), } } From 6c3396be260f7822182581b245e45c741b62cde1 Mon Sep 17 00:00:00 2001 From: Julia Mertz Date: Sat, 17 Aug 2024 23:14:09 +0200 Subject: [PATCH 5/5] fix formatting and variable name --- spotify_player/src/config/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spotify_player/src/config/mod.rs b/spotify_player/src/config/mod.rs index e69cf5c6..397d596b 100644 --- a/spotify_player/src/config/mod.rs +++ b/spotify_player/src/config/mod.rs @@ -143,10 +143,10 @@ pub struct Command { } impl Command { - /// Execute a command, returning stdout if succeeded or stderr if failed + /// Execute a command, returning stdout if succeeded or stderr if failed pub fn execute(&self, extra_args: Option>) -> anyhow::Result { let mut args = self.args.clone(); - args.extend(extra.unwrap_or_default()); + args.extend(extra_args.unwrap_or_default()); let output = std::process::Command::new(&self.command) .args(&args)