Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move file-specific options under OutFile #529

Merged
merged 2 commits into from
Sep 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 39 additions & 24 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,15 +76,36 @@ pub mod internal_tests {

#[derive(Clone, Debug)]
pub enum OutFile {
/// Path(None) means same as input
Path(Option<PathBuf>),
/// Don't actually write any output, just calculate the best results.
None,
/// Write output to a file.
///
/// * `path`: Path to write the output file. `None` means same as input.
/// * `preserve_attrs`: Ensure the output file has the same permissions & timestamps as the input file.
Path {
path: Option<PathBuf>,
preserve_attrs: bool,
},
/// Write to standard output.
StdOut,
}

impl OutFile {
/// Construct a new `OutFile` with the given path.
///
/// This is a convenience method for `OutFile::Path { path: Some(path), preserve_attrs: false }`.
pub fn from_path(path: PathBuf) -> Self {
OutFile::Path {
path: Some(path),
preserve_attrs: false,
}
}
andrews05 marked this conversation as resolved.
Show resolved Hide resolved

pub fn path(&self) -> Option<&Path> {
match *self {
OutFile::Path(Some(ref p)) => Some(p.as_path()),
OutFile::Path {
path: Some(ref p), ..
} => Some(p.as_path()),
_ => None,
}
}
Expand Down Expand Up @@ -138,18 +159,10 @@ pub struct Options {
///
/// Default: `false`
pub check: bool,
/// Don't actually write any output, just calculate the best results.
///
/// Default: `false`
pub pretend: bool,
/// Write to output even if there was no improvement in compression.
///
/// Default: `false`
pub force: bool,
/// Ensure the output file has the same permissions as the input file.
///
/// Default: `false`
pub preserve_attrs: bool,
/// Which RowFilters to try on the file
///
/// Default: `None,Sub,Entropy,Bigrams`
Expand Down Expand Up @@ -296,10 +309,8 @@ impl Default for Options {
Options {
backup: false,
check: false,
pretend: false,
fix_errors: false,
force: false,
preserve_attrs: false,
filter: indexset! {RowFilter::None, RowFilter::Sub, RowFilter::Entropy, RowFilter::Bigrams},
interlace: Some(Interlacing::None),
optimize_alpha: false,
Expand Down Expand Up @@ -416,7 +427,13 @@ pub fn optimize(input: &InFile, output: &OutFile, opts: &Options) -> PngResult<(
let opt_metadata_preserved;
let in_data = match *input {
InFile::Path(ref input_path) => {
if opts.preserve_attrs {
if matches!(
output,
OutFile::Path {
preserve_attrs: true,
..
}
) {
opt_metadata_preserved = input_path
.metadata()
.map_err(|err| {
Expand Down Expand Up @@ -458,8 +475,8 @@ pub fn optimize(input: &InFile, output: &OutFile, opts: &Options) -> PngResult<(
if is_fully_optimized(in_data.len(), optimized_output.len(), opts) {
match (output, input) {
// if p is None, it also means same as the input path
(OutFile::Path(ref p), InFile::Path(ref input_path))
if p.as_ref().map_or(true, |p| p == input_path) =>
(OutFile::Path { path, .. }, InFile::Path(ref input_path))
if path.as_ref().map_or(true, |p| p == input_path) =>
{
info!("{}: Could not optimize further, no change written", input);
return Ok(());
Expand All @@ -484,20 +501,18 @@ pub fn optimize(input: &InFile, output: &OutFile, opts: &Options) -> PngResult<(
)
};

if opts.pretend {
info!("{}: Running in pretend mode, no output", savings);
return Ok(());
}

match (output, input) {
(&OutFile::StdOut, _) | (&OutFile::Path(None), &InFile::StdIn) => {
(OutFile::None, _) => {
info!("{}: Running in pretend mode, no output", savings);
}
(&OutFile::StdOut, _) | (&OutFile::Path { path: None, .. }, &InFile::StdIn) => {
let mut buffer = BufWriter::new(stdout());
buffer
.write_all(&optimized_output)
.map_err(|e| PngError::new(&format!("Unable to write to stdout: {}", e)))?;
}
(OutFile::Path(ref output_path), _) => {
let output_path = output_path
(OutFile::Path { path, .. }, _) => {
let output_path = path
.as_ref()
.map(|p| p.as_path())
.unwrap_or_else(|| input.path().unwrap());
Expand Down
29 changes: 17 additions & 12 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -376,12 +376,16 @@ fn collect_files(
}
continue;
};
let out_file = if let Some(ref out_dir) = *out_dir {
let out_path = Some(out_dir.join(input.file_name().unwrap()));
OutFile::Path(out_path)
} else {
(*out_file).clone()
};
let out_file =
if let (Some(out_dir), &OutFile::Path { preserve_attrs, .. }) = (out_dir, out_file) {
let path = Some(out_dir.join(input.file_name().unwrap()));
OutFile::Path {
path,
preserve_attrs,
}
} else {
(*out_file).clone()
};
let in_file = if using_stdin {
InFile::StdIn
} else {
Expand Down Expand Up @@ -459,10 +463,15 @@ fn parse_opts_into_struct(
None
};

let out_file = if matches.get_flag("stdout") {
let out_file = if matches.get_flag("pretend") {
OutFile::None
} else if matches.get_flag("stdout") {
OutFile::StdOut
} else {
OutFile::Path(matches.get_one::<PathBuf>("output_file").cloned())
OutFile::Path {
path: matches.get_one::<PathBuf>("output_file").cloned(),
preserve_attrs: matches.get_flag("preserve"),
}
};

opts.optimize_alpha = matches.get_flag("alpha");
Expand All @@ -482,10 +491,6 @@ fn parse_opts_into_struct(

opts.check = matches.get_flag("check");

opts.pretend = matches.get_flag("pretend");

opts.preserve_attrs = matches.get_flag("preserve");

opts.bit_depth_reduction = !matches.get_flag("no-bit-reduction");

opts.color_type_reduction = !matches.get_flag("no-color-reduction");
Expand Down
5 changes: 1 addition & 4 deletions tests/filters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,7 @@ fn get_opts(input: &Path) -> (OutFile, oxipng::Options) {
filter.insert(RowFilter::None);
options.filter = filter;

(
OutFile::Path(Some(input.with_extension("out.png"))),
options,
)
(OutFile::from_path(input.with_extension("out.png")), options)
}

fn test_it_converts(
Expand Down
11 changes: 5 additions & 6 deletions tests/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,7 @@ fn get_opts(input: &Path) -> (OutFile, oxipng::Options) {
filter.insert(RowFilter::None);
options.filter = filter;

(
OutFile::Path(Some(input.with_extension("out.png"))),
options,
)
(OutFile::from_path(input.with_extension("out.png")), options)
}

/// Add callback to allow checks before the output file is deleted again
Expand Down Expand Up @@ -518,8 +515,10 @@ fn preserve_attrs() {
#[cfg(feature = "filetime")]
let mtime_canon = RefCell::new(filetime::FileTime::from_unix_time(0, 0));

let (output, mut opts) = get_opts(&input);
opts.preserve_attrs = true;
let (mut output, opts) = get_opts(&input);
if let OutFile::Path { preserve_attrs, .. } = &mut output {
*preserve_attrs = true;
}

#[cfg(feature = "filetime")]
let callback_pre = |path_in: &Path| {
Expand Down
5 changes: 1 addition & 4 deletions tests/interlaced.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,7 @@ fn get_opts(input: &Path) -> (OutFile, oxipng::Options) {
filter.insert(RowFilter::None);
options.filter = filter;

(
OutFile::Path(Some(input.with_extension("out.png"))),
options,
)
(OutFile::from_path(input.with_extension("out.png")), options)
}

fn test_it_converts(
Expand Down
5 changes: 1 addition & 4 deletions tests/interlacing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,7 @@ fn get_opts(input: &Path) -> (OutFile, oxipng::Options) {
filter.insert(RowFilter::None);
options.filter = filter;

(
OutFile::Path(Some(input.with_extension("out.png"))),
options,
)
(OutFile::from_path(input.with_extension("out.png")), options)
}

fn test_it_converts(
Expand Down
6 changes: 3 additions & 3 deletions tests/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ fn optimize_from_memory_apng() {
fn optimize() {
let result = oxipng::optimize(
&"tests/files/fully_optimized.png".into(),
&OutFile::Path(None),
&OutFile::None,
&Options::default(),
);
assert!(result.is_ok());
Expand All @@ -47,7 +47,7 @@ fn optimize() {
fn optimize_corrupted() {
let result = oxipng::optimize(
&"tests/files/corrupted_header.png".into(),
&OutFile::Path(None),
&OutFile::None,
&Options::default(),
);
assert!(result.is_err());
Expand All @@ -57,7 +57,7 @@ fn optimize_corrupted() {
fn optimize_apng() {
let result = oxipng::optimize(
&"tests/files/apng_file.png".into(),
&OutFile::Path(None),
&OutFile::None,
&Options::from_preset(0),
);
assert!(result.is_ok());
Expand Down
5 changes: 1 addition & 4 deletions tests/reduction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,7 @@ fn get_opts(input: &Path) -> (OutFile, oxipng::Options) {
filter.insert(RowFilter::None);
options.filter = filter;

(
OutFile::Path(Some(input.with_extension("out.png"))),
options,
)
(OutFile::from_path(input.with_extension("out.png")), options)
}

fn test_it_converts(
Expand Down
7 changes: 2 additions & 5 deletions tests/regression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,7 @@ fn get_opts(input: &Path) -> (OutFile, oxipng::Options) {
filter.insert(RowFilter::None);
options.filter = filter;

(
OutFile::Path(Some(input.with_extension("out.png"))),
options,
)
(OutFile::from_path(input.with_extension("out.png")), options)
}

fn test_it_converts(
Expand Down Expand Up @@ -295,7 +292,7 @@ fn issue_92_filter_5() {
let input = "tests/files/issue-92.png";
let (_, mut opts) = get_opts(Path::new(input));
opts.filter = [RowFilter::MinSum].iter().cloned().collect();
let output = OutFile::Path(Some(Path::new(input).with_extension("-f5-out.png")));
let output = OutFile::from_path(Path::new(input).with_extension("-f5-out.png"));

test_it_converts(
input,
Expand Down
5 changes: 1 addition & 4 deletions tests/strategies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,7 @@ fn get_opts(input: &Path) -> (OutFile, oxipng::Options) {
filter.insert(RowFilter::None);
options.filter = filter;

(
OutFile::Path(Some(input.with_extension("out.png"))),
options,
)
(OutFile::from_path(input.with_extension("out.png")), options)
}

fn test_it_converts(
Expand Down