Merge pull request #7 from alexwlchan/more-improvements
- ID
b7d58cf- date
2024-08-20 12:22:14+00:00- author
Alex Chan <alex@alexwlchan.net>- parents
7861191,343adc7- message
Merge pull request #7 from alexwlchan/more-improvements Add the remaining tests and features to call this a 1.0- changed files
4 files, 255 additions, 197 deletions
Changed files
README.md (466) → README.md (623)
diff --git a/README.md b/README.md
index 75f4d7a..81a0f47 100644
--- a/README.md
+++ b/README.md
@@ -2,21 +2,13 @@ create-thumbnail PATH [--width=WIDTH | --height=HEIGHT] --out-dir=OUT_DIR
focusing on a small piece of code makes it better
-* CLI:
- -> width only
- -> height only
-
-* happy path:
- -> small file
- -> test dimensions
-
-* errors:
- -> creates thumbnail directory
- -> /dev/null
- -> thumbnail dir is file?
- -> try to overwrite original file?
- -> non-image format
- -> unrecognised image format
- -> non-existent file
-
-* print name of thumbnail
\ No newline at end of file
+"DRY" / rule of three / https://en.wikipedia.org/wiki/Rule_of_three_(computer_programming)
+good at doing inside a single project
+but not across project
+this is thumbnailing code that I've written across at least half a dozen projects
+but it's always an adjunct to the main rpoject
+never made it a project on its own so it's always a slightly rushed, half-baked bit of code
+pul it out into its own project!
+
+thumbnails = don't feel need in local dev, cos everything is whizzy fast
+much better here
src/create_thumbnail.rs (2224) → src/create_thumbnail.rs (6886)
diff --git a/src/create_thumbnail.rs b/src/create_thumbnail.rs
index 4eb9e3c..9a4ae53 100644
--- a/src/create_thumbnail.rs
+++ b/src/create_thumbnail.rs
@@ -5,6 +5,138 @@ use std::str;
use image::imageops::FilterType;
+use crate::create_parent_directory::create_parent_directory;
+use crate::get_thumbnail_dimensions::{get_thumbnail_dimensions, TargetDimension};
+use crate::is_animated_gif::is_animated_gif;
+
+/// Create a thumbnail for the image, and return the relative path of
+/// the thumbnail within the collection folder.
+pub fn create_thumbnail(
+ path: &PathBuf,
+ out_dir: &PathBuf,
+ target: TargetDimension,
+) -> io::Result<PathBuf> {
+ let thumbnail_path = out_dir.join(path.file_name().unwrap());
+ create_parent_directory(&thumbnail_path)?;
+
+ // Make sure we don't overwrite the original image with a thumbnail
+ if *path == thumbnail_path {
+ return Err(io::Error::new(
+ io::ErrorKind::InvalidData,
+ "Cannot write thumbnail to the same directory as the original image",
+ ));
+ }
+ assert!(*path != thumbnail_path);
+
+ let (new_width, new_height) = get_thumbnail_dimensions(&path, target)
+ .map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e.to_string()))?;
+
+ if is_animated_gif(path)? {
+ create_animated_gif_thumbnail(path, out_dir, new_width, new_height)
+ } else {
+ create_static_thumbnail(path, out_dir, new_width, new_height)
+ }
+}
+
+#[cfg(test)]
+mod test_create_thumbnail {
+ use std::path::PathBuf;
+
+ use super::create_thumbnail;
+ use crate::get_thumbnail_dimensions::TargetDimension;
+ use crate::test_utils::{get_dimensions, test_dir};
+
+ #[test]
+ fn creates_an_animated_gif_thumbnail() {
+ let gif_path = PathBuf::from("src/tests/animated_squares.gif");
+ let out_dir = test_dir();
+ let target = TargetDimension::MaxWidth(16);
+
+ let thumbnail_path = create_thumbnail(&gif_path, &out_dir, target).unwrap();
+
+ assert_eq!(thumbnail_path, out_dir.join("animated_squares.mp4"));
+ assert!(thumbnail_path.exists());
+ }
+
+ #[test]
+ fn creates_a_static_gif_thumbnail() {
+ let img_path = PathBuf::from("src/tests/yellow.gif");
+ let out_dir = test_dir();
+ let target = TargetDimension::MaxWidth(16);
+
+ let thumbnail_path = create_thumbnail(&img_path, &out_dir, target).unwrap();
+
+ assert_eq!(thumbnail_path, out_dir.join("yellow.gif"));
+ assert!(thumbnail_path.exists());
+ assert_eq!(get_dimensions(&thumbnail_path), (16, 8));
+ }
+
+ #[test]
+ fn creates_a_png_thumbnail() {
+ let img_path = PathBuf::from("src/tests/red.png");
+ let out_dir = test_dir();
+ let target = TargetDimension::MaxWidth(16);
+
+ let thumbnail_path = create_thumbnail(&img_path, &out_dir, target).unwrap();
+
+ assert_eq!(thumbnail_path, out_dir.join("red.png"));
+ assert!(thumbnail_path.exists());
+ assert_eq!(get_dimensions(&thumbnail_path), (16, 32));
+ }
+
+ #[test]
+ fn creates_a_jpeg_thumbnail() {
+ let img_path = PathBuf::from("src/tests/noise.jpg");
+ let out_dir = test_dir();
+ let target = TargetDimension::MaxWidth(16);
+
+ let thumbnail_path = create_thumbnail(&img_path, &out_dir, target).unwrap();
+
+ assert_eq!(thumbnail_path, out_dir.join("noise.jpg"));
+ assert!(thumbnail_path.exists());
+ assert_eq!(get_dimensions(&thumbnail_path), (16, 32));
+ }
+
+ #[test]
+ fn creates_a_tif_thumbnail() {
+ let img_path = PathBuf::from("src/tests/green.tiff");
+ let out_dir = test_dir();
+ let target = TargetDimension::MaxHeight(16);
+
+ let thumbnail_path = create_thumbnail(&img_path, &out_dir, target).unwrap();
+
+ assert_eq!(thumbnail_path, out_dir.join("green.tiff"));
+ assert!(thumbnail_path.exists());
+ assert_eq!(get_dimensions(&thumbnail_path), (16, 16));
+ }
+
+ #[test]
+ fn creates_a_webp_thumbnail() {
+ let img_path = PathBuf::from("src/tests/purple.webp");
+ let out_dir = test_dir();
+ let target = TargetDimension::MaxWidth(16);
+
+ let thumbnail_path = create_thumbnail(&img_path, &out_dir, target).unwrap();
+
+ assert_eq!(thumbnail_path, out_dir.join("purple.webp"));
+ assert!(thumbnail_path.exists());
+ assert_eq!(get_dimensions(&thumbnail_path), (16, 16));
+ }
+
+ #[test]
+ fn it_creates_an_equal_size_thumbnail_if_dimension_larger_than_original() {
+ let img_path = PathBuf::from("src/tests/noise.jpg");
+ let out_dir = test_dir();
+ let target = TargetDimension::MaxWidth(500);
+
+ let thumbnail_path = create_thumbnail(&img_path, &out_dir, target).unwrap();
+
+ assert_eq!(thumbnail_path, out_dir.join("noise.jpg"));
+ assert!(thumbnail_path.exists());
+ assert_eq!(get_dimensions(&thumbnail_path), (128, 256));
+ }
+}
+
/// Create a thumbnail for an animated GIF.
///
/// This will use `ffmpeg` to create an MP4 file of the desired dimensions
src/get_thumbnail_dimensions.rs (4497) → src/get_thumbnail_dimensions.rs (3810)
diff --git a/src/get_thumbnail_dimensions.rs b/src/get_thumbnail_dimensions.rs
index d596f7c..95c9f1e 100644
--- a/src/get_thumbnail_dimensions.rs
+++ b/src/get_thumbnail_dimensions.rs
@@ -2,6 +2,14 @@ use std::path::PathBuf;
use image::GenericImageView;
+/// Represents the target dimensions of the thumbnail.
+///
+/// The user can choose a max width, or a max height, but not both.
+pub enum TargetDimension {
+ MaxWidth(u32),
+ MaxHeight(u32),
+}
+
/// Given the path to the original image and the target width/height,
/// calculate the dimensions of the new image.
///
@@ -17,25 +25,16 @@ use image::GenericImageView;
///
pub fn get_thumbnail_dimensions(
path: &PathBuf,
- target_width: Option<u32>,
- target_height: Option<u32>,
+ target: TargetDimension,
) -> Result<(u32, u32), image::error::ImageError> {
- // Assert that exactly one of `target_width` and `target_height` are defined
- assert!(target_width.is_some() || target_height.is_some());
- assert!(target_width.is_none() || target_height.is_none());
-
- // Open the image, and compare its dimensions to the target
let img = image::open(path)?;
- // Calculate the new width/height of the image
- let (new_width, new_height) = match (target_width, target_height) {
- (Some(w), None) if w >= img.width() => img.dimensions(),
- (None, Some(h)) if h >= img.height() => img.dimensions(),
-
- (Some(w), None) => (w, w * img.height() / img.width()),
- (None, Some(h)) => (h * img.width() / img.height(), h),
+ let (new_width, new_height) = match target {
+ TargetDimension::MaxWidth(w) if w >= img.width() => img.dimensions(),
+ TargetDimension::MaxHeight(h) if h >= img.height() => img.dimensions(),
- _ => unreachable!(),
+ TargetDimension::MaxWidth(w) => (w, w * img.height() / img.width()),
+ TargetDimension::MaxHeight(h) => (h * img.width() / img.height(), h),
};
Ok((new_width, new_height))
@@ -53,10 +52,9 @@ mod test_get_thumbnail_dimensions {
fn with_target_width() {
let p = PathBuf::from("src/tests/red.png");
- let target_width: Option<u32> = Some(50);
- let target_height: Option<u32> = None;
+ let target = TargetDimension::MaxWidth(50);
- let dimensions = get_thumbnail_dimensions(&p, target_width, target_height);
+ let dimensions = get_thumbnail_dimensions(&p, target);
assert_eq!(dimensions.unwrap(), (50, 100));
}
@@ -64,10 +62,9 @@ mod test_get_thumbnail_dimensions {
fn with_target_height() {
let p = PathBuf::from("src/tests/red.png");
- let target_width: Option<u32> = None;
- let target_height: Option<u32> = Some(100);
+ let target = TargetDimension::MaxHeight(100);
- let dimensions = get_thumbnail_dimensions(&p, target_width, target_height);
+ let dimensions = get_thumbnail_dimensions(&p, target);
assert_eq!(dimensions.unwrap(), (50, 100));
}
@@ -75,10 +72,9 @@ mod test_get_thumbnail_dimensions {
fn leaves_image_as_is_if_target_width_greater_than_width() {
let p = PathBuf::from("src/tests/red.png");
- let target_width: Option<u32> = Some(500);
- let target_height: Option<u32> = None;
+ let target = TargetDimension::MaxWidth(500);
- let dimensions = get_thumbnail_dimensions(&p, target_width, target_height);
+ let dimensions = get_thumbnail_dimensions(&p, target);
assert_eq!(dimensions.unwrap(), (100, 200));
}
@@ -86,10 +82,9 @@ mod test_get_thumbnail_dimensions {
fn leaves_image_as_is_if_target_width_equal_to_width() {
let p = PathBuf::from("src/tests/red.png");
- let target_width: Option<u32> = Some(500);
- let target_height: Option<u32> = None;
+ let target = TargetDimension::MaxWidth(500);
- let dimensions = get_thumbnail_dimensions(&p, target_width, target_height);
+ let dimensions = get_thumbnail_dimensions(&p, target);
assert_eq!(dimensions.unwrap(), (100, 200));
}
@@ -97,10 +92,9 @@ mod test_get_thumbnail_dimensions {
fn leaves_image_as_is_if_target_height_greater_than_height() {
let p = PathBuf::from("src/tests/red.png");
- let target_width: Option<u32> = None;
- let target_height: Option<u32> = Some(500);
+ let target = TargetDimension::MaxHeight(500);
- let dimensions = get_thumbnail_dimensions(&p, target_width, target_height);
+ let dimensions = get_thumbnail_dimensions(&p, target);
assert_eq!(dimensions.unwrap(), (100, 200));
}
@@ -108,10 +102,9 @@ mod test_get_thumbnail_dimensions {
fn leaves_image_as_is_if_target_height_equal_to_height() {
let p = PathBuf::from("src/tests/red.png");
- let target_width: Option<u32> = None;
- let target_height: Option<u32> = Some(500);
+ let target = TargetDimension::MaxHeight(500);
- let dimensions = get_thumbnail_dimensions(&p, target_width, target_height);
+ let dimensions = get_thumbnail_dimensions(&p, target);
assert_eq!(dimensions.unwrap(), (100, 200));
}
@@ -119,10 +112,9 @@ mod test_get_thumbnail_dimensions {
fn errors_if_image_does_not_exist() {
let p = PathBuf::from("src/tests/doesnotexist.png");
- let target_width: Option<u32> = Some(50);
- let target_height: Option<u32> = None;
+ let target = TargetDimension::MaxWidth(50);
- let dimensions = get_thumbnail_dimensions(&p, target_width, target_height);
+ let dimensions = get_thumbnail_dimensions(&p, target);
assert!(dimensions.is_err());
}
@@ -130,10 +122,9 @@ mod test_get_thumbnail_dimensions {
fn errors_if_cannot_read_image() {
let p = PathBuf::from("README.md");
- let target_width: Option<u32> = Some(50);
- let target_height: Option<u32> = None;
+ let target = TargetDimension::MaxWidth(50);
- let dimensions = get_thumbnail_dimensions(&p, target_width, target_height);
+ let dimensions = get_thumbnail_dimensions(&p, target);
assert!(dimensions.is_err());
}
}
src/main.rs (8684) → src/main.rs (6924)
diff --git a/src/main.rs b/src/main.rs
index 6d0f2d1..3568f6a 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -1,6 +1,5 @@
#![deny(warnings)]
-use std::io;
use std::path::PathBuf;
use clap::{ArgGroup, Parser};
@@ -10,139 +9,8 @@ mod create_thumbnail;
mod get_thumbnail_dimensions;
mod is_animated_gif;
-use crate::create_parent_directory::create_parent_directory;
-use crate::get_thumbnail_dimensions::get_thumbnail_dimensions;
-use crate::is_animated_gif::is_animated_gif;
-
-/// Create a thumbnail for the image, and return the relative path of
-/// the thumbnail within the collection folder.
-///
-/// TODO: Having two Option<u32> arguments is confusing because they could
-/// easily be swapped. Replace this with some sort of struct!
-pub fn create_thumbnail(
- path: &PathBuf,
- out_dir: &PathBuf,
- width: Option<u32>,
- height: Option<u32>,
-) -> io::Result<PathBuf> {
- let thumbnail_path = out_dir.join(path.file_name().unwrap());
- create_parent_directory(&thumbnail_path)?;
-
- // TODO: Does this check do what I think?
- assert!(*path != thumbnail_path);
-
- println!("w = {:?}", width);
- println!("h = {:?}", height);
-
- let (new_width, new_height) = get_thumbnail_dimensions(&path, width, height)
- .map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e.to_string()))?;
-
- println!("w = {:?}, h = {:?}", new_width, new_height);
-
- //
- if is_animated_gif(path)? {
- create_thumbnail::create_animated_gif_thumbnail(path, out_dir, new_width, new_height)
- } else {
- create_thumbnail::create_static_thumbnail(path, out_dir, new_width, new_height)
- }
-}
-
-#[cfg(test)]
-mod test_create_thumbnail {
- use std::path::PathBuf;
-
- use super::create_thumbnail;
- use crate::test_utils::{get_dimensions, test_dir};
-
- #[test]
- fn creates_an_animated_gif_thumbnail() {
- let gif_path = PathBuf::from("src/tests/animated_squares.gif");
- let out_dir = test_dir();
- let target_width = Some(16);
- let target_height = None;
-
- let thumbnail_path =
- create_thumbnail(&gif_path, &out_dir, target_width, target_height).unwrap();
-
- assert_eq!(thumbnail_path, out_dir.join("animated_squares.mp4"));
- assert!(thumbnail_path.exists());
- }
-
- #[test]
- fn creates_a_static_gif_thumbnail() {
- let gif_path = PathBuf::from("src/tests/yellow.gif");
- let out_dir = test_dir();
- let target_width = Some(16);
- let target_height = None;
-
- let thumbnail_path =
- create_thumbnail(&gif_path, &out_dir, target_width, target_height).unwrap();
-
- assert_eq!(thumbnail_path, out_dir.join("yellow.gif"));
- assert!(thumbnail_path.exists());
- assert_eq!(get_dimensions(&thumbnail_path), (16, 8));
- }
-
- #[test]
- fn creates_a_png_thumbnail() {
- let gif_path = PathBuf::from("src/tests/red.png");
- let out_dir = test_dir();
- let target_width = Some(16);
- let target_height = None;
-
- let thumbnail_path =
- create_thumbnail(&gif_path, &out_dir, target_width, target_height).unwrap();
-
- assert_eq!(thumbnail_path, out_dir.join("red.png"));
- assert!(thumbnail_path.exists());
- assert_eq!(get_dimensions(&thumbnail_path), (16, 32));
- }
-
- #[test]
- fn creates_a_jpeg_thumbnail() {
- let gif_path = PathBuf::from("src/tests/noise.jpg");
- let out_dir = test_dir();
- let target_width = Some(16);
- let target_height = None;
-
- let thumbnail_path =
- create_thumbnail(&gif_path, &out_dir, target_width, target_height).unwrap();
-
- assert_eq!(thumbnail_path, out_dir.join("noise.jpg"));
- assert!(thumbnail_path.exists());
- assert_eq!(get_dimensions(&thumbnail_path), (16, 32));
- }
-
- #[test]
- fn creates_a_tif_thumbnail() {
- let gif_path = PathBuf::from("src/tests/green.tiff");
- let out_dir = test_dir();
- let target_width = Some(16);
- let target_height = None;
-
- let thumbnail_path =
- create_thumbnail(&gif_path, &out_dir, target_width, target_height).unwrap();
-
- assert_eq!(thumbnail_path, out_dir.join("green.tiff"));
- assert!(thumbnail_path.exists());
- assert_eq!(get_dimensions(&thumbnail_path), (16, 16));
- }
-
- #[test]
- fn creates_a_webp_thumbnail() {
- let gif_path = PathBuf::from("src/tests/purple.webp");
- let out_dir = test_dir();
- let target_width = Some(16);
- let target_height = None;
-
- let thumbnail_path =
- create_thumbnail(&gif_path, &out_dir, target_width, target_height).unwrap();
-
- assert_eq!(thumbnail_path, out_dir.join("purple.webp"));
- assert!(thumbnail_path.exists());
- assert_eq!(get_dimensions(&thumbnail_path), (16, 16));
- }
-}
+use crate::create_thumbnail::create_thumbnail;
+use crate::get_thumbnail_dimensions::TargetDimension;
#[derive(Debug, Parser)]
#[clap(version, about)]
@@ -171,19 +39,51 @@ struct Cli {
fn main() {
let cli = Cli::parse();
- println!("args = {:?}", cli);
-
- create_thumbnail(&cli.path, &cli.out_dir, cli.width, cli.height).unwrap();
+ let target = match (cli.width, cli.height) {
+ (Some(w), None) => TargetDimension::MaxWidth(w),
+ (None, Some(h)) => TargetDimension::MaxHeight(h),
+ _ => unreachable!(),
+ };
+
+ match create_thumbnail(&cli.path, &cli.out_dir, target) {
+ Ok(thumbnail_path) => print!("{}", thumbnail_path.display()),
+ Err(e) => {
+ eprintln!("{}", e);
+ std::process::exit(1);
+ }
+ };
}
#[cfg(test)]
mod test_cli {
+ use std::path::PathBuf;
+
use regex::Regex;
- use crate::test_utils::{get_failure, get_success};
+ use crate::test_utils::{get_dimensions, get_failure, get_success};
+
+ #[test]
+ fn it_creates_a_thumbnail_with_max_width() {
+ let output = get_success(&["src/tests/red.png", "--width=50", "--out-dir=/tmp"]);
+
+ assert_eq!(output.exit_code, 0);
+ assert_eq!(output.stdout, "/tmp/red.png");
+ assert_eq!(output.stderr, "");
+ assert_eq!(get_dimensions(&PathBuf::from("/tmp/red.png")), (50, 100));
+ }
+
+ #[test]
+ fn it_creates_a_thumbnail_with_max_height() {
+ let output = get_success(&["src/tests/noise.jpg", "--height=128", "--out-dir=/tmp"]);
+
+ assert_eq!(output.exit_code, 0);
+ assert_eq!(output.stdout, "/tmp/noise.jpg");
+ assert_eq!(output.stderr, "");
+ assert_eq!(get_dimensions(&PathBuf::from("/tmp/noise.jpg")), (64, 128));
+ }
#[test]
- fn it_errors_if_you_pass_width_and_height() {
+ fn it_fails_if_you_pass_width_and_height() {
let output = get_failure(&[
"src/tests/red.png",
"--width=100",
@@ -201,7 +101,7 @@ mod test_cli {
}
#[test]
- fn it_errors_if_you_pass_neither_width_nor_height() {
+ fn it_fails_if_you_pass_neither_width_nor_height() {
let output = get_failure(&["src/tests/red.png", "--out-dir=/tmp"]);
let re = Regex::new(r"the following required arguments were not provided:").unwrap();
@@ -212,6 +112,49 @@ mod test_cli {
}
#[test]
+ fn it_fails_if_you_pass_a_non_existent_file() {
+ let output = get_failure(&["doesnotexist.txt", "--width=50", "--out-dir=/tmp"]);
+
+ assert_eq!(output.exit_code, 1);
+ assert_eq!(output.stderr, "No such file or directory (os error 2)\n");
+ assert_eq!(output.stdout, "");
+ }
+
+ #[test]
+ fn it_fails_if_you_pass_a_non_image() {
+ let output = get_failure(&["Cargo.toml", "--width=50", "--out-dir=/tmp"]);
+
+ assert_eq!(output.exit_code, 1);
+ assert_eq!(output.stderr, "The image format could not be determined\n");
+ assert_eq!(output.stdout, "");
+ }
+
+ // TODO: Improve this error message.
+ //
+ // It's good to know the tool won't completely break when this happens, but ideally
+ // we'd return a more meaningful error message in this case.
+ #[test]
+ fn it_fails_if_out_dir_is_a_file() {
+ let output = get_failure(&["src/images/noise.jpg", "--width=50", "--out-dir=README.md"]);
+
+ assert_eq!(output.exit_code, 1);
+ assert_eq!(output.stderr, "File exists (os error 17)\n");
+ assert_eq!(output.stdout, "");
+ }
+
+ #[test]
+ fn it_fails_if_you_try_to_overwrite_the_original_file() {
+ let output = get_failure(&["src/images/noise.jpg", "--width=50", "--out-dir=src/images"]);
+
+ assert_eq!(output.exit_code, 1);
+ assert_eq!(
+ output.stderr,
+ "Cannot write thumbnail to the same directory as the original image\n"
+ );
+ assert_eq!(output.stdout, "");
+ }
+
+ #[test]
fn it_prints_the_version() {
let output = get_success(&["--version"]);