From e02c6a25d7b2266244f472e7e9714809f0d375b8 Mon Sep 17 00:00:00 2001 From: Ivan Kuchin Date: Tue, 5 Aug 2014 14:25:18 +0200 Subject: [PATCH 1/9] add used_bins method to worker for preresolving --- lib/image_optim/worker.rb | 5 +++++ lib/image_optim/worker/jhead.rb | 4 ++++ lib/image_optim/worker/jpegtran.rb | 4 ++++ 3 files changed, 13 insertions(+) diff --git a/lib/image_optim/worker.rb b/lib/image_optim/worker.rb index 10eddb7e..f371e593 100644 --- a/lib/image_optim/worker.rb +++ b/lib/image_optim/worker.rb @@ -80,6 +80,11 @@ def run_order 0 end + # List of bins used by worker + def used_bins + [self.class.bin_sym] + end + # Check if operation resulted in optimized file def optimized?(src, dst) dst.size? && dst.size < src.size diff --git a/lib/image_optim/worker/jhead.rb b/lib/image_optim/worker/jhead.rb index 5c4b394f..61c242e3 100644 --- a/lib/image_optim/worker/jhead.rb +++ b/lib/image_optim/worker/jhead.rb @@ -17,6 +17,10 @@ def run_order -10 end + def used_bins + [:jhead, :jpegtran] + end + def optimize(src, dst) if (2..8).include?(EXIFR::JPEG.new(src.to_s).orientation.to_i) src.copy(dst) diff --git a/lib/image_optim/worker/jpegtran.rb b/lib/image_optim/worker/jpegtran.rb index 8cfbd5f3..32bbce3d 100644 --- a/lib/image_optim/worker/jpegtran.rb +++ b/lib/image_optim/worker/jpegtran.rb @@ -17,6 +17,10 @@ class Jpegtran < Worker option(:jpegrescan, false, 'Use jpegtran through jpegrescan, '\ 'ignore progressive option'){ |v| !!v } + def used_bins + jpegrescan ? [:jpegtran, :jpegrescan] : [:jpegtran] + end + def optimize(src, dst) if jpegrescan args = %W[#{src} #{dst}] From c371dd4ecaf628d815d0a67b6fbccd0677b8c3cd Mon Sep 17 00:00:00 2001 From: Ivan Kuchin Date: Fri, 29 Aug 2014 01:49:13 +0200 Subject: [PATCH 2/9] tell about disabling worker using an argument as well as using an option --- lib/image_optim/worker.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/image_optim/worker.rb b/lib/image_optim/worker.rb index f371e593..a52b6c9d 100644 --- a/lib/image_optim/worker.rb +++ b/lib/image_optim/worker.rb @@ -106,7 +106,8 @@ def resolve_bin!(bin) rescue BinResolver::Error => e name = self.class.bin_sym raise e, "#{name} worker: #{e.message}; please provide proper binary or "\ - "disable this worker (`:#{name} => false`)", e.backtrace + "disable this worker (--no-#{name} argument or "\ + "`:#{name} => false` through options)", e.backtrace end # Run command setting priority and hiding output From 06906bcd4dab7f4e698409a099bc696da16a1296 Mon Sep 17 00:00:00 2001 From: Ivan Kuchin Date: Wed, 3 Sep 2014 15:52:03 +0200 Subject: [PATCH 3/9] extract Worker#wrap_resolver_error_message --- lib/image_optim/worker.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/image_optim/worker.rb b/lib/image_optim/worker.rb index a52b6c9d..4223448c 100644 --- a/lib/image_optim/worker.rb +++ b/lib/image_optim/worker.rb @@ -104,10 +104,14 @@ def assert_no_unknown_options!(options) def resolve_bin!(bin) @image_optim.resolve_bin!(bin) rescue BinResolver::Error => e + raise e, wrap_resolver_error_message(e.message), e.backtrace + end + + def wrap_resolver_error_message(message) name = self.class.bin_sym - raise e, "#{name} worker: #{e.message}; please provide proper binary or "\ + "#{name} worker: #{message}; please provide proper binary or "\ "disable this worker (--no-#{name} argument or "\ - "`:#{name} => false` through options)", e.backtrace + "`:#{name} => false` through options)" end # Run command setting priority and hiding output From 61b129fa2cb42085228c9c64c4cfb069ae8e36ce Mon Sep 17 00:00:00 2001 From: Ivan Kuchin Date: Wed, 3 Sep 2014 15:53:46 +0200 Subject: [PATCH 4/9] add Worker#resolve_used_bins! --- lib/image_optim/worker.rb | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/lib/image_optim/worker.rb b/lib/image_optim/worker.rb index 4223448c..82fdb9b5 100644 --- a/lib/image_optim/worker.rb +++ b/lib/image_optim/worker.rb @@ -85,6 +85,20 @@ def used_bins [self.class.bin_sym] end + # Resolve used bins, raise exception mergin all messages + def resolve_used_bins! + errors = [] + used_bins.each do |bin| + begin + @image_optim.resolve_bin!(bin) + rescue BinResolver::Error => e + errors << e + end + end + return if errors.empty? + fail BinResolver::Error, wrap_resolver_error_message(errors.join(', ')) + end + # Check if operation resulted in optimized file def optimized?(src, dst) dst.size? && dst.size < src.size From 0d4895aff1afb555ff1f708994e114cda580b044 Mon Sep 17 00:00:00 2001 From: Ivan Kuchin Date: Wed, 3 Sep 2014 15:56:42 +0200 Subject: [PATCH 5/9] add Worker.create_all --- lib/image_optim.rb | 10 ++++------ lib/image_optim/worker.rb | 9 +++++++++ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/lib/image_optim.rb b/lib/image_optim.rb index 5dc1cdab..4675d896 100644 --- a/lib/image_optim.rb +++ b/lib/image_optim.rb @@ -167,17 +167,15 @@ def env_path # Create hash with format mapped to list of workers sorted by run order def create_workers_by_format(&options_proc) by_format = {} - Worker.klasses.each do |klass| - next unless (options = options_proc[klass]) - worker = klass.new(self, options) + workers = Worker.create_all(self, &options_proc) + sorted = workers.sort_by.with_index{ |worker, i| [worker.run_order, i] } + sorted.each do |worker| worker.image_formats.each do |format| by_format[format] ||= [] by_format[format] << worker end end - by_format.each do |_format, workers| - workers.sort_by!.with_index{ |worker, i| [worker.run_order, i] } - end + by_format end # Run method for each item in list diff --git a/lib/image_optim/worker.rb b/lib/image_optim/worker.rb index 82fdb9b5..fd5098ac 100644 --- a/lib/image_optim/worker.rb +++ b/lib/image_optim/worker.rb @@ -37,6 +37,15 @@ def option(name, default, type, description = nil, &proc) option_definitions << OptionDefinition.new(name, default, type, description, &proc) end + + # Initialize all workers using options from calling options_proc with + # klass + def create_all(image_optim, &options_proc) + Worker.klasses.map do |klass| + next unless (options = options_proc[klass]) + klass.new(image_optim, options) + end.compact + end end # Configure (raises on extra options) From 7719d40048f9d1603cca6341e67a647119d93797 Mon Sep 17 00:00:00 2001 From: Ivan Kuchin Date: Wed, 3 Sep 2014 16:13:50 +0200 Subject: [PATCH 6/9] extract BinResolver.collect_errors --- lib/image_optim/bin_resolver.rb | 13 +++++++++++++ lib/image_optim/worker.rb | 9 ++------- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/lib/image_optim/bin_resolver.rb b/lib/image_optim/bin_resolver.rb index 8adc1d0d..a3b25b58 100644 --- a/lib/image_optim/bin_resolver.rb +++ b/lib/image_optim/bin_resolver.rb @@ -58,6 +58,19 @@ def env_path [dir, ENV['PATH'], VENDOR_PATH].compact.join(':') end + # Collect resolving errors when running block over items of enumerable + def self.collect_errors(enumerable) + errors = [] + enumerable.each do |item| + begin + yield item + rescue Error => e + errors << e + end + end + errors + end + private def resolving(name) diff --git a/lib/image_optim/worker.rb b/lib/image_optim/worker.rb index fd5098ac..055d22ee 100644 --- a/lib/image_optim/worker.rb +++ b/lib/image_optim/worker.rb @@ -96,13 +96,8 @@ def used_bins # Resolve used bins, raise exception mergin all messages def resolve_used_bins! - errors = [] - used_bins.each do |bin| - begin - @image_optim.resolve_bin!(bin) - rescue BinResolver::Error => e - errors << e - end + errors = BinResolver.collect_errors(used_bins) do |bin| + @image_optim.resolve_bin!(bin) end return if errors.empty? fail BinResolver::Error, wrap_resolver_error_message(errors.join(', ')) From d23a94d48a588452ddf478518ee7ff739ed58c14 Mon Sep 17 00:00:00 2001 From: Ivan Kuchin Date: Wed, 3 Sep 2014 16:45:26 +0200 Subject: [PATCH 7/9] remove creation of Abc worker in specs --- spec/image_optim/config_spec.rb | 6 +++++- spec/image_optim/worker_spec.rb | 7 +++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/spec/image_optim/config_spec.rb b/spec/image_optim/config_spec.rb index f6249bde..19183bc2 100644 --- a/spec/image_optim/config_spec.rb +++ b/spec/image_optim/config_spec.rb @@ -60,7 +60,11 @@ end describe 'for_worker' do - Abc = Class.new(ImageOptim::Worker) do + Abc = Class.new do + def self.bin_sym + :abc + end + def image_formats [] end diff --git a/spec/image_optim/worker_spec.rb b/spec/image_optim/worker_spec.rb index 20cd6b87..13099a8f 100644 --- a/spec/image_optim/worker_spec.rb +++ b/spec/image_optim/worker_spec.rb @@ -6,13 +6,12 @@ Worker = ImageOptim::Worker describe 'optimize' do - it 'should raise NotImplementedError unless overriden' do - class Abc < ImageOptim::Worker; end - + it 'should raise NotImplementedError' do image_optim = ImageOptim.new + worker = Worker.new(image_optim, {}) expect do - Abc.new(image_optim, {}).optimize(double, double) + worker.optimize(double, double) end.to raise_error NotImplementedError end end From d10d52b7a67e4d944e42b61db08e09fd64d47f6d Mon Sep 17 00:00:00 2001 From: Ivan Kuchin Date: Thu, 4 Sep 2014 17:28:24 +0200 Subject: [PATCH 8/9] add Worker.resolve_all! --- lib/image_optim.rb | 1 + lib/image_optim/worker.rb | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/lib/image_optim.rb b/lib/image_optim.rb index 4675d896..26446980 100644 --- a/lib/image_optim.rb +++ b/lib/image_optim.rb @@ -168,6 +168,7 @@ def env_path def create_workers_by_format(&options_proc) by_format = {} workers = Worker.create_all(self, &options_proc) + Worker.resolve_all!(workers) sorted = workers.sort_by.with_index{ |worker, i| [worker.run_order, i] } sorted.each do |worker| worker.image_formats.each do |format| diff --git a/lib/image_optim/worker.rb b/lib/image_optim/worker.rb index 055d22ee..19e9702d 100644 --- a/lib/image_optim/worker.rb +++ b/lib/image_optim/worker.rb @@ -46,6 +46,15 @@ def create_all(image_optim, &options_proc) klass.new(image_optim, options) end.compact end + + # Resolve all bins of all workers failing with one joint exception + def resolve_all!(workers) + errors = BinResolver.collect_errors(workers) do |worker| + worker.resolve_used_bins! + end + return if errors.empty? + fail BinResolver::Error, ['Bin resolving errors:', *errors].join("\n") + end end # Configure (raises on extra options) From e31106a1467b4ce0ea37f7a19c4457b740b6daf0 Mon Sep 17 00:00:00 2001 From: Ivan Kuchin Date: Thu, 4 Sep 2014 17:28:48 +0200 Subject: [PATCH 9/9] increase Style/ClassLength to 150 --- .rubocop.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.rubocop.yml b/.rubocop.yml index ea27c335..206401e6 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -12,7 +12,7 @@ Style/CaseIndentation: IndentWhenRelativeTo: end Style/ClassLength: - Max: 120 + Max: 150 Style/CyclomaticComplexity: Max: 10