-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: jsipfs ls -r (Recursive list directory) #1222
Conversation
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.
LGTM as long as CI is passing :) Thanks @JonKrone !!
@@ -11,6 +11,8 @@ module.exports = { | |||
|
|||
handler (argv) { | |||
let path = argv.key | |||
// `ipfs file ls` is deprecated. See https://ipfs.io/docs/commands/#ipfs-file-ls | |||
print(`This functionality is deprecated, and will be removed in future versions. If possible, please use 'ipfs ls' instead.`) |
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.
We should be suggesting files ls
, right? per ipfs/specs#98 (comment)
copied this from ipfs.io, will change.
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.
Still just ipfs ls
for now until both APIs are united. Currently, ipfs ls !== ipfs files ls
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.
Yeah, I thought so but was a bit confused here. Tracing code, I found that ipfs ls
is an alias of ipfs files ls
in js-ipfs/core -
Line 118 in 905bdc0
this.ls = this.files.lsImmutable |
There is also no /files/ls
route handler in js-ipfs and a jsipfs files ls
command with a daemon running triggers a js-ipfs-api /ls
, not /files/ls
, request. So there are a couple places where it seems ipfs ls == ipfs files ls
. I'm sure there's something that I'm missing, let's go over it Wednesday.
@JonKrone could you rebase master onto this branch for a clean merge (CI green)? |
…n to the ipfs.lsImmutable call instead of querying the server for each explored subdir.
…re related. Also need to understand mfs more, and differences between 'ipfs ls|file ls|files ls'
…hile I test master
83ac5d4
to
e20c9de
Compare
@JonKrone just released the js-ipfs-api update you needed. Once you update the deps here you should be all good :) |
…he test is cb or promise based?!
}) | ||
|
||
it('recursively follows folders, -r', function () { | ||
this.slow(2000) |
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.
@JonKrone what does this call do?
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.
got it, it tells mocha to inform us that it is slow after 2000ms.
* fix: change name-api examples to new api * fix: change files-api example to new api * fix: change upload example to new api * fix: change bundle-webpack to support new api * fix: remove browserify example doesnt support imports * fix: change pubsub example to the new api * Update examples/bundle-webpack/src/App.js Co-Authored-By: Alan Shaw <alan.shaw@protocol.ai> * Update examples/bundle-webpack/src/App.js Co-Authored-By: Alan Shaw <alan.shaw@protocol.ai> * Update examples/files-api/files-api.js Co-Authored-By: Alan Shaw <alan.shaw@protocol.ai> * Update examples/upload-file-via-browser/src/App.js Co-Authored-By: Alan Shaw <alan.shaw@protocol.ai> * Update examples/upload-file-via-browser/src/App.js Co-Authored-By: Alan Shaw <alan.shaw@protocol.ai> Co-authored-by: Alan Shaw <alan.shaw@protocol.ai>
Adds an -r flag to
ipfs ls
.This is just a minor change with a little bit of cleanup. Moves the ls cli tests to their own suite and prints a deprecation notice on
ipfs file ls
.I thought there might be useful information to show under an option like
ls -l
but I don't know of any information available that's important enough. I was at various times a little confused by the three ls commands,file ls
files ls
andls
, and how they differed and inherited so I hope I've consolidated them into a single impl.Requires a small change to
js-ipfs-api
: ipfs-inactive/js-ipfs-http-client#693related issue: https://github.com/ipfs/interface-ipfs-core/issues/92