Merge pull request #22 from Sekkyo/main

- Fixes CBZ creation on macOS
- Fixes page ordering on macOS
- Resolves clippy warnings
- Adds Windows ARM64 to automated release builds
- Fixes typos
- Improves code quality

Author: shloop

.github/workflows/release.yml

@@ -26,6 +26,11 @@ jobs:
             target: i686-pc-windows-msvc
             bin: gbscraper.exe
             final: gbscraper-windows-i686.zip
+          - os_name: Windows-aarch64
+            os: windows-latest
+            target: aarch64-pc-windows-msvc
+            bin: gbscraper.exe
+            final: gbscraper-windows-aarch64.zip
           - os_name: macOS-x86_64
             os: macOS-latest
             target: x86_64-apple-darwin

Cargo.toml

@@ -26,7 +26,7 @@ serde = { version = "1.0.24" , features = ["derive"] }
 serde_json = { version = "1.0.122" }
 clap = { version = "4.5.13", features = ["derive"] }
 bitflags = "2.6.0"
-zip = "2.1.6"
+zip = { version = "2.2.0", default-features = false, features = ["deflate-miniz"] }
 image = "0.25.2"
 sanitise-file-name = "1.0.0"
 time = "0.3.36"

src/lib/scraper/batching.rs

@@ -12,10 +12,10 @@ pub fn download_period(url: &str, dest: &str, options: &mut ScraperOptions) -> i
     let url = sanitize_url(url)?;
 
     if options.verbose {
-        println!("Attemping download of period page with url: {url}");
+        println!("Attempting download of period page with url: {url}");
     }
 
-    for issue_url in get_issue_urls_in_period(&url, &options)? {
+    for issue_url in get_issue_urls_in_period(&url, options)? {
         if let Err(x) = download_issue(&issue_url, dest, options) {
             eprintln!("Error downloading issue {issue_url}: {}", x);
         }
@@ -28,10 +28,10 @@ pub fn download_all(url: &str, dest: &str, options: &mut ScraperOptions) -> io::
     let url = sanitize_url(url)?;
 
     if options.verbose {
-        println!("Attemping download of base page with url: {url}");
+        println!("Attempting download of base page with url: {url}");
     }
 
-    for period_url in get_period_urls(&url, &options)? {
+    for period_url in get_period_urls(&url, options)? {
         if let Err(x) = download_period(&period_url, dest, options) {
             eprintln!("Error downloading period {period_url}: {}", x);
         }
@@ -43,7 +43,7 @@ pub fn download_all(url: &str, dest: &str, options: &mut ScraperOptions) -> io::
 pub fn get_period_urls(url: &str, options: &ScraperOptions) -> io::Result<Vec<String>> {
     let mut ret = Vec::new();
 
-    let res = try_download(&url, options.download_attempts)?;
+    let res = try_download(url, options.download_attempts)?;
     let body = res.text().to_result()?;
 
     let doc = Html::parse_document(&body);
@@ -72,7 +72,7 @@ pub fn get_period_urls(url: &str, options: &ScraperOptions) -> io::Result<Vec<St
 pub fn get_issue_urls_in_period(url: &str, options: &ScraperOptions) -> io::Result<Vec<String>> {
     let mut ret = Vec::new();
 
-    let res = try_download(&url, options.download_attempts)?;
+    let res = try_download(url, options.download_attempts)?;
     let body = res.text().to_result()?;
     let doc = Html::parse_document(&body);
 

src/lib/scraper/helpers.rs

@@ -14,7 +14,7 @@ pub(crate) fn id_from_url(url: &str) -> io::Result<String> {
         None => url_obj
             .path_segments()
             .to_result(INVALID_URL)?
-            .last()
+            .next_back()
             .to_result(INVALID_URL)?
             .to_string(),
     })
@@ -41,49 +41,42 @@ pub(crate) fn sanitize_url(url: &str) -> io::Result<String> {
     const PERIOD_TAG: &str = "atm_aiy";
     let url_obj = Url::try_from(url).to_result()?;
     match url_obj.query_pairs().find(|x| x.0 == PERIOD_TAG) {
-        Some(x) => Ok(std::format!("{base_url}&{PERIOD_TAG}={}", x.1.to_string())),
+        Some(x) => Ok(std::format!("{base_url}&{PERIOD_TAG}={}", x.1)),
         None => Ok(base_url),
     }
 }
 
-// Methods to convert between option/result types for error propogation.
+// Methods to convert between option/result types for error propagation.
 
 pub(crate) trait ToResult<T> {
-    ///
     fn to_result(self) -> std::io::Result<T>;
 }
 
 impl<T, E: Display> ToResult<T> for std::result::Result<T, E> {
     fn to_result(self) -> std::io::Result<T> {
         match self {
             Ok(x) => Ok(x),
-            Err(x) => Err(std::io::Error::new(io::ErrorKind::Other, x.to_string())),
+            Err(x) => Err(std::io::Error::other(x.to_string())),
         }
     }
 }
 
 pub(crate) trait ToResultErrorMessage<T> {
-    ///
     fn to_result(self, msg: &str) -> std::io::Result<T>;
 }
 
 impl<T> ToResultErrorMessage<T> for Option<T> {
     fn to_result(self, msg: &str) -> std::io::Result<T> {
         match self {
             Some(x) => Ok(x),
-            None => Err(std::io::Error::new(io::ErrorKind::Other, msg)),
+            None => Err(std::io::Error::other(msg.to_string())),
         }
     }
 }
 
 /// Generate filename for image.
 pub(crate) fn generate_image_filename(page_number: &usize, page_id: &str, ext: &str) -> String {
-    std::format!(
-        "{0}-{1}.{2}",
-        std::format!("{:0>5}", page_number),
-        page_id,
-        ext
-    )
+    std::format!("{:0>5}-{page_id}.{ext}", page_number)
 }
 
 /// Determine image extension by the content header.
@@ -109,7 +102,7 @@ pub(crate) fn get_image_ext(res: &reqwest::blocking::Response) -> io::Result<Str
 /// Determine image extension by the content header.
 pub(crate) fn try_download(url: &str, mut attempts: u32) -> io::Result<reqwest::blocking::Response> {
     let indefinite = attempts == 0;
-    let mut res: io::Result<reqwest::blocking::Response> = Err(io::Error::new(io::ErrorKind::Other, ""));
+    let mut res: io::Result<reqwest::blocking::Response> = Err(io::Error::other(""));
     while indefinite || attempts > 0 {
         res = reqwest::blocking::get(url).to_result();
         if let Ok(res) = res {

src/lib/scraper/mod.rs

@@ -1,5 +1,6 @@
 pub mod batching;
 mod helpers;
+#[allow(clippy::module_inception)]
 pub mod scraper;
 pub mod types;
 

src/lib/scraper/scraper.rs

@@ -129,7 +129,7 @@ pub fn download_issue(
     let mut first_page = "1".to_string();
     let mut i_page = 1;
     for page in issue.page {
-        if let None = page.src {
+        if page.src.is_none() {
             page_number_lookup.insert(page.pid.clone(), i_page);
             pages_to_download.push_back(page.pid.clone());
             if i_page == 1 {
@@ -172,12 +172,8 @@ pub fn download_issue(
         // Download images linked in JSON.
         // Note: JSON will contain an entry for every page in book. Requested page should have accompanying source URL, and adjacent pages may as well.
         for page in &issue.page {
-            // Skip if already downloaded.
-            if let None = &page.src {
-                continue;
-            }
-            // Skip if no download link.
-            else if pages_downloaded.contains(&page.pid) {
+            // Skip if no download link or already downloaded.
+            if page.src.is_none() || pages_downloaded.contains(&page.pid) {
                 continue;
             }
 
@@ -209,8 +205,8 @@ pub fn download_issue(
 
                     let mut any_png = false;
                     let mut canvas = image::DynamicImage::new(
-                        size_info.width.into(),
-                        size_info.height.into(),
+                        size_info.width,
+                        size_info.height,
                         image::ColorType::Rgb8,
                     );
 

src/lib/scraper/types.rs

@@ -139,6 +139,7 @@ pub enum ContentType {
 }
 
 #[derive(Debug, PartialEq, Eq)]
+#[allow(clippy::large_enum_variant)]
 pub enum DownloadStatus {
     Skipped,
     Complete(BookMetadata),
@@ -176,9 +177,9 @@ impl BookMetadata {
     }
 
     fn parse_length(text: &str) -> io::Result<u32> {
-        Ok(Self::remove_and_extract(text, Self::SUFFIX_PAGES)
+        Self::remove_and_extract(text, Self::SUFFIX_PAGES)
             .parse::<u32>()
-            .to_result()?)
+            .to_result()
     }
 
     fn remove_and_extract(source: &str, to_remove: &str) -> String {
@@ -225,8 +226,7 @@ impl BookMetadata {
             .select(&Selector::parse("#metadata").to_result()?)
             .next()
         {
-            let mut i: u32 = 0;
-            for child in e.text() {
+            for (i, child) in e.text().enumerate() {
                 if i == 0 {
                     publish_date = child.to_string();
                 } else if child.starts_with(Self::PREFIX_PUBLISHER) {
@@ -238,8 +238,6 @@ impl BookMetadata {
                 } else {
                     volume = child.to_string();
                 }
-
-                i += 1;
             }
         };
 

src/lib/writer/cbz.rs

@@ -12,25 +12,29 @@ use zip::write::SimpleFileOptions;
 /// * `target_filename` - Path to save CBZ to, including filename and extension.
 pub fn create_cbz(image_dir: &str, target_filename: &str) -> io::Result<()> {
     let dir_entry = std::path::Path::new(target_filename);
-    let file = std::fs::File::create(dir_entry).unwrap();
+    let file = std::fs::File::create(dir_entry)?;
 
     let mut zip = zip::ZipWriter::new(file);
     let options = SimpleFileOptions::default().compression_method(zip::CompressionMethod::Deflated);
 
-    let read_dir = fs::read_dir(image_dir)?;
-    for dir_entry in read_dir {
-        if let Ok(dir_entry) = dir_entry {
-            if let Ok(mut file) = std::fs::File::open(dir_entry.path()) {
-                let filename = dir_entry.file_name().into_string().unwrap();
-                let _ = file.seek(io::SeekFrom::Start(0));
+    let mut entries: Vec<_> = fs::read_dir(image_dir)?
+        .collect::<io::Result<_>>()?;
+    entries.sort_by_key(|e| e.file_name());
+    for dir_entry in entries {
+        let mut file = std::fs::File::open(dir_entry.path())?;
+        let filename = dir_entry.file_name().into_string().map_err(|file_name| {
+            io::Error::new(
+                io::ErrorKind::InvalidData,
+                format!("image filename is not valid UTF-8: {:?}", file_name),
+            )
+        })?;
+        file.seek(io::SeekFrom::Start(0))?;
 
-                zip.start_file(filename, options)?;
+        zip.start_file(filename, options)?;
 
-                let mut buffer = Vec::new();
-                let _ = file.read_to_end(&mut buffer)?;
-                zip.write_all(&buffer)?;
-            }
-        }
+        let mut buffer = Vec::new();
+        file.read_to_end(&mut buffer)?;
+        zip.write_all(&buffer)?;
     }
 
     zip.finish()?;