search

mraleph / irhydra

Tool for displaying IR used by V8 and Dart VM optimizing compilers
Apache License 2.0
433 stars 32 forks source link

Add artifact loading from gists #43

Closed brendankenny closed 9 years ago

brendankenny commented 9 years ago

I've wanted to be able to share irhydra views via URL for some time. When I went in there today to see the feasibility of this, I noticed you had already added support for loading from a Drive account, so rather than filing an issue I just adapted that code to load from gists, as long as the gist has a hydrogen.cfg and a code.asm file.

Not sure if you had plans around loading files, and I haven't written any Dart in about 2 years, so feel free to close this PR without hurting my feelings :)

I put up a demo on my fork. The artifacts in https://gist.github.com/brendankenny/75d281c83d98d7f0e070 are loaded by adding #gist:brendankenny/75d281c83d98d7f0e070 to the URL. Demo Page

mraleph commented 9 years ago

That's an awesome sauce! Thanks. Quick question: are you sure the user name is needed? I think hash is enough to identify the gist.

brendankenny commented 9 years ago

Hmm, you may be right, but I can't get it to work with any combination of the parts of https://gist.githubusercontent.com/brendankenny/75d281c83d98d7f0e070/raw/code.asm Do you know what the URL pattern should be?

mraleph commented 9 years ago

Apparently https://gist.githubusercontent.com/raw/75d281c83d98d7f0e070/code.asm redirects to the right place.

I am not sure if HttpRequest correctly follows the redirect though

Vyacheslav Egorov

On Fri, Apr 3, 2015 at 11:01 AM, Brendan Kenny notifications@github.com wrote:

Hmm, you may be right, but I can't get it to work with any combination of the parts of https://gist.githubusercontent.com/brendankenny/75d281c83d98d7f0e070/raw/code.asm to work. Do you know what the URL pattern should be?

— Reply to this email directly or view it on GitHub https://github.com/mraleph/irhydra/pull/43#issuecomment-89228829.

mraleph commented 9 years ago

Confirmed HttpRequest.getString("https://gist.githubusercontent.com/raw/75d281c83d98d7f0e070/code.asm").then(print) works, so I think if you change the base URL to this I will merge the PR.

brendankenny commented 9 years ago

Good catch! I had no idea that worked.

Updated, PTAL. Github does a 302 redirect and still serves with Access-Control-Allow-Origin: *, so everything looks good with this.

mraleph commented 9 years ago

LGTM, thanks!

I will redeploy some time today.

mraleph commented 9 years ago

Redeployed.

brendankenny commented 9 years ago

Looks good, and the new loading toasts are great.

mraleph commented 9 years ago

Yeah, when I was testing I noticed that I did not use this toast on all code paths and even when I used it - it showed the wrong toast because I broke it at some point. Guh.

Testing... I totally need some tests set up fro this thing.