add: functionality to add code from url and github gists

[henilp105]

Implments #78 #81 functionality to directly add code from a url or a github gist into the frontend.

proposed functionality:

To support github gists
http://localhost:3000/?github=certik/b6d162bee7fdd7711722221a90729a2a

To support code in url (similar to fortran playground)
code must be URI encoded to retain the format of the code.

http://localhost:3000/?code=program%20hello%0A%20%20!%20This%20is%20a%20comment%20line%3B%20it%20is%20ignored%20by%20the%20compiler%0A%20%20print%20*%2C%20'Hello%2C%20World!'%0Aend%20program%20hello%0A

Thanks and Regards,
Henil Panchal

CC: @certik @Shaikh-Ubaid

[certik]

Beautiful, great job @henilp105 !

Let me try to push from my account so that we get the preview. Done: #96 (comment)

@Shaikh-Ubaid, the link in that comment does not work, it says "404 Page not found". Any idea why?

[Shaikh-Ubaid]

it says "404 Page not found". Any idea why?

Looking into it.

[certik]

We probably also want to show the error using the nextjs popup, so that the user knows that something went wrong.

[certik]

Also, if the user supplies anything else than code or gist, let's give an error message?

[henilp105]

sure, Thanks @certik.

[Shaikh-Ubaid]

Also, if the user supplies anything else than code or gist, let's give an error message?

With the latest updates, I think we probably do not catch error for params other than code, gist. I think we can tackle it in this PR or we can support it separately. Any approach seems fine to me.

[Shaikh-Ubaid]

The github deployment job https://github.com/lfortran/pull_request_preview/actions/runs/6017281074/job/16323524902 does not seem to succeed. Seems like an issue at GitHubs end.

[Shaikh-Ubaid]

The particular github deployment job https://github.com/lfortran/pull_request_preview/actions/runs/6017281074/job/16323524902 at pull_request_preview seems to fail.

I submitted a dummy PR #97 to trigger the CI to build and check if it works. The CI passed there. Consequently the CI build for https://lfortran.github.io/pull_request_preview/lfortran/96/ also got rerendered (since every PR shares the same project/repository and we build the whole pull_request_preview directory at the CI.) The site at https://lfortran.github.io/pull_request_preview/lfortran/96/ now seems to work.

[certik]

I think the website is way too big to deploy, by the nature of just adding to it (probably 5MB per deploy). I think we just need to force push into the deploy repository and make it smaller.

[certik]

This seems to almost work: https://lfortran.github.io/pull_request_preview/lfortran/96?github=certik/b6d162bee7fdd7711722221a90729a2a, the right panel never loads, but that might be an unrelated bug.

[certik]

I created a new gist with the correct code. It still does not load correctly:

https://lfortran.github.io/pull_request_preview/lfortran/96?github=certik/646df20112d01abee7250e0bafeddd35

But if you copy & paste this code into dev.lfortran.org, then it works. So I think there is a bug somewhere that we need to fix.

[Shaikh-Ubaid]

The changes seem to work locally for me.

This seems to almost work: https://lfortran.github.io/pull_request_preview/lfortran/96?github=certik/b6d162bee7fdd7711722221a90729a2a, the right panel never loads, but that might be an unrelated bug.

Yes, it is unrelated. The links to lfortran.* do not point at the right place. I think/guess the issue could be here

lcompilers_frontend/components/LoadLFortran.js

Line 176 in f625905

<Script src={`./lfortran.js`} onLoad={setupLFortran}></Script>

[certik]

If it works locally, but not in the test deployment, what does that imply for production deployment --- will it work, or not?

I would fix it to work in test deployment, then we have a robust testing approach.

[certik]

Unrelated question: I thought you are not allowed to load data from a different domain? We host at lfortran.org or at github.io, but we load data from gist.github.com, which is a different domain. Isn't this Cross-Origin Resource Sharing (CORS), which is not allowed?

[Shaikh-Ubaid]

If it works locally, but not in the test deployment, what does that imply for production deployment --- will it work, or not?

I think it would work in the production deployment. But as you shared we should ideally fix the test deployment first.

[Shaikh-Ubaid]

Unrelated question: I thought you are not allowed to load data from a different domain?

From #81 (comment),

Some sites/APIs allow Cross Origin Resource Sharing, while many (in my experience) do not allow. In the above case, it seems that the GitHub API is CORS Enabled (thus allowing data to be fetched from client browser, probably using some authorization token)

It seems Github Pages or github.io or https://gist.githubusercontent.com allow CORS (we also know this from here #24 (comment)).

[Shaikh-Ubaid]

With respect to this PR or the issues it addresses,

The concern in implementing #78 (using a code in the url) is that we cannot share code of size more than (I guess around) 2000 characters or around 1.4KB in size. That is, we cannot share many code examples like the originial mandelbrot code, you shared you had to minify it to fit in 2000 characters or 1.4KBs. Thus this method does not seem super useful.

To overcome the restriction in the above sharing code in the url approach, we planned of another approach #81 where we would supply the url of the fortran file. The concerns here are:

• It works with gist (and might work with github files) but is not universal. For example, we might not be able to load fortran files at https://web.stanford.edu/class/me200c/f77_routines/ (I did not check it but generally fetching files from any random websites is not allowed by browsers due to CORS policy.)
• While discussing about Support lfortran.wasm and lfortran.js of different commits #24 (comment) (in one of the meets) you shared that github (or github.io) for now supports CORS, but we do not yet know if/when it would be stopped. Thus, this approach of fetching fortran files does not seem to be robust.

[certik]

It looks like github supports CORS for the github pages and gist. So I would support it. If they remove the support, then it will break of course. But is there an alternative?

I agree sharing via the URL is limited to very short codes, but might be helpful for short snippets, like some calculator style "print *, sin(0.5)".

So let's try both the URL and gist, as long as it is implemented nicely, and see how it goes.

Let's however first fix the test deployment, so that we can ensure it actually works.

[Shaikh-Ubaid]

But is there an alternative?

There can probably be a Third-Party API that allows fetching of files from websites. We need to explore. (It could come with limits, but they should be such that they are sufficient for our use case.)

[Shaikh-Ubaid]

Hey Henil,

Thank you so much for the PR. Your initiative and willingness to contribute are highly appreciated and truly embody the spirit of open-source collaboration.

I noticed that there was a discussion thread at #81 related to the changes you've incorporated into the pull request. While there might not have been direct participation in the discussion, I'd like to encourage you to consider engaging with the community in such conversations. Your insights and perspective are invaluable, and your involvement can lead to even more effective solutions.

On a separate note, I'd like to suggest using a distinct branch for future pull requests. While this isn't a strict requirement, it can help prevent any conflicts during the development and review process. Personally, I encountered a situation where I had to rename my local 'main' branch to fetch your main branch avoiding any conflicts.

This PR looks good to me and there are some minor comments/enhancements above. Thank you once again for your commitment to enhancing our project. Your contributions are making a positive impact. Keep up the great work!

Warm regards,
Ubaid

[Shaikh-Ubaid]

It works with gist (and might work with github files)

It seems we can also support fetching files from GitHub repositories (https://stackoverflow.com/a/63131670). We might need to process/replace the url provided by the user to point to raw.githubusercontent.com.

[certik]

Great. Let's just do the bare minimum to merge this PR, I think that's a good start. Then we can improve upon it.

[henilp105]

Thanks @certik @Shaikh-Ubaid for the review, I am really grateful for the help, I would apologise for the late response due to time difference.

There can probably be a Third-Party API that allows fetching of files from websites. We need to explore. (It could come with limits, but they should be such that they are sufficient for our use case.)

The github supports CORS on the raw files and gists, I had checked for the github api but it would require 3 API calls to fetch the code with the Github Token ( on client side) , instead of our current single API call and many 3rd party APIs would require the auth token on the client side.

@Shaikh-Ubaid have implemented the required changes, but I am having some difficulty to call handleUserTabChange("STDOUT") , to run the code snippets after loading them.

I would surely make a new branch for the upcoming PRs.

Regarding the CI failure , it seems that you would have set the time limit for a CI to 10mins, for the preview you may increase to prevent the timeout in the CI ( it seems to be a likely cause, as the CI crash was only in the deploy phase).

Thanks and Regards,
Henil

[certik]

Thanks @henilp105. Yes, it seems the easiest is to use CORS as you did. If GitHub removes it, we'll have to find another solution that might require the user to login, which is less ideal. Until then let's use this.

@Shaikh-Ubaid how difficult would be to fix the testing deployment?

[Shaikh-Ubaid]

We have the preview at https://lfortran.github.io/pull_request_preview/lfortran/96

It seems when no url param is passed, the user gets the notification "Unknown parameter Supplied, loading default code." which probably is not needed. In this case, the user might be trying to use the site interactively without any preloading code. Also, I guess we should show the notification pop only when the fetch failed. A popup probably need not be displayed when it succeeds, since the user can already verify based on the updated source code present in the editor.

[Shaikh-Ubaid]

Henil, it seems the code in this PR currently uses tabs with spaces of 2, while the rest of the code base, I think uses tabs of spaces 4. It would be great if we can follow a consistent tab space as the rest of the code base.

[Shaikh-Ubaid]

@Shaikh-Ubaid have implemented the required changes, but I am having some difficulty to call handleUserTabChange("STDOUT") , to run the code snippets after loading them.

For now, the following diff seems to make the loaded source code to run and/or obtain output. I am unsure if it is robust (probably not), because it works assuming that the fetching of the external file will be earlier that the loading of the lfortran module. But I think if the loading of lfortran took place earlier compared of the fetching of external file (for example if the external file is large is size), then I think the fetched external file would not be run. That is the output shown would be the one for the previous source code. In this case, the user would need to explicitly press the run button to execute his fetched code. I think to solve this issue we might probably need another useState variable which stores whether external file fetching is complete and based on this we can do the call to handleUserTabChange(). I guess we can fix this in subsequent PRs.

diff --git a/pages/index.js b/pages/index.js
index f490122..beeb657 100644
--- a/pages/index.js
+++ b/pages/index.js
@@ -49,18 +49,14 @@ export default function Home() {
 
     const myHeight = ((!isMobile) ? "calc(100vh - 170px)" : "calc(50vh - 85px)");
 
-    useEffect(() => {
-        if(moduleReady) { fetchData();  handleUserTabChange("STDOUT"); }
-      }, [moduleReady]);
-
     async function fetchData() {
         const url = window.location.search;
         const gist = "https://gist.githubusercontent.com/";
         const urlParams = new URLSearchParams(url);
@@ -146,6 +141,14 @@ export default function Home() {
         setActiveTab(key);
     }
 
+    useEffect(() => {
+        fetchData();
+    }, []);
+
+    useEffect(() => {
+        if(moduleReady) { handleUserTabChange("STDOUT"); }
+      }, [moduleReady]);
+
     return (
         <>
             <LoadLFortran
(END)

[henilp105]

Thanks @Shaikh-Ubaid , I have implemented the required changes.

[Shaikh-Ubaid]

Great work, Henil! Thank you so much! I really appreciate it. I left minor comments.

[henilp105]

Thanks a lot @Shaikh-Ubaid , I am really grateful for the help.

[Shaikh-Ubaid]

The preview is available at https://lfortran.github.io/pull_request_preview/lfortran/96/. It seems fine to me.

[Shaikh-Ubaid]

This looks good to me. Thank you so much for this @henilp105!

[Shaikh-Ubaid]

@certik we should probably squash merge this PR.

[certik]

Awesome, I played with it. I discovered the following issues:

• I used the old link https://lfortran.github.io/pull_request_preview/lfortran/96?github=certik/7e2652943bbff7f0d0963dd4fcf1813a, and I would expect it to give an error message, but it just got ignored; Give an error message for incorrect url #100
• The new link works: https://lfortran.github.io/pull_request_preview/lfortran/96?gist=certik/7e2652943bbff7f0d0963dd4fcf1813a
• It first loads the mandelbrot example, then it replaces it with the gist. I would expect it to show blank text editor (not mandelbrot), then load it with gist; Loading gist should not load the default mandelbrot first #101
• When I updated a gist, it still loads the old gist. Here is an example: https://lfortran.github.io/pull_request_preview/lfortran/96?gist=certik/b6d162bee7fdd7711722221a90729a2a, in a private window it loads the new gist, but in the old window it loads the old gist. Updated gist sometimes loads the old one #102
• Also I noticed sometimes it loads the new gist, but it does not run it. I have to manually click on Run. Gist is not always run automatically #99

I am fine to fix these in subsequent PRs, but we should make this facility more robust. It's nice, very usable. I expect this to be frequently used to share user code, say on the Fortran Discourse.

[Shaikh-Ubaid]

Also I noticed sometimes it loads the new gist, but it does not run it. I have to manually click on Run.

I think/guess this happens due to #94 (comment).

[certik]

@Shaikh-Ubaid go ahead and merge this PR (after polishing the history if needed). We can iteratively improve upon it in subsequent PRs.

[certik]

Thanks. I opened up dedicated issues for the above problems.