-
Notifications
You must be signed in to change notification settings - Fork 325
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.
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
Adds self served directory listings #323
Conversation
add-on/_locales/en/messages.json
Outdated
@@ -36,7 +36,7 @@ | |||
"description": "A label in Node status section of Browser Action pop-up" | |||
}, | |||
"panel_quickUpload": { | |||
"message": "Quick Upload", | |||
"message": "Share files via IPFS", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I must say, it is a much better name for this feature 👍
package.json
Outdated
@@ -69,10 +69,16 @@ | |||
}, | |||
"dependencies": { | |||
"choo": "6.6.0", | |||
"doc-sniff": "^1.0.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fyi I've found two ways to avoid the chore of removing ^
from package.json
- while doing
npm install
of new dependency, one can do it with--save --save-exact
- to make it default behaviour globally:
npm config set save-exact true
960109d
to
a1f8cfe
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dir listing is super neat ✨
As a note to self.... We'll want to extract the trailing slash normalisation logic soon, as it causes issues when trying to render webpages with relative links. I'll raise seperate issue for that.
@lidel when you get a moment to look at this pr, this compare view to just check the changes without all of #320 in too, is helpful: tableflip/ipfs-companion@84b2ada...tableflip:feat/dir-view#diff-ca7aa692aef78b2031d7a5da30e03248R10 |
try { | ||
data = await ipfs.files.cat(path) | ||
} catch (err) { | ||
if (err.message.toLowerCase() === 'this dag node is a directory') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ouch, is this the only way we can detect directories?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I probably should have explained this a little...here goes:
It's similar to how the js-ipfs http gateway works. It tries to "resolve" a path (e.g. /ipfs/$hash/path/to/a/file) by using ipfs.dag.get
to walk the path and eventually find the hash of the file at the end. It uses Unixfs.unmarshal
on the end file to detect a directory and will raise an error if so "this dag node is a directory". If this error occurs then it'll try to get the links using ipfs.object.get
and render a directory view.
Instead of walking the path using ipfs.dag.get
(which isn't available in js-ipfs-api) we use ipfs.files.cat
which has the advantage of being able to pass a path to it (so the walking is done for us) and is what we'd call to get file content anyway. It also returns the same error - "this dag node is a directory" for directories. We use ipfs.ls
instead of ipfs.object.get
because:
ipfs.files.cat
resolved the hash but didn't return it to us so we can't useipfs.object.get
anyway since it needs a hash- We can pass a path to
ipfs.ls
and it'll give us the links
Is there a better way? @diasdavid
@alanshaw If you resolve conflict with master, I think we can merge this before it gets cold. 👍 |
🌟 |
\o/ Directory viewer in Brave for handled
ipfs://
requests!