solidity icon indicating copy to clipboard operation
solidity copied to clipboard

Fix 'Open In Remix' in docs

Open Aniket-Engg opened this issue 2 years ago • 2 comments

  • [ ] Remix requires version in URL params with commit id like version=soljson-v0.8.7+commit.e28d00a7.js . Currently from Solidity docs, version is sent as version=0.8.15 which only set compiler to latest local version in Remix

  • [ ] From Yul docs here https://docs.soliditylang.org/en/v0.8.15/yul.html, if I try to open Erc20 Example in Remix (from the bottom of the docs), it show 413 error Screenshot 2022-07-26 at 1 43 51 PM

Aniket-Engg avatar Jul 26 '22 08:07 Aniket-Engg

Remix requires version in URL params with commit id like version=soljson-v0.8.7+commit.e28d00a7.js . Currently from Solidity docs, version is sent as version=0.8.15 which only set compiler to latest local version in Remix

Well, I intentionally did not include the commit part in the link because getting it into built docs is tricky. The documentation build process knows only the x.y.z part of the version string because that's what is in the repo. It does not have access to the whole list of published binaries and really should not depend on what's on that list. Maybe we could get the hash using git, assuming the build is always done inside a repo (might not always be the case), but I'm not even sure it's feasible to run things like git on ReadTheDocs from within Sphinx. Event then, there's a problem with identifying the commit. Only for releases it's the current one. Outside of that we'd have to find the commit based on tags and tags are not always available - for example in translation repositories tags are not present and when they are they actually indicate something different (the commit that contains the complete translation for a particular version).

So this is messy and I think a much better solution is to translate the short version like v0.8.15 into the full version with commit further down the stack. I actually submitted a feature request for that when I implemented the "open in Remix" links (https://github.com/ethereum/solidity/issues/11415#issuecomment-899702428), but I see it's still open: https://github.com/ethereum/remix-project/issues/1490.

Are there any concerns with that feature request? What do you think about it? I did not receive any feedback in the issue so far.

if I try to open Erc20 Example in Remix (from the bottom of the docs), it show 413 error

I think this is because the encoded example exceeds maximum URL length. That's one of the longer examples. I actually thought that it would be mostly a client-side concern (different browsers have different limits) but looks like Cloudflare has some server-side limit too.

The simplest way to "solve" it would be to remove the links above some specific length and that's easy but I didn't want to remove it for everyone just because some browsers are more strict than others about it. I guess if you're using Cloudflare then everyone has the same upper limit and we could lower it to that value at least.

A proper solution would require some changes on your side. Preferably some way to compress the example.

Alternatively, you could accept a POST request but that would also require a much more complex solution involving JS on our side (currently it's dead simple - everything happens in Python at build time and it's just a static link).

cameel avatar Jul 27 '22 19:07 cameel

Thanks for the explanation @cameel . I have added https://github.com/ethereum/remix-project/issues/1490 in the current project plan.

For that long URL, may be for now, you can just shorten this one specifically.

Aniket-Engg avatar Jul 28 '22 06:07 Aniket-Engg

To summarize what's left to do here:

  • Find out what the URL length limit in CloudFlare is
  • Set MAX_SAFE_URL_LENGTH to that limit: https://github.com/ethereum/solidity/blob/2ba8fc1db517f38cb0896ae3dc1870d6a151d424/docs/ext/remix_code_links.py#L7
  • Adjust the plugin to make it omit Remix links when URL length exceeds MAX_SAFE_URL_LENGTH. Currently it issues a warning, which makes docs build fail (due to warnings being treated as errors). https://github.com/ethereum/solidity/blob/2ba8fc1db517f38cb0896ae3dc1870d6a151d424/docs/ext/remix_code_links.py#L59-L65 We should keep the warning but change it to info so that the build does not fail.

cameel avatar Oct 04 '22 14:10 cameel

@r0qs I think this would also be a task for you.

cameel avatar Oct 04 '22 14:10 cameel

Screenshot 2023-05-22 at 13 01 52 I can safely open the ERC20 example on Remix.

NunoFilipeSantos avatar May 22 '23 12:05 NunoFilipeSantos