From ee9777e99b96597fc3403535c592ce818f909277 Mon Sep 17 00:00:00 2001 From: Ashwin Maroli Date: Mon, 4 May 2020 22:21:14 +0530 Subject: [PATCH 01/11] Initialize include-files as Jekyll objects --- .rubocop.yml | 2 +- lib/jekyll.rb | 2 + lib/jekyll/inclusion.rb | 50 +++++++++++++++++++++ lib/jekyll/reader.rb | 1 + lib/jekyll/readers/include_file_reader.rb | 54 +++++++++++++++++++++++ lib/jekyll/site.rb | 3 +- lib/jekyll/tags/include.rb | 47 +++++++++++++++++++- lib/jekyll/theme.rb | 5 +++ test/test_tags.rb | 32 +++++++------- 9 files changed, 177 insertions(+), 19 deletions(-) create mode 100644 lib/jekyll/inclusion.rb create mode 100644 lib/jekyll/readers/include_file_reader.rb diff --git a/.rubocop.yml b/.rubocop.yml index fb736c5e6c6..e01ea15bc4c 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -52,7 +52,7 @@ Lint/Void: Exclude: - lib/jekyll/site.rb Metrics/AbcSize: - Max: 21 + Max: 24 Metrics/BlockLength: Exclude: - test/**/*.rb diff --git a/lib/jekyll.rb b/lib/jekyll.rb index 7989dd2e756..0e2f92cdf78 100644 --- a/lib/jekyll.rb +++ b/lib/jekyll.rb @@ -54,9 +54,11 @@ module Jekyll autoload :FrontmatterDefaults, "jekyll/frontmatter_defaults" autoload :Hooks, "jekyll/hooks" autoload :Layout, "jekyll/layout" + autoload :Inclusion, "jekyll/inclusion" autoload :Cache, "jekyll/cache" autoload :CollectionReader, "jekyll/readers/collection_reader" autoload :DataReader, "jekyll/readers/data_reader" + autoload :IncludeFileReader, "jekyll/readers/include_file_reader" autoload :LayoutReader, "jekyll/readers/layout_reader" autoload :PostReader, "jekyll/readers/post_reader" autoload :PageReader, "jekyll/readers/page_reader" diff --git a/lib/jekyll/inclusion.rb b/lib/jekyll/inclusion.rb new file mode 100644 index 00000000000..7d537c8350e --- /dev/null +++ b/lib/jekyll/inclusion.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +module Jekyll + class Inclusion + attr_reader :name, :path, :relative_path + + def initialize(site, base, name) + @site = site + @name = name + @path = PathManager.join(base, name) + @relative_path = compute_relative_path + end + + def template + @template ||= site.liquid_renderer.file(relative_path).parse(content) + end + + def content + @content ||= File.read(path, **site.file_read_opts) + end + + def inspect + "#{self.class} #{@relative_path.inspect}" + end + alias_method :to_s, :inspect + + private + + attr_reader :site + + def dir_path + if site.theme && path.start_with?(site.theme.includes_path) + PathManager.join site.theme.basename, "_includes" + else + site.config["includes_dir"] + end + end + + def compute_relative_path + case path + when site.theme && %r!#{site.theme.includes_path}/(?.*)!o + PathManager.join dir_path, Regexp.last_match[:rel_path] + when %r!#{site.in_source_dir(site.config["includes_dir"], "/")}(?.*)!o + PathManager.join dir_path, Regexp.last_match[:rel_path] + else + path.sub(site.in_source_dir("/"), "") + end + end + end +end diff --git a/lib/jekyll/reader.rb b/lib/jekyll/reader.rb index 381d9d3165f..effc849f760 100644 --- a/lib/jekyll/reader.rb +++ b/lib/jekyll/reader.rb @@ -18,6 +18,7 @@ def read sort_files! @site.data = DataReader.new(site).read(site.config["data_dir"]) CollectionReader.new(site).read + IncludeFileReader.new(site).read ThemeAssetsReader.new(site).read end diff --git a/lib/jekyll/readers/include_file_reader.rb b/lib/jekyll/readers/include_file_reader.rb new file mode 100644 index 00000000000..20a6384abb9 --- /dev/null +++ b/lib/jekyll/readers/include_file_reader.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +module Jekyll + class IncludeFileReader + attr_reader :site + def initialize(site) + @site = site + @inclusions = {} + end + + def read + site.includes_load_paths.each do |load_path| + entries_in(load_path)&.each do |name| + @inclusions[name] ||= initialize_validated(load_path, name) + end + end + @inclusions.each_value { |item| item && site.inclusions << item } + end + + private + + def initialize_validated(load_path, name) + path = PathManager.join(load_path, name) + return unless valid_include_path?(load_path, path) + + Inclusion.new(site, load_path, name) + end + + def valid_include_path?(directory, path) + !outside_scope(directory, path) && !File.directory?(path) + end + + def outside_scope(directory, path) + site.safe && !realpath_prefixed_with?(directory, path) + end + + def realpath_prefixed_with?(dir, path) + File.exist?(path) && File.realpath(path).start_with?(dir) + rescue StandardError + false + end + + def entries_in(directory) + entries = nil + return entries unless File.exist?(directory) + + Dir.chdir(directory) do + entries = EntryFilter.new(site).filter(Dir["**/*"]) + end + + entries + end + end +end diff --git a/lib/jekyll/site.rb b/lib/jekyll/site.rb index 5195a377e1e..785fc0c81aa 100644 --- a/lib/jekyll/site.rb +++ b/lib/jekyll/site.rb @@ -3,7 +3,7 @@ module Jekyll class Site attr_reader :source, :dest, :cache_dir, :config - attr_accessor :layouts, :pages, :static_files, :drafts, + attr_accessor :layouts, :pages, :static_files, :drafts, :inclusions, :exclude, :include, :lsi, :highlighter, :permalink_style, :time, :future, :unpublished, :safe, :plugins, :limit_posts, :show_drafts, :keep_files, :baseurl, :data, :file_read_opts, @@ -96,6 +96,7 @@ def reset Time.now end self.layouts = {} + self.inclusions = [] self.pages = [] self.static_files = [] self.data = {} diff --git a/lib/jekyll/tags/include.rb b/lib/jekyll/tags/include.rb index 49ffcb9efbf..b979110cf33 100644 --- a/lib/jekyll/tags/include.rb +++ b/lib/jekyll/tags/include.rb @@ -192,6 +192,51 @@ def could_not_locate_message(file, includes_dirs, safe) end end + # Do not inherit from this class. + # TODO: Merge into the `Jekyll::Tags::IncludeTag` in v5.0 + class OptimizedIncludeTag < IncludeTag + def render(context) + @site ||= context.registers[:site] + + file = render_variable(context) || @file + validate_file_name(file) + inclusion = locate_include(file) + return unless inclusion + + add_include_to_dependency(@site, inclusion, context) + partial = inclusion.template + + context.stack do + context["include"] = parse_params(context) if @params + begin + partial.render!(context) + rescue Liquid::Error => e + e.template_name = path + e.markup_context = "included " if e.markup_context.nil? + raise e + end + end + end + + private + + def locate_include(file) + inclusion = @site.inclusions.find { |item| item.name == file } + return inclusion if inclusion + + raise IOError, could_not_locate_message(file, @site.includes_load_paths, @site.safe) + end + + def add_include_to_dependency(site, inclusion, context) + return unless context.registers[:page]&.key?("path") + + site.regenerator.add_dependency( + site.in_source_dir(context.registers[:page]["path"]), + inclusion.path + ) + end + end + class IncludeRelativeTag < IncludeTag def tag_includes_dirs(context) Array(page_path(context)).freeze @@ -217,5 +262,5 @@ def page_path(context) end end -Liquid::Template.register_tag("include", Jekyll::Tags::IncludeTag) +Liquid::Template.register_tag("include", Jekyll::Tags::OptimizedIncludeTag) Liquid::Template.register_tag("include_relative", Jekyll::Tags::IncludeRelativeTag) diff --git a/lib/jekyll/theme.rb b/lib/jekyll/theme.rb index 5ef4d044504..3e5172f7ab0 100644 --- a/lib/jekyll/theme.rb +++ b/lib/jekyll/theme.rb @@ -21,6 +21,11 @@ def root "or includes a symbolic link loop" end + # The name of theme directory + def basename + @basename ||= File.basename(root) + end + def includes_path @includes_path ||= path_for "_includes" end diff --git a/test/test_tags.rb b/test/test_tags.rb index d0eae0d030f..2b973c6e1dc 100644 --- a/test/test_tags.rb +++ b/test/test_tags.rb @@ -709,7 +709,7 @@ def highlight_block_with_opts(options_string) "permalink" => "pretty", "source" => source_dir, "destination" => dest_dir, - "read_posts" => true, + "read_all" => true, "safe" => true) end @result ||= "" @@ -730,7 +730,7 @@ def highlight_block_with_opts(options_string) "permalink" => "pretty", "source" => source_dir, "destination" => dest_dir, - "read_posts" => true, + "read_all" => true, "safe" => true) end assert_match( @@ -758,7 +758,7 @@ def highlight_block_with_opts(options_string) "permalink" => "pretty", "source" => source_dir, "destination" => dest_dir, - "read_posts" => true) + "read_all" => true) end should "correctly output include variable" do @@ -786,7 +786,7 @@ def highlight_block_with_opts(options_string) "permalink" => "pretty", "source" => source_dir, "destination" => dest_dir, - "read_posts" => true) + "read_all" => true) end should "correctly output include variable" do @@ -814,7 +814,7 @@ def highlight_block_with_opts(options_string) "permalink" => "pretty", "source" => source_dir, "destination" => dest_dir, - "read_posts" => true) + "read_all" => true) end should "correctly output include variable" do @@ -841,7 +841,7 @@ def highlight_block_with_opts(options_string) "permalink" => "pretty", "source" => source_dir, "destination" => dest_dir, - "read_posts" => true) + "read_all" => true) end content = <<~CONTENT @@ -857,7 +857,7 @@ def highlight_block_with_opts(options_string) "permalink" => "pretty", "source" => source_dir, "destination" => dest_dir, - "read_posts" => true) + "read_all" => true) end end end @@ -875,7 +875,7 @@ def highlight_block_with_opts(options_string) "permalink" => "pretty", "source" => source_dir, "destination" => dest_dir, - "read_posts" => true) + "read_all" => true) end should "list all parameters" do @@ -901,7 +901,7 @@ def highlight_block_with_opts(options_string) "permalink" => "pretty", "source" => source_dir, "destination" => dest_dir, - "read_posts" => true) + "read_all" => true) end should "include file with empty parameters" do @@ -923,7 +923,7 @@ def highlight_block_with_opts(options_string) "permalink" => "pretty", "source" => source_dir, "destination" => dest_dir, - "read_posts" => true) + "read_all" => true) end should "include file from custom directory" do @@ -944,7 +944,7 @@ def highlight_block_with_opts(options_string) "permalink" => "pretty", "source" => source_dir, "destination" => dest_dir, - "read_posts" => true) + "read_all" => true) end should "include file with empty parameters within if statement" do @@ -969,7 +969,7 @@ def highlight_block_with_opts(options_string) "permalink" => "pretty", "source" => source_dir, "destination" => dest_dir, - "read_posts" => true) + "read_all" => true) end assert_match( "Could not locate the included file 'missing.html' in any of " \ @@ -1063,7 +1063,7 @@ def highlight_block_with_opts(options_string) "permalink" => "pretty", "source" => source_dir, "destination" => dest_dir, - "read_posts" => true) + "read_all" => true) end assert_match "Could not locate the included file 'missing.html' in any of " \ "[\"#{source_dir}\"].", exception.message @@ -1087,7 +1087,7 @@ def highlight_block_with_opts(options_string) "permalink" => "pretty", "source" => source_dir, "destination" => dest_dir, - "read_posts" => true) + "read_all" => true) end assert_equal( "Invalid syntax for include tag. File contains invalid characters or " \ @@ -1115,7 +1115,7 @@ def highlight_block_with_opts(options_string) "permalink" => "pretty", "source" => source_dir, "destination" => dest_dir, - "read_posts" => true, + "read_all" => true, "safe" => true) end @result ||= "" @@ -1136,7 +1136,7 @@ def highlight_block_with_opts(options_string) "permalink" => "pretty", "source" => source_dir, "destination" => dest_dir, - "read_posts" => true, + "read_all" => true, "safe" => true) end assert_match( From 797da2e5985c6e133cf004daef6e3265d0e75971 Mon Sep 17 00:00:00 2001 From: Ashwin Maroli Date: Mon, 4 May 2020 23:31:06 +0530 Subject: [PATCH 02/11] Fix failing Cucumber tests --- lib/jekyll/inclusion.rb | 9 ++++++++- lib/jekyll/tags/include.rb | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/jekyll/inclusion.rb b/lib/jekyll/inclusion.rb index 7d537c8350e..e89adaaf520 100644 --- a/lib/jekyll/inclusion.rb +++ b/lib/jekyll/inclusion.rb @@ -12,7 +12,14 @@ def initialize(site, base, name) end def template - @template ||= site.liquid_renderer.file(relative_path).parse(content) + @template ||= \ + begin + site.liquid_renderer.file(relative_path).parse(content) + rescue Liquid::Error => e + e.template_name = path + e.markup_context = "included " if e.markup_context.nil? + raise e + end end def content diff --git a/lib/jekyll/tags/include.rb b/lib/jekyll/tags/include.rb index b979110cf33..de1ae22a50a 100644 --- a/lib/jekyll/tags/include.rb +++ b/lib/jekyll/tags/include.rb @@ -211,7 +211,7 @@ def render(context) begin partial.render!(context) rescue Liquid::Error => e - e.template_name = path + e.template_name = inclusion.path e.markup_context = "included " if e.markup_context.nil? raise e end From 9f3b5714baf35ef5d588541b869ba06e0c2ebd20 Mon Sep 17 00:00:00 2001 From: Ashwin Maroli Date: Mon, 4 May 2020 23:31:27 +0530 Subject: [PATCH 03/11] Appease RuboCop --- .rubocop.yml | 2 +- lib/jekyll/inclusion.rb | 2 ++ lib/jekyll/reader.rb | 3 +++ lib/jekyll/site.rb | 2 ++ 4 files changed, 8 insertions(+), 1 deletion(-) diff --git a/.rubocop.yml b/.rubocop.yml index e01ea15bc4c..fb736c5e6c6 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -52,7 +52,7 @@ Lint/Void: Exclude: - lib/jekyll/site.rb Metrics/AbcSize: - Max: 24 + Max: 21 Metrics/BlockLength: Exclude: - test/**/*.rb diff --git a/lib/jekyll/inclusion.rb b/lib/jekyll/inclusion.rb index e89adaaf520..434e395f6f2 100644 --- a/lib/jekyll/inclusion.rb +++ b/lib/jekyll/inclusion.rb @@ -43,6 +43,7 @@ def dir_path end end + # rubocop:disable Metrics/AbcSize def compute_relative_path case path when site.theme && %r!#{site.theme.includes_path}/(?.*)!o @@ -53,5 +54,6 @@ def compute_relative_path path.sub(site.in_source_dir("/"), "") end end + # rubocop:enable Metrics/AbcSize end end diff --git a/lib/jekyll/reader.rb b/lib/jekyll/reader.rb index effc849f760..641b712af3e 100644 --- a/lib/jekyll/reader.rb +++ b/lib/jekyll/reader.rb @@ -8,6 +8,8 @@ def initialize(site) @site = site end + # rubocop:disable Metrics/AbcSize + # # Read Site data from disk and load it into internal data structures. # # Returns nothing. @@ -21,6 +23,7 @@ def read IncludeFileReader.new(site).read ThemeAssetsReader.new(site).read end + # rubocop:enable Metrics/AbcSize # Sorts posts, pages, and static files. def sort_files! diff --git a/lib/jekyll/site.rb b/lib/jekyll/site.rb index 785fc0c81aa..8d71a3ea570 100644 --- a/lib/jekyll/site.rb +++ b/lib/jekyll/site.rb @@ -84,6 +84,7 @@ def print_stats Jekyll.logger.info @liquid_renderer.stats_table end + # rubocop:disable Metrics/AbcSize # rubocop:disable Metrics/MethodLength # # Reset Site details. @@ -116,6 +117,7 @@ def reset Jekyll::Hooks.trigger :site, :after_reset, self end # rubocop:enable Metrics/MethodLength + # rubocop:enable Metrics/AbcSize # Load necessary libraries, plugins, converters, and generators. # From c9640f21f102b919c916f7b6a833f24edbe6cd2f Mon Sep 17 00:00:00 2001 From: Ashwin Maroli Date: Wed, 6 May 2020 11:52:57 +0530 Subject: [PATCH 04/11] EntryFilter wasn't involved in the super's render --- lib/jekyll/readers/include_file_reader.rb | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/jekyll/readers/include_file_reader.rb b/lib/jekyll/readers/include_file_reader.rb index 20a6384abb9..db6d9c78f21 100644 --- a/lib/jekyll/readers/include_file_reader.rb +++ b/lib/jekyll/readers/include_file_reader.rb @@ -44,10 +44,7 @@ def entries_in(directory) entries = nil return entries unless File.exist?(directory) - Dir.chdir(directory) do - entries = EntryFilter.new(site).filter(Dir["**/*"]) - end - + Dir.chdir(directory) { entries = Dir["**/*"] } entries end end From 7e62659574515ba0fff207e17af6f697ce2a0934 Mon Sep 17 00:00:00 2001 From: Ashwin Maroli Date: Wed, 6 May 2020 11:57:03 +0530 Subject: [PATCH 05/11] Combine duplicated code --- lib/jekyll/inclusion.rb | 16 +++++++--------- lib/jekyll/tags/include.rb | 8 +------- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/lib/jekyll/inclusion.rb b/lib/jekyll/inclusion.rb index 434e395f6f2..d834f9277e5 100644 --- a/lib/jekyll/inclusion.rb +++ b/lib/jekyll/inclusion.rb @@ -11,15 +11,13 @@ def initialize(site, base, name) @relative_path = compute_relative_path end - def template - @template ||= \ - begin - site.liquid_renderer.file(relative_path).parse(content) - rescue Liquid::Error => e - e.template_name = path - e.markup_context = "included " if e.markup_context.nil? - raise e - end + def render(context) + @template ||= site.liquid_renderer.file(relative_path).parse(content) + @template.render!(context) + rescue Liquid::Error => e + e.template_name = path + e.markup_context = "included " if e.markup_context.nil? + raise e end def content diff --git a/lib/jekyll/tags/include.rb b/lib/jekyll/tags/include.rb index de1ae22a50a..4347dbe57dc 100644 --- a/lib/jekyll/tags/include.rb +++ b/lib/jekyll/tags/include.rb @@ -208,13 +208,7 @@ def render(context) context.stack do context["include"] = parse_params(context) if @params - begin - partial.render!(context) - rescue Liquid::Error => e - e.template_name = inclusion.path - e.markup_context = "included " if e.markup_context.nil? - raise e - end + inclusion.render(context) end end From 0b93dbdf9138f795fbb02f4ca2ef7fb570316ab8 Mon Sep 17 00:00:00 2001 From: Ashwin Maroli Date: Wed, 6 May 2020 11:59:55 +0530 Subject: [PATCH 06/11] Refactor to reduce computations --- lib/jekyll/inclusion.rb | 9 ++++++--- lib/jekyll/readers/include_file_reader.rb | 5 ++--- lib/jekyll/site.rb | 2 +- lib/jekyll/tags/include.rb | 20 +++++++------------- 4 files changed, 16 insertions(+), 20 deletions(-) diff --git a/lib/jekyll/inclusion.rb b/lib/jekyll/inclusion.rb index d834f9277e5..67877571c16 100644 --- a/lib/jekyll/inclusion.rb +++ b/lib/jekyll/inclusion.rb @@ -2,13 +2,16 @@ module Jekyll class Inclusion - attr_reader :name, :path, :relative_path + attr_reader :name, :path def initialize(site, base, name) @site = site @name = name @path = PathManager.join(base, name) - @relative_path = compute_relative_path + end + + def relative_path + @relative_path ||= compute_relative_path end def render(context) @@ -25,7 +28,7 @@ def content end def inspect - "#{self.class} #{@relative_path.inspect}" + "#{self.class} #{relative_path.inspect}" end alias_method :to_s, :inspect diff --git a/lib/jekyll/readers/include_file_reader.rb b/lib/jekyll/readers/include_file_reader.rb index db6d9c78f21..98ff7b4e421 100644 --- a/lib/jekyll/readers/include_file_reader.rb +++ b/lib/jekyll/readers/include_file_reader.rb @@ -5,16 +5,15 @@ class IncludeFileReader attr_reader :site def initialize(site) @site = site - @inclusions = {} end def read site.includes_load_paths.each do |load_path| entries_in(load_path)&.each do |name| - @inclusions[name] ||= initialize_validated(load_path, name) + inclusion = initialize_validated(load_path, name) + inclusion && site.inclusions[name] ||= inclusion end end - @inclusions.each_value { |item| item && site.inclusions << item } end private diff --git a/lib/jekyll/site.rb b/lib/jekyll/site.rb index 8d71a3ea570..26495ad3095 100644 --- a/lib/jekyll/site.rb +++ b/lib/jekyll/site.rb @@ -97,7 +97,7 @@ def reset Time.now end self.layouts = {} - self.inclusions = [] + self.inclusions = {} self.pages = [] self.static_files = [] self.data = {} diff --git a/lib/jekyll/tags/include.rb b/lib/jekyll/tags/include.rb index 4347dbe57dc..9b75ae82103 100644 --- a/lib/jekyll/tags/include.rb +++ b/lib/jekyll/tags/include.rb @@ -200,11 +200,8 @@ def render(context) file = render_variable(context) || @file validate_file_name(file) - inclusion = locate_include(file) - return unless inclusion - - add_include_to_dependency(@site, inclusion, context) - partial = inclusion.template + inclusion = @site.inclusions[file] || raise(IOError, could_not_locate_message(file)) + add_include_to_dependency(inclusion, context) if @site.incremental? context.stack do context["include"] = parse_params(context) if @params @@ -214,18 +211,15 @@ def render(context) private - def locate_include(file) - inclusion = @site.inclusions.find { |item| item.name == file } - return inclusion if inclusion - - raise IOError, could_not_locate_message(file, @site.includes_load_paths, @site.safe) + def could_not_locate_message(file) + super(file, @site.includes_load_paths, @site.safe) end - def add_include_to_dependency(site, inclusion, context) + def add_include_to_dependency(inclusion, context) return unless context.registers[:page]&.key?("path") - site.regenerator.add_dependency( - site.in_source_dir(context.registers[:page]["path"]), + @site.regenerator.add_dependency( + @site.in_source_dir(context.registers[:page]["path"]), inclusion.path ) end From 8e1d624013e3f4355bbe06c33e9012a74f7bbb2c Mon Sep 17 00:00:00 2001 From: Ashwin Maroli Date: Wed, 6 May 2020 17:16:47 +0530 Subject: [PATCH 07/11] Reduce duplicate string count --- lib/jekyll/tags/include.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/jekyll/tags/include.rb b/lib/jekyll/tags/include.rb index 9b75ae82103..d381a10f7bb 100644 --- a/lib/jekyll/tags/include.rb +++ b/lib/jekyll/tags/include.rb @@ -18,12 +18,13 @@ class IncludeTag < Liquid::Tag def initialize(tag_name, markup, tokens) super - matched = markup.strip.match(VARIABLE_SYNTAX) + markup = markup.strip + matched = markup.match(VARIABLE_SYNTAX) if matched @file = matched["variable"].strip @params = matched["params"].strip else - @file, @params = markup.strip.split(%r!\s+!, 2) + @file, @params = markup.split(%r!\s+!, 2) end validate_params if @params @tag_name = tag_name From 1129e5b7e22d56b2daacdd2cd963f21161196920 Mon Sep 17 00:00:00 2001 From: Ashwin Maroli Date: Wed, 6 May 2020 23:20:21 +0530 Subject: [PATCH 08/11] Fold IncludeFileReader into OptimizedIncludeTag --- lib/jekyll.rb | 1 - lib/jekyll/reader.rb | 4 -- lib/jekyll/readers/include_file_reader.rb | 50 ----------------------- lib/jekyll/tags/include.rb | 27 ++++++++++-- test/test_tags.rb | 32 +++++++-------- 5 files changed, 40 insertions(+), 74 deletions(-) delete mode 100644 lib/jekyll/readers/include_file_reader.rb diff --git a/lib/jekyll.rb b/lib/jekyll.rb index 0e2f92cdf78..9add96121b9 100644 --- a/lib/jekyll.rb +++ b/lib/jekyll.rb @@ -58,7 +58,6 @@ module Jekyll autoload :Cache, "jekyll/cache" autoload :CollectionReader, "jekyll/readers/collection_reader" autoload :DataReader, "jekyll/readers/data_reader" - autoload :IncludeFileReader, "jekyll/readers/include_file_reader" autoload :LayoutReader, "jekyll/readers/layout_reader" autoload :PostReader, "jekyll/readers/post_reader" autoload :PageReader, "jekyll/readers/page_reader" diff --git a/lib/jekyll/reader.rb b/lib/jekyll/reader.rb index 641b712af3e..381d9d3165f 100644 --- a/lib/jekyll/reader.rb +++ b/lib/jekyll/reader.rb @@ -8,8 +8,6 @@ def initialize(site) @site = site end - # rubocop:disable Metrics/AbcSize - # # Read Site data from disk and load it into internal data structures. # # Returns nothing. @@ -20,10 +18,8 @@ def read sort_files! @site.data = DataReader.new(site).read(site.config["data_dir"]) CollectionReader.new(site).read - IncludeFileReader.new(site).read ThemeAssetsReader.new(site).read end - # rubocop:enable Metrics/AbcSize # Sorts posts, pages, and static files. def sort_files! diff --git a/lib/jekyll/readers/include_file_reader.rb b/lib/jekyll/readers/include_file_reader.rb deleted file mode 100644 index 98ff7b4e421..00000000000 --- a/lib/jekyll/readers/include_file_reader.rb +++ /dev/null @@ -1,50 +0,0 @@ -# frozen_string_literal: true - -module Jekyll - class IncludeFileReader - attr_reader :site - def initialize(site) - @site = site - end - - def read - site.includes_load_paths.each do |load_path| - entries_in(load_path)&.each do |name| - inclusion = initialize_validated(load_path, name) - inclusion && site.inclusions[name] ||= inclusion - end - end - end - - private - - def initialize_validated(load_path, name) - path = PathManager.join(load_path, name) - return unless valid_include_path?(load_path, path) - - Inclusion.new(site, load_path, name) - end - - def valid_include_path?(directory, path) - !outside_scope(directory, path) && !File.directory?(path) - end - - def outside_scope(directory, path) - site.safe && !realpath_prefixed_with?(directory, path) - end - - def realpath_prefixed_with?(dir, path) - File.exist?(path) && File.realpath(path).start_with?(dir) - rescue StandardError - false - end - - def entries_in(directory) - entries = nil - return entries unless File.exist?(directory) - - Dir.chdir(directory) { entries = Dir["**/*"] } - entries - end - end -end diff --git a/lib/jekyll/tags/include.rb b/lib/jekyll/tags/include.rb index d381a10f7bb..8e612032d96 100644 --- a/lib/jekyll/tags/include.rb +++ b/lib/jekyll/tags/include.rb @@ -201,7 +201,10 @@ def render(context) file = render_variable(context) || @file validate_file_name(file) - inclusion = @site.inclusions[file] || raise(IOError, could_not_locate_message(file)) + + @site.inclusions[file] ||= locate_include_file(file) + inclusion = @site.inclusions[file] + add_include_to_dependency(inclusion, context) if @site.incremental? context.stack do @@ -212,8 +215,26 @@ def render(context) private - def could_not_locate_message(file) - super(file, @site.includes_load_paths, @site.safe) + def locate_include_file(file) + @site.includes_load_paths.each do |dir| + path = PathManager.join(dir, file) + return Inclusion.new(@site, dir, file) if valid_include_file?(path, dir) + end + raise IOError, could_not_locate_message(file, @site.includes_load_paths, @site.safe) + end + + def valid_include_file?(path, dir) + File.exist?(path) && !File.directory?(path) && !outside_scope?(path, dir) + end + + def outside_scope?(path, dir) + @site.safe && !realpath_prefixed_with?(path, dir) + end + + def realpath_prefixed_with?(path, dir) + File.realpath(path).start_with?(dir) + rescue StandardError + false end def add_include_to_dependency(inclusion, context) diff --git a/test/test_tags.rb b/test/test_tags.rb index 2b973c6e1dc..d0eae0d030f 100644 --- a/test/test_tags.rb +++ b/test/test_tags.rb @@ -709,7 +709,7 @@ def highlight_block_with_opts(options_string) "permalink" => "pretty", "source" => source_dir, "destination" => dest_dir, - "read_all" => true, + "read_posts" => true, "safe" => true) end @result ||= "" @@ -730,7 +730,7 @@ def highlight_block_with_opts(options_string) "permalink" => "pretty", "source" => source_dir, "destination" => dest_dir, - "read_all" => true, + "read_posts" => true, "safe" => true) end assert_match( @@ -758,7 +758,7 @@ def highlight_block_with_opts(options_string) "permalink" => "pretty", "source" => source_dir, "destination" => dest_dir, - "read_all" => true) + "read_posts" => true) end should "correctly output include variable" do @@ -786,7 +786,7 @@ def highlight_block_with_opts(options_string) "permalink" => "pretty", "source" => source_dir, "destination" => dest_dir, - "read_all" => true) + "read_posts" => true) end should "correctly output include variable" do @@ -814,7 +814,7 @@ def highlight_block_with_opts(options_string) "permalink" => "pretty", "source" => source_dir, "destination" => dest_dir, - "read_all" => true) + "read_posts" => true) end should "correctly output include variable" do @@ -841,7 +841,7 @@ def highlight_block_with_opts(options_string) "permalink" => "pretty", "source" => source_dir, "destination" => dest_dir, - "read_all" => true) + "read_posts" => true) end content = <<~CONTENT @@ -857,7 +857,7 @@ def highlight_block_with_opts(options_string) "permalink" => "pretty", "source" => source_dir, "destination" => dest_dir, - "read_all" => true) + "read_posts" => true) end end end @@ -875,7 +875,7 @@ def highlight_block_with_opts(options_string) "permalink" => "pretty", "source" => source_dir, "destination" => dest_dir, - "read_all" => true) + "read_posts" => true) end should "list all parameters" do @@ -901,7 +901,7 @@ def highlight_block_with_opts(options_string) "permalink" => "pretty", "source" => source_dir, "destination" => dest_dir, - "read_all" => true) + "read_posts" => true) end should "include file with empty parameters" do @@ -923,7 +923,7 @@ def highlight_block_with_opts(options_string) "permalink" => "pretty", "source" => source_dir, "destination" => dest_dir, - "read_all" => true) + "read_posts" => true) end should "include file from custom directory" do @@ -944,7 +944,7 @@ def highlight_block_with_opts(options_string) "permalink" => "pretty", "source" => source_dir, "destination" => dest_dir, - "read_all" => true) + "read_posts" => true) end should "include file with empty parameters within if statement" do @@ -969,7 +969,7 @@ def highlight_block_with_opts(options_string) "permalink" => "pretty", "source" => source_dir, "destination" => dest_dir, - "read_all" => true) + "read_posts" => true) end assert_match( "Could not locate the included file 'missing.html' in any of " \ @@ -1063,7 +1063,7 @@ def highlight_block_with_opts(options_string) "permalink" => "pretty", "source" => source_dir, "destination" => dest_dir, - "read_all" => true) + "read_posts" => true) end assert_match "Could not locate the included file 'missing.html' in any of " \ "[\"#{source_dir}\"].", exception.message @@ -1087,7 +1087,7 @@ def highlight_block_with_opts(options_string) "permalink" => "pretty", "source" => source_dir, "destination" => dest_dir, - "read_all" => true) + "read_posts" => true) end assert_equal( "Invalid syntax for include tag. File contains invalid characters or " \ @@ -1115,7 +1115,7 @@ def highlight_block_with_opts(options_string) "permalink" => "pretty", "source" => source_dir, "destination" => dest_dir, - "read_all" => true, + "read_posts" => true, "safe" => true) end @result ||= "" @@ -1136,7 +1136,7 @@ def highlight_block_with_opts(options_string) "permalink" => "pretty", "source" => source_dir, "destination" => dest_dir, - "read_all" => true, + "read_posts" => true, "safe" => true) end assert_match( From 5b81aa945351b99c5f034e1c3b5eaa7e775d41db Mon Sep 17 00:00:00 2001 From: Ashwin Maroli Date: Thu, 14 May 2020 15:12:19 +0530 Subject: [PATCH 09/11] Make `Inclusion#relative_path` a private method --- lib/jekyll/inclusion.rb | 31 +++++++++++-------------------- 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/lib/jekyll/inclusion.rb b/lib/jekyll/inclusion.rb index 67877571c16..1d813fd6981 100644 --- a/lib/jekyll/inclusion.rb +++ b/lib/jekyll/inclusion.rb @@ -10,10 +10,6 @@ def initialize(site, base, name) @path = PathManager.join(base, name) end - def relative_path - @relative_path ||= compute_relative_path - end - def render(context) @template ||= site.liquid_renderer.file(relative_path).parse(content) @template.render!(context) @@ -36,23 +32,18 @@ def inspect attr_reader :site - def dir_path - if site.theme && path.start_with?(site.theme.includes_path) - PathManager.join site.theme.basename, "_includes" - else - site.config["includes_dir"] - end - end - # rubocop:disable Metrics/AbcSize - def compute_relative_path - case path - when site.theme && %r!#{site.theme.includes_path}/(?.*)!o - PathManager.join dir_path, Regexp.last_match[:rel_path] - when %r!#{site.in_source_dir(site.config["includes_dir"], "/")}(?.*)!o - PathManager.join dir_path, Regexp.last_match[:rel_path] - else - path.sub(site.in_source_dir("/"), "") + def relative_path + @relative_path ||= begin + case path + when site.theme && %r!#{site.theme.includes_path}/(?.*)!o + dir_path = PathManager.join(site.theme.basename, "_includes") + PathManager.join(dir_path, Regexp.last_match[:rel_path]) + when %r!#{site.in_source_dir(site.config["includes_dir"], "/")}(?.*)!o + PathManager.join(site.config["includes_dir"], Regexp.last_match[:rel_path]) + else + path.sub(site.in_source_dir("/"), "") + end end end # rubocop:enable Metrics/AbcSize From ba36d074dc31cf9b142baff0e8235fae5a683828 Mon Sep 17 00:00:00 2001 From: Ashwin Maroli Date: Thu, 14 May 2020 15:39:07 +0530 Subject: [PATCH 10/11] Remove relative_path attribute from Inclusion objs --- lib/jekyll/inclusion.rb | 27 ++++----------------------- 1 file changed, 4 insertions(+), 23 deletions(-) diff --git a/lib/jekyll/inclusion.rb b/lib/jekyll/inclusion.rb index 1d813fd6981..179ddb045cb 100644 --- a/lib/jekyll/inclusion.rb +++ b/lib/jekyll/inclusion.rb @@ -2,7 +2,8 @@ module Jekyll class Inclusion - attr_reader :name, :path + attr_reader :site, :name, :path + private :site def initialize(site, base, name) @site = site @@ -11,7 +12,7 @@ def initialize(site, base, name) end def render(context) - @template ||= site.liquid_renderer.file(relative_path).parse(content) + @template ||= site.liquid_renderer.file(path).parse(content) @template.render!(context) rescue Liquid::Error => e e.template_name = path @@ -24,28 +25,8 @@ def content end def inspect - "#{self.class} #{relative_path.inspect}" + "#{self.class} #{path.inspect}" end alias_method :to_s, :inspect - - private - - attr_reader :site - - # rubocop:disable Metrics/AbcSize - def relative_path - @relative_path ||= begin - case path - when site.theme && %r!#{site.theme.includes_path}/(?.*)!o - dir_path = PathManager.join(site.theme.basename, "_includes") - PathManager.join(dir_path, Regexp.last_match[:rel_path]) - when %r!#{site.in_source_dir(site.config["includes_dir"], "/")}(?.*)!o - PathManager.join(site.config["includes_dir"], Regexp.last_match[:rel_path]) - else - path.sub(site.in_source_dir("/"), "") - end - end - end - # rubocop:enable Metrics/AbcSize end end From 78798b0c60e2912ea16712e8b067526d160697af Mon Sep 17 00:00:00 2001 From: Ashwin Maroli Date: Thu, 14 May 2020 15:42:03 +0530 Subject: [PATCH 11/11] File.file? checks if path exists and is a file --- lib/jekyll/tags/include.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/jekyll/tags/include.rb b/lib/jekyll/tags/include.rb index 8e612032d96..cc6b145102f 100644 --- a/lib/jekyll/tags/include.rb +++ b/lib/jekyll/tags/include.rb @@ -224,7 +224,7 @@ def locate_include_file(file) end def valid_include_file?(path, dir) - File.exist?(path) && !File.directory?(path) && !outside_scope?(path, dir) + File.file?(path) && !outside_scope?(path, dir) end def outside_scope?(path, dir)