Bugfix: Sort comments by date ascending

[deadlydog]

This is an enhancement or feature.

Summary

Currently the order that comments appear on a post are random. This PR sorts the comments by date in ascending order, so oldest comments appear at the top, and newest comments down at the bottom.

Unrelated

Also, would you consider adding the hacktoberfest label to your GitHub repository. This encourages more people to contribute to your repository, and allows PRs like this to count towards hacktoberfest. Thanks!

[iBug]

FYI: Please check out #2719 for Michael's answer to your "unrelated" request. Not sure if things have changed, though.

[deadlydog]

I added the hacktoberfest-accepted label to your PR. Didn't do the global label as I had to limit contributions at the start of the month as I was getting garbage PRs just to cash in on the free tshirt.

Ah, perfect. I'm cool with simply adding the hacktoberfest-accepted label to this PR :)

[mmistakes]

@deadlydog Do you have an example repo that shows comments out of order? I haven't used the Staticman implimentation in some time with this theme, but my recollection it showed comments in reverse chron as expected.

I'm wondering if soemthing changed with how dates are formatted in the YAML front matter of a comment that's changed that.

[deadlydog]

Haha, well, I manually applied the fix in my blog already to test it out. I can revert the fix temporarily if you like to see them out of order. You can see just from the code though that the sort filter doesn't specify what to sort by; in my PR I tell it to sort by the date field.

I've reverted the fix to my blog; check out this post to see an example of how the comments get sorted without this PR.

Here's a screenshot as well of the default sort order:

Thanks @mmistakes :)

[mmistakes]

What's the YAML look like for one of your comments?

Back when the demo site used Staticman date came in as a string, which the sort filter worked as is on.
If Staticman is now passing dates correctly, then I can see why a plain sort doesn't work for you.

MM comment examples:
https://github.com/mmistakes/minimal-mistakes/blob/master/docs/_data/comments

message: Cooooookies! Yum! (Thanks for this awesome them.. and... recipe..)
name: Markus
email: f6f9be6ae6e174661ea7c87a520ffef8
url: 'http:https://www.markusgiesen.com'
hidden: ''
date: '2016-09-14T16:23:32.840Z'

[mmistakes]

I just tried building the demo site that's in this repo at /docs and comments are in correct order... oldest to newest.

With your fix I get the same results.
Not sure why it's fine on the demo site but for you, you get them out of order. Only thing I can think of is there's something different in the YAML files... either the date field or maybe it's the filenames?

Just trying to determine what the problem is in case this breaks something. The logic for sorting Staticman comments has been the same since I shipped it years ago. So either it's been fine for all this time, or no one uses it to have noticed 😝

[deadlydog]

Haha, for sure, don't want to accidentally break it, as I've only tested my fix on my site.

Here's a link to one of the comment yaml files so you can check their format and file names out.

Thanks @mmistakes :)

[mmistakes]

Ah I see, it's your filenames. I bet the default behavior is to sort alphabetically.

comment-David C-1621270309470.yml
comment-Ian-1617816373178.yml
comment-Joshua Silverman-1613060427957.yml
comment-Nathan-1586332247533.yml
comment-Ralf-1612346809157.yml
comment-SkyHome-1621516671771.yml
comment-chandrika-1587371626665.yml
comment-deadlydog-1612365758774.yml

demo site:

comment-1473870213530.yml
comment-1478213467992.yml
comment-1500181304581.yml
comment-1500214855350.yml

It works on the demo site because the filenames have the same base filename, where as your's has the comment author names in there which throws the order off.

That's probably why it hasn't been reported as a bug up until now because everyone who used Staticman used the same naming convention I did (e.g., filename: "comment-{@timestamp}").

Your fix looks sound to me, I'll just need to do some additional testing. But I think it's safe to assume it'll be OK as it worked fine with my quick test.