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

Remove conditional assignment from Document URL #2575

Merged
merged 4 commits into from Jul 10, 2014

Conversation

Projects
None yet
4 participants
@alfredxing
Member

alfredxing commented Jul 7, 2014

The conditional assignment was making results from url quite outdated, causing #2557.

This PR provides a fix for the issue as well as a test covering document permalinks.

@@ -127,7 +127,7 @@ def permalink
#
# Returns the computed URL for the document.
def url
@url ||= URL.new({
@url = URL.new({

This comment has been minimized.

@parkr

parkr Jul 7, 2014

Member

Does the permalink method not take from data?

@parkr

parkr Jul 7, 2014

Member

Does the permalink method not take from data?

This comment has been minimized.

@alfredxing

alfredxing Jul 7, 2014

Member

@parkr It does, but when url is first called at the beginning of read, data is not yet initialized, so the URL ends up not taking into consideration the permalink at all.

@alfredxing

alfredxing Jul 7, 2014

Member

@parkr It does, but when url is first called at the beginning of read, data is not yet initialized, so the URL ends up not taking into consideration the permalink at all.

@iloveip

This comment has been minimized.

Show comment
Hide comment
@iloveip

iloveip Jul 10, 2014

Can you please include this PR in v. 2.1.1? I would really like to be able to set custom permalinks for documents inside collection, as I can not get the navigation to work the right way otherwise (see #2560 ).

iloveip commented Jul 10, 2014

Can you please include this PR in v. 2.1.1? I would really like to be able to set custom permalinks for documents inside collection, as I can not get the navigation to work the right way otherwise (see #2560 ).

@parkr parkr merged commit 36c5017 into jekyll:master Jul 10, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

parkr added a commit that referenced this pull request Jul 10, 2014

@alfredxing alfredxing deleted the alfredxing:test-document-permalink branch Jul 10, 2014

@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.