Skip to content
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.

Sign up for GitHub

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

Jump to bottom

WIP: rewriting ipfs uris to fs: prefix and linkifying inline urls #35

Closed
wants to merge 1 commit into from
Closed

WIP: rewriting ipfs uris to fs: prefix and linkifying inline urls #35

wants to merge 1 commit into from

Conversation

the8472
Copy link
Contributor

@the8472 the8472 commented Dec 21, 2015

Very experimental for now, but I can rewrite pretty much everything to fs:/ URIs while doing http in the background.

lidel
lidel reviewed Dec 23, 2015
View reviewed changes
lib/redirects.js

//console.log("comp", ipfsURI.spec, channel.originalURI.spec, ipfsURI.spec == channel.originalURI.spec)

if(prefs.useCustomGateway || !channel.originalURI || channel.originalURI.spec != ipfsURI.spec ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

redirects.js:30: we probably want to use gw.isEnabled() here as ReferenceError: prefs is not defined (I got this error while testing b5d785b)

@lidel
Copy link
Member

lidel commented Dec 23, 2015

I understand it is work in progress and some things do not work yet, but it looks quite cool.
I really like the canonization idea (forwarding ipfs:, ipns: and other custom protocol handlers to fs: one).

Keep up good work 👍

Some random bugs:

  • when rewriteAs=fs:
    • ipfs:///QmTAsnXoWmLZQEpvyZscrReFzqxP3pvULfGVgpJuayrp1w redirects to fs:/ipfs/QmTAsnXoWmLZQEpvyZscrReFzqxP3pvULfGVgpJuayrp1w (OK) but ipfs://QmTAsnXoWmLZQEpvyZscrReFzqxP3pvULfGVgpJuayrp1w redirects to fs:/ipfs/ (BUG)
  • when rewriteAs=gateway (Rewrite IPFS URIs in the UI as Public Gateway) the redirect to the custom gateway does not seem to work -- it is probably caused by WIP: rewriting ipfs uris to fs: prefix and linkifying inline urls #35 (diff)

@lidel lidel mentioned this pull request Jan 7, 2016
Closed
@lidel lidel mentioned this pull request Jan 23, 2016
Closed
@lidel lidel force-pushed the master branch 3 times, most recently from 963e9f0 to 7d4c0a2 Compare January 30, 2016 21:35
@lidel
Copy link
Member

lidel commented Feb 6, 2016

@the8472 noot noot 🐧, are you still working on this? A lot of code changed since December 😢
I'll understand if you lost motivation, just let me know so that I can move #48 forward without duplicating our efforts.

@the8472
Copy link
Contributor Author

the8472 commented Feb 7, 2016

Yeah, still on it.
Specifically I have a cut-down version that only works in non-e10s, doesn't try to do any of the URI fixups and doesn't rewrite http -> fs either. In other words it only keeps the fs: scheme if you're opening a fs: uri.

If you want to I can make a new PR for that. e10s support and the other things could be built on top of it.

@lidel
Copy link
Member

lidel commented Feb 7, 2016

I could merge new PR with cut-down version, but only if it comes with e10 support (otherwise we have a regression).

If it is not something you can easily add, then we can wait for the full solution, no problem. 👍

@the8472
Copy link
Contributor Author

the8472 commented Feb 11, 2016

Obsoleted by #70

@the8472 the8472 closed this Feb 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@the8472 @lidel