lashman/pixstrip
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

Fix 26 bugs, edge cases, and consistency issues from fifth audit pass

Browse Source 
Critical: undo toast now trashes only batch output files (not entire dir),
JPEG scanline write errors propagated, selective metadata write result returned.

High: zero-dimension guards in ResizeConfig/fit_within, negative aspect ratio
rejection, FM integration toggle infinite recursion guard, saturating counter
arithmetic in executor.

Medium: PNG compression level passed to oxipng, pct mode updates job_config,
external file loading updates step indicator, CLI undo removes history entries,
watch config write failures reported, fast-copy path reads image dimensions for
rename templates, discovery excludes unprocessable formats (heic/svg/ico/jxl),
CLI warns on invalid algorithm/overwrite values, resolve_collision trailing dot
fix, generation guards on all preview threads to cancel stale results, default
DPI aligned to 0, watermark text width uses char count not byte length.

Low: binary path escaped in Nautilus extension, file dialog filter aligned with
discovery, reset_wizard clears preset_mode and output_dir.
This commit is contained in:
lashman
2026-03-07 19:47:23 +02:00
parent 270a7db60d
commit b432cc7431
44 changed files with 5748 additions and 2221 deletions
Download Patch File Download Diff File Expand all files Collapse all files

301
pixstrip-cli/src/main.rs
View File

@@ -6,6 +6,7 @@ use pixstrip_core::pipeline::ProcessingJob;
use pixstrip_core::preset::Preset;
use pixstrip_core::storage::{HistoryStore, PresetStore};
use pixstrip_core::types::*;
use std::io::Write;
use std::path::PathBuf;
#[derive(Parser)]
@@ -67,7 +68,7 @@ enum Commands {
watermark_position: String,
/// Watermark opacity (0.0-1.0)
#[arg(long, default_value = "0.5")]
#[arg(long, default_value = "0.5", value_parser = parse_opacity)]
watermark_opacity: f32,
/// Rename with prefix
@@ -275,7 +276,10 @@ fn cmd_process(args: CmdProcessArgs) {
.unwrap_or_else(|| input_dir.join("processed"));
let mut job = if let Some(ref name) = args.preset {
let preset = find_preset(name);
let preset = find_preset(name).unwrap_or_else(|| {
eprintln!("Preset '{}' not found. Use 'pixstrip preset list' to see available presets.", name);
std::process::exit(1);
});
println!("Using preset: {}", preset.name);
preset.to_job(&input_dir, &output_dir)
} else {
@@ -286,14 +290,12 @@ fn cmd_process(args: CmdProcessArgs) {
if let Some(ref resize_str) = args.resize {
job.resize = Some(parse_resize(resize_str));
}
if let Some(ref fmt_str) = args.format
&& let Some(fmt) = parse_format(fmt_str)
{
if let Some(ref fmt_str) = args.format {
let fmt = parse_format(fmt_str).unwrap_or_else(|| std::process::exit(1));
job.convert = Some(ConvertConfig::SingleFormat(fmt));
}
if let Some(ref q_str) = args.quality
&& let Some(preset) = parse_quality(q_str)
{
if let Some(ref q_str) = args.quality {
let preset = parse_quality(q_str).unwrap_or_else(|| std::process::exit(1));
job.compress = Some(CompressConfig::Preset(preset));
}
if args.strip_metadata {
@@ -320,13 +322,28 @@ fn cmd_process(args: CmdProcessArgs) {
});
}
if args.rename_prefix.is_some() || args.rename_suffix.is_some() || args.rename_template.is_some() {
if let Some(ref tmpl) = args.rename_template {
if args.rename_prefix.is_some() || args.rename_suffix.is_some() {
eprintln!("Warning: --rename-template overrides --rename-prefix/--rename-suffix");
}
if !tmpl.contains('{') {
eprintln!("Warning: rename template '{}' has no placeholders. Use {{name}}, {{counter}}, {{ext}}, etc.", tmpl);
}
if !tmpl.contains("{ext}") && !tmpl.contains('.') {
eprintln!("Warning: rename template has no {{ext}} or file extension - output files may lack extensions");
}
}
job.rename = Some(RenameConfig {
prefix: args.rename_prefix.unwrap_or_default(),
suffix: args.rename_suffix.unwrap_or_default(),
counter_start: 1,
counter_padding: 3,
counter_enabled: true,
counter_position: 3,
template: args.rename_template,
case_mode: 0,
replace_spaces: 0,
special_chars: 0,
regex_find: String::new(),
regex_replace: String::new(),
});
@@ -336,13 +353,21 @@ fn cmd_process(args: CmdProcessArgs) {
"catmullrom" | "catmull-rom" => ResizeAlgorithm::CatmullRom,
"bilinear" => ResizeAlgorithm::Bilinear,
"nearest" => ResizeAlgorithm::Nearest,
_ => ResizeAlgorithm::Lanczos3,
"lanczos3" | "lanczos" => ResizeAlgorithm::Lanczos3,
other => {
eprintln!("Warning: unknown algorithm '{}', using lanczos3. Valid: lanczos3, catmullrom, bilinear, nearest", other);
ResizeAlgorithm::Lanczos3
}
};
job.overwrite_behavior = match args.overwrite.to_lowercase().as_str() {
"overwrite" | "always" => OverwriteBehavior::Overwrite,
"skip" => OverwriteBehavior::Skip,
_ => OverwriteBehavior::AutoRename,
"overwrite" | "always" => OverwriteAction::Overwrite,
"skip" => OverwriteAction::Skip,
"auto-rename" | "autorename" | "rename" => OverwriteAction::AutoRename,
other => {
eprintln!("Warning: unknown overwrite mode '{}', using auto-rename. Valid: auto-rename, overwrite, skip", other);
OverwriteAction::AutoRename
}
};
for file in &source_files {
@@ -357,6 +382,7 @@ fn cmd_process(args: CmdProcessArgs) {
"\r[{}/{}] {}...",
update.current, update.total, update.current_file
);
let _ = std::io::stderr().flush();
})
.unwrap_or_else(|e| {
eprintln!("\nProcessing failed: {}", e);
@@ -395,20 +421,39 @@ fn cmd_process(args: CmdProcessArgs) {
// Save to history
let history = HistoryStore::new();
let output_ext = match job.convert {
Some(ConvertConfig::SingleFormat(fmt)) => fmt.extension(),
_ => "",
};
let output_files: Vec<String> = source_files
.iter()
.map(|f| {
output_dir
.join(f.file_name().unwrap_or_default())
.to_string_lossy()
.into()
.enumerate()
.map(|(i, f)| {
let stem = f.file_stem().and_then(|s| s.to_str()).unwrap_or("file");
let ext = if output_ext.is_empty() {
f.extension().and_then(|e| e.to_str()).unwrap_or("jpg")
} else {
output_ext
};
let name = if let Some(ref rename) = job.rename {
if let Some(ref tmpl) = rename.template {
pixstrip_core::operations::rename::apply_template(
tmpl, stem, ext, rename.counter_start.saturating_add(i as u32), None,
)
} else {
rename.apply_simple(stem, ext, (i as u32).saturating_add(1))
}
} else {
format!("{}.{}", stem, ext)
};
output_dir.join(name).to_string_lossy().into()
})
.collect();
let _ = history.add(pixstrip_core::storage::HistoryEntry {
if let Err(e) = history.add(pixstrip_core::storage::HistoryEntry {
timestamp: chrono_timestamp(),
input_dir: input_dir.to_string_lossy().into(),
output_dir: output_dir.to_string_lossy().into(),
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,
total: result.total,
succeeded: result.succeeded,
@@ -417,7 +462,9 @@ fn cmd_process(args: CmdProcessArgs) {
total_output_bytes: result.total_output_bytes,
elapsed_ms: result.elapsed_ms,
output_files,
});
}, 50, 30) {
eprintln!("Warning: failed to save history (undo may not work): {}", e);
}
}
fn cmd_preset_list() {
@@ -439,7 +486,10 @@ fn cmd_preset_list() {
}
fn cmd_preset_export(name: &str, output: &str) {
let preset = find_preset(name);
let preset = find_preset(name).unwrap_or_else(|| {
eprintln!("Preset '{}' not found. Use 'pixstrip preset list' to see available presets.", name);
std::process::exit(1);
});
let store = PresetStore::new();
match store.export_to_file(&preset, &PathBuf::from(output)) {
Ok(()) => println!("Exported '{}' to '{}'", name, output),
@@ -499,18 +549,23 @@ fn cmd_history() {
}
fn cmd_undo(count: usize) {
if count == 0 {
eprintln!("Must undo at least 1 batch");
std::process::exit(1);
}
let history = HistoryStore::new();
match history.list() {
Ok(entries) => {
Ok(mut entries) => {
if entries.is_empty() {
println!("No processing history to undo.");
return;
}
let to_undo = entries.iter().rev().take(count);
let undo_count = count.min(entries.len());
let to_undo = entries.split_off(entries.len() - undo_count);
let mut total_trashed = 0;
for entry in to_undo {
for entry in &to_undo {
if entry.output_files.is_empty() {
println!(
"Batch from {} has no recorded output files - cannot undo",
@@ -527,7 +582,6 @@ fn cmd_undo(count: usize) {
for file_path in &entry.output_files {
let path = PathBuf::from(file_path);
if path.exists() {
// Move to OS trash using the trash crate
match trash::delete(&path) {
Ok(()) => {
total_trashed += 1;
@@ -540,6 +594,11 @@ fn cmd_undo(count: usize) {
}
}
// Remove undone entries from history
if let Err(e) = history.write_all(&entries) {
eprintln!("Warning: failed to update history after undo: {}", e);
}
println!("{} files moved to trash", total_trashed);
}
Err(e) => {
@@ -551,12 +610,19 @@ fn cmd_undo(count: usize) {
fn cmd_watch_add(path: &str, preset_name: &str, recursive: bool) {
// Verify the preset exists
let _preset = find_preset(preset_name);
if find_preset(preset_name).is_none() {
eprintln!("Preset '{}' not found. Use 'pixstrip preset list' to see available presets.", preset_name);
std::process::exit(1);
}
let watch_path = PathBuf::from(path);
if !watch_path.exists() {
eprintln!("Watch folder does not exist: {}", path);
std::process::exit(1);
}
if !watch_path.is_dir() {
eprintln!("Watch path is not a directory: {}", path);
std::process::exit(1);
}
// Save watch folder config
let watch = pixstrip_core::watcher::WatchFolder {
@@ -568,7 +634,8 @@ fn cmd_watch_add(path: &str, preset_name: &str, recursive: bool) {
// Store in config
let config_dir = dirs::config_dir()
.unwrap_or_else(|| PathBuf::from("~/.config"))
.or_else(|| dirs::home_dir().map(|h| h.join(".config")))
.unwrap_or_else(|| std::env::temp_dir())
.join("pixstrip");
let watches_path = config_dir.join("watches.json");
let mut watches: Vec<pixstrip_core::watcher::WatchFolder> = if watches_path.exists() {
@@ -587,15 +654,27 @@ fn cmd_watch_add(path: &str, preset_name: &str, recursive: bool) {
}
watches.push(watch);
let _ = std::fs::create_dir_all(&config_dir);
let _ = std::fs::write(&watches_path, serde_json::to_string_pretty(&watches).unwrap());
if let Err(e) = std::fs::create_dir_all(&config_dir) {
eprintln!("Warning: failed to create config directory: {}", e);
}
match serde_json::to_string_pretty(&watches) {
Ok(json) => {
if let Err(e) = std::fs::write(&watches_path, json) {
eprintln!("Warning: failed to write watch config: {}", e);
}
}
Err(e) => {
eprintln!("Warning: failed to serialize watch config: {}", e);
}
}
println!("Added watch: {} -> preset '{}'", path, preset_name);
}
fn cmd_watch_list() {
let config_dir = dirs::config_dir()
.unwrap_or_else(|| PathBuf::from("~/.config"))
.or_else(|| dirs::home_dir().map(|h| h.join(".config")))
.unwrap_or_else(|| std::env::temp_dir())
.join("pixstrip");
let watches_path = config_dir.join("watches.json");
@@ -630,7 +709,8 @@ fn cmd_watch_list() {
fn cmd_watch_remove(path: &str) {
let config_dir = dirs::config_dir()
.unwrap_or_else(|| PathBuf::from("~/.config"))
.or_else(|| dirs::home_dir().map(|h| h.join(".config")))
.unwrap_or_else(|| std::env::temp_dir())
.join("pixstrip");
let watches_path = config_dir.join("watches.json");
@@ -652,21 +732,33 @@ fn cmd_watch_remove(path: &str) {
return;
}
let _ = std::fs::write(&watches_path, serde_json::to_string_pretty(&watches).unwrap());
if let Ok(json) = serde_json::to_string_pretty(&watches) {
let _ = std::fs::write(&watches_path, json);
}
println!("Removed watch folder: {}", path);
}
fn cmd_watch_start() {
let config_dir = dirs::config_dir()
.unwrap_or_else(|| PathBuf::from("~/.config"))
.or_else(|| dirs::home_dir().map(|h| h.join(".config")))
.unwrap_or_else(|| std::env::temp_dir())
.join("pixstrip");
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()
match std::fs::read_to_string(&watches_path) {
Ok(content) => match serde_json::from_str(&content) {
Ok(w) => w,
Err(e) => {
eprintln!("Warning: failed to parse watches.json: {}. Using empty config.", e);
Vec::new()
}
},
Err(e) => {
eprintln!("Warning: failed to read watches.json: {}", e);
Vec::new()
}
}
} else {
Vec::new()
};
@@ -700,9 +792,15 @@ fn cmd_watch_start() {
match event {
pixstrip_core::watcher::WatchEvent::NewImage(path) => {
println!("New image: {}", path.display());
// Find which watcher this came from and use its preset
if let Some((_, preset_name)) = watchers.first() {
let preset = find_preset(preset_name);
// Find which watcher owns this path and use its preset
let matched = active.iter()
.find(|w| path.starts_with(&w.path))
.map(|w| w.preset_name.clone());
if let Some(preset_name) = matched.as_deref().or_else(|| watchers.first().map(|(_, n)| n.as_str())) {
let Some(preset) = find_preset(preset_name) else {
eprintln!(" Preset '{}' not found, skipping", preset_name);
continue;
};
let input_dir = path.parent().unwrap_or_else(|| std::path::Path::new(".")).to_path_buf();
let output_dir = input_dir.join("processed");
let mut job = preset.to_job(&input_dir, &output_dir);
@@ -728,23 +826,29 @@ fn cmd_watch_start() {
// --- Helpers ---
fn find_preset(name: &str) -> Preset {
// Check builtins first
fn find_preset(name: &str) -> Option<Preset> {
// Check builtins first (case-insensitive)
let lower = name.to_lowercase();
for preset in Preset::all_builtins() {
if preset.name.to_lowercase() == lower {
return preset;
return Some(preset);
}
}
// Check user presets
// Check user presets - try exact match first, then case-insensitive
let store = PresetStore::new();
if let Ok(preset) = store.load(name) {
return preset;
return Some(preset);
}
if let Ok(presets) = store.list() {
for preset in presets {
if preset.name.to_lowercase() == lower {
return Some(preset);
}
}
}
eprintln!("Preset '{}' not found. Use 'pixstrip preset list' to see available presets.", name);
std::process::exit(1);
None
}
fn parse_resize(s: &str) -> ResizeConfig {
@@ -757,12 +861,20 @@ fn parse_resize(s: &str) -> ResizeConfig {
eprintln!("Invalid resize height: '{}'", h);
std::process::exit(1);
});
if width == 0 || height == 0 {
eprintln!("Resize dimensions must be greater than zero");
std::process::exit(1);
}
ResizeConfig::Exact(Dimensions { width, height })
} else {
let width: u32 = s.parse().unwrap_or_else(|_| {
eprintln!("Invalid resize value: '{}'. Use a width like '1200' or dimensions like '1200x900'", s);
std::process::exit(1);
});
if width == 0 {
eprintln!("Resize width must be greater than zero");
std::process::exit(1);
}
ResizeConfig::ByWidth(width)
}
}
@@ -840,6 +952,14 @@ fn parse_watermark_position(s: &str) -> WatermarkPosition {
}
}
fn parse_opacity(s: &str) -> std::result::Result<f32, String> {
let v: f32 = s.parse().map_err(|e: std::num::ParseFloatError| e.to_string())?;
if !(0.0..=1.0).contains(&v) {
return Err("opacity must be between 0.0 and 1.0".into());
}
Ok(v)
}
fn format_bytes(bytes: u64) -> String {
if bytes < 1024 {
format!("{} B", bytes)
@@ -872,3 +992,88 @@ fn chrono_timestamp() -> String {
.unwrap_or_default();
format!("{}", duration.as_secs())
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn parse_format_valid() {
assert_eq!(parse_format("jpeg"), Some(ImageFormat::Jpeg));
assert_eq!(parse_format("jpg"), Some(ImageFormat::Jpeg));
assert_eq!(parse_format("PNG"), Some(ImageFormat::Png));
assert_eq!(parse_format("webp"), Some(ImageFormat::WebP));
assert_eq!(parse_format("avif"), Some(ImageFormat::Avif));
}
#[test]
fn parse_format_invalid() {
assert_eq!(parse_format("bmp"), None);
assert_eq!(parse_format("xyz"), None);
}
#[test]
fn parse_quality_valid() {
assert_eq!(parse_quality("maximum"), Some(QualityPreset::Maximum));
assert_eq!(parse_quality("max"), Some(QualityPreset::Maximum));
assert_eq!(parse_quality("high"), Some(QualityPreset::High));
assert_eq!(parse_quality("medium"), Some(QualityPreset::Medium));
assert_eq!(parse_quality("med"), Some(QualityPreset::Medium));
assert_eq!(parse_quality("low"), Some(QualityPreset::Low));
assert_eq!(parse_quality("web"), Some(QualityPreset::WebOptimized));
}
#[test]
fn parse_quality_invalid() {
assert_eq!(parse_quality("ultra"), None);
assert_eq!(parse_quality(""), None);
}
#[test]
fn parse_opacity_valid() {
assert!(parse_opacity("0.5").is_ok());
assert!(parse_opacity("0.0").is_ok());
assert!(parse_opacity("1.0").is_ok());
}
#[test]
fn parse_opacity_invalid() {
assert!(parse_opacity("1.5").is_err());
assert!(parse_opacity("-0.1").is_err());
assert!(parse_opacity("abc").is_err());
}
#[test]
fn format_bytes_ranges() {
assert_eq!(format_bytes(500), "500 B");
assert_eq!(format_bytes(1024), "1.0 KB");
assert_eq!(format_bytes(1024 * 1024), "1.0 MB");
assert_eq!(format_bytes(1024 * 1024 * 1024), "1.0 GB");
}
#[test]
fn format_duration_ranges() {
assert_eq!(format_duration(500), "500ms");
assert_eq!(format_duration(1500), "1.5s");
assert_eq!(format_duration(90_000), "1m 30s");
}
#[test]
fn find_preset_builtins() {
assert!(find_preset("Blog Photos").is_some());
assert!(find_preset("blog photos").is_some());
assert!(find_preset("nonexistent preset xyz").is_none());
}
#[test]
fn parse_resize_width_only() {
let config = parse_resize("1200");
assert!(matches!(config, ResizeConfig::ByWidth(1200)));
}
#[test]
fn parse_resize_exact() {
let config = parse_resize("1200x900");
assert!(matches!(config, ResizeConfig::Exact(Dimensions { width: 1200, height: 900 })));
}
}