-
Notifications
You must be signed in to change notification settings - Fork 462
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Show supported features and audio backend in --version #1312
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,7 @@ use librespot_playback::{ | |
dither::{mk_ditherer, DithererBuilder, TriangularDitherer}, | ||
}; | ||
use log::{error, info, warn}; | ||
use once_cell::sync::Lazy; | ||
use serde::{de::Error, de::Unexpected, Deserialize, Deserializer}; | ||
use sha1::{Digest, Sha1}; | ||
use std::{fmt, fs, path::Path, path::PathBuf, str::FromStr}; | ||
|
@@ -332,34 +333,102 @@ impl From<AudioFormat> for LSAudioFormat { | |
} | ||
} | ||
|
||
#[derive(Debug, Default, StructOpt)] | ||
pub static ABOUT_INFO: Lazy<String> = Lazy::new(format_features_info); | ||
|
||
// Features functionality | ||
fn format_features_info() -> String { | ||
let features = get_enabled_features(); | ||
let backends = get_enabled_backends(); | ||
|
||
let features_str = if features.is_empty() { | ||
"none".to_string() | ||
} else { | ||
features.join(", ") | ||
}; | ||
|
||
// Get the version from the Cargo environment variable | ||
let version = env!("CARGO_PKG_VERSION"); | ||
|
||
// Format the string with version under the title, and avoid repeating version elsewhere | ||
format!( | ||
"A Spotify daemon\nspotifyd {}\nfeatures: {}\naudio backends: {}", | ||
version, | ||
features_str, | ||
backends.join(", ") | ||
) | ||
} | ||
|
||
pub fn get_enabled_backends() -> Vec<&'static str> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be basically the same as |
||
let backends = vec![ | ||
#[cfg(feature = "alsa_backend")] | ||
"alsa", | ||
#[cfg(feature = "pulseaudio_backend")] | ||
"pulseaudio", | ||
#[cfg(feature = "rodio_backend")] | ||
"rodio", | ||
#[cfg(feature = "portaudio_backend")] | ||
"portaudio", | ||
#[cfg(feature = "rodiojack_backend")] | ||
"rodiojack", | ||
#[cfg(feature = "pipe_backend")] | ||
"pipe", | ||
]; | ||
|
||
backends | ||
} | ||
|
||
pub fn get_enabled_features() -> Vec<&'static str> { | ||
let mut features = Vec::new(); | ||
|
||
#[cfg(feature = "dbus_mpris")] | ||
features.push("MPRIS"); | ||
|
||
#[cfg(feature = "dbus_keyring")] | ||
features.push("keyring"); | ||
|
||
features | ||
} | ||
Comment on lines
+380
to
+390
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I would also prefer the &[
#[cfg(feature = "dbus_mpris")]
"MPRIS",
// ...
] style here, but not that important. |
||
|
||
#[derive(Debug, StructOpt)] | ||
#[structopt( | ||
about = "A Spotify daemon", | ||
about = &**ABOUT_INFO, // Use double dereference to get `&str` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of replacing |
||
author, | ||
name = "spotifyd", | ||
setting(AppSettings::ColoredHelp) | ||
)] | ||
pub struct CliConfig { | ||
/// The path to the config file to use | ||
// The path to the config file to use | ||
#[structopt(long, value_name = "string")] | ||
pub config_path: Option<PathBuf>, | ||
|
||
/// If set, starts spotifyd without detaching | ||
// If set, starts spotifyd without detaching | ||
#[structopt(long)] | ||
pub no_daemon: bool, | ||
|
||
/// Prints more verbose output | ||
// Prints more verbose output | ||
#[structopt(long)] | ||
pub verbose: bool, | ||
|
||
/// Path to PID file. | ||
// Path to PID file. | ||
#[structopt(long)] | ||
pub pid: Option<PathBuf>, | ||
|
||
#[structopt(flatten)] | ||
pub shared_config: SharedConfigValues, | ||
} | ||
|
||
impl Default for CliConfig { | ||
fn default() -> Self { | ||
CliConfig { | ||
config_path: None, | ||
no_daemon: false, | ||
verbose: false, | ||
pid: None, | ||
shared_config: SharedConfigValues::default(), | ||
} | ||
} | ||
} | ||
|
||
Comment on lines
+420
to
+431
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doesn't this exactly do, what |
||
// A struct that holds all allowed config fields. | ||
// The actual config file is made up of two sections, spotifyd and global. | ||
#[derive(Clone, Default, Deserialize, PartialEq, StructOpt)] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
use crate::config::CliConfig; | ||
use crate::config::ABOUT_INFO; | ||
#[cfg(unix)] | ||
use color_eyre::eyre::eyre; | ||
use color_eyre::{ | ||
|
@@ -98,6 +99,12 @@ fn main() -> eyre::Result<()> { | |
|
||
color_eyre::install().wrap_err("Couldn't initialize error reporting")?; | ||
|
||
// Handle --version explicitly | ||
if std::env::args().any(|arg| arg == "--version") { | ||
println!("{}", &**ABOUT_INFO); | ||
return Ok(()); | ||
} | ||
Comment on lines
+102
to
+106
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we specify |
||
|
||
let mut cli_config: CliConfig = CliConfig::from_args(); | ||
|
||
let is_daemon = !cli_config.no_daemon; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In theory, since recently, the standard library also has support for those Lazy cells e.g. with
std::cell::LazyCell
. But to make use of that, we would need a MSRV bump, so this would also be fine for now! Whatever suits you best. :)