Skip to content
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

Fix regression of .Truncated evaluation in manual summaries #2989

Merged
merged 1 commit into from Feb 19, 2017
Merged

Fix regression of .Truncated evaluation in manual summaries #2989

merged 1 commit into from Feb 19, 2017

Conversation

socialhacker
Copy link
Contributor

This fixes the behavior of .Truncated that was introduced with commit
bef496b which was later broken. The
desired behavior is that .Truncated would evaluate to false when there
was nothing after the user defined summary marker.

This also adds a simple unit test to ensure that this feature isn't
broken again. The check for content after the user defined summary
marker is done on the raw content instead of the working copy because
some of the markup renderers add elements after the marker, making it
difficult to determine if there is actually any content.

The behavior (evaluating to false when there is no content, just
summary) is also now documented.

@CLAassistant
Copy link

CLAassistant commented Jan 26, 2017

CLA assistant check
All committers have signed the CLA.

hugolib/page.go Outdated
// the raw content because different markup engines (rst and asciidoc in
// particular) add div and p elements after the summary marker.
func (p *Page) anyContentAfterUserDefinedSummary() bool {
sections := bytes.Split(p.rawContent, helpers.SummaryDivider)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I care about performance, and I'm pretty sure we have done the above split before. So no need to do it again.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Performance is a good goal, and a nice feature of Hugo, and I'm happy to improve this. This split is not actually done anywhere on the raw content, it is done on the work content once it has been modified by renderContent, but that is too late. The same approximate amount of work is also done on the work content right before renderContent in commonConvert (after shortcode and emoji processing). That is where the is replaced by the internal marker. It should be fine to do this check there instead of on the true raw content, so I'll look into changing how that replacement is done so that the truncate flag information can be computed then.

This fixes the behavior of .Truncated that was introduced with commit
bef496b which was later broken.  The
desired behavior is that .Truncated would evaluate to false when there
was nothing after the user defined summary marker.

This also adds a simple unit test to ensure that this feature isn't
broken again.  The check for content after the user defined summary
marker is done on the raw content instead of the working copy because
some of the markup renderers add elements after the marker, making it
difficult to determine if there is actually any content.

The behavior (evaluating to false when there is no content, just
summary) is also now documented.
@socialhacker
Copy link
Contributor Author

Hi Bjørn, I've updated to only do the raw data split once, I've also included in this version the removal of some dead code, that might have been a mistake though. Let me know if you would like me to make a separate pull request for the removal.

@bep bep merged commit 99fbc75 into gohugoio:master Feb 19, 2017
@socialhacker socialhacker deleted the fix_truncated_regression branch February 19, 2017 15:34
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants