refactor: simplify matching and interface flows

.github/workflows/release.yml

@@ -12,21 +12,17 @@ jobs:
     runs-on: ubuntu-22.04
     permissions:
       contents: read
+    outputs:
+      package_version: ${{ steps.release_version.outputs.package_version }}
     steps:
       - name: Checkout repository
         uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7
 
-      - name: Validate release tag
-        if: github.event_name == 'push'
+      - name: Read and validate release version
+        id: release_version
         shell: bash
         run: |
           set -euo pipefail
-          tag="$GITHUB_REF_NAME"
-          if [[ ! "$tag" =~ ^v(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)$ ]]; then
-            echo "Release tag must match vX.Y.Z: $tag" >&2
-            exit 1
-          fi
-
           package_version="$(awk '
             /^\[package\]$/ { in_package = 1; next }
             /^\[/ { in_package = 0 }
@@ -38,15 +34,23 @@ jobs:
               exit
             }
           ' Cargo.toml)"
-          tag_version="${tag#v}"
-
           if [[ -z "$package_version" ]]; then
             echo "Could not read package.version from Cargo.toml" >&2
             exit 1
           fi
-          if [[ "$tag_version" != "$package_version" ]]; then
-            echo "Tag version $tag_version does not match Cargo.toml package.version $package_version" >&2
-            exit 1
+          echo "package_version=$package_version" >> "$GITHUB_OUTPUT"
+
+          if [[ "$GITHUB_EVENT_NAME" == "push" ]]; then
+            tag="$GITHUB_REF_NAME"
+            if [[ ! "$tag" =~ ^v(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)$ ]]; then
+              echo "Release tag must match vX.Y.Z: $tag" >&2
+              exit 1
+            fi
+            tag_version="${tag#v}"
+            if [[ "$tag_version" != "$package_version" ]]; then
+              echo "Tag version $tag_version does not match Cargo.toml package.version $package_version" >&2
+              exit 1
+            fi
           fi
 
   build:
@@ -98,30 +102,11 @@ jobs:
       - name: Set package metadata
         id: package_metadata
         shell: bash
+        env:
+          PACKAGE_VERSION: ${{ needs.validate.outputs.package_version }}
         run: |
           set -euo pipefail
-          package_version="$(awk '
-            /^\[package\]$/ { in_package = 1; next }
-            /^\[/ { in_package = 0 }
-            in_package && /^version = "/ {
-              value = $0
-              sub(/^version = "/, "", value)
-              sub(/"[[:space:]]*$/, "", value)
-              print value
-              exit
-            }
-          ' Cargo.toml)"
-          if [[ -z "$package_version" ]]; then
-            echo "Could not read package.version from Cargo.toml" >&2
-            exit 1
-          fi
-
-          if [[ "$GITHUB_EVENT_NAME" == "push" ]]; then
-            version="${GITHUB_REF_NAME#v}"
-          else
-            version="$package_version"
-          fi
-          package_name="beaver-v${version}-${{ matrix.target }}"
+          package_name="beaver-v${PACKAGE_VERSION}-${{ matrix.target }}"
           echo "package_name=$package_name" >> "$GITHUB_OUTPUT"
 
       - name: Package Unix archive
@@ -217,8 +202,8 @@ jobs:
           set -euo pipefail
           shopt -s nullglob
           archives=(beaver-v*.tar.gz beaver-v*.zip)
-          if [[ "${#archives[@]}" -ne 5 ]]; then
-            echo "Expected five release archives, found ${#archives[@]}" >&2
+          if [[ "${#archives[@]}" -eq 0 ]]; then
+            echo "No release archives found" >&2
             exit 1
           fi
           sha256sum "${archives[@]}" | sort -k2 > SHA256SUMS
@@ -243,9 +228,6 @@ jobs:
     permissions:
       contents: read
     steps:
-      - name: Checkout beaver
-        uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7
-
       - name: Update tap formula
         shell: bash
         env:

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "beaver"
-version = "1.0.3"
+version = "1.0.4"
 edition = "2021"
 rust-version = "1.88"
 description = "Rename subtitle files to match their videos, from a terminal UI or a CLI."

src/applying.rs

@@ -20,7 +20,6 @@ use crate::planning::{RenameOp, RenamePlan};
 /// A cheap fingerprint of a path, used to spot changes between two points in time.
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct FileState {
-    exists: bool,
     is_file: bool,
     identity: Option<(u64, u64)>,
     size: Option<u64>,
@@ -31,15 +30,13 @@ impl FileState {
     pub fn capture(path: &Path) -> Self {
         let Ok(metadata) = fs::metadata(path) else {
             return Self {
-                exists: false,
                 is_file: false,
                 identity: None,
                 size: None,
                 modified: None,
             };
         };
         Self {
-            exists: true,
             is_file: metadata.is_file(),
             identity: file_identity(&metadata),
             size: Some(metadata.len()),

src/cli.rs

@@ -9,12 +9,14 @@ use std::io::{self, IsTerminal, Write};
 use std::path::{Path, PathBuf};
 use std::process::ExitCode;
 
-use clap::{Parser, ValueEnum};
+use clap::Parser;
 
 use crate::applying::{apply_operations, prepare_operations};
 use crate::paths::{display_path, file_name};
 use crate::planning::{plan_directory, PlanOptions, RenameOp, RenamePlan};
-use crate::presentation::{match_badge, skip_label, MatchLevel};
+use crate::presentation::{match_badge, skip_label};
+
+pub use crate::presentation::MatchLevel as Level;
 
 #[derive(Parser, Debug)]
 #[command(
@@ -73,33 +75,14 @@ pub struct Cli {
     pub force: bool,
 }
 
-#[derive(Copy, Clone, Debug, PartialEq, Eq, ValueEnum)]
-pub enum Level {
-    Relaxed,
-    Balanced,
-    Cautious,
-}
-
-impl From<Level> for MatchLevel {
-    fn from(level: Level) -> Self {
-        match level {
-            Level::Relaxed => Self::Relaxed,
-            Level::Balanced => Self::Balanced,
-            Level::Cautious => Self::Cautious,
-        }
-    }
-}
-
 impl Cli {
     fn plan_options(&self) -> PlanOptions {
         let defaults = PlanOptions::default();
         PlanOptions {
             recursive: self.recursive,
             strict: self.strict,
             overwrite_existing: self.force,
-            min_score: self
-                .min_score
-                .unwrap_or_else(|| MatchLevel::from(self.level).score()),
+            min_score: self.min_score.unwrap_or_else(|| self.level.score()),
             video_exts: if self.video_ext.is_empty() {
                 defaults.video_exts
             } else {
@@ -250,7 +233,8 @@ fn confirm(count: usize) -> bool {
     if io::stdin().read_line(&mut answer).is_err() {
         return false;
     }
-    matches!(answer.trim().to_lowercase().as_str(), "y" | "yes")
+    let answer = answer.trim();
+    answer.eq_ignore_ascii_case("y") || answer.eq_ignore_ascii_case("yes")
 }
 
 #[cfg(test)]
@@ -276,7 +260,7 @@ mod tests {
     #[test]
     fn a_level_maps_onto_a_threshold() {
         let cli = Cli::parse_from(["beaver", "/tmp", "--level", "cautious"]);
-        assert_eq!(cli.plan_options().min_score, MatchLevel::Cautious.score());
+        assert_eq!(cli.plan_options().min_score, Level::Cautious.score());
 
         let cli = Cli::parse_from([
             "beaver",

src/planning.rs

@@ -16,6 +16,9 @@ use crate::similarity::ratio;
 
 pub const VIDEO_EXTS_DEFAULT: &[&str] = &["mkv", "mp4", "avi", "mov", "wmv", "m4v", "webm"];
 pub const SUB_EXTS_DEFAULT: &[&str] = &["ass", "srt", "ssa", "vtt", "sub"];
+pub(crate) const RELAXED_MIN_SCORE: f64 = 0.60;
+pub(crate) const BALANCED_MIN_SCORE: f64 = 0.72;
+pub(crate) const CAUTIOUS_MIN_SCORE: f64 = 0.84;
 
 /// How far ahead of the runner-up the best fuzzy match has to be.
 ///
@@ -94,7 +97,7 @@ impl Default for PlanOptions {
             recursive: false,
             strict: false,
             overwrite_existing: false,
-            min_score: crate::presentation::MatchLevel::default().score(),
+            min_score: BALANCED_MIN_SCORE,
             video_exts: VIDEO_EXTS_DEFAULT
                 .iter()
                 .map(|ext| ext.to_string())
@@ -260,6 +263,7 @@ fn collect_files(root: &Path, recursive: bool) -> std::io::Result<Vec<PathBuf>>
                         directories.push(path);
                     }
                 }
+                Ok(file_type) if file_type.is_file() => files.push(path),
                 // Follows symlinks, so a link to a video counts as one.
                 _ if path.is_file() => files.push(path),
                 _ => {}
@@ -280,7 +284,7 @@ fn create_plan(
         .values_mut()
         .chain(subtitles_by_directory.values_mut())
     {
-        candidates.sort_by_key(|candidate| sort_key(&candidate.path));
+        candidates.sort_by_cached_key(|candidate| sort_key(&candidate.path));
     }
 
     let mut directories: Vec<PathBuf> = videos_by_directory
@@ -290,22 +294,24 @@ fn create_plan(
         .collect::<HashSet<_>>()
         .into_iter()
         .collect();
-    directories.sort_by_key(|directory| sort_key(directory));
+    directories.sort_by_cached_key(|directory| sort_key(directory));
 
     let video_count = videos_by_directory.values().map(Vec::len).sum();
     let subtitle_count = subtitles_by_directory.values().map(Vec::len).sum();
     let directory_count = directories.len();
 
     let mut operations = Vec::new();
     let mut skipped = Vec::new();
-    let empty: Vec<Candidate> = Vec::new();
-
     for directory in &directories {
-        let subtitles = subtitles_by_directory.get(directory).unwrap_or(&empty);
+        let subtitles = subtitles_by_directory
+            .get(directory)
+            .map_or(&[][..], Vec::as_slice);
         if subtitles.is_empty() {
             continue;
         }
-        let videos = videos_by_directory.get(directory).unwrap_or(&empty);
+        let videos = videos_by_directory
+            .get(directory)
+            .map_or(&[][..], Vec::as_slice);
         if videos.is_empty() {
             skipped.extend(subtitles.iter().map(|subtitle| SkippedRename {
                 path: subtitle.path.clone(),
@@ -450,16 +456,17 @@ fn choose_destination(
     let directory = video.path.parent()?;
     let video_stem = video.path.file_stem()?.to_string_lossy().into_owned();
 
-    let base = directory.join(format!("{video_stem}.{extension}"));
     // Renaming a file onto its own name is not a collision; the caller reports
     // that case as "already matches".
-    if base == subtitle.path {
-        return Some(base);
-    }
     let taken = |candidate: &Path| {
         planned.contains(candidate) || (!overwrite_existing && path_exists(candidate))
     };
-    if !taken(&base) {
+    let available = |candidate: PathBuf| {
+        (candidate == subtitle.path || !taken(&candidate)).then_some(candidate)
+    };
+
+    let base = directory.join(format!("{video_stem}.{extension}"));
+    if let Some(base) = available(base) {
         return Some(base);
     }
     if strict {
@@ -468,21 +475,15 @@ fn choose_destination(
 
     if let Some(tag) = language_tag(&subtitle.path.file_stem()?.to_string_lossy()) {
         let tagged = directory.join(format!("{video_stem}.{tag}.{extension}"));
-        if tagged == subtitle.path {
-            return Some(tagged);
-        }
-        if !taken(&tagged) {
+        if let Some(tagged) = available(tagged) {
             return Some(tagged);
         }
     }
 
     // Bounded so a pathological directory cannot spin here forever.
     for number in 2..1000 {
         let numbered = directory.join(format!("{video_stem}.{number}.{extension}"));
-        if numbered == subtitle.path {
-            return Some(numbered);
-        }
-        if !taken(&numbered) {
+        if let Some(numbered) = available(numbered) {
             return Some(numbered);
         }
     }
@@ -502,21 +503,27 @@ fn choose_unique_best<'a>(
         return (None, 0.0);
     }
 
-    let mut scored: Vec<(f64, &Candidate)> = videos
-        .iter()
-        .filter(|video| !video.stem_norm.is_empty())
-        .map(|video| (ratio(&subtitle.stem_norm, &video.stem_norm), video))
-        .collect();
-    // Stable, so equal scores keep directory order and the choice is repeatable.
-    scored.sort_by(|left, right| right.0.total_cmp(&left.0));
+    let mut best = None;
+    let mut runner_up = 0.0;
+    for video in videos.iter().filter(|video| !video.stem_norm.is_empty()) {
+        let score = ratio(&subtitle.stem_norm, &video.stem_norm);
+        match best {
+            Some((best_score, _)) if score > best_score => {
+                runner_up = best_score;
+                best = Some((score, video));
+            }
+            Some(_) if score > runner_up => runner_up = score,
+            None => best = Some((score, video)),
+            _ => {}
+        }
+    }
 
-    let Some(&(best_score, best)) = scored.first() else {
+    let Some((best_score, best)) = best else {
         return (None, 0.0);
     };
     if best_score < min_score {
         return (None, best_score);
     }
-    let runner_up = scored.get(1).map_or(0.0, |entry| entry.0);
     if best_score - runner_up < MIN_SCORE_MARGIN {
         return (None, best_score);
     }