SCSS converter: expand @config["source"] to be "safer". #55
Conversation
|
|
||
| private | ||
| def site_source | ||
| @site_source ||= File.expand_path(@config["source"]).freeze |
There was a problem hiding this comment.
If it's being passed to Jekyll.sanitized_path in 2/3 places is this necessary?
There was a problem hiding this comment.
Yeah. Jekyll.sanitized_path does not expand the base directory, so we have to do that here.
There was a problem hiding this comment.
Jekyll.sanitized_path does not expand the base directory, so we have to do that here.
Would the smarter fix be to do that there?
There was a problem hiding this comment.
On the one hand, that would mean you can pass any directory and it'll auto-expand, on the other File.expand_path calls are pretty expensive and we use this function a lot.
There was a problem hiding this comment.
It'd also lock us to a particular Jekyll version.
| end | ||
|
|
||
| # Expand file globs (e.g. `node_modules/*/node_modules` ) | ||
| Dir.chdir(@config["source"]) do | ||
| Dir.chdir(site_source) do |
There was a problem hiding this comment.
Does this need Jekyll.sanitized_path?
There was a problem hiding this comment.
I don't believe so – Jekyll.sanitized_path just ensures the first argument is a prefix for the second argument, so we wouldn't be saving anything there.
|
@jekyllbot: merge +bug |
* 'master' of https://github.com/jekyll/jekyll-sass-converter: Update history to reflect merge of #55 [ci skip] Ok fine, Jekyll 2. Clean up our test matrix. Get our scripts in order. SCSS converter: expand @config["source"] to be "safer".
Addresses a potential issue from #50.
/cc @jekyll/gh-pages