greghendershott / frog

Frog is a static blog generator implemented in Racket, targeting Bootstrap and able to use Pygments.
916 stars 96 forks source link

BS4 older newer links fixes #222

Closed tfeb closed 6 years ago

tfeb commented 6 years ago

This PR contains fixes for #221. They are currently a bit rough, but they do seem to work.

886b7fb12c4d32fafd04b5883997183ca2b2cab8 modifies older/newer-links to accept a keyword argument, for-bs which can be either 3 or 4: if the former it does what it already did, if 4 it does what the function defined in post-template.html used to do. post-template.html now calls it with the right argument for BS4 and the function defined in it is gone.

a94b7f775b095cc983677181baa6bd7d3fa39eb2 attempts to fix the index page stuff. It does this by just inserting suitable classes for BS4 in the generated HTML. I have not checked this looks OK with BS3 and I don't really know how to make it work properly if it looks bad there.

6d868944e092fbfa87bac7c0b2ee7af69466a73a is a gratuitous extra change which installs all the example posts: this means the example blog has more than one post, but it means you have to delete more to make it into a real blog.

I am probably about to give up on BS4 because it looks terrible (mathjax is now a joke, layout is a joke), but I'll try to keep beating my head against it long enought to make any fixes to these needed.

tfeb commented 6 years ago

I've checked what things look like using this with a BS3-based blog (this only matters for the index-page links: the things changed by a94b7f7). The result is it's fine: there are extra classes on things but no CSS styles associated with them so nothing happens.

greghendershott commented 6 years ago

Thanks for looking at this!

PR #207 originally changed older/newer-links -- but I discouraged that, thinking it would break for people who updated Frog, but were unwilling or unable to change their site to use BS4.

Speaking of which, I'm still in that camp. I haven't changed my own blog to use BS4, yet.

That may mean, however, that I'm not dogfooding the BS4 changes very well. So I do appreciate you looking at this, and the other changes in the PR!

greghendershott commented 6 years ago

OK actually I'm also not sure about commit 6d86894 -- copying over all the example posts. It's my fault for naming them badly, but I thinking of them more like "smoke tests". Definitely not "examples" in the sense of role models. Many are just blah blah blah exercise foo blah blah blah. So, I feel unsure about dumping them all into new-initialized sites?

greghendershott commented 6 years ago

Commit a94b7f7 seems great!

tfeb commented 6 years ago

@greghendershott: I think that 6d868944e092fbfa87bac7c0b2ee7af69466a73a doesn't belong in this PR in any case: it's at best peripherally related to BS4. So I'll revert it I think. What I think would be good is if there was a way to quickly fabricate a blog which had more than one post, as that lets you see things like forward-backward links (I think it's not practical to have enough posts to create multiple index pages. So perhaps a compromise would be just to select two of the smoke-test entries rather than one? If that seems reasonable I could submit a PR for that, but it might be easier just to add it on master directly without the PR stuff?

tfeb commented 6 years ago

With regards to older/newer-links: my change should mean that no existing BS3 (or in fact BS4) blog sees any change. Any existing incantation won't use #:for-bs ... and so will create the existing, BS3, HTML. A new blog will use the provided post-template.html and this file does include #:for-bs 4, so it will create the right syntax for BS4: that's right, because the version of BS that ships with frog is 4 as well, so it kind of needs to.

So I think this is a safe change.

greghendershott commented 6 years ago

With regards to older/newer-links: my change should mean that no existing BS3 (or in fact BS4) blog sees any change.

Oh. Dang. I didn't look carefully enough -- sorry! (I think my brain was maxed out last night; big chunk of racket-mode work I was trying to wrap up.)

greghendershott commented 6 years ago

Now that this PR effectively nets out to just commit 886b7fb and commit a94b7f7 (but not 6d86894) rebased to master, I'd like to merge it.

I'd slightly prefer to have it squashed down to two commits, before merging. (Usually I prefer a straight line history of ff-only commits; usually don't love merge commits.)

If you'd like to squash yourself, please go ahead?

In any case, thank you again, and I'll try to merge this within the next 24h.

tfeb commented 6 years ago

Agree about clean commits. How do I do this in a way which won't cause something to puke? Normally what I'd do (if it was only me) is fix the branch up locally (rebase it of master, squash the bogus commits away) and then force-push it to blow away the history. Do pull requests cope with that?

(Sorry, not very used to using PRs, I almost entirely use GH for replicating my repos around the place rather than for the workflow).

tfeb commented 6 years ago

OK, I read the manual and it looks like my fix/force-push is explicitly a workflow they allow, so I've done that: the branch now should be properly ahead of master.