From efeb87a0322a8dad98ad57beac816d403b96d6bc Mon Sep 17 00:00:00 2001 From: Joseph Wynn Date: Sun, 25 Sep 2016 20:23:06 +0100 Subject: [PATCH] Treat paths as relative to Jekyll `source` directory (#34) * Move 'fixtures' to 'test-site' because, like, that's what it is * Refactor to make reasoning about file paths easier * Write a failing test * Conventions, conventions * Always treat paths as relative to the Jekyll site source --- features/extra-image-generation.feature | 21 +++++++++- features/image-generation.feature | 18 ++++++++ features/responsive-image-tag.feature | 1 + features/step_definitions/jekyll_steps.rb | 10 +++++ features/support/env.rb | 3 -- features/support/hooks.rb | 4 +- .../_includes/base-url.html | 0 .../_includes/custom-template.html | 0 .../_includes/responsive-image.html | 0 ...ody-loves-jalapeño-pineapple-cornbread.png | Bin .../assets/subdir/test.png | Bin lib/jekyll/responsive_image/block.rb | 33 +++++---------- lib/jekyll/responsive_image/common.rb | 39 +++++++++++++++--- .../responsive_image/extra_image_generator.rb | 5 ++- .../responsive_image/image_processor.rb | 2 +- lib/jekyll/responsive_image/resize_handler.rb | 16 +++---- lib/jekyll/responsive_image/tag.rb | 23 +---------- 17 files changed, 108 insertions(+), 67 deletions(-) rename features/{fixtures => test-site}/_includes/base-url.html (100%) rename features/{fixtures => test-site}/_includes/custom-template.html (100%) rename features/{fixtures => test-site}/_includes/responsive-image.html (100%) rename features/{fixtures => test-site}/assets/everybody-loves-jalapeño-pineapple-cornbread.png (100%) rename features/{fixtures => test-site}/assets/subdir/test.png (100%) diff --git a/features/extra-image-generation.feature b/features/extra-image-generation.feature index e1a24b7..6cecce1 100644 --- a/features/extra-image-generation.feature +++ b/features/extra-image-generation.feature @@ -8,7 +8,6 @@ Feature: Extra image generation """ sizes: - width: 100 - extra_images: - assets/everybody-loves-jalapeño-pineapple-cornbread.png """ @@ -16,13 +15,13 @@ Feature: Extra image generation And I have a file "index.html" with "Hello, world!" When I run Jekyll Then the image "assets/resized/everybody-loves-jalapeño-pineapple-cornbread-100x50.png" should have the dimensions "100x50" + And the file "_site/assets/resized/everybody-loves-jalapeño-pineapple-cornbread-100x50.png" should exist Scenario: Using glob patterns Given I have a responsive_image configuration with: """ sizes: - width: 100 - extra_images: - assets/*.png """ @@ -30,6 +29,24 @@ Feature: Extra image generation And I have a file "index.html" with "Hello, world!" When I run Jekyll Then the image "assets/resized/everybody-loves-jalapeño-pineapple-cornbread-100x50.png" should have the dimensions "100x50" + And the file "_site/assets/resized/everybody-loves-jalapeño-pineapple-cornbread-100x50.png" should exist + + Scenario: Honouring Jekyll 'source' configuration + Given I have copied my site to "sub-dir/my-site-copy" + And I have a configuration with: + """ + source: sub-dir/my-site-copy + responsive_image: + sizes: + - width: 100 + extra_images: + - assets/*.png + """ + + And I have a file "index.html" with "Hello, world!" + When I run Jekyll + Then the image "sub-dir/my-site-copy/assets/resized/everybody-loves-jalapeño-pineapple-cornbread-100x50.png" should have the dimensions "100x50" + And the file "_site/assets/resized/everybody-loves-jalapeño-pineapple-cornbread-100x50.png" should exist Scenario: No extra images Given I have a responsive_image configuration with: diff --git a/features/image-generation.feature b/features/image-generation.feature index 8929cb8..4fbfb51 100644 --- a/features/image-generation.feature +++ b/features/image-generation.feature @@ -14,6 +14,7 @@ Feature: Responsive image generation And I have a file "index.html" with "{% responsive_image path: assets/everybody-loves-jalapeño-pineapple-cornbread.png alt: Foobar %}" When I run Jekyll Then the image "assets/resized/everybody-loves-jalapeño-pineapple-cornbread-100x50.png" should have the dimensions "100x50" + And the file "_site/assets/resized/everybody-loves-jalapeño-pineapple-cornbread-100x50.png" should exist Scenario: Handling subdirectories Given I have a responsive_image configuration with: @@ -33,4 +34,21 @@ Feature: Responsive image generation When I run Jekyll Then the file "assets/resized/everybody-loves-jalapeño-pineapple-cornbread-100.png" should exist + And the file "_site/assets/resized/everybody-loves-jalapeño-pineapple-cornbread-100.png" should exist And the file "assets/resized/subdir/test-100.png" should exist + And the file "_site/assets/resized/subdir/test-100.png" should exist + + Scenario: Honouring Jekyll 'source' configuration + Given I have copied my site to "sub-dir/my-site-copy" + And I have a configuration with: + """ + source: sub-dir/my-site-copy + responsive_image: + template: _includes/responsive-image.html + sizes: + - width: 100 + """ + And I have a file "sub-dir/my-site-copy/index.html" with "{% responsive_image path: assets/everybody-loves-jalapeño-pineapple-cornbread.png %}" + When I run Jekyll + Then the image "sub-dir/my-site-copy/assets/resized/everybody-loves-jalapeño-pineapple-cornbread-100x50.png" should have the dimensions "100x50" + And the file "_site/assets/resized/everybody-loves-jalapeño-pineapple-cornbread-100x50.png" should exist diff --git a/features/responsive-image-tag.feature b/features/responsive-image-tag.feature index cdee79b..ddad2f9 100644 --- a/features/responsive-image-tag.feature +++ b/features/responsive-image-tag.feature @@ -77,3 +77,4 @@ Feature: Jekyll responsive_image tag When I run Jekyll Then I should see "/assets/everybody-loves-jalapeño-pineapple-cornbread.png-resized/100/everybody-loves-jalapeño-pineapple-cornbread-50.png 100w" in "_site/index.html" And the file "assets/everybody-loves-jalapeño-pineapple-cornbread.png-resized/100/everybody-loves-jalapeño-pineapple-cornbread-50.png" should exist + And the file "_site/assets/everybody-loves-jalapeño-pineapple-cornbread.png-resized/100/everybody-loves-jalapeño-pineapple-cornbread-50.png" should exist diff --git a/features/step_definitions/jekyll_steps.rb b/features/step_definitions/jekyll_steps.rb index 07899c9..7284284 100644 --- a/features/step_definitions/jekyll_steps.rb +++ b/features/step_definitions/jekyll_steps.rb @@ -8,6 +8,16 @@ Then /^Jekyll should throw a "(.+)"$/ do |error_class| assert_raise(Object.const_get(error_class)) { run_jekyll } end +Given /^I have copied my site to "(.+)"$/ do |path| + new_site_dir = File.join(TEST_DIR, path) + + FileUtils.mkdir_p(new_site_dir) + + Dir.glob(File.join(TEST_DIR, '*')) + .reject { |f| File.basename(f) == File.dirname(path) } + .each { |f| FileUtils.mv(f, new_site_dir) } +end + Given /^I have a configuration with:$/ do |config| write_file('_config.yml', config) end diff --git a/features/support/env.rb b/features/support/env.rb index 0a6e910..eaed1ae 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -9,9 +9,6 @@ require 'jekyll/responsive_image' TEST_DIR = File.join('/', 'tmp', 'jekyll') def run_jekyll(options = {}) - options['source'] ||= TEST_DIR - options['destination'] ||= File.join(TEST_DIR, '_site') - options = Jekyll.configuration(options) site = Jekyll::Site.new(options) diff --git a/features/support/hooks.rb b/features/support/hooks.rb index ad0bb57..f5528f5 100644 --- a/features/support/hooks.rb +++ b/features/support/hooks.rb @@ -2,8 +2,8 @@ Before do FileUtils.rm_rf(TEST_DIR) if File.exist?(TEST_DIR) FileUtils.mkdir_p(TEST_DIR) - fixtures = File.expand_path('../../fixtures', __FILE__) - FileUtils.cp_r(Dir.glob("#{fixtures}/*"), TEST_DIR) + test_site = File.expand_path('../../test-site', __FILE__) + FileUtils.cp_r(Dir.glob("#{test_site}/*"), TEST_DIR) Dir.chdir(TEST_DIR) end diff --git a/features/fixtures/_includes/base-url.html b/features/test-site/_includes/base-url.html similarity index 100% rename from features/fixtures/_includes/base-url.html rename to features/test-site/_includes/base-url.html diff --git a/features/fixtures/_includes/custom-template.html b/features/test-site/_includes/custom-template.html similarity index 100% rename from features/fixtures/_includes/custom-template.html rename to features/test-site/_includes/custom-template.html diff --git a/features/fixtures/_includes/responsive-image.html b/features/test-site/_includes/responsive-image.html similarity index 100% rename from features/fixtures/_includes/responsive-image.html rename to features/test-site/_includes/responsive-image.html diff --git a/features/fixtures/assets/everybody-loves-jalapeño-pineapple-cornbread.png b/features/test-site/assets/everybody-loves-jalapeño-pineapple-cornbread.png similarity index 100% rename from features/fixtures/assets/everybody-loves-jalapeño-pineapple-cornbread.png rename to features/test-site/assets/everybody-loves-jalapeño-pineapple-cornbread.png diff --git a/features/fixtures/assets/subdir/test.png b/features/test-site/assets/subdir/test.png similarity index 100% rename from features/fixtures/assets/subdir/test.png rename to features/test-site/assets/subdir/test.png diff --git a/lib/jekyll/responsive_image/block.rb b/lib/jekyll/responsive_image/block.rb index 7c01bf9..935635f 100644 --- a/lib/jekyll/responsive_image/block.rb +++ b/lib/jekyll/responsive_image/block.rb @@ -1,3 +1,12 @@ + + + + + + + + + module Jekyll class ResponsiveImage class Block < Liquid::Block @@ -5,29 +14,7 @@ module Jekyll def render(context) attributes = YAML.load(super) - - cache_key = attributes.to_s - result = attributes['cache'] ? RenderCache.get(cache_key) : nil - - if result.nil? - site = context.registers[:site] - config = make_config(site) - - image_template = attributes['template'] || config['template'] - - image = ImageProcessor.process(attributes['path'], config) - attributes['original'] = image[:original] - attributes['resized'] = image[:resized] - - partial = File.read(image_template) - template = Liquid::Template.parse(partial) - - result = template.render!(attributes.merge(site.site_payload)) - - RenderCache.set(cache_key, result) - end - - result + render_responsive_image(context, attributes) end end end diff --git a/lib/jekyll/responsive_image/common.rb b/lib/jekyll/responsive_image/common.rb index 40676e5..51cb0b2 100644 --- a/lib/jekyll/responsive_image/common.rb +++ b/lib/jekyll/responsive_image/common.rb @@ -4,15 +4,42 @@ module Jekyll include Jekyll::ResponsiveImage::Utils def make_config(site) - config = ResponsiveImage.defaults.dup.merge(site.config['responsive_image']).merge(:site_dest => site.dest) + ResponsiveImage.defaults.dup + .merge(site.config['responsive_image']) + .merge(:site_source => site.source, :site_dest => site.dest) + end - # Not very nice, but this is needed to create a clean path to add to keep_files - output_dir = format_output_path(config['output_path_format'], config['base_path'], '*', '*', '*') - output_dir = "#{File.dirname(output_dir)}/*" + def keep_resized_image!(site, image) + keep_dir = File.dirname(image['path']) + site.config['keep_files'] << keep_dir unless site.config['keep_files'].include?(keep_dir) + end - site.config['keep_files'] << output_dir unless site.config['keep_files'].include?(output_dir) + def render_responsive_image(context, attributes) + cache_key = attributes.to_s + result = attributes['cache'] ? RenderCache.get(cache_key) : nil - config + if result.nil? + site = context.registers[:site] + config = make_config(site) + + source_image_path = site.in_source_dir(attributes['path'].to_s) + image = ImageProcessor.process(source_image_path, config) + attributes['original'] = image[:original] + attributes['resized'] = image[:resized] + + attributes['resized'].each { |resized| keep_resized_image!(site, resized) } + + image_template = site.in_source_dir(attributes['template'] || config['template']) + partial = File.read(image_template) + template = Liquid::Template.parse(partial) + + result = template.render!(attributes.merge(site.site_payload)) + + + RenderCache.set(cache_key, result) + end + + result end end end diff --git a/lib/jekyll/responsive_image/extra_image_generator.rb b/lib/jekyll/responsive_image/extra_image_generator.rb index dc72f34..566f9eb 100644 --- a/lib/jekyll/responsive_image/extra_image_generator.rb +++ b/lib/jekyll/responsive_image/extra_image_generator.rb @@ -7,7 +7,10 @@ module Jekyll config = make_config(site) config['extra_images'].each do |pathspec| - Dir.glob(pathspec) { |path| ImageProcessor.process(path, config) } + Dir.glob(site.in_source_dir(pathspec)) do |path| + result = ImageProcessor.process(path, config) + result[:resized].each { |image| keep_resized_image!(site, image) } + end end end end diff --git a/lib/jekyll/responsive_image/image_processor.rb b/lib/jekyll/responsive_image/image_processor.rb index cbae275..afc7689 100644 --- a/lib/jekyll/responsive_image/image_processor.rb +++ b/lib/jekyll/responsive_image/image_processor.rb @@ -4,7 +4,7 @@ module Jekyll include ResponsiveImage::Utils def process(image_path, config) - raise SyntaxError.new("Invalid image path specified: #{image_path}") unless File.exist?(image_path.to_s) + raise SyntaxError.new("Invalid image path specified: #{image_path}") unless File.file?(image_path) resize_handler = ResizeHandler.new img = Magick::Image::read(image_path).first diff --git a/lib/jekyll/responsive_image/resize_handler.rb b/lib/jekyll/responsive_image/resize_handler.rb index 4b32a1d..04b5e8d 100644 --- a/lib/jekyll/responsive_image/resize_handler.rb +++ b/lib/jekyll/responsive_image/resize_handler.rb @@ -17,22 +17,24 @@ module Jekyll filepath = format_output_path(config['output_path_format'], config['base_path'], image_path, width, height) resized.push(image_hash(config['base_path'], filepath, width, height)) - # Don't resize images more than once - next if File.exist?(filepath) + site_source_filepath = File.expand_path(filepath, config[:site_source]) + site_dest_filepath = File.expand_path(filepath, config[:site_dest]) - ensure_output_dir_exists!(File.dirname(filepath)) + # Don't resize images more than once + next if File.exist?(site_source_filepath) + + ensure_output_dir_exists!(File.dirname(site_source_filepath)) + ensure_output_dir_exists!(File.dirname(site_dest_filepath)) Jekyll.logger.info "Generating #{filepath}" i = img.scale(ratio) - i.write(filepath) do |f| + i.write(site_source_filepath) do |f| f.quality = size['quality'] || config['default_quality'] end # Ensure the generated file is copied to the _site directory - site_dest_filepath = File.expand_path(filepath, config[:site_dest]) - ensure_output_dir_exists!(File.dirname(site_dest_filepath)) - FileUtils.copy(filepath, site_dest_filepath) + FileUtils.copy_file(site_source_filepath, site_dest_filepath) i.destroy! end diff --git a/lib/jekyll/responsive_image/tag.rb b/lib/jekyll/responsive_image/tag.rb index f601aca..7994c2b 100644 --- a/lib/jekyll/responsive_image/tag.rb +++ b/lib/jekyll/responsive_image/tag.rb @@ -15,28 +15,7 @@ module Jekyll end def render(context) - cache_key = @attributes.to_s - result = @attributes['cache'] ? RenderCache.get(cache_key) : nil - - if result.nil? - site = context.registers[:site] - config = make_config(site) - - image = ImageProcessor.process(@attributes['path'], config) - @attributes['original'] = image[:original] - @attributes['resized'] = image[:resized] - - image_template = @attributes['template'] || config['template'] - - partial = File.read(image_template) - template = Liquid::Template.parse(partial) - - result = template.render!(@attributes.merge(site.site_payload)) - - RenderCache.set(cache_key, result) - end - - result + render_responsive_image(context, @attributes) end end end