ysk24ok / jekyll-linkpreview

Jekyll plugin to generate link preview
https://rubygems.org/gems/jekyll-linkpreview
MIT License
30 stars 8 forks source link

Take "source" config into consideration #31

Closed Clpsplug closed 1 year ago

Clpsplug commented 2 years ago

The issue

Directories that this plugin would use are all expected to be present in the project root. However, Jekyll has a configuration to change the 'content root' of the site. It can change where Jekyll looks for the contents to build a site.

# This config value makes `$PROJECT_ROOT/_content` the 'content root.' Anything outside of it isn't included on the site.
# This is useful if you have so many files you don't want to include in the build that your `exclude` list is ridiculously long.
source: "_content"

My site makes use of this configuration value. However, because the plugin only looks at the project root, I have to create a weird directory structure like this:

$PROJECT_ROOT
|-_content
| |-_posts
| |-_includes
|   |-head.liquid
|-_cache
|-_includes
  |-linkpreview.html
  |-linkpreview_nog.html

Notice that I have to repeat _includes dir twice.
Since the outer _includes is outside of the 'content root,' Jekyll cannot auto-rebuild the site on a change to linkpreview.html when running the command jekyll serve.

Potential fix

In Jekyll, it is possible to pull the configuration value from within the Tag class. This is done in a context-aware manner; for example, if you use the --config option in the jekyll command, it will use the value in a specified file instead of _config.yml.

class MyTag < Jekyll::Tag
  def render(context)
    # prints the config value! We *must* use the `config` variable available here, though.
    p context.registers[:site].config['source']  
    "Output for this tag"
  end
end

I gave a quick read of the source code, and it looks that capturing this value in the method below and prepending it to @@template_dir should do the trick.

https://github.com/ysk24ok/jekyll-linkpreview/blob/181981c56c9daffa7da4082ae20a5f9dc64d1d48/lib/jekyll-linkpreview.rb#L147-L151

I'm not sure about the @@cache_dir, though. The required change is precisely the same, but the _cache dir looks fine to stay outside the content root for me. But again, that directory being aware of the "source" config also makes total sense.

I'd be happy to hear your opinion on this matter. Thanks for a great plugin! 😄

ysk24ok commented 1 year ago

Thank you for your detailed issue.

We must use the config variable available here, though.

What do you mean by this? Does it mean we have to use --config option when we want to use _context.registers[:site].config['source'] ? If there is a variable that holds the value of source, I think we can just add it to @@template_dir.

Clpsplug commented 1 year ago

We must use the config variable available here, though. What do you mean by this?

By this part, I meant that only the context parameter for the render method holds the object that we need. Whether it's from _config.yml or from the --config command line flag override, the source config value will appear as context.registers[:site].config['source'].

So yes, I think the fix is to prepend it to @@template_dir, too. I'm making a PR, and I'm about to test it on my end. I'll also try modifying RSpec tests but it may become a bit complicated because there are Jekyll's configuration values taken into consideration.