-
-
Notifications
You must be signed in to change notification settings - Fork 9.9k
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
include: fix 'no implicit conversion of nil to String' #5750
Conversation
This is when either 'dir' or 'file' is nil.
@@ -163,7 +163,7 @@ def load_cached_partial(path, context) | |||
end | |||
|
|||
def valid_include_file?(path, dir, safe) | |||
!(outside_site_source?(path, dir, safe) || !File.exist?(path)) | |||
!(outside_site_source?(path, dir, safe) || !File.file?(path)) |
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.
We don't want this to be a directory.
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.
why not !File.directory?(path)
?
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.
I believe checking to see if it's a file is easier to read - affirmations are easier for us to grok than negations.
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.
checking to see if it's a file
but its !File.file?(path)
instead of File.file?(path)
...
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.
yeah, If anything I'd write this as:
!outside_site_source?(path, dir, safe) && File.file?(path)
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.
Works for me! Updated to Ben's suggestion.
@jekyllbot: merge +bug |
Are we releasing a v3.3.2? Thought the |
This is when either 'dir' or 'file' is
nil
and passedFile.join
.Backtrace