-
Notifications
You must be signed in to change notification settings - Fork 1
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
Flag options for different stat categories #12
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,14 +4,20 @@ use ::zip; | |
use std::collections::HashMap; | ||
use std::fs; | ||
|
||
pub fn zips_to_json(file_paths: &[&str], exclude: &str) -> String { | ||
let multi_archive = MultiArchiveJsonWriter::from(file_paths, exclude); | ||
serde_json::to_string(&multi_archive).unwrap() | ||
use zip_info::StatArgs; | ||
|
||
pub fn zips_to_json(file_paths: &[&str], exclude: &str, stat_args: &StatArgs) -> String { | ||
zips_to_json_with_printer(file_paths, exclude, stat_args, serde_json::to_string) | ||
} | ||
|
||
pub fn zips_to_json_pretty(file_paths: &[&str], exclude: &str, stat_args: &StatArgs) -> String { | ||
zips_to_json_with_printer(file_paths, exclude, stat_args, serde_json::to_string_pretty) | ||
} | ||
|
||
pub fn zips_to_json_pretty(file_paths: &[&str], exclude: &str) -> String { | ||
let multi_archive = MultiArchiveJsonWriter::from(file_paths,exclude); | ||
serde_json::to_string_pretty(&multi_archive).unwrap() | ||
fn zips_to_json_with_printer<F>(file_paths: &[&str], exclude: &str, stat_args: &StatArgs, printer: F) -> String | ||
where F: Fn(&MultiArchiveJsonWriter) -> serde_json::Result<String> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great refactoring using the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I accidentally hit "start review" instead of just "single comment"--note that there are still comments, too |
||
let multi_archive = MultiArchiveJsonWriter::from(file_paths, exclude, stat_args); | ||
printer(&multi_archive).unwrap() | ||
} | ||
|
||
#[derive(Serialize, Deserialize, Debug, PartialEq)] | ||
|
@@ -28,13 +34,13 @@ impl MultiArchiveJsonWriter { | |
/// Create and fill MultiArchiveJsonWriter representing | ||
/// zero to many .zip files: | ||
pub fn from( | ||
file_paths: &[&str], exclude: &str) -> MultiArchiveJsonWriter { | ||
file_paths: &[&str], exclude: &str, stat_args: &StatArgs) -> MultiArchiveJsonWriter { | ||
let mut multi_archive = MultiArchiveJsonWriter::new(); | ||
|
||
for path in file_paths { | ||
multi_archive.archives.insert( | ||
String::from(*path), | ||
ZipArchiveJsonWriter::from(*path, exclude), | ||
ZipArchiveJsonWriter::from(*path, exclude, stat_args), | ||
); | ||
} | ||
|
||
|
@@ -55,7 +61,7 @@ impl ZipArchiveJsonWriter { | |
|
||
/// Create and fill ZipArchiveJsonWriter representing a | ||
/// .zip file: | ||
pub fn from(file_path: &str, exclude: &str) -> ZipArchiveJsonWriter { | ||
pub fn from(file_path: &str, exclude: &str, stat_args: &StatArgs) -> ZipArchiveJsonWriter { | ||
let mut archive_writer = ZipArchiveJsonWriter::new(); | ||
let exclude_pattern = glob::Pattern::new(exclude).unwrap(); | ||
|
||
|
@@ -71,10 +77,15 @@ impl ZipArchiveJsonWriter { | |
if !exclude_pattern.matches(zip_object.name()) { | ||
archive_writer.objects.insert( | ||
String::from(zip_object.name()), | ||
// I wanted to prevent the evaluation of as many of these parameters at this level as possible | ||
// but it seems that because compression rate depends on both .size() and .compressed_size(), we | ||
// need both to be passed in to calculate compression_rate if needed. (No way to encode this | ||
// dependency here without dependent types.) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No problem |
||
ZipObjectJsonWriter::new( | ||
zip_object.compression(), | ||
if stat_args.flag_compression_type { Some(zip_object.compression()) } else { None }, | ||
zip_object.size(), | ||
zip_object.compressed_size(), | ||
stat_args, | ||
), | ||
); | ||
} | ||
|
@@ -84,33 +95,51 @@ impl ZipArchiveJsonWriter { | |
} | ||
} | ||
|
||
// We are going to use Option types here to denote that not | ||
// all of these are going to be calculated, based on certain | ||
// command-line flags given to this program. | ||
// | ||
// TODO: Figure out how to get the JSON serializer to dump fields | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you satisfying the |
||
// that are set to null, if this is something that we want in the | ||
// feature. | ||
#[derive(Serialize, Deserialize, Debug, PartialEq)] | ||
struct ZipObjectJsonWriter { | ||
compression_type: String, | ||
original_size: u64, | ||
compressed_size: u64, | ||
compression_rate: String, | ||
#[serde(skip_serializing_if="Option::is_none")] | ||
compression_type: Option<String>, | ||
#[serde(skip_serializing_if="Option::is_none")] | ||
original_size: Option<u64>, | ||
#[serde(skip_serializing_if="Option::is_none")] | ||
compressed_size: Option<u64>, | ||
#[serde(skip_serializing_if="Option::is_none")] | ||
compression_rate: Option<String>, | ||
} | ||
|
||
impl ZipObjectJsonWriter { | ||
pub fn new( | ||
compression_type: zip::CompressionMethod, | ||
compression_type: Option<zip::CompressionMethod>, | ||
original_size: u64, | ||
compressed_size: u64, | ||
stat_args: &StatArgs, | ||
) -> ZipObjectJsonWriter { | ||
let compression_rate = match original_size { | ||
0 => 0.0 as f64, | ||
_ => { | ||
(original_size as f64 - compressed_size as f64) | ||
/ original_size as f64 | ||
}, | ||
}; | ||
let compression_rate = | ||
if stat_args.flag_compression_rate { | ||
Some(match original_size { | ||
0 => 0.0 as f64, | ||
_ => { | ||
(original_size as f64 - compressed_size as f64) | ||
/ original_size as f64 | ||
}, | ||
}) | ||
} | ||
else { | ||
None | ||
}; | ||
|
||
ZipObjectJsonWriter { | ||
compression_type: format!("{}", compression_type), | ||
original_size: original_size, | ||
compressed_size: compressed_size, | ||
compression_rate: format!("{:.*}%", 2, compression_rate * 100.0), | ||
compression_type: compression_type.and_then(|v| Some(format!("{}", v))), | ||
original_size: if stat_args.flag_original_size { Some(original_size) } else { None }, | ||
compressed_size: if stat_args.flag_compressed_size { Some(compressed_size) } else { None }, | ||
compression_rate: compression_rate.and_then(|v| Some(format!("{:.*}%", 2, v * 100.0))), | ||
} | ||
} | ||
} | ||
|
@@ -121,10 +150,10 @@ mod tests { | |
|
||
fn get_zip_object() -> ZipObjectJsonWriter { | ||
ZipObjectJsonWriter { | ||
compression_type: format!("{}", zip::CompressionMethod::Deflated), | ||
original_size: 100, | ||
compressed_size: 50, | ||
compression_rate: String::from("50%"), | ||
compression_type: Some(format!("{}", zip::CompressionMethod::Deflated)), | ||
original_size: Some(100), | ||
compressed_size: Some(50), | ||
compression_rate: Some(String::from("50%")), | ||
} | ||
} | ||
|
||
|
@@ -137,28 +166,31 @@ mod tests { | |
#[test] | ||
fn test_new_zip_object_calculates_percentages() { | ||
let zip_object = ZipObjectJsonWriter::new( | ||
zip::CompressionMethod::Deflated, | ||
Some(zip::CompressionMethod::Deflated), | ||
100, | ||
50, | ||
&StatArgs::default(), | ||
); | ||
|
||
assert_eq!("50.00%", zip_object.compression_rate); | ||
assert_eq!("50.00%", zip_object.compression_rate.unwrap_or_default()); | ||
|
||
let zip_object_empty = ZipObjectJsonWriter::new( | ||
zip::CompressionMethod::Stored, | ||
Some(zip::CompressionMethod::Stored), | ||
0, | ||
0, | ||
&StatArgs::default(), | ||
); | ||
|
||
assert_eq!("0.00%", zip_object_empty.compression_rate); | ||
assert_eq!("0.00%", zip_object_empty.compression_rate.unwrap_or_default()); | ||
|
||
let zip_object_grew = ZipObjectJsonWriter::new( | ||
zip::CompressionMethod::Deflated, | ||
Some(zip::CompressionMethod::Deflated), | ||
100, | ||
150, | ||
&StatArgs::default(), | ||
); | ||
|
||
assert_eq!("-50.00%", zip_object_grew.compression_rate); | ||
assert_eq!("-50.00%", zip_object_grew.compression_rate.unwrap_or_default()); | ||
} | ||
|
||
#[test] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,10 +15,12 @@ mod flat_writer; | |
mod json_writer; | ||
|
||
use zip_info::WriteZipInfo; | ||
use zip_info::Args; | ||
use zip_info::StatArgs; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can move all of these on one line using the shortcut described here. |
||
|
||
/// The Docopt usage string | ||
const USAGE: &'static str = " | ||
Usage: zi [-j | -p] [--exclude=<glob>] <path> ... | ||
Usage: zi [-j | -p] [--exclude=<glob>] [options] <path> ... | ||
zi --help | ||
|
||
zi presents information about Zip archives. | ||
|
@@ -29,21 +31,20 @@ Common options: | |
-p, --pretty-json Structure the output in easy-to-read JSON | ||
--exclude=<glob> Ignore objects in the archives whose name | ||
is like this glob pattern. | ||
More options: | ||
--compression-type Show the compression type of each file. | ||
--original-size Show the original size of each file. | ||
--compressed-size Show the compressed size of each file. | ||
--compression-rate Show the compression rate of each file. | ||
"; | ||
|
||
#[derive(Debug, RustcDecodable)] | ||
struct Args { | ||
arg_path: Vec<String>, | ||
flag_json: bool, | ||
flag_pretty_json: bool, | ||
flag_exclude: String, | ||
} | ||
|
||
fn main() { | ||
let args: Args = Docopt::new(USAGE) | ||
.and_then(|d| d.decode()) | ||
.unwrap_or_else(|e| e.exit()); | ||
|
||
let stat_args = StatArgs::new(&args); | ||
|
||
// XXX We divide up the display functionality here since | ||
// the original flat writer works differently than the JSON | ||
// writer. Ultimately, the JSON writer should be a generic | ||
|
@@ -54,7 +55,7 @@ fn main() { | |
args.arg_path.as_slice() | ||
); | ||
|
||
println!("{}", multiwriter.write_zip_info(args.flag_exclude.as_str())); | ||
println!("{}", multiwriter.write_zip_info(args.flag_exclude.as_str(), &stat_args)); | ||
} else { | ||
// Convert String to &str for json printing since | ||
// Docopt appears not to be able to handle Vec<&str>: | ||
|
@@ -69,6 +70,6 @@ fn main() { | |
_ => json_writer::zips_to_json_pretty, | ||
}; | ||
|
||
println!("{}", s(path_names.as_slice(), args.flag_exclude.as_str())); | ||
println!("{}", s(path_names.as_slice(), args.flag_exclude.as_str(), &stat_args)); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,59 @@ | ||
pub trait WriteZipInfo { | ||
fn write_zip_info(&mut self, exclude: &str) -> String; | ||
fn write_zip_info(&mut self, exclude: &str, stat_args: &StatArgs) -> String; | ||
} | ||
|
||
// Struct for our command-line arguments | ||
#[derive(Debug, RustcDecodable)] | ||
pub struct Args { | ||
pub arg_path: Vec<String>, | ||
pub flag_json: bool, | ||
pub flag_pretty_json: bool, | ||
pub flag_exclude: String, | ||
pub flag_compression_type: bool, | ||
pub flag_original_size: bool, | ||
pub flag_compressed_size: bool, | ||
pub flag_compression_rate: bool, | ||
} | ||
|
||
// We will need to pass this down in the main thread, to the individual printers, | ||
// and there is no point in passing down the whole Args struct, in the spirit of encapsulation. | ||
#[derive(Debug)] | ||
pub struct StatArgs { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This feels a little un-DRY. For the sake of encapsulation, it definitely makes sense to pass the stat args around as their own struct, but I'm wondering whether there's a way we can eliminate the fields from (That, or we can just rename |
||
pub flag_compression_type: bool, | ||
pub flag_original_size: bool, | ||
pub flag_compressed_size: bool, | ||
pub flag_compression_rate: bool, | ||
} | ||
|
||
impl StatArgs { | ||
pub fn new(args: &Args) -> StatArgs { | ||
let mut stat_args = StatArgs { | ||
flag_compression_type: args.flag_compression_type, | ||
flag_original_size: args.flag_original_size, | ||
flag_compressed_size: args.flag_compressed_size, | ||
flag_compression_rate: args.flag_compression_rate, | ||
}; | ||
|
||
// If none of these are explicitly specified, we print all of them. | ||
if stat_args.is_all_false() { | ||
stat_args = StatArgs::default(); | ||
} | ||
|
||
stat_args | ||
} | ||
|
||
pub fn default() -> StatArgs { | ||
StatArgs { | ||
flag_compression_type: true, | ||
flag_original_size: true, | ||
flag_compressed_size: true, | ||
flag_compression_rate: true, | ||
} | ||
} | ||
|
||
fn is_all_false(&self) -> bool { | ||
!(self.flag_compression_type || self.flag_original_size || self.flag_compressed_size || self.flag_compression_rate) | ||
} | ||
} | ||
|
||
#[cfg(test)] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same shortcut as in main.rs