Adds ability to link to all files #5199

Merged
merged 3 commits into from Sep 13, 2016

Projects

None yet

6 participants

@jeffkole
Contributor
jeffkole commented Aug 3, 2016

Fixes request made in #4624 and bug found in #5182

@jeffkole jeffkole Adds ability to link to all files
Fixes request made in #4624 and bug found in #5182
82d6657
@envygeeks
Member

What bug? I see no bug in that documentation.

@DirtyF
Member
DirtyF commented Aug 4, 2016 edited

Thanks @jeffkole link to pages and files works fine now :)

I tested it a bit and noticed that you have to use add a / before the path to a file or Jekyll won't find the file.

{% link /assets/files/doc.pdf %}

Also, just so you know I didn't manage to link to a data file with {% link _data/file.yml %} but's it's out of the scope of this fix I guess.

@envygeeks it wasn't a bug in the documentation but rather in the implementation of the brand new linktag, hence this PR

@parkr
Member
parkr commented Aug 5, 2016

@jeffkole Because you added a file to the test fixture site, you have to update another test: https://travis-ci.org/jekyll/jekyll/jobs/149605617

@jeffkole jeffkole Fixes tests to account for new info.md file
05445b8
@jeffkole
Contributor
jeffkole commented Aug 5, 2016

@DirtyF That is annoying, isn't it. Looks like static files get a leading slash added to their relative path property. I'll check in a fix.

@jeffkole jeffkole Fixes `link` tag to work with static files
e0ce4a0
@parkr
Member
parkr commented Aug 6, 2016

LGTM!

@parkr parkr added a commit that referenced this pull request Aug 7, 2016
@parkr parkr templates.md: {% link %} tag only accepts collection documents
Remove examples of pages/static file links until it's released.
#5182 #5199
ab4abb1
@DirtyF
Member
DirtyF commented Sep 13, 2016 edited
@envygeeks
Member

I like that it has tests. Thanks for adding tests. LGTM.

@envygeeks
Member

Since I don't remember the tag for a feature +minor? I'll let @parkr or somebody else on @jekyll/core deal with the merge.

@mattr-
Member
mattr- commented Sep 13, 2016

@jekyllbot: merge +minor

@jekyllbot jekyllbot merged commit 4888b84 into jekyll:master Sep 13, 2016

2 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jekyll/lgtm Approved by @parkr and @envygeeks.
@mattr-
Member
mattr- commented Sep 13, 2016

@envygeeks you were right. It's +minor. 😃

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