Eliminate the need for prev_section and next_section metadata. #3292

Merged
merged 2 commits into from Jan 13, 2015

Conversation

Projects
None yet
5 participants
@flyinprogrammer
Contributor

flyinprogrammer commented Jan 12, 2015

Not sure if we want this code...

This PR started with me looking into how we do all this awesome documentation/collection stuff, and then I saw that we keep track of nav metadata in two places, the yaml file and in the document as metadata, and that made me really sad. I also hadn't ever done work with liquid - it's kinda cool, but kinda gross [the creation of the doc list makes me sad]. Regardless, here we are, and believe it or not this code doesn't seem to increase build time all that much.

site/_includes/section_nav.html
+{% endcomment %}
+
+{% for document in document_array %}
+ {% assign document_url = document | prepend:"/docs/" | append:"/" %}

This comment has been minimized.

@parkr

parkr Jan 12, 2015

Member

It would be cool if you could do site.docs | where: "url",document or something.

@parkr

parkr Jan 12, 2015

Member

It would be cool if you could do site.docs | where: "url",document or something.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jan 12, 2015

Member

Not bad. Lots of fancy Liquid going on there, but looks pretty solid.

How does this impact build performance?

Member

parkr commented Jan 12, 2015

Not bad. Lots of fancy Liquid going on there, but looks pretty solid.

How does this impact build performance?

@jaybe-jekyll

This comment has been minimized.

Show comment
Hide comment
@jaybe-jekyll

jaybe-jekyll Jan 13, 2015

Member

@flyinprogrammer Wonderful to see someone focusing on such aspects. At the time, the metadata-driven navigation was an improvement; prior-- it was static. 🔢

I look forward to exploring your concepts. I am also curious about the differential in build times.

Member

jaybe-jekyll commented Jan 13, 2015

@flyinprogrammer Wonderful to see someone focusing on such aspects. At the time, the metadata-driven navigation was an improvement; prior-- it was static. 🔢

I look forward to exploring your concepts. I am also curious about the differential in build times.

@flyinprogrammer

This comment has been minimized.

Show comment
Hide comment
@flyinprogrammer

flyinprogrammer Jan 13, 2015

Contributor

Got to thinking about chained filters and low and behold, I could dry up some code :)

With out change:
0m11.518s
0m11.693s
0m11.559s

With latest (b31d8a1) change:
0m11.782s
0m11.684s
0m11.933s

It makes me sad that iterations are so expensive. That said though, I feel like trading some time for possible human error is worth it - but I might also be biased. 😉

Contributor

flyinprogrammer commented Jan 13, 2015

Got to thinking about chained filters and low and behold, I could dry up some code :)

With out change:
0m11.518s
0m11.693s
0m11.559s

With latest (b31d8a1) change:
0m11.782s
0m11.684s
0m11.933s

It makes me sad that iterations are so expensive. That said though, I feel like trading some time for possible human error is worth it - but I might also be biased. 😉

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jan 13, 2015

Member

Worth it. Thanks!

Member

parkr commented Jan 13, 2015

Worth it. Thanks!

@parkr parkr merged commit 06b6e86 into jekyll:master Jan 13, 2015

1 check passed

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

parkr added a commit that referenced this pull request Jan 13, 2015

@troyswanson

This comment has been minimized.

Show comment
Hide comment
@troyswanson

troyswanson Jan 14, 2015

Member

Awesome job. I've been wanting to do this since 7f3b351 (exactly one year ago!) but couldn't figure out how to do it. Your solution is exactly what I should have come up with.

Thanks 👍

Member

troyswanson commented Jan 14, 2015

Awesome job. I've been wanting to do this since 7f3b351 (exactly one year ago!) but couldn't figure out how to do it. Your solution is exactly what I should have come up with.

Thanks 👍

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