Propagate _data folder from remote theme#89
Propagate _data folder from remote theme#89mgerzabek wants to merge 23 commits intobenbalter:masterfrom
Conversation
All files from _data folder within remote theme are accessible in consumer project. The behaviour is exactly the same as for overrides of _includes and _layouts. Data files with the same name in consumer project override data files within remote theme.
|
Welcome! Congrats on your first pull request to Jekyll Remote Theme. If you haven't already, please be sure to check out the contributing guidelines. |
|
Thanks for this @mgerzabek. Can you provide a bit more detail as to the use case where a theme should be injecting data into the consumer site? |
|
Well, I have a catalogue of text modules for a layout file concerned with GDPR. And I want them to be in the remote theme to have changes to this catalogue be available for all projects that use my theme. Further so, I see that in putting a catalogue in a (remote) theme becoming a curated collection of possibly anything would give a better user experience/consumer experience of themes. |
|
My current remote theme has icons stored in _data that have to be manually copied to work. |
|
@duboisp we could finally use the _data folder to store theme settings and variables |
|
Since this is my first pull request and I couldn’t find a reviewer, may I ask @benbalter how the stage of this PR could be pushed forward. As I understand from the timeline I need a review from someone with write access to this repo. Unfortunately the Reviewers section is empty, even the owner of the repo is not listed, so I cannot ask for a review. Is this some kind of a chicken-egg-related problem? Any help/clearing words appreciated. |
Solange der Inhalt des _data Folders aus dem remote theme nicht gelesen werden kann, müssen die Datenschutztexte (_data/datenschutz.json) direkt im Template Repository gehostet werden. TODO Sobald mein PR akzeptiert wurde [1] - _data/datenschutz.json entfernen - .gitignore wieder den _data Folder als nicht gehostet einfügen [1] benbalter/jekyll-remote-theme#89
|
Another use case would be i18n files within the theme that can be accessed by the site using the theme. Just one of many, many cases. For myself, we use a remote theme for our foundation website that would store things like events which the foundation website and multiple sub-websites would then display. Our use-case is somewhat unusual: example case: https://owasp.org
|
|
I think this could be a really useful feature for all Jekyll themes. Would you consider submitting the change to the Jekyll project upstream? |
Mhm… okay, I‘m just a Jekyll end user and not a ruby pro. Beside that, of course, I’d do it, but I‘m not sure where to start. Any hints where and how to go about it? |
|
@benbalter What do we need to do to get this PR merged? A review? |
|
From what I understand, yes. A review from someone who can do this. Until yesterday the checks where green, then I klicked the merge button to synchronize the PR with the latest sources and now even the checks don‘t run with the message “Merging can be performed automatically with 1 approving review.“ |
|
For those interested in this feature and deploying their site using GitHub Actions, |
…und wird dann im lokalen Repository verlinkt. Achtung! Eine Verzeichnisstruktur wie bei mir am MacBook ist erforderlich. Also alle Repositories müssen innerhalb eines git Elternverzeichnisses angelegt sein.
|
With the success of PR 8815 for Jekyll this PR becomes obsolete. When v4.3 is released the desired behaviour should be within Jekyll standard distro. Closing this. |
|
@mgerzabek It would be good to validate that. It's my understanding that at least the Theme class in this repo may need updating to support the data loading from the theme. |
|
@parkr you’re right. Thanks for the hint! |
@mgerzabek You need not wait for an official release. # Gemfile
gem "jekyll", github: "jekyll/jekyll"
... |
|
@parkr + @ashmaroli rolled back to the minimal required changes to this PR to work with the new additions for Jekyll. Also sorry for the noise… I enhanced my local copy of this PR to circumvene the downloading of remote theme when it’s present at my local machine and didn’t realize this morning that I have to redo these changes. Everything should be fine now. |
parkr
left a comment
There was a problem hiding this comment.
I'd love to see a test added for this to ensure that a remote theme _data files are accurately read. 😄
@mgerzabek I don't see why you have to wait for a downstream merge.. # _config.yml
remote_theme: mgerzabek/primer@add_data_folder_for_remote_theme |
|
I this actually still needed with Jekyll 4.3.2? With me it already works since the remote theme download and extracts the whole theme repo in temp, and the theme source is set to |
Perhaps for cross-compatibility with older versions of Jekyll? This plugin is whitelisted for use with |
|
Hi there MG, ... I came across your PR because I've been facing a similar problem. In my case, I developed a theme that had a custom I suppose my want is a mix of this PR and somewhat the description covered in issue #96. I did look at your code though and tried to see if this was a good starting point - but I have some concerns.
Here's wishing BB takes am interest in this ... and creates an answer that I'm not capable of :) |
|
Hello @vinorodrigues Jekyll 4.3 has this feature built-in already. So, feel free to try this feature out with latest version of Jekyll and any version of the |
¿? Which one? Remote theme file inclusion, like Also ... we're speaking mostly of usage on gh-pages ... I thought that was still on 3.10.0. (Or so they say.)
|
There was a problem hiding this comment.
Copilot reviewed 4 out of 8 changed files in this pull request and generated 7 comments.
Files not reviewed (4)
- .idea/.gitignore: Language not supported
- .idea/jekyll-remote-theme.iml: Language not supported
- .idea/modules.xml: Language not supported
- .idea/vcs.xml: Language not supported
Comments suppressed due to low confidence (2)
README.md:53
- The word "catalogues" is unclear in this context. The sentence should read "data files" instead of "catalogues" for clarity. Additionally, there are trailing spaces at the end of this line that should be removed.
In addition to the default directories `assets`, `_includes`, `_layouts`, and `_sass` also catalogues in the remote theme's `_data` directory are considered.
lib/jekyll-remote-theme/theme.rb:68
- The new data_path method lacks test coverage. Given that the repository has comprehensive test coverage for other theme methods (as seen in spec/jekyll-remote-theme/theme_spec.rb), a test should be added to verify that data_path returns the correct path.
def data_path
@data_path ||= File.join(@root, "_data")
end
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <module type="RUBY_MODULE" version="4"> | ||
| <component name="ModuleRunConfigurationManager"> | ||
| <shared /> | ||
| </component> | ||
| <component name="NewModuleRootManager"> | ||
| <content url="file://$MODULE_DIR$"> | ||
| <excludeFolder url="file://$MODULE_DIR$/spec/fixtures/site/.jekyll-cache" /> | ||
| <excludeFolder url="file://$MODULE_DIR$/tmp" /> | ||
| </content> | ||
| <orderEntry type="jdk" jdkName="rbenv: 2.7.1" jdkType="RUBY_SDK" /> | ||
| <orderEntry type="sourceFolder" forTests="false" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="addressable (v2.7.0, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="ast (v2.4.2, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="bundler (v2.1.4, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="coderay (v1.1.3, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="colorator (v1.1.0, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="concurrent-ruby (v1.1.8, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="crack (v0.4.5, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="diff-lcs (v1.4.4, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="em-websocket (v0.5.2, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="eventmachine (v1.2.7, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="faraday (v1.3.0, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="faraday-net_http (v1.0.1, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="ffi (v1.15.0, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="forwardable-extended (v2.6.0, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="hashdiff (v1.0.1, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="http_parser.rb (v0.6.0, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="i18n (v1.8.9, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="jaro_winkler (v1.5.4, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="jekyll (v4.2.0, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="jekyll-github-metadata (v2.13.0@c0d0fa, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="jekyll-sass-converter (v2.1.0, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="jekyll-seo-tag (v2.7.1, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="jekyll-theme-primer (v0.5.4, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="jekyll-watch (v2.2.1, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="jekyll_test_plugin_malicious (v0.2.0, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="kramdown (v2.3.0, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="kramdown-parser-gfm (v1.1.0, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="liquid (v4.0.3, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="listen (v3.4.1, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="mercenary (v0.4.0, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="method_source (v1.0.0, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="multipart-post (v2.1.1, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="octokit (v4.20.0, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="parallel (v1.20.1, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="parser (v3.0.0.0, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="pathutil (v0.16.2, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="pry (v0.14.0, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="public_suffix (v4.0.6, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="rainbow (v3.0.0, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="rb-fsevent (v0.10.4, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="rb-inotify (v0.10.1, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="rexml (v3.2.4, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="rouge (v3.26.0, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="rspec (v3.10.0, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="rspec-core (v3.10.1, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="rspec-expectations (v3.10.1, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="rspec-mocks (v3.10.2, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="rspec-support (v3.10.2, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="rubocop (v0.80.1, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="rubocop-jekyll (v0.11.0, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="rubocop-performance (v1.6.1, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="ruby-progressbar (v1.11.0, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="ruby2_keywords (v0.0.4, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="rubyzip (v2.3.2, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="safe_yaml (v1.0.5, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="sassc (v2.4.0, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="sawyer (v0.8.2, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="terminal-table (v2.0.0, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="unicode-display_width (v1.6.1, rbenv: 2.7.1) [gem]" level="application" /> | ||
| <orderEntry type="library" scope="PROVIDED" name="webmock (v3.12.1, rbenv: 2.7.1) [gem]" level="application" /> | ||
| </component> | ||
| </module> No newline at end of file |
There was a problem hiding this comment.
IDE configuration files should not be committed to the repository. These are personal development environment settings that vary between developers and should be excluded via .gitignore. Please remove these files from the commit.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
All files from _data folder within remote theme are accessible in consumer projects. The behaviour is exactly the same as for overrides of _includes and _layouts. Data files with the same name in consumer project override data files within remote theme.
As supposed by @hblankenship and @desima in #68
View rendered README.md