New Liquid tag for listing files: directory #585

Closed
wants to merge 4 commits into
from
@sillylogger

This is a follow up to @mojombo's response to @robru's gallery pull request. I simplified the name and dir param to simply a path and changed the date-stamp files to use Jekyll's convention. Usage is similar to @robru pull request:

<ul>
  {% directory path: images/vacation %}
    <li> <img src="{{ file.url }}" /> </li>
  {% enddirectory %}
</ul>

It provides this data to the templates:

file.url        # absolute path to file
file.name       # basename
file.date       # date extracted from beginning of filename, if present (optional)
file.slug       # basename with the date and file extension stripped off
file.title      # file.slug with hyphens converted to spaces, and put into Title Case

The forloop maintains its usual context.

My updates to this patch also removed the YAML globbing from files as that now throws errors for files without YAML. Lastly I added an exclude param that rejects files based on a RegExp. @mojombo wasn't a big fan of defaults, so the default exclusion of /.html$/ might not fly...

@andycasey

Thoughts on this @mojombo?

@paperdigits

Github needs a feature wherein I can express my want (perhaps monetarily... like a "sponsor this feature" kind of thing) of a certain feature. That would clear the comments for real discussion, instead of cluttering it with comments like this one-- where I express my want of this particular feature.

@parkr
Jekyll member

What's really awesome about open source projects released under an MIT license is that you can make whatever derivative you want!

Jekyll is meant to be extensible, which is how you were able to write this plugin. If you really want it to be pushed to GitHub Pages, maintain your source locally and push up some deploy directory (compile to _site, rsync with _deploy, perhaps) which has the compiled HTML.

@paperdigits

@parkr This pull request is to make it whatever we want... Otherwise I'm confused a bit by your comments. This pull would add the feature to the jekyll gem, thus negating the needs to keep this feature as a plugin.

@parkr
Jekyll member

Sorry for the confusion. My comments were meant to suggest an alternative route to getting what you want: compile the site locally and push the generated HTML, etc up to GH Pages.

@fbnlsr

This is a must-have.

@paperdigits

does this mean we'll see this in jekyll soon?

@mattr-
Jekyll member

It hasn't been merged yet, so no. Another issue was closed in favor of this one.

@parkr
Jekyll member

I'd like @mojombo to take a look at this and approve it before we merge.

@SirCmpwn

Dropping in to express support for this. Ping @mojombo

@SirCmpwn

A potentially valuable addition to this PR would be exposing the files YAML front-matter (if they had any) to liquid, and the contents of the file if plaintext.

@kelvinst

So @parkr, will @mojombo have a time someday to look at this? I don't know if someone already revised it, but I'll revise it anyway.

@kelvinst kelvinst commented on an outdated diff Oct 2, 2013
lib/jekyll/tags/directory.rb
+ class DirectoryTag < Liquid::Block
+ include Convertible
+
+ MATCHER = /^(.+\/)*(\d+-\d+-\d+)-(.*)(\.[^.]+)$/
+
+ attr_accessor :content, :data
+
+ def initialize(tag_name, markup, tokens)
+ attributes = {}
+
+ # Parse parameters
+ markup.scan(Liquid::TagAttributes) do |key, value|
+ attributes[key] = value
+ end
+
+ @path = attributes['path'] || '.'
@kelvinst
kelvinst added a line comment Oct 2, 2013

Too many spaces after attributes['path']

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@kelvinst kelvinst commented on an outdated diff Oct 2, 2013
lib/jekyll/tags/directory.rb
@@ -0,0 +1,115 @@
+# Title: Dynamic directories for Jekyll
+# Author: Tommy Sullivan http://superawesometommy.com, Robert Park http://exolucere.ca
+# Description: The directory tag lets you iterate over files at a particular path. If files conform to the standard Jekyll format, YYYY-MM-DD-file-title, then those attributes will be populated on the yielded file object. The `forloop` object maintains [its usual context](http://wiki.shopify.com/UsingLiquid#For_loops).
+#
+# Syntax:
+#
+# {% directory path: path/from/source [reverse] [exclude] %}
@kelvinst
kelvinst added a line comment Oct 2, 2013

In the feature tests you used reverse: true and exclude: 'regexp', I guess that is the right way, isn't it?

If yes, this might be {% directory path: path/from/source [reverse: boolean_value] [exclude: 'regexp'] %}, to be more explicit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@kelvinst kelvinst and 1 other commented on an outdated diff Oct 2, 2013
lib/jekyll/tags/directory.rb
+
+ MATCHER = /^(.+\/)*(\d+-\d+-\d+)-(.*)(\.[^.]+)$/
+
+ attr_accessor :content, :data
+
+ def initialize(tag_name, markup, tokens)
+ attributes = {}
+
+ # Parse parameters
+ markup.scan(Liquid::TagAttributes) do |key, value|
+ attributes[key] = value
+ end
+
+ @path = attributes['path'] || '.'
+ @exclude = Regexp.new(attributes['exclude'] || '.html$', Regexp::EXTENDED | Regexp::IGNORECASE)
+ @rev = attributes['reverse'].nil?
@kelvinst
kelvinst added a line comment Oct 2, 2013

Sorry, but I think this is wrong. And if I use {% directory path: downloads/files reverse: false %}{% enddirectory %}? Or will the feature tests wrong? See at this line

@sillylogger
sillylogger added a line comment Dec 5, 2013

You're right, it is wrong.
But now we get into the question of how well should one parse the String version of a boolean 😔
I'll just take 'true' as true and everything else as false..

@kelvinst
kelvinst added a line comment Dec 5, 2013

Nice, this is the best way, I guess.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@kelvinst kelvinst commented on the diff Oct 2, 2013
lib/jekyll/tags/directory.rb
+ def initialize(tag_name, markup, tokens)
+ attributes = {}
+
+ # Parse parameters
+ markup.scan(Liquid::TagAttributes) do |key, value|
+ attributes[key] = value
+ end
+
+ @path = attributes['path'] || '.'
+ @exclude = Regexp.new(attributes['exclude'] || '.html$', Regexp::EXTENDED | Regexp::IGNORECASE)
+ @rev = attributes['reverse'].nil?
+
+ super
+ end
+
+ def render(context)
@kelvinst
kelvinst added a line comment Oct 2, 2013

So, I think this method is too big, maybe if you can split it... 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@just-boris

Now a topic starter is unavailable 😞
@sillylogger, your PR has been reviewed!

@sillylogger

@just-boris sorry sorry, I will try to get to this shortly. Thanks @kelvinst for reviewing!

@parkr
Jekyll member

Just a head's up, if we were to include this tag, it would likely be disabled in safe mode unless I can get the 👍 from @github/security.

@kelvinst

You're absowelcomelutely. 😄

@just-boris

I think, that the github security wouldn't like that with this tag can directory content out of project context could be listed. I made pull request with fix for it into sillylogger/jekyll-directory#2. Current PR based on that jekyll plugin

@SirCmpwn

I express strong support for inclusion in the github-pages build. I've actually been waiting on this pull request to get specific features implemented.

@ghost

@SirCmpwn have you been waiting 3 years, like I have?

@SirCmpwn

It's not a contest, @robru.

@jamesfzhang

+1 for this.

@pedrocorreia

This would be really helpful.

@parkr
Jekyll member

This isn't something I'd like to see in core, but is there a plugin that does this?

@just-boris

@parkr yes. Topic starter has separate version of this tag as plugin: https://github.com/sillylogger/jekyll-directory

@mattr-
Jekyll member

@parkr. Agreed. I'd prefer to have this outside of core as well.

Closing.

@mattr- mattr- closed this Feb 20, 2014
@just-boris

@mattr- oh no! we are want this feature in Github.Pages, so this feature should be in core, because Pages builds site without plugins

@amattn

Would love for this to be in core (github pages) as well. It's the only place I use jekyll.

@amattn

Can I ask what the rationale is for keeping this outside of core in a plugin?

@parkr
Jekyll member

Giving access to the FS on GHP is something they (rightly) want to keep to a minimum. Security audits take time so it means we have to wait just that much longer to allow any of the other features of Jekyll go up. At least with a plugin, everything is completely isolated and we can ask the GHP team to consider bringing it in later.

@amattn

@parkr
Thanks so much! Makes tons of sense. I'll look for an alternative solution for now.

@parkr parkr referenced this pull request Mar 12, 2014
Closed

How to iterate directories? #2136

@cirosantilli

@paperdigits Thanks for the link, but I need it in GH pages. Starred them in the hope GitHub will it it someday.

@parkr
Jekyll member

@cirosantilli If you want to see a feature on GitHub or GitHub Pages, email support! We've already opted out of this in core, but the GHP team has added support for plugins.

@illus0r illus0r referenced this pull request in Praqma/code-conf.com Apr 21, 2016
Closed

Browse press material #116

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment