-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Add JavaScript-based redirects to supplement Read the Docs redirects #11304
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
Add JavaScript-based redirects to supplement Read the Docs redirects #11304
Conversation
2bb8148 to
07f0af0
Compare
07f0af0 to
1140dca
Compare
This is addressed in #11315.
I think the easiest solution might be to place |
Wouldn't that break RTD redirects though? We still want them to work when possible to reduce the reliance on JavaScript, as it's less optimal. Edit: Nevermind, this is handled by https://github.com/godotengine/godot-docs/blob/master/_tools/redirects/create_redirects.py rather than CSV file itself. Edit 2: Done. The redirects CSV is now in |
01ce012 to
1db2aaa
Compare
|
Awesome! Does that leave the PR in a reviewable / potentially mergable state? |
1db2aaa to
66f44d7
Compare
|
I've added support for redirecting to absolute URLs. Note that the existing |
Ah, dang, sorry. I think it's fine to add them here. |
66f44d7 to
1931017
Compare
Done 🙂 |
mhilbrunner
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Would be good to add a comment to create_redirects.py that it is now obsolete, pointing to custom.js instead.
Will test this a bit.
05442c0 to
bc126e1
Compare
Read the Docs redirects don't reliably apply when there is a large amount of redirects defined. While this JavaScript-based solution is not as seamless, it has no limit on how many redirects can be defined.
bc126e1 to
e69b689
Compare
|
Alright, lets try this. Thanks both of you! We'll see how this goes. |
|
Cherrypicked to 4.5 in #11448. |
Read the Docs has an upper limit on the number of redirects that canapply. This limit is exceeded by the current
redirects.csv,which means many redirects don't apply correctly.While this JavaScript-based solution is not as seamless from the user's perspective, it has no limit on how many redirects can be defined.
To test this PR, build the website locally and run
simple-http-server --try-file 404.htmlin_build/html. It's important to instruct the local web server to use404.htmlas a custom 404 page, or you won't be able to test this.Note that
redirects.csvisn't modified by this PR yet, so this won't perform all redirects from the previouscontributingsection tocontributing.godotengine.org.We'll also probably want to change the redirects CSV URL to match the current branch in the long run. I'm not sure how we should do this in a way that keeps working with local web servers (which may not have
/latest,/stableor/4.5in the URL). Maybe we should just modifycustom.jsfor each branch?