Add display_format config, fix restart command, clean up config code

- Add display_format option with get/set/list/unset for customizing track display
- Add Track::format() supporting $ARTIST, $ALBUM, $TRACK variables
- Update queue, status, notifications to use display_format
- Fix restart command using wrong subcommand 'start' -> 'start-daemon'
- Remove socket_path from config (now computed at runtime from XDG_RUNTIME_DIR)
- Consolidate duplicate save() and save_all() into single save()
- Fix redundant .clone().clone() in config.rs
- Add todo item for caching config in daemon hot paths

5 files changed, 237 insertions, 66 deletions

diff --git a/src/cli.rs b/src/cli.rs
index bd81983..e0cea63 100644
--- a/src/cli.rs
+++ b/src/cli.rs
@@ -14,9 +14,9 @@ pub struct Cli {
 #[derive(Subcommand)]
 pub enum Commands {
     /// Start the daemon
-    Start,
+    StartDaemon,
     /// Stop the daemon
-    Stop,
+    StopDaemon,
     /// Restart the daemon (kill existing and start new)
     Restart,
     /// Play a track, album, or artist
@@ -37,7 +37,7 @@ pub enum Commands {
     /// Toggle play/pause
     Toggle,
     /// Stop playback
-    StopPlayback,
+    Stop,
     /// Skip to next track
     Next,
     /// Skip to previous track
@@ -101,10 +101,10 @@ pub enum SettingsCommands {
 impl Cli {
     pub fn run(&self) -> Result<(), Box<dyn std::error::Error>> {
         match &self.command {
-            Commands::Start => {
+            Commands::StartDaemon => {
                 crate::daemon::start_daemon()?;
             }
-            Commands::Stop => {
+            Commands::StopDaemon => {
                 crate::daemon::stop_daemon()?;
             }
             Commands::Restart => {
@@ -135,7 +135,7 @@ impl Cli {
             Commands::Toggle => {
                 crate::daemon::send_command("toggle".to_string())?;
             }
-            Commands::StopPlayback => {
+            Commands::Stop => {
                 crate::daemon::send_command("stop".to_string())?;
             }
             Commands::Next => {
diff --git a/src/config.rs b/src/config.rs
index 42573ed..5fd2f1b 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -9,13 +9,13 @@ pub struct Config {
     pub config_path: PathBuf,
     pub music_paths: Vec<PathBuf>,
     pub scan_interval: Option<u64>,
-    pub socket_path: Option<PathBuf>,
     pub lastfm_api_key: Option<String>,
     pub lastfm_api_secret: Option<String>,
     pub lastfm_session_key: Option<String>,
     pub listenbrainz_token: Option<String>,
     pub volume: Option<u8>,
     pub shuffle: Option<String>,
+    pub display_format: Option<String>,
 }
 
 impl Config {
@@ -43,8 +43,13 @@ impl Config {
                 config_path: config_path.clone(),
                 music_paths: vec![dirs::audio_dir().unwrap_or_else(|| PathBuf::from("~/Music"))],
                 scan_interval: Some(3600),
-                socket_path: None,
-                ..Default::default()
+                lastfm_api_key: None,
+                lastfm_api_secret: None,
+                lastfm_session_key: None,
+                listenbrainz_token: None,
+                volume: Some(50),
+                shuffle: None,
+                display_format: Some(String::from("$ARTIST - $TRACK")),
             };
             default.save()?;
             return Ok(default);
@@ -53,15 +58,72 @@ impl Config {
         let content = fs::read_to_string(&config_path)?;
         let mut config: Self = toml::from_str(&content)?;
         config.config_path = config_path;
+        if config.music_paths.is_empty() {
+            config.music_paths =
+                vec![dirs::audio_dir().unwrap_or_else(|| PathBuf::from("~/Music"))];
+        }
+        if config.scan_interval.is_none() {
+            config.scan_interval = Some(3600);
+        }
+        config.save()?;
         Ok(config)
     }
 
     pub fn save(&self) -> Result<(), Box<dyn std::error::Error>> {
-        let content = toml::to_string_pretty(self)?;
+        let mut output = String::new();
+        let paths_str = self
+            .music_paths
+            .iter()
+            .map(|p| format!("\"{}\"", p.display()))
+            .collect::<Vec<_>>()
+            .join(", ");
+        output.push_str(&format!("music_paths = [{}]\n", paths_str));
+        output.push_str(&format!(
+            "scan_interval = {}\n",
+            self.scan_interval.unwrap_or(3600)
+        ));
+        let api_key = self
+            .lastfm_api_key
+            .as_ref()
+            .filter(|s| !s.is_empty())
+            .map(|s| s.as_str())
+            .unwrap_or("");
+        output.push_str(&format!("lastfm_api_key = \"{}\"\n", api_key));
+        let api_secret = self
+            .lastfm_api_secret
+            .as_ref()
+            .filter(|s| !s.is_empty())
+            .map(|s| s.as_str())
+            .unwrap_or("");
+        output.push_str(&format!("lastfm_api_secret = \"{}\"\n", api_secret));
+        let session_key = self
+            .lastfm_session_key
+            .as_ref()
+            .filter(|s| !s.is_empty())
+            .map(|s| s.as_str())
+            .unwrap_or("");
+        output.push_str(&format!("lastfm_session_key = \"{}\"\n", session_key));
+        let lb_token = self
+            .listenbrainz_token
+            .as_ref()
+            .filter(|s| !s.is_empty())
+            .map(|s| s.as_str())
+            .unwrap_or("");
+        output.push_str(&format!("listenbrainz_token = \"{}\"\n", lb_token));
+        output.push_str(&format!("volume = {}\n", self.volume.unwrap_or(0)));
+        let shuffle = self
+            .shuffle
+            .as_ref()
+            .filter(|s| !s.is_empty())
+            .map(|s| s.as_str())
+            .unwrap_or("");
+        output.push_str(&format!("shuffle = \"{}\"\n", shuffle));
+        let fmt = self.display_format.clone().unwrap_or_default();
+        output.push_str(&format!("display_format = \"{}\"\n", fmt));
         if let Some(parent) = self.config_path.parent() {
             fs::create_dir_all(parent)?;
         }
-        fs::write(&self.config_path, content)?;
+        fs::write(&self.config_path, output)?;
         Ok(())
     }
 
@@ -77,32 +139,39 @@ impl Config {
                 .scan_interval
                 .map(|i| i.to_string())
                 .unwrap_or_else(|| "not set".to_string())),
-            "socket_path" => Ok(self
-                .socket_path
-                .as_ref()
-                .map(|p| p.display().to_string())
-                .unwrap_or_else(|| "default".to_string())),
             "lastfm_api_key" => Ok(self
                 .lastfm_api_key
-                .clone()
+                .as_ref()
+                .filter(|s| !s.is_empty())
+                .map(|_| "<set>".to_string())
                 .unwrap_or_else(|| "not set".to_string())),
             "lastfm_api_secret" => Ok(self
                 .lastfm_api_secret
-                .clone()
+                .as_ref()
+                .filter(|s| !s.is_empty())
+                .map(|_| "<set>".to_string())
                 .unwrap_or_else(|| "not set".to_string())),
             "lastfm_session_key" => Ok(self
                 .lastfm_session_key
-                .clone()
+                .as_ref()
+                .filter(|s| !s.is_empty())
+                .map(|_| "<set>".to_string())
                 .unwrap_or_else(|| "not set".to_string())),
             "listenbrainz_token" => Ok(self
                 .listenbrainz_token
-                .clone()
+                .as_ref()
+                .filter(|s| !s.is_empty())
+                .map(|_| "<set>".to_string())
                 .unwrap_or_else(|| "not set".to_string())),
             "volume" => Ok(self
                 .volume
                 .map(|v| v.to_string())
                 .unwrap_or_else(|| "not set".to_string())),
             "shuffle" => Ok(self.shuffle.clone().unwrap_or_else(|| "off".to_string())),
+            "display_format" => Ok(self
+                .display_format
+                .clone()
+                .unwrap_or_else(|| "not set".to_string())),
             _ => Err(format!("Unknown setting: {}", key).into()),
         }
     }
@@ -115,9 +184,6 @@ impl Config {
             "scan_interval" => {
                 self.scan_interval = Some(value.parse()?);
             }
-            "socket_path" => {
-                self.socket_path = Some(PathBuf::from(value));
-            }
             "lastfm_api_key" => {
                 self.lastfm_api_key = Some(value.to_string());
             }
@@ -142,45 +208,97 @@ impl Config {
                     return Err(format!("Invalid shuffle value: {}", value).into());
                 }
             }
+            "display_format" => {
+                self.display_format = Some(value.to_string());
+            }
             _ => return Err(format!("Unknown setting: {}", key).into()),
         }
         Ok(())
     }
 
     pub fn list(&self) {
-        println!("music_paths = {:?}", self.music_paths);
-        println!("scan_interval = {:?}", self.scan_interval);
-        println!("socket_path = {:?}", self.socket_path);
-        println!("lastfm_api_key = {:?}", self.lastfm_api_key);
-        println!("lastfm_api_secret = {:?}", self.lastfm_api_secret);
-        println!("lastfm_session_key = {:?}", self.lastfm_session_key);
-        println!("listenbrainz_token = {:?}", self.listenbrainz_token);
-        println!("volume = {:?}", self.volume);
-        println!("shuffle = {:?}", self.shuffle);
+        println!(
+            "music_paths = {}",
+            self.music_paths
+                .iter()
+                .map(|p| p.display().to_string())
+                .collect::<Vec<_>>()
+                .join(", ")
+        );
+        println!(
+            "scan_interval = {}",
+            self.scan_interval
+                .map_or("not set".to_string(), |v| v.to_string())
+        );
+        println!(
+            "lastfm_api_key = {}",
+            self.lastfm_api_key
+                .as_ref()
+                .filter(|s| !s.is_empty())
+                .map_or("not set".to_string(), |_| "<set>".to_string())
+        );
+        println!(
+            "lastfm_api_secret = {}",
+            self.lastfm_api_secret
+                .as_ref()
+                .filter(|s| !s.is_empty())
+                .map_or("not set".to_string(), |_| "<set>".to_string())
+        );
+        println!(
+            "lastfm_session_key = {}",
+            self.lastfm_session_key
+                .as_ref()
+                .filter(|s| !s.is_empty())
+                .map_or("not set".to_string(), |_| "<set>".to_string())
+        );
+        println!(
+            "listenbrainz_token = {}",
+            self.listenbrainz_token
+                .as_ref()
+                .filter(|s| !s.is_empty())
+                .map_or("not set".to_string(), |_| "<set>".to_string())
+        );
+        println!(
+            "volume = {}",
+            self.volume
+                .filter(|&v| v != 0)
+                .map_or("not set".to_string(), |v| v.to_string())
+        );
+        println!(
+            "shuffle = {}",
+            self.shuffle
+                .clone()
+                .filter(|s| !s.is_empty())
+                .unwrap_or_else(|| "off".to_string())
+        );
+        println!(
+            "display_format = {}",
+            self.display_format
+                .clone()
+                .unwrap_or_else(|| "not set".to_string())
+        );
     }
 
     pub fn unset(&mut self, key: &str) -> Result<(), Box<dyn std::error::Error>> {
         match key {
             "music_paths" => self.music_paths.clear(),
             "scan_interval" => self.scan_interval = None,
-            "socket_path" => self.socket_path = None,
             "lastfm_api_key" => self.lastfm_api_key = None,
             "lastfm_api_secret" => self.lastfm_api_secret = None,
             "lastfm_session_key" => self.lastfm_session_key = None,
             "listenbrainz_token" => self.listenbrainz_token = None,
             "volume" => self.volume = None,
             "shuffle" => self.shuffle = None,
+            "display_format" => self.display_format = None,
             _ => return Err(format!("Unknown setting: {}", key).into()),
         }
         Ok(())
     }
 
-    pub fn get_socket_path(&self) -> PathBuf {
-        self.socket_path.clone().unwrap_or_else(|| {
-            let runtime_dir = std::env::var("XDG_RUNTIME_DIR")
-                .map(PathBuf::from)
-                .unwrap_or_else(|_| PathBuf::from("/tmp"));
-            runtime_dir.join("rsplayer").join("daemon.sock")
-        })
+    pub fn get_socket_path() -> PathBuf {
+        let runtime_dir = std::env::var("XDG_RUNTIME_DIR")
+            .map(PathBuf::from)
+            .unwrap_or_else(|_| PathBuf::from("/tmp"));
+        runtime_dir.join("rsplayer").join("daemon.sock")
     }
 }
diff --git a/src/daemon.rs b/src/daemon.rs
index b79e67a..acc7729 100644
--- a/src/daemon.rs
+++ b/src/daemon.rs
@@ -12,7 +12,7 @@ use std::time::Duration;
 
 pub fn start_daemon() -> Result<(), Box<dyn std::error::Error>> {
     let config = Config::load()?;
-    let socket_path = config.get_socket_path();
+    let socket_path = Config::get_socket_path();
     let pid_path = get_pid_path(&config);
 
     // Check if daemon is already running
@@ -55,7 +55,6 @@ pub fn start_daemon() -> Result<(), Box<dyn std::error::Error>> {
     let queue = Arc::new(Mutex::new(Queue::new()));
 
     // Load and apply saved settings
-    let config = Config::load().unwrap_or_default();
     if let Some(vol) = config.volume {
         audio_engine.set_volume(vol as f32 / 100.0);
     }
@@ -69,6 +68,9 @@ pub fn start_daemon() -> Result<(), Box<dyn std::error::Error>> {
         }
     }
 
+    // Save full config
+    config.save()?;
+
     // Accept connections until shutdown
     listener.set_nonblocking(true)?;
     // Helper: poll until a specific track actually starts playing or timeout
@@ -106,7 +108,8 @@ pub fn start_daemon() -> Result<(), Box<dyn std::error::Error>> {
                     audio_engine.play(next_track.clone());
                     if wait_for_start(&audio_engine, &next_track, 500) {
                         started = true;
-                        notification::notify_track(&Track::from_path(next_track));
+                        let fmt = Config::load().ok().and_then(|c| c.display_format.clone());
+                        notification::notify_track(&Track::from_path(next_track), fmt.as_deref());
                         break;
                     }
                 } else {
@@ -182,7 +185,7 @@ pub fn stop_daemon() -> Result<(), Box<dyn std::error::Error>> {
             }
 
             // Clean up files
-            let socket_path = config.get_socket_path();
+            let socket_path = Config::get_socket_path();
             std::fs::remove_file(&socket_path).ok();
             std::fs::remove_file(&pid_path).ok();
 
@@ -203,7 +206,7 @@ pub fn restart_daemon() -> Result<(), Box<dyn std::error::Error>> {
     // Start new daemon in background
     let current_exe = std::env::current_exe()?;
     std::process::Command::new(current_exe)
-        .arg("start")
+        .arg("start-daemon")
         .spawn()?;
 
     println!("Daemon restarted");
@@ -216,8 +219,7 @@ pub fn send_command(cmd: String) -> Result<(), Box<dyn std::error::Error>> {
 }
 
 fn send_command_raw(cmd: String) -> Result<(), Box<dyn std::error::Error>> {
-    let config = Config::load()?;
-    let socket_path = config.get_socket_path();
+    let socket_path = Config::get_socket_path();
 
     let mut stream = UnixStream::connect(socket_path)?;
     stream.write_all(cmd.as_bytes())?;
@@ -254,7 +256,11 @@ fn process_command(
             if audio.is_playing() {
                 if let Some(track_path) = audio.current_track() {
                     let track = Track::from_path(track_path);
-                    let track_str = track.display_string(0, 1);
+                    let fmt = Config::load().ok().and_then(|c| c.display_format.clone());
+                    let track_str = match fmt.as_deref() {
+                        Some(f) => track.format(f),
+                        None => track.display_string(0, 1),
+                    };
                     format!("Playing\nTrack: {}\n", track_str)
                 } else {
                     "Playing\n".to_string()
@@ -296,7 +302,11 @@ fn process_command(
             if let Ok(mut q) = queue.lock() {
                 if let Some(next_track) = q.next() {
                     audio.play(next_track.clone());
-                    notification::notify_track(&Track::from_path(next_track));
+                    let fmt = Config::load().ok().and_then(|c| c.display_format.clone());
+                    notification::notify_track(
+                        &Track::from_path(next_track.clone()),
+                        fmt.as_deref(),
+                    );
                     "Playing next track\n".to_string()
                 } else {
                     audio.stop();
@@ -311,7 +321,11 @@ fn process_command(
             if let Ok(mut q) = queue.lock() {
                 if let Some(prev_track) = q.previous() {
                     audio.play(prev_track.clone());
-                    notification::notify_track(&Track::from_path(prev_track));
+                    let fmt = Config::load().ok().and_then(|c| c.display_format.clone());
+                    notification::notify_track(
+                        &Track::from_path(prev_track.clone()),
+                        fmt.as_deref(),
+                    );
                     "Playing previous track\n".to_string()
                 } else {
                     "No previous track\n".to_string()
@@ -371,7 +385,13 @@ fn process_command(
             } else {
                 ("last", rest)
             };
-            let path_str = path_str.to_string();
+            let path_str = if path_str.starts_with("~/") {
+                std::env::var("HOME")
+                    .map(|h| path_str.replacen("~", &h, 1))
+                    .unwrap_or_else(|_| path_str.to_string())
+            } else {
+                path_str.to_string()
+            };
 
             if path_str.is_empty() {
                 return "Usage: play [now|next|last] <path>\n".to_string();
@@ -390,7 +410,9 @@ fn process_command(
                             }
                             if let Some(track) = q.track_at(q.position()) {
                                 audio.play(track.path.clone());
-                                notification::notify_track(track);
+                                let fmt =
+                                    Config::load().ok().and_then(|c| c.display_format.clone());
+                                notification::notify_track(track, fmt.as_deref());
                             }
                             format!("Playing now: {}\n", path_str)
                         }
@@ -405,7 +427,9 @@ fn process_command(
                             if was_empty && !was_playing {
                                 let track = Track::from_path(path.clone());
                                 audio.play(path);
-                                notification::notify_track(&track);
+                                let fmt =
+                                    Config::load().ok().and_then(|c| c.display_format.clone());
+                                notification::notify_track(&track, fmt.as_deref());
                                 format!("Playing: {}\n", path_str)
                             } else {
                                 format!("Queued: {}\n", path_str)
@@ -431,7 +455,10 @@ fn process_command(
                                     }
                                     if let Some(track) = q.track_at(q.position()) {
                                         audio.play(track.path.clone());
-                                        notification::notify_track(track);
+                                        let fmt = Config::load()
+                                            .ok()
+                                            .and_then(|c| c.display_format.clone());
+                                        notification::notify_track(track, fmt.as_deref());
                                     }
                                     format!(
                                         "Playing now: {} tracks from {}\n",
@@ -459,7 +486,10 @@ fn process_command(
                                         && let Some(track) = q.current()
                                     {
                                         audio.play(track.path.clone());
-                                        notification::notify_track(track);
+                                        let fmt = Config::load()
+                                            .ok()
+                                            .and_then(|c| c.display_format.clone());
+                                        notification::notify_track(track, fmt.as_deref());
                                     }
                                     format!("Queued {} tracks from: {}\n", files.len(), path_str)
                                 }
@@ -512,9 +542,13 @@ fn process_command(
                         q.shuffled_indices().to_vec()
                     };
                     let is_playing = audio.is_playing();
+                    let fmt = Config::load().ok().and_then(|c| c.display_format.clone());
                     for (display_idx, &track_idx) in indices.iter().enumerate() {
                         if let Some(track) = q.track_at(track_idx) {
-                            let track_str = track.display_string(display_idx, q.len());
+                            let track_str = match fmt.as_deref() {
+                                Some(f) => track.format(f),
+                                None => track.display_string(display_idx, q.len()),
+                            };
                             if track_idx == playing_track_idx && is_playing {
                                 output.push_str(&format!("> {} (playing)\n", track_str));
                             } else {
diff --git a/src/notification.rs b/src/notification.rs
index 2f7baa2..4947e6f 100644
--- a/src/notification.rs
+++ b/src/notification.rs
@@ -1,15 +1,18 @@
 use crate::queue::Track;
 
-pub fn notify_track(track: &Track) {
-    let body = match (track.artist.as_deref(), track.title.as_deref()) {
-        (Some(a), Some(t)) => format!("{} - {}", a, t),
-        (_, Some(t)) => t.to_string(),
-        _ => track
-            .path
-            .file_name()
-            .and_then(|n| n.to_str())
-            .unwrap_or("Unknown")
-            .to_string(),
+pub fn notify_track(track: &Track, format: Option<&str>) {
+    let body = match format {
+        Some(fmt) => track.format(fmt),
+        None => match (track.artist.as_deref(), track.title.as_deref()) {
+            (Some(a), Some(t)) => format!("{} - {}", a, t),
+            (_, Some(t)) => t.to_string(),
+            _ => track
+                .path
+                .file_name()
+                .and_then(|n| n.to_str())
+                .unwrap_or("Unknown")
+                .to_string(),
+        },
     };
 
     let _ = notify_rust::Notification::new()
diff --git a/src/queue.rs b/src/queue.rs
index 2c20356..d42203f 100644
--- a/src/queue.rs
+++ b/src/queue.rs
@@ -117,6 +117,22 @@ impl Track {
             format!("{:0width$}. {} - {}", index + 1, album_artist, title)
         }
     }
+
+    pub fn format(&self, format_str: &str) -> String {
+        let artist = self.artist.clone().unwrap_or_default();
+        let album = self.album.clone().unwrap_or_default();
+        let title = self.title.clone().unwrap_or_else(|| {
+            self.path
+                .file_name()
+                .and_then(|n| n.to_str())
+                .unwrap_or("Unknown")
+                .to_string()
+        });
+        format_str
+            .replace("$ARTIST", &artist)
+            .replace("$ALBUM", &album)
+            .replace("$TRACK", &title)
+    }
 }
 
 pub struct Queue {