Clean up low-severity code quality issues

- Share IMAGE_EXTENSIONS between discovery.rs and watcher.rs (DRY)
- Extract compute_renamed_path() to deduplicate ~100 lines in executor
- Extract estimate_text_dimensions() to deduplicate watermark calc (3 copies)
- Fix encoder fallback defaults: WebP 85, AVIF 63 (match QualityPreset::High)
- Extract watch_config_dir() and load_watches() helpers in CLI (4 copies)
- Remove redundant else branches after unwrap_or_default()
- Rename misleading chrono_timestamp() to unix_timestamp()
This commit is contained in:
2026-03-08 00:22:24 +02:00
parent 7e5d19ab03
commit 8d754017fa
6 changed files with 97 additions and 133 deletions

View File

@@ -9,6 +9,20 @@ use pixstrip_core::types::*;
use std::io::Write;
use std::path::PathBuf;
fn watch_config_dir() -> PathBuf {
dirs::config_dir()
.or_else(|| dirs::home_dir().map(|h| h.join(".config")))
.unwrap_or_else(|| std::env::temp_dir())
.join("pixstrip")
}
fn load_watches(watches_path: &std::path::Path) -> Vec<pixstrip_core::watcher::WatchFolder> {
std::fs::read_to_string(watches_path)
.ok()
.and_then(|s| serde_json::from_str(&s).ok())
.unwrap_or_default()
}
#[derive(Parser)]
#[command(name = "pixstrip")]
#[command(about = "Batch image processor - resize, convert, compress, strip metadata, watermark, and rename")]
@@ -440,7 +454,7 @@ fn cmd_process(args: CmdProcessArgs) {
// Save to history - use actual output paths from the executor
let history = HistoryStore::new();
if let Err(e) = history.add(pixstrip_core::storage::HistoryEntry {
timestamp: chrono_timestamp(),
timestamp: unix_timestamp(),
input_dir: input_dir.canonicalize().unwrap_or_else(|_| input_dir.to_path_buf()).to_string_lossy().into(),
output_dir: output_dir.canonicalize().unwrap_or_else(|_| output_dir.to_path_buf()).to_string_lossy().into(),
preset_name: args.preset,
@@ -640,19 +654,9 @@ fn cmd_watch_add(path: &str, preset_name: &str, recursive: bool) {
};
// Store in config
let config_dir = dirs::config_dir()
.or_else(|| dirs::home_dir().map(|h| h.join(".config")))
.unwrap_or_else(|| std::env::temp_dir())
.join("pixstrip");
let config_dir = watch_config_dir();
let watches_path = config_dir.join("watches.json");
let mut watches: Vec<pixstrip_core::watcher::WatchFolder> = if watches_path.exists() {
std::fs::read_to_string(&watches_path)
.ok()
.and_then(|s| serde_json::from_str(&s).ok())
.unwrap_or_default()
} else {
Vec::new()
};
let mut watches = load_watches(&watches_path);
// Don't add duplicate paths
if watches.iter().any(|w| w.path == watch.path) {
@@ -682,20 +686,10 @@ fn cmd_watch_add(path: &str, preset_name: &str, recursive: bool) {
}
fn cmd_watch_list() {
let config_dir = dirs::config_dir()
.or_else(|| dirs::home_dir().map(|h| h.join(".config")))
.unwrap_or_else(|| std::env::temp_dir())
.join("pixstrip");
let config_dir = watch_config_dir();
let watches_path = config_dir.join("watches.json");
let watches: Vec<pixstrip_core::watcher::WatchFolder> = if watches_path.exists() {
std::fs::read_to_string(&watches_path)
.ok()
.and_then(|s| serde_json::from_str(&s).ok())
.unwrap_or_default()
} else {
Vec::new()
};
let watches = load_watches(&watches_path);
if watches.is_empty() {
println!("No watch folders configured.");
@@ -718,20 +712,10 @@ fn cmd_watch_list() {
}
fn cmd_watch_remove(path: &str) {
let config_dir = dirs::config_dir()
.or_else(|| dirs::home_dir().map(|h| h.join(".config")))
.unwrap_or_else(|| std::env::temp_dir())
.join("pixstrip");
let config_dir = watch_config_dir();
let watches_path = config_dir.join("watches.json");
let mut watches: Vec<pixstrip_core::watcher::WatchFolder> = if watches_path.exists() {
std::fs::read_to_string(&watches_path)
.ok()
.and_then(|s| serde_json::from_str(&s).ok())
.unwrap_or_default()
} else {
Vec::new()
};
let mut watches = load_watches(&watches_path);
let original_len = watches.len();
let target = PathBuf::from(path);
@@ -759,10 +743,7 @@ fn cmd_watch_remove(path: &str) {
}
fn cmd_watch_start() {
let config_dir = dirs::config_dir()
.or_else(|| dirs::home_dir().map(|h| h.join(".config")))
.unwrap_or_else(|| std::env::temp_dir())
.join("pixstrip");
let config_dir = watch_config_dir();
let watches_path = config_dir.join("watches.json");
let watches: Vec<pixstrip_core::watcher::WatchFolder> = if watches_path.exists() {
@@ -876,7 +857,7 @@ fn cmd_watch_start() {
println!(" Processed: {} -> {}", format_bytes(r.total_input_bytes), format_bytes(r.total_output_bytes));
let history = HistoryStore::new();
let _ = history.add(pixstrip_core::storage::HistoryEntry {
timestamp: chrono_timestamp(),
timestamp: unix_timestamp(),
input_dir: input_dir.to_string_lossy().into(),
output_dir: output_dir.to_string_lossy().into(),
preset_name: Some(preset_name.to_string()),
@@ -1065,7 +1046,7 @@ fn format_duration(ms: u64) -> String {
}
}
fn chrono_timestamp() -> String {
fn unix_timestamp() -> String {
// Store as Unix seconds string (must match GTK format for pruning compatibility)
std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)

View File

@@ -2,11 +2,11 @@ use std::path::{Path, PathBuf};
use walkdir::WalkDir;
const IMAGE_EXTENSIONS: &[&str] = &[
pub const IMAGE_EXTENSIONS: &[&str] = &[
"jpg", "jpeg", "png", "webp", "avif", "gif", "tiff", "tif", "bmp",
];
fn is_image_extension(ext: &str) -> bool {
pub fn is_image_extension(ext: &str) -> bool {
IMAGE_EXTENSIONS.contains(&ext.to_lowercase().as_str())
}

View File

@@ -40,8 +40,8 @@ impl OutputEncoder {
match format {
ImageFormat::Jpeg => self.encode_jpeg(img, quality.unwrap_or(85)),
ImageFormat::Png => self.encode_png(img, quality.unwrap_or(3)),
ImageFormat::WebP => self.encode_webp(img, quality.unwrap_or(80)),
ImageFormat::Avif => self.encode_avif(img, quality.unwrap_or(80)),
ImageFormat::WebP => self.encode_webp(img, quality.unwrap_or(85)),
ImageFormat::Avif => self.encode_avif(img, quality.unwrap_or(63)),
ImageFormat::Gif => self.encode_fallback(img, image::ImageFormat::Gif),
ImageFormat::Tiff => self.encode_fallback(img, image::ImageFormat::Tiff),
ImageFormat::Bmp => self.encode_fallback(img, image::ImageFormat::Bmp),

View File

@@ -325,6 +325,46 @@ impl PipelineExecutor {
Ok(result)
}
/// Compute the renamed output path for a source file.
/// `ext` is the output extension, `dims` is (width, height) or None.
fn compute_renamed_path(
job: &ProcessingJob,
rename: &crate::operations::RenameConfig,
source: &crate::types::ImageSource,
ext: &str,
index: usize,
dims: Option<(u32, u32)>,
) -> std::path::PathBuf {
let stem = source.path.file_stem().and_then(|s| s.to_str()).unwrap_or("output");
if let Some(ref template) = rename.template {
let original_ext = source.path.extension().and_then(|e| e.to_str());
let working_stem = crate::operations::rename::apply_regex_replace(
stem, &rename.regex_find, &rename.regex_replace,
);
let working_stem = crate::operations::rename::apply_space_replacement(&working_stem, rename.replace_spaces);
let working_stem = crate::operations::rename::apply_special_chars(&working_stem, rename.special_chars);
let new_name = crate::operations::rename::apply_template_full(
template, &working_stem, ext,
rename.counter_start.saturating_add(index as u32),
dims, original_ext, Some(&source.path), None,
);
let new_name = if rename.case_mode > 0 {
if let Some(dot_pos) = new_name.rfind('.') {
let (name_part, ext_part) = new_name.split_at(dot_pos);
format!("{}{}", crate::operations::rename::apply_case_conversion(name_part, rename.case_mode), ext_part)
} else {
crate::operations::rename::apply_case_conversion(&new_name, rename.case_mode)
}
} else {
new_name
};
job.output_dir.join(sanitize_filename(&new_name))
} else {
let new_name = rename.apply_simple(stem, ext, (index as u32).saturating_add(1));
job.output_dir.join(sanitize_filename(&new_name))
}
}
fn process_single_static(
job: &ProcessingJob,
source: &crate::types::ImageSource,
@@ -350,40 +390,17 @@ impl PipelineExecutor {
// just copy the file instead of decoding/re-encoding.
if !job.needs_pixel_processing() && !metadata_needs_reencode {
let output_path = if let Some(ref rename) = job.rename {
let stem = source.path.file_stem().and_then(|s| s.to_str()).unwrap_or("output");
let ext = source.path.extension().and_then(|e| e.to_str()).unwrap_or("jpg");
if let Some(ref template) = rename.template {
// Read dimensions without full decode for {width}/{height} templates
let dims = image::ImageReader::open(&source.path)
let dims = if rename.template.is_some() {
image::ImageReader::open(&source.path)
.ok()
.and_then(|r| r.with_guessed_format().ok())
.and_then(|r| r.into_dimensions().ok());
// Apply rename transformations on stem (matching the pixel path)
let working_stem = crate::operations::rename::apply_regex_replace(
stem, &rename.regex_find, &rename.regex_replace,
);
let working_stem = crate::operations::rename::apply_space_replacement(&working_stem, rename.replace_spaces);
let working_stem = crate::operations::rename::apply_special_chars(&working_stem, rename.special_chars);
let new_name = crate::operations::rename::apply_template_full(
template, &working_stem, ext,
rename.counter_start.saturating_add(index as u32),
dims, Some(ext), Some(&source.path), None,
);
let new_name = if rename.case_mode > 0 {
if let Some(dot_pos) = new_name.rfind('.') {
let (name_part, ext_part) = new_name.split_at(dot_pos);
format!("{}{}", crate::operations::rename::apply_case_conversion(name_part, rename.case_mode), ext_part)
.and_then(|r| r.into_dimensions().ok())
} else {
crate::operations::rename::apply_case_conversion(&new_name, rename.case_mode)
}
} else {
new_name
None
};
job.output_dir.join(sanitize_filename(&new_name))
} else {
let new_name = rename.apply_simple(stem, ext, (index as u32).saturating_add(1));
job.output_dir.join(sanitize_filename(&new_name))
}
Self::compute_renamed_path(job, rename, source, ext, index, dims)
} else {
job.output_path_for(source, None)
};
@@ -503,49 +520,9 @@ impl PipelineExecutor {
// Determine output path (with rename if configured)
let output_path = if let Some(ref rename) = job.rename {
let stem = source
.path
.file_stem()
.and_then(|s| s.to_str())
.unwrap_or("output");
let ext = output_format.extension();
if let Some(ref template) = rename.template {
let dims = Some((img.width(), img.height()));
let original_ext = source.path.extension()
.and_then(|e| e.to_str());
// Apply rename transformations on the stem before template expansion
let working_stem = crate::operations::rename::apply_regex_replace(
stem, &rename.regex_find, &rename.regex_replace,
);
let working_stem = crate::operations::rename::apply_space_replacement(&working_stem, rename.replace_spaces);
let working_stem = crate::operations::rename::apply_special_chars(&working_stem, rename.special_chars);
let new_name = crate::operations::rename::apply_template_full(
template,
&working_stem,
ext,
rename.counter_start.saturating_add(index as u32),
dims,
original_ext,
Some(&source.path),
None,
);
// Apply case conversion to the final name (without extension)
let new_name = if rename.case_mode > 0 {
if let Some(dot_pos) = new_name.rfind('.') {
let (name_part, ext_part) = new_name.split_at(dot_pos);
format!("{}{}", crate::operations::rename::apply_case_conversion(name_part, rename.case_mode), ext_part)
} else {
crate::operations::rename::apply_case_conversion(&new_name, rename.case_mode)
}
} else {
new_name
};
job.output_dir.join(sanitize_filename(&new_name))
} else {
let new_name = rename.apply_simple(stem, ext, (index as u32).saturating_add(1));
job.output_dir.join(sanitize_filename(&new_name))
}
Self::compute_renamed_path(job, rename, source, ext, index, dims)
} else {
job.output_path_for(source, Some(output_format))
};

View File

@@ -189,6 +189,18 @@ fn walkdir_depth_inner(dir: &std::path::Path, max_depth: u32, results: &mut Vec<
}
}
/// Estimate text dimensions for a given string and font size.
/// Returns (width, height) in pixels.
fn estimate_text_dimensions(text: &str, font_size: f32) -> (u32, u32) {
let w = ((text.chars().count().min(10_000) as f32 * font_size.min(1000.0) * 0.6) as u32)
.saturating_add(4)
.min(16384);
let h = ((font_size.min(1000.0) * 1.4) as u32)
.saturating_add(4)
.min(4096);
(w, h)
}
/// Render text onto a transparent RGBA buffer and return it as a DynamicImage
fn render_text_to_image(
text: &str,
@@ -198,8 +210,7 @@ fn render_text_to_image(
opacity: f32,
) -> image::RgbaImage {
let scale = ab_glyph::PxScale::from(font_size);
let text_width = ((text.chars().count().min(10_000) as f32 * font_size.min(1000.0) * 0.6) as u32).saturating_add(4).min(16384);
let text_height = ((font_size.min(1000.0) * 1.4) as u32).saturating_add(4).min(4096);
let (text_width, text_height) = estimate_text_dimensions(text, font_size);
let alpha = (opacity * 255.0).clamp(0.0, 255.0) as u8;
let draw_color = Rgba([color[0], color[1], color[2], alpha]);
@@ -296,11 +307,10 @@ fn apply_text_watermark(
} else {
// No rotation - draw text directly (faster)
let scale = ab_glyph::PxScale::from(font_size);
let text_width = ((text.chars().count().min(10_000) as f32 * font_size.min(1000.0) * 0.6) as u32).saturating_add(4).min(16384);
let text_height = ((font_size.min(1000.0) * 1.4) as u32).saturating_add(4).min(4096);
let (tw, th) = estimate_text_dimensions(text, font_size);
let text_dims = Dimensions {
width: text_width,
height: text_height,
width: tw,
height: th,
};
let image_dims = Dimensions {
width: img.width(),
@@ -366,17 +376,16 @@ fn apply_tiled_text_watermark(
let alpha = (opacity * 255.0).clamp(0.0, 255.0) as u8;
let draw_color = Rgba([color[0], color[1], color[2], alpha]);
let text_width = ((text.chars().count().min(10_000) as f32 * font_size.min(1000.0) * 0.6) as i64 + 4).min(16384);
let text_height = ((font_size.min(1000.0) * 1.4) as i64 + 4).min(4096);
let (text_width, text_height) = estimate_text_dimensions(text, font_size);
let mut y: i64 = 0;
while y < ih as i64 {
let mut x: i64 = 0;
while x < iw as i64 {
draw_text_mut(&mut rgba, draw_color, x as i32, y as i32, scale, &font, text);
x += text_width + spacing as i64;
x += text_width as i64 + spacing as i64;
}
y += text_height + spacing as i64;
y += text_height as i64 + spacing as i64;
}
}

View File

@@ -112,10 +112,7 @@ impl Default for FolderWatcher {
}
fn is_image_file(path: &Path) -> bool {
let supported = [
"jpg", "jpeg", "png", "webp", "avif", "gif", "tiff", "tif", "bmp",
];
path.extension()
.and_then(|e| e.to_str())
.is_some_and(|ext| supported.contains(&ext.to_lowercase().as_str()))
.is_some_and(crate::discovery::is_image_extension)
}