Automatic redirection of old-type URLs #5056
Conversation
|
A critical and unhandled exception has occurred: |
|
There's the bug again. Can ethereum/EIP-Bot#63 be fixed sooner rather than later? Active editor CC: @lightclient, @MicahZoltu, @SamWilsn |
|
Huh, I did not expect that to work first try. I am pleasantly surprised. Tests: |
For greater forwards compatibility
There is no longer a mistake in the regex
|
I'm temporarily changing the GitHub pages branch to #5055 to debug why the LICENCE file isn't working. |
|
This feels like unnecessary complexity. I think EIP-20 is the only EIP that would match the regex, and no one should be using that link anymore. I would rather just set an EOL on the mis-named EIP-20 and delete it once we reach that EOL. |
|
Is there any harm in merging this? It's not adding too much additional complexity (6 lines of pretty understandable JS code, which are now commented). The rationale for this is so that we can remove the EIP 20 href, but still have the links redirect. This also has the advantage of also redirecting links that mistakenly add Again, for 6 lines of code, I think it's worth it (to me, it seems to be a no-brainer). I understand the EIP editors may disagree. so I will leave this open until there is consensus. |
Is this a common problem? Do we have examples of live links that include While I agree that 6 lines isn't much, many instances of "just 6 lines" can really add up in a project over time and that can turn into technical debt and bit rot. In general, I think that any feature addition, even if it is a single character code change, must reach a certain bar of necessity before being included in a project and to me it feels like this particular issue doesn't quite meet that bar. |
It fixes the second typo too. If the user puts I doubt it's a common problem. I very much don't think my code will contribute to code rot at all (it's pretty well documented), but I can add another comment. Editor CC: @lightclient @SamWilsn @axic @gcolvin |
| @@ -2,6 +2,15 @@ | |||
| layout: default | |||
| --- | |||
|
|
|||
| <script> | |||
| let thePath = window.location.pathname; // Fetches the path (e.g. "/EIPS/eip-20-token-standard" or "/core") | |||
| let theRegex = /^(.*\/EIPS\/eip-\d+)[^\d\s\/][^\/]*$/gm; // This regex matches the EIP path (e.g. "/EIPS/EIP-20") if the original path isn't valid | |||
There was a problem hiding this comment.
While I don't think this is exploitable (because you use pathname), I am uncomfortable with a redirect to a location in the URL.
|
@Pandapip1 thank you for this contribution! I think it is a good idea, maybe something to consider if we move the ERCs out of this repository. However, it doesn't seem terribly important given the current state of the repo. As most editors are against the change, I'm going to close this PR. |
Fixes #5054, #5052