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

Render page prior to splitting on summary #1667

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
5 participants
@OJ

OJ commented Dec 3, 2015

This is my first contribution to Hugo so I expect it to be strongly critiqued as I learn the ropes!

The issue that I found, which I briefly mentioned here, is that the summary generation doesn't work correctly in some cases when Markdown is used.

Hugo splits the page based on the summary marker, and then renders the header separately. If users make use of the references facility that comes with Markdown, then all the links in the reference list at the bottom of the page are no present when the markdown is rendered. Eg:

Long and fabulous [summary][] goes here!
.
.
<!--more-->
.
.
  [summary]: http://random.com/

With the links missing, the page ends up with Markdown creeping into the summary.

This PR contains an attempt at fixing this issue by rendering the page first. I don't think it's perfect, and so I would appreciate some commentary on what would be a better option. One thing that stands out for me is that the whitespace isn't trimmed due to blank lines being convered to <p></p>, but rather that cludge a fix together for that I thought I'd throw it open to commentary.

Thanks again for Hugo, I love it.

@CLAassistant

This comment has been minimized.

Show comment
Hide comment
@CLAassistant

CLAassistant Dec 3, 2015

CLA assistant check
All committers have accepted the CLA.

CLAassistant commented Dec 3, 2015

CLA assistant check
All committers have accepted the CLA.

Render page prior to splitting on summary
This commit forces the page to be rendered prior to the summary section being determined. This is due to the fact that in the case of markdown files, references can be cut from the post and hence markdown links won't be rendered correctly.
@OJ

This comment has been minimized.

Show comment
Hide comment
@OJ

OJ Dec 3, 2015

I'm looking into fixing the build issues. Bear with me :)

OJ commented Dec 3, 2015

I'm looking into fixing the build issues. Bear with me :)

@OJ

This comment has been minimized.

Show comment
Hide comment
@OJ

OJ Dec 3, 2015

It looks like fixing this isn't going to be as simple as I thought. Forcing rendering before splitting wraps the <!--more--> tag in other markup in certain circumstances, and so splitting after causes other kinds of issues. The few ideas I have come up with so far that work around this feel like kludges at best.

The cheap fix is to force the use of [inline](links), but I am not a fan of those, and I don't really use them at all in my posts.

Keen to hear some ideas on how best to resolve this. I'll mull over some options as well. Thanks!

OJ commented Dec 3, 2015

It looks like fixing this isn't going to be as simple as I thought. Forcing rendering before splitting wraps the <!--more--> tag in other markup in certain circumstances, and so splitting after causes other kinds of issues. The few ideas I have come up with so far that work around this feel like kludges at best.

The cheap fix is to force the use of [inline](links), but I am not a fan of those, and I don't really use them at all in my posts.

Keen to hear some ideas on how best to resolve this. I'll mull over some options as well. Thanks!

@spf13

This comment has been minimized.

Show comment
Hide comment
@spf13

spf13 Dec 16, 2015

Contributor

One idea would be to replace the with another string that may still be discoverable but would have different characteristics.

Contributor

spf13 commented Dec 16, 2015

One idea would be to replace the with another string that may still be discoverable but would have different characteristics.

@bling

This comment has been minimized.

Show comment
Hide comment
@bling

bling Dec 31, 2015

@OJ can you provide an example of where the more tag gets wrapped in other tags? i merged your changes locally and it appears to work...but then again, i always have split on its own line.

bling commented Dec 31, 2015

@OJ can you provide an example of where the more tag gets wrapped in other tags? i merged your changes locally and it appears to work...but then again, i always have split on its own line.

@OJ

This comment has been minimized.

Show comment
Hide comment
@OJ

OJ Jan 2, 2016

@bling: sure mate. Basically anything that looks like this:

A paragraph goes like this.<!--more-->Then another starts like this.

The unit tests for the project have such examples as well.

Cheers!

OJ commented Jan 2, 2016

@bling: sure mate. Basically anything that looks like this:

A paragraph goes like this.<!--more-->Then another starts like this.

The unit tests for the project have such examples as well.

Cheers!

@bling

This comment has been minimized.

Show comment
Hide comment
@bling

bling Jan 2, 2016

ah, i see what you mean. i had my <a> tags styled with display:block so i didn't even notice (even with <!--more--> inline like what you mentioned). it looks like jekyll does it by appending all the references to the summary after splitting. seems like it would work here.

bling commented Jan 2, 2016

ah, i see what you mean. i had my <a> tags styled with display:block so i didn't even notice (even with <!--more--> inline like what you mentioned). it looks like jekyll does it by appending all the references to the summary after splitting. seems like it would work here.

@spf13

This comment has been minimized.

Show comment
Hide comment
@spf13

spf13 Jan 4, 2016

Contributor

I think we should follow the approach that @bling suggests here.

Contributor

spf13 commented Jan 4, 2016

I think we should follow the approach that @bling suggests here.

@OJ

This comment has been minimized.

Show comment
Hide comment
@OJ

OJ Jan 6, 2016

+1.

I'll get onto this as soon as I've got the internet reconnected at home. I'll close this PR and open a new one. Thanks to all for the input!

OJ commented Jan 6, 2016

+1.

I'll get onto this as soon as I've got the internet reconnected at home. I'll close this PR and open a new one. Thanks to all for the input!

@OJ OJ closed this Jan 6, 2016

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