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

rubocop: lib/jekyll/regenerator.rb #5025

Merged
merged 2 commits into from
Jun 23, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 0 additions & 1 deletion .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ AllCops:
- lib/jekyll/configuration.rb
- lib/jekyll/convertible.rb
- lib/jekyll/document.rb
- lib/jekyll/regenerator.rb
- lib/jekyll/renderer.rb
- lib/jekyll/utils.rb
- bin/**/*
Expand Down
75 changes: 46 additions & 29 deletions lib/jekyll/regenerator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,14 @@ def initialize(site)
def regenerate?(document)
case document
when Page
document.asset_file? || document.data['regenerate'] ||
source_modified_or_dest_missing?(
site.in_source_dir(document.relative_path), document.destination(@site.dest)
)
regenerate_page?(document)
when Document
!document.write? || document.data['regenerate'] ||
source_modified_or_dest_missing?(
document.path, document.destination(@site.dest)
)
regenerate_document?(document)
else
source_path = document.respond_to?(:path) ? document.path : nil
dest_path = document.respond_to?(:destination) ? document.destination(@site.dest) : nil
source_path = document.respond_to?(:path) ? document.path : nil
dest_path = if document.respond_to?(:destination)
document.destination(@site.dest)
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was wrong with the previous two lines? Just too long?

It seems like your solution for source_path and for dest_path are two ways of doing the same thing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, dest_path line was 98 chars long.
Is there another way to make it shorter?)

source_modified_or_dest_missing?(source_path, dest_path)
end
end
Expand All @@ -44,7 +40,7 @@ def add(path)

metadata[path] = {
"mtime" => File.mtime(path),
"deps" => []
"deps" => []
}
cache[path] = true
end
Expand Down Expand Up @@ -94,23 +90,7 @@ def modified?(path)
return cache[path]
end

# Check path that exists in metadata
data = metadata[path]
if data
data["deps"].each do |dependency|
if modified?(dependency)
return cache[dependency] = cache[path] = true
end
end
if File.exist?(path) && data["mtime"].eql?(File.mtime(path))
return cache[path] = false
else
return add(path)
end
end

# Path does not exist in metadata, add it
return add(path)
check_path_exists(path)
Copy link
Member

@parkr parkr Jun 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment on the old line 97 only referenced the old line 98. 99 thru 110 determined if the file was modified, so it has nothing to do with the comment. This is three operations:

  1. If we have seen this file before and one of its dependencies have been modified, set the regeneration bit for both the dependency and the file to true
  2. If we have seen this file before and it has not been modified, set the regeneration bit to false. If it has been modified, set it to true.
  3. If we have not seen this file before, add it to the metadata and regenerate it.

We should encapsulate it into these three operations.

end

# Add a dependency of a path
Expand Down Expand Up @@ -139,7 +119,7 @@ def write_metadata
#
# Returns the String path of the file.
def metadata_file
site.in_source_dir('.jekyll-metadata')
site.in_source_dir(".jekyll-metadata")
end

# Check if metadata has been disabled
Expand Down Expand Up @@ -173,5 +153,42 @@ def read_metadata
{}
end
end

private
def regenerate_page?(document)
document.asset_file? || document.data["regenerate"] ||
source_modified_or_dest_missing?(
site.in_source_dir(document.relative_path), document.destination(@site.dest)
)
end

private
def regenerate_document?(document)
!document.write? || document.data["regenerate"] ||
source_modified_or_dest_missing?(
document.path, document.destination(@site.dest)
)
end

# Private: Check path that exists in metadata
#
# Returns Boolean
private
def check_path_exists(path)
data = metadata[path]
if data
data["deps"].each do |dependency|
if modified?(dependency)
return cache[dependency] = cache[path] = true
end
end
if File.exist?(path) && data["mtime"].eql?(File.mtime(path))
return cache[path] = false
end
end

# Path does not exist in metadata, add it
add(path)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check_path_exists should not modify anything.

end
end
end