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 internal links with question marks #242

Merged
merged 3 commits into from
Dec 30, 2017
Merged

Fix internal links with question marks #242

merged 3 commits into from
Dec 30, 2017

Conversation

h-m-f-t
Copy link
Contributor

@h-m-f-t h-m-f-t commented Dec 29, 2017

This change makes links work again!

When the site shifted to its new template, it likely deployed with a newer version of Jekyll than before (...or something) that in the creation of HTML omits question marks (and other non-alphanumerics) in the creation of header IDs. It previously used the percent-encoding %3f for question marks.

There may be other things that the shift shifted, but I think I've removed all the %3fs in the repo.

@konklone
Copy link
Contributor

konklone commented Dec 30, 2017

Thank you! I didn't notice this at all, though I bet it's an artifact of me updating dependencies in #241. This included an update to redcarpet, which is responsible for generating the anchor slugs from header names. There are a number of entries in the changelog related to anchor generation between 3.1.2 and 3.4.0 that could explain the change.

Ideally, we'd restore the old behavior, so as not to break past links. But the changes in redcarpet look to be positive changes to fix bugs and handle non-ASCII characters better, and there's no obvious option to opt in to the old behavior.

So I guess we'll just deal with anchor breakage. This isn't nearly as bad as URL breakage, since it degrades to loading the intended page anyway.

I really appreciate you fixing this!

@konklone konklone merged commit 28f2274 into master Dec 30, 2017
@konklone konklone deleted the fix-internal-links branch December 30, 2017 03:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants