Adds link Liquid tag to make generation of URL's easier
#4624
Conversation
This tag mirrors the post_tag functionality but for collections instead of just posts.
|
This is a possible solution to #2252. If the design looks good and the code is good, I can also add docs to describe how to use it. |
|
What's the benefit to linking to a collection? I'm not seeing one at the moment since collections refer to a group of things and a post is a single entity. |
|
Perhaps this is just misnamed. The tag links to a specific item in a collection, not to the collection itself. If you have a collection that generates pages (ie, |
|
Not commenting on the PR, necessarily, but the use-case you bring up, @jeffkole, can be accomplished in 3.1 with something like this: |
|
@budparr Let's say that I have another collection that is of books I have read and written summaries of, and that I like to link to those summary pages in posts that I write. Don't you think it would be nicer for the user to write than The first strikes me as a nice clean interface to get a specific job done. It can be documented and tested. The second is obviously more flexible, but it requires knowing more about the internals of Jekyll to use it. |
|
@jeffkole - Neither one seems all that easy (I think if I were doing that I'd grab the url directly). But, I had/have no intention to argue the elegance of the solution. I only brought it up because being able to access a collection item's Cheers! |
|
@jeffkole I'm interested in a multi-purpose tag which can work for more than just collection documents. What do you think about |
|
@parkr Updated to be a |
|
IMO, rebasing is dead now. Github just made it easy to review changes without rebasing. |
| @@ -14,6 +14,9 @@ def create_post(content, override = {}, converter_class = Jekyll::Converters::Ma | |||
| if override['read_posts'] | |||
| site.posts.docs.concat(PostReader.new(site).read_posts('')) | |||
| end | |||
| if override['read_collections'] | |||
There was a problem hiding this comment.
It would be preferable to make both these if's single line and stack them above the other.
There was a problem hiding this comment.
Like this?
site.posts.docs.concat(PostReader.new(site).read_posts('')) if override['read_posts']
CollectionReader.new(site).read if override['read_collections']
There was a problem hiding this comment.
Yes, that looks much cleaner!
There was a problem hiding this comment.
@parkr the bot (and I believe Rubocop too) doesn't track line width on Jekyll/Jekyll for now because we have a bunch of lines way over that and it was impacting the score for something so trivial that we could fix over time and didn't impact the actual code or it's readability for the most part.
| end | ||
|
|
||
| should "have the url to the \"site/generate\" item" do | ||
| assert_match %r{2\s/methods/site/generate}, @result |
There was a problem hiding this comment.
Should these have file extensions?
|
Thanks, @jeffkole. One comment above about the tests then I'm 👍 to 🚢. |
|
Thanks for the review @parkr and @envygeeks. I think this should be good to go now. |
link Liquid tag to make generation of URL's easier
|
@jekyllbot: merge +minor |
|
Thanks a lot, @jeffkole! |
* origin/master: (65 commits) Update history to reflect merge of #4703 [ci skip] Update history to reflect merge of #4712 [ci skip] Highlight the test code Update history to reflect merge of #4640 [ci skip] readded "env=prod"-condition Update history to reflect merge of #3849 [ci skip] Update history to reflect merge of #4624 [ci skip] Update history to reflect merge of #4704 [ci skip] Update history to reflect merge of #4706 [ci skip] Checks for link file extension in tests Updating assets documentation Fix test teardown for cleaner. Update history to reflect merge of #4542 [ci skip] Add explanation of site variables in the example _config.yml Use double quotes in the gemfile Add test for creation of Gemfile by 'jekyll new' Add comment about github-pages Update history to reflect merge of #4533 [ci skip] Ensure Rouge closes its div/figure properly after highlighting ends. Add Site#config= which can be used to set the config ...
|
Should this tag be documented on the website? It's mentioned in the release notes for 3.2. |
|
@kainjow Huh. Yeah, it should be. It should be documented like |
|
@jeffkole A few questions regarding the current implementation:
|
|
@DirtyF You should be able to link to any document that Jekyll is generating, including pages. Is that not working? As for the file extension, it could certainly be done. I'll leave it to @parkr for guidance, since that kind of stylistic decision is more up to him than me. If he likes it, I can make the changes. |
@DirtyF What's the reason for this? |
Fixes request made in jekyll#4624 and bug found in jekyll#5182
|
Opened #5199 to link to pages and static files. |
This tag mirrors the post_tag functionality but for collections instead of just posts.