Whole-post excerpts should match the post content #4004

Merged
merged 1 commit into from Oct 16, 2015

Conversation

Projects
None yet
5 participants
@kevinoid
Contributor

kevinoid commented Oct 1, 2015

When a post does not contain an excerpt_separator, meaning the excerpt includes the entire post, the excerpt should contain exactly the post content. This is desirable both from a correctness standpoint, that the excerpt should not introduce any new content, and more practically to allow fast and easy detection of whole-post excerpts in Liquid templates using post.excerpt == post.content. A common use-case is deciding whether to render "Read More" links to a post on a page containing post excerpts.

Currently whole-post excerpts include additional newlines at the end of the excerpt content, which makes it difficult to detect whether an excerpt includes the whole post (unless there is an easy method I am overlooking). This PR fixes the issue and adds tests to ensure it stays fixed.

If there's anything I've overlooked or style changes that you'd like to see, let me know.

Thanks,
Kevin

Whole-post excerpts should match the post content
When a post does not contain an excerpt_separator, meaning the excerpt
includes the entire post, the excerpt should contain exactly the post
content.

This is desirable both from a correctness standpoint, that the excerpt
should not introduce any new content, and more practically to allow fast
and easy detection of whole-post excerpts in Liquid templates using
`post.excerpt == post.content`.  A common use-case is deciding whether
to render "Read More" links on a page containing post excerpts.

This commit does exactly that.  It avoids adding additional newlines to
the excerpt content when the excerpt includes the whole post and adds
tests to ensure that this behavior is correct and preserved going
forward.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Oct 2, 2015

Member

Seems logical to me. Any objections, @jekyll/core?

Member

parkr commented Oct 2, 2015

Seems logical to me. Any objections, @jekyll/core?

@parkr parkr added the fix label Oct 2, 2015

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Oct 2, 2015

Contributor

I'm 👍 there was only once I wished this was the case but it was a wish none-the-less.

Contributor

envygeeks commented Oct 2, 2015

I'm 👍 there was only once I wished this was the case but it was a wish none-the-less.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Oct 2, 2015

Member

Cool. :shipit:

Member

parkr commented Oct 2, 2015

Cool. :shipit:

@mattr-

This comment has been minimized.

Show comment
Hide comment
@mattr-

mattr- Oct 16, 2015

Member

I'm good with this.

Member

mattr- commented Oct 16, 2015

I'm good with this.

@mattr- mattr- merged commit 20303de into jekyll:master Oct 16, 2015

1 check passed

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

mattr- added a commit that referenced this pull request Oct 16, 2015

@kevinoid

This comment has been minimized.

Show comment
Hide comment
@kevinoid

kevinoid Oct 16, 2015

Contributor

Thanks guys, much appreciated!

Contributor

kevinoid commented Oct 16, 2015

Thanks guys, much appreciated!

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