Fix general bugs (#7)

* validate before saving

* isolate temp dirs so cleanup only removes the current session

* add required user agent header for OS

* validate and store only non-empty values for magneturl and infohash

* return None from build_request for TV media when episode information is not available

* limit season/episode to 1–99

* replace silentfallback with explicit error handling to avoid saving invalid timestamps

* reset the field index when entering Settings

* fix cursor misposition with unicode

* fmt

* regex lazily

Author: van-sprundel

Cargo.lock

@@ -38,7 +38,10 @@ which = "8"
 discord-rich-presence = "1.0"
 futures = "0.3"
 itertools = "0.13"
+once_cell = "1.20"
 unicode-truncate = "2.0"
+unicode-width = "0.2"
+uuid = { version = "1", features = ["v4"] }
 
 [dev-dependencies]
 wiremock = "0.6"

Cargo.toml

@@ -188,6 +188,7 @@ impl Config {
     }
 
     pub fn save(&self) -> Result<(), ConfigError> {
+        self.validate()?;
         let path = Self::config_path()?;
         // Ensure parent directory exists
         if let Some(parent) = path.parent() {

src/config.rs

@@ -4,6 +4,16 @@ pub mod trakt;
 pub use discord::DiscordExtension;
 pub use trakt::TraktExtension;
 
+use once_cell::sync::Lazy;
+use regex::Regex;
+
+// Compiled regexes for episode parsing (compiled once at startup)
+static SXEX_RE: Lazy<Regex> = Lazy::new(|| Regex::new(r"(?i)[Ss](\d{1,2})[Ee](\d{1,3})").unwrap());
+static X_RE: Lazy<Regex> = Lazy::new(|| Regex::new(r"(?i)(\d{1,2})x(\d{1,3})").unwrap());
+static FULL_RE: Lazy<Regex> =
+    Lazy::new(|| Regex::new(r"(?i)season[.\s]*(\d{1,2}).*episode[.\s]*(\d{1,3})").unwrap());
+static COMPACT_RE: Lazy<Regex> = Lazy::new(|| Regex::new(r"[.\s](\d{1,2})(\d{2})[.\s]").unwrap());
+
 /// Information about the currently playing media
 #[derive(Debug, Clone)]
 pub struct MediaInfo {
@@ -32,29 +42,24 @@ pub struct MediaInfo {
 /// - Season 1 Episode 2
 /// - .102. (season 1, episode 02)
 pub fn parse_episode_info(filename: &str) -> (Option<u32>, Option<u32>) {
-    use regex::Regex;
-
     // S01E02, S1E2 format (most common)
-    let sxex_re = Regex::new(r"(?i)[Ss](\d{1,2})[Ee](\d{1,3})").unwrap();
-    if let Some(caps) = sxex_re.captures(filename)
+    if let Some(caps) = SXEX_RE.captures(filename)
         && let (Some(s), Some(e)) = (caps.get(1), caps.get(2))
         && let (Ok(season), Ok(episode)) = (s.as_str().parse(), e.as_str().parse())
     {
         return (Some(season), Some(episode));
     }
 
     // 1x02, 01x02 format
-    let x_re = Regex::new(r"(?i)(\d{1,2})x(\d{1,3})").unwrap();
-    if let Some(caps) = x_re.captures(filename)
+    if let Some(caps) = X_RE.captures(filename)
         && let (Some(s), Some(e)) = (caps.get(1), caps.get(2))
         && let (Ok(season), Ok(episode)) = (s.as_str().parse(), e.as_str().parse())
     {
         return (Some(season), Some(episode));
     }
 
     // Season 1 Episode 2 format (also handles dots instead of spaces)
-    let full_re = Regex::new(r"(?i)season[.\s]*(\d{1,2}).*episode[.\s]*(\d{1,3})").unwrap();
-    if let Some(caps) = full_re.captures(filename)
+    if let Some(caps) = FULL_RE.captures(filename)
         && let (Some(s), Some(e)) = (caps.get(1), caps.get(2))
         && let (Ok(season), Ok(episode)) = (s.as_str().parse(), e.as_str().parse())
     {
@@ -63,13 +68,17 @@ pub fn parse_episode_info(filename: &str) -> (Option<u32>, Option<u32>) {
 
     // .102. or .1002. format (season 1, episode 02 or season 10, episode 02)
     // Must be surrounded by dots/spaces to avoid matching years
-    let compact_re = Regex::new(r"[.\s](\d)(\d{2})[.\s]").unwrap();
-    if let Some(caps) = compact_re.captures(filename)
+    if let Some(caps) = COMPACT_RE.captures(filename)
         && let (Some(s), Some(e)) = (caps.get(1), caps.get(2))
         && let (Ok(season), Ok(episode)) = (s.as_str().parse::<u32>(), e.as_str().parse::<u32>())
     {
-        // Only valid if episode isn't too high (avoid matching years like 1999)
-        if (1..=99).contains(&season) && (1..=99).contains(&episode) {
+        // Reconstruct the full matched number to check if it's a year
+        let full_number = season * 100 + episode;
+        // Only valid if episode isn't too high and the full number isn't a year (1900-2099)
+        if (1..=99).contains(&season)
+            && (1..=99).contains(&episode)
+            && !(1900..=2099).contains(&full_number)
+        {
             return (Some(season), Some(episode));
         }
     }
@@ -214,4 +223,38 @@ mod tests {
         assert_eq!(parse_episode_info("show.S01e02.mkv"), (Some(1), Some(2)));
         assert_eq!(parse_episode_info("show.s01E02.mkv"), (Some(1), Some(2)));
     }
+
+    #[test]
+    fn test_compact_format_season_10_episode_02() {
+        // The comment in parse_episode_info states this format is supported
+        let result = parse_episode_info("Show.Name.1002.mkv");
+
+        assert_eq!(
+            result,
+            (Some(10), Some(2)),
+            "According to the comment, .1002. should parse as season 10, episode 2"
+        );
+    }
+
+    #[test]
+    fn test_compact_format_single_digit_season() {
+        // Verify single-digit seasons still work
+        assert_eq!(parse_episode_info("Show.Name.102.mkv"), (Some(1), Some(2)));
+        assert_eq!(parse_episode_info("Show.Name.315.mkv"), (Some(3), Some(15)));
+    }
+
+    #[test]
+    fn test_compact_format_rejects_years() {
+        // Should not match years that look like they could be season/episode
+        assert_eq!(
+            parse_episode_info("Movie.1999.BluRay.mkv"),
+            (None, None),
+            "1999 should be treated as a year, not season 19 episode 99"
+        );
+        assert_eq!(
+            parse_episode_info("Movie.2020.1080p.mkv"),
+            (None, None),
+            "2020 should be treated as a year, not season 20 episode 20"
+        );
+    }
 }

src/extensions/mod.rs

@@ -92,12 +92,12 @@ impl TraktExtension {
                     })
                 }
                 _ => {
-                    tracing::debug!(
+                    tracing::warn!(
                         title = %media.title,
                         filename = %media.file_name,
-                        "trakt: no episode info found in filename"
+                        "trakt: cannot scrobble TV show without episode info"
                     );
-                    None
+                    return None;
                 }
             };
 

src/extensions/trakt.rs

@@ -102,10 +102,13 @@ impl WatchHistory {
 
     /// Update watch progress
     pub fn update(&mut self, key: String, title: String, progress_percent: f64) {
-        let now = std::time::SystemTime::now()
-            .duration_since(std::time::UNIX_EPOCH)
-            .map(|d| d.as_secs())
-            .unwrap_or(0);
+        let now = match std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH) {
+            Ok(d) => d.as_secs(),
+            Err(e) => {
+                error!("failed to get system time, skipping history update: {}", e);
+                return;
+            }
+        };
 
         self.entries.insert(
             key,
@@ -143,10 +146,13 @@ impl WatchHistory {
 
     /// Clear entries older than given days
     pub fn cleanup_old(&mut self, days: u64) {
-        let now = std::time::SystemTime::now()
-            .duration_since(std::time::UNIX_EPOCH)
-            .map(|d| d.as_secs())
-            .unwrap_or(0);
+        let now = match std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH) {
+            Ok(d) => d.as_secs(),
+            Err(e) => {
+                error!("failed to get system time, skipping cleanup: {}", e);
+                return;
+            }
+        };
         let cutoff = now.saturating_sub(days * 24 * 60 * 60);
 
         self.entries.retain(|_, e| e.last_watched >= cutoff);

src/history.rs

@@ -54,8 +54,15 @@ pub struct SubtitleDownload {
 
 impl OpenSubtitlesClient {
     pub fn new(api_key: &str) -> Self {
+        let user_agent = format!("{}/{}", env!("CARGO_PKG_NAME"), env!("CARGO_PKG_VERSION"));
+
+        let client = Client::builder()
+            .user_agent(user_agent)
+            .build()
+            .expect("Failed to build HTTP client");
+
         Self {
-            client: Client::new(),
+            client,
             api_key: api_key.to_string(),
         }
     }