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

Expose file path to Liquid #951

Merged
merged 3 commits into from Apr 12, 2013

Conversation

Projects
None yet
6 participants
@maul-esel
Contributor

maul-esel commented Apr 12, 2013

This exposes the source file path to liquid as requested in #633.

@parkr

This comment has been minimized.

Member

parkr commented Apr 12, 2013

Can you tell me more about how and why you'd use this?

@benbalter: Could this be useful for your GitHub Teach project as well? Could be a cool way to jump to "Edit" or "View Jekyll Source".

@benbalter

This comment has been minimized.

Contributor

benbalter commented Apr 12, 2013

Not for that particular project, but in general, yes, very.

I use a dirty, dirty hack on my personal site, for example, because it's very hard to go from permalink back to source.

Can't tell by the changeset, but the only thing I'd caution is to only expose it relative to the site's root. Knowing a file is ./about.md is one thing, but knowing it's /foo/bar/sites/ben/about.md is a whole different story.

If the goal is to get to the file's source (e.g., link to GitHub repo), I'd then have to figure out the site's base directory and run a replace filter. Barely better off, not to mention, probably wont sit so great with some security conscious folks.

@maul-esel

This comment has been minimized.

Contributor

maul-esel commented Apr 12, 2013

@benbalter: yes, it is relative to the source directory. Otherwise it would also be incorrect as soon as the repo is moved anywhere.

@parkr: While I can for sure see how this is useful for the cases described in the issue, I probably wouldn't use it myself any time soon (rarely using jekyll so far). I just picked an issue I thought I could solve ;) Whether you want that feature or not is up to you.

@parkr

This comment has been minimized.

Member

parkr commented Apr 12, 2013

@benbalter: Know anyone I can chat with who works on GitHub Pages? I know there's no team per se, but perhaps you know someone who can run a quick security check on relative paths? Considering these can be built anyway, what added security issue can you see by having them natively served to Liquid templates?

@maul-esel Well, thanks so much for contributing your time!

@benbalter

This comment has been minimized.

Contributor

benbalter commented Apr 12, 2013

@sr any red flags with exposing a file's relative path to Liquid?

@sr

This comment has been minimized.

sr commented Apr 12, 2013

/cc @github/security

@parkr

This comment has been minimized.

Member

parkr commented Apr 12, 2013

@sr This repo doesn't seem to have that group capability, or I can't see it because it's not a public team. Somehow, it's not linked.

Thanks to you both for drawing attention to this. I think it should be fine, but it'll run on your servers soon, so we should ensure it is safe for general use with GitHub Pages.

@sr

This comment has been minimized.

sr commented Apr 12, 2013

Ugh yeah. Thanks for pointing that out. Doing it manually instead then:

/cc @shawndavenport @mastahyeti @sroberts

@mastahyeti

This comment has been minimized.

Contributor

mastahyeti commented Apr 12, 2013

Yeah, looks like @dir is coming from Jekyll::Site.source, which is assumed to be a relative path. The source appears to normally come from the config, but we don't allow that setting for GitHub pages anyway. 👍

@parkr

This comment has been minimized.

Member

parkr commented Apr 12, 2013

@mastahyeti Awesome, thanks for that confirmation.

parkr added a commit that referenced this pull request Apr 12, 2013

@parkr parkr merged commit 92db4ed into jekyll:master Apr 12, 2013

1 check passed

default The Travis build passed
Details

parkr added a commit that referenced this pull request Apr 12, 2013

@maul-esel maul-esel deleted the maul-esel:liquid-file-path branch Apr 12, 2013

parkr added a commit that referenced this pull request Apr 12, 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.