Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

display single files from gist #861

Merged
merged 4 commits into from Mar 17, 2013

Conversation

Projects
None yet
5 participants
@danielgrieve
Copy link
Contributor

danielgrieve commented Mar 15, 2013

No description provided.

Daniel Grieve
@danielgrieve

This comment has been minimized.

Copy link
Contributor Author

danielgrieve commented Mar 15, 2013

This was requested by @pathawks in issue #839

@jwebcat

This comment has been minimized.

Copy link

jwebcat commented Mar 16, 2013

@danielgrieve isn't there a plugin for this already?

@danielgrieve

This comment has been minimized.

Copy link
Contributor Author

danielgrieve commented Mar 16, 2013

Well the plugin that comes with standard jekyll at the moment can only display full gists, so this adds the capability to display single files. Yes, there are plugins that you can add which will do what I've added here, but not part of a base jekyll installation.

@jwebcat

This comment has been minimized.

Copy link

jwebcat commented Mar 16, 2013

@danielgrieve Fantastic, and thanks bro :+1 looking forward to using this on gh pages.

On Mar 16, 2013 5:46 AM, "Daniel Grieve" notifications@github.com wrote:

Well the plugin that comes with standard jekyll at the moment can only
display full gists, so this adds the capability to display single files.
Yes, there are plugins that you can add which will do what I've added here,
but not part of a base jekyll installation.


Reply to this email directly or view it on GitHubhttps://github.com//pull/861#issuecomment-15004269
.

super
@gist = gist.strip
def render(context)
if tag_contents = @markup.match(/(\d+) (.*)/)

This comment has been minimized.

Copy link
@parkr

parkr Mar 16, 2013

Member

I'm a bit worried about the freedom of the second part of this Regexp. It's probably fine, but I just want to double-check.

@mojombo will have to verify that this won't be a vunerability to GitHub's systems.

This comment has been minimized.

Copy link
@mojombo

mojombo Mar 17, 2013

Contributor

@parkr I don't see any security problem here, what kind of problem were you envisioning?

Regardless, it's probably wise to make the regex more strict. Something like:

@markup.strip.match(/\A(\d+) ?(\S*)\Z/)

Note the .strip, the anchoring of beginning and end of string, optional space, and the non-whitespace-only match in filename part.

This comment has been minimized.

Copy link
@parkr

parkr Mar 17, 2013

Member

@mojombo Mostly worried about someone hacking this through a security vulnerability in Regexp in the version of Ruby that GitHub Pages uses to run jekyll on sites you host. I'd rather be safe than sorry, so I just wanted to check!


def gist_script_tag(gist_id, filename=nil)
if filename.empty?
"<script src=\"https://gist.github.com/#{gist_id}.js\">\s</script>"

This comment has been minimized.

Copy link
@parkr

parkr Mar 16, 2013

Member

What do you accomplish with the \s?

This comment has been minimized.

Copy link
@danielgrieve

danielgrieve Mar 16, 2013

Author Contributor

I'm not sure how that made it in there, it should've just stayed as a space!

Daniel Grieve added some commits Mar 16, 2013

Daniel Grieve
@danielgrieve

This comment has been minimized.

Copy link
Contributor Author

danielgrieve commented Mar 17, 2013

Took the suggestions from @mojombo there for the regex matching.

@parkr

This comment has been minimized.

Copy link
Member

parkr commented Mar 17, 2013

@danielgrieve Bomb, thanks man. One last fix and I'll merge it in!

Daniel Grieve

parkr added a commit that referenced this pull request Mar 17, 2013

Merge pull request #861 from danielgrieve/gist-tag
display single files from gist

@parkr parkr merged commit a054ce2 into jekyll:master Mar 17, 2013

1 check passed

default The Travis build passed
Details

parkr added a commit that referenced this pull request Mar 17, 2013

@jekyll jekyll locked and limited conversation to collaborators Feb 27, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.