From 80740d523f452ecde540cc1d9d6b14a2c8c63417 Mon Sep 17 00:00:00 2001
From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com>
Date: Wed, 11 Sep 2024 12:38:36 -0700
Subject: [PATCH] Rename `find_snapshots` (#596)

Renames & comments, for clarity
---
 cargo-insta/src/cargo.rs | 5 +++++
 cargo-insta/src/cli.rs   | 8 ++++----
 cargo-insta/src/walk.rs  | 4 ++--
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/cargo-insta/src/cargo.rs b/cargo-insta/src/cargo.rs
index 8dc7b34a..15e2f07f 100644
--- a/cargo-insta/src/cargo.rs
+++ b/cargo-insta/src/cargo.rs
@@ -2,6 +2,8 @@ use std::path::PathBuf;
 
 pub(crate) use cargo_metadata::Package;
 
+/// Find snapshot roots within a package
+// (I'm not sure how necessary this is; relative to just using all paths?)
 pub(crate) fn find_snapshot_roots(package: &Package) -> Vec<PathBuf> {
     let mut roots = Vec::new();
 
@@ -28,6 +30,9 @@ pub(crate) fn find_snapshot_roots(package: &Package) -> Vec<PathBuf> {
         roots.push(root.to_path_buf());
     }
 
+    // TODO: I think this root reduction is duplicative over the logic in
+    // `make_snapshot_walker`; could try removing.
+
     // reduce roots to avoid traversing into paths twice.  If we have both
     // /foo and /foo/bar as roots we would only walk into /foo.  Otherwise
     // we would encounter paths twice.  If we don't skip them here we run
diff --git a/cargo-insta/src/cli.rs b/cargo-insta/src/cli.rs
index b2de4e88..2390b486 100644
--- a/cargo-insta/src/cli.rs
+++ b/cargo-insta/src/cli.rs
@@ -17,7 +17,7 @@ use crate::cargo::{find_snapshot_roots, Package};
 use crate::container::{Operation, SnapshotContainer};
 use crate::utils::cargo_insta_version;
 use crate::utils::{err_msg, QuietExit};
-use crate::walk::{find_snapshots, make_snapshot_walker, FindFlags};
+use crate::walk::{find_pending_snapshots, make_snapshot_walker, FindFlags};
 
 use clap::{Args, Parser, Subcommand, ValueEnum};
 
@@ -451,7 +451,7 @@ fn load_snapshot_containers<'a>(
     for package in &loc.packages {
         for root in find_snapshot_roots(package) {
             roots.insert(root.clone());
-            for snapshot_container in find_snapshots(&root, &loc.exts, loc.find_flags) {
+            for snapshot_container in find_pending_snapshots(&root, &loc.exts, loc.find_flags) {
                 snapshot_containers.push((snapshot_container?, package));
             }
         }
@@ -762,7 +762,7 @@ fn handle_unreferenced_snapshots(
     let mut encountered_any = false;
 
     for package in loc.packages.clone() {
-        let walker = make_snapshot_walker(
+        let unreferenced_snapshots = make_snapshot_walker(
             package.manifest_path.parent().unwrap().as_std_path(),
             &[".snap"],
             FindFlags {
@@ -781,7 +781,7 @@ fn handle_unreferenced_snapshots(
         .filter_map(|e| e.path().canonicalize().ok())
         .filter(|path| !files.contains(path));
 
-        for path in walker {
+        for path in unreferenced_snapshots {
             if !encountered_any {
                 match action {
                     Action::Delete => {
diff --git a/cargo-insta/src/walk.rs b/cargo-insta/src/walk.rs
index 0d8a10e6..5ff9f754 100644
--- a/cargo-insta/src/walk.rs
+++ b/cargo-insta/src/walk.rs
@@ -21,8 +21,8 @@ fn is_hidden(entry: &DirEntry) -> bool {
         .unwrap_or(false)
 }
 
-/// Finds all snapshots
-pub(crate) fn find_snapshots<'a>(
+/// Finds all pending snapshots
+pub(crate) fn find_pending_snapshots<'a>(
     package_root: &Path,
     extensions: &'a [&'a str],
     flags: FindFlags,