-
Notifications
You must be signed in to change notification settings - Fork 1.2k
files add, cat, get core + cli offline #193
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,16 @@ | ||
'use strict' | ||
|
||
const Command = require('ronin').Command | ||
const IPFS = require('../../../core') | ||
const utils = require('../../utils') | ||
const debug = require('debug') | ||
const log = debug('cli:version') | ||
log.error = debug('cli:version:error') | ||
const bs58 = require('bs58') | ||
const Readable = require('stream').Readable | ||
const fs = require('fs') | ||
const async = require('async') | ||
const pathj = require('path') | ||
const glob = require('glob') | ||
|
||
module.exports = Command.extend({ | ||
desc: 'Add a file to IPFS using the UnixFS data format', | ||
|
@@ -19,15 +24,71 @@ module.exports = Command.extend({ | |
}, | ||
|
||
run: (recursive, path) => { | ||
var node = new IPFS() | ||
path = process.cwd() + '/' + path | ||
node.files.add(path, { | ||
recursive: recursive | ||
}, (err, stats) => { | ||
let rs | ||
|
||
if (!path) { | ||
throw new Error('Error: Argument \'path\' is required') | ||
} | ||
|
||
var s = fs.statSync(path) | ||
|
||
if (s.isDirectory() && recursive === false) { | ||
throw new Error('Error: ' + process.cwd() + ' is a directory, use the \'-r\' flag to specify directories') | ||
} | ||
if (path === '.' && recursive === true) { | ||
path = process.cwd() | ||
s = fs.statSync(process.cwd()) | ||
} else if (path === '.' && recursive === false) { | ||
s = fs.statSync(process.cwd()) | ||
if (s.isDirectory()) { | ||
throw new Error('Error: ' + process.cwd() + ' is a directory, use the \'-r\' flag to specify directories') | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the goal of this chunk of code? Can it be extracted to a function with a good name? At first glance seems madness to me :D There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's to check for all of the inputs you could give to add command. variations i could think of are...
I could make a checkpath() function but i'm not sure how that would simplify any of the logic There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Creating a function and adding that as a comment describing what it is doing would be considerably much better. |
||
|
||
glob(pathj.join(path, '/**/*'), (err, res) => { | ||
if (err) { | ||
return console.log(err) | ||
throw err | ||
} | ||
console.log('added', bs58.encode(stats.Hash).toString(), stats.Name) | ||
utils.getIPFS((err, ipfs) => { | ||
if (err) { | ||
throw err | ||
} | ||
const i = ipfs.files.add() | ||
i.on('data', (file) => { | ||
console.log('added', bs58.encode(file.multihash).toString(), file.path) | ||
}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You never listen for the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What would I need to do when it is complete? It's just printing the responses until there is nothing left to print There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The process should exit gracefully |
||
if (res.length !== 0) { | ||
const index = path.lastIndexOf('/') | ||
async.eachLimit(res, 10, (element, callback) => { | ||
rs = new Readable() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can declare There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 80:11 error 'rs' is not defined no-undef There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah you will have to define it again on line 83 here, remember let and const are block scoped There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. doing that will give a lint error about defining twice, the only reason i moved those decelerations to the top was to avoid lint errors |
||
const addPath = element.substring(index + 1, element.length) | ||
if (fs.statSync(element).isDirectory()) { | ||
callback() | ||
} else { | ||
const buffered = fs.readFileSync(element) | ||
rs.push(buffered) | ||
rs.push(null) | ||
const filePair = {path: addPath, stream: rs} | ||
i.write(filePair) | ||
callback() | ||
} | ||
}, (err) => { | ||
if (err) { | ||
throw err | ||
} | ||
i.end() | ||
}) | ||
} else { | ||
rs = new Readable() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. these 'forced' reuse of variables feels weird. A variable named should be reused if several points in the code will look at it and react to its change. If you instantiate a new object, but then have code that treats that object specifically, no need to reuse variables. (remember that in JS, each time you reassign a variable, you give V8 an headache :) ) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't fully understand what you mean. Will try to ponder on this a bit. The decelerations come in different parts of the logic. Or there shouldn't be a time when the if statement is satisfied for both conditions meaning that it is necessary to instantiate 'rs' in both logic choices. Let me know if I am missing something here. If you are talking about |
||
const buffered = fs.readFileSync(path) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why reading the file all together and shuve it on a readable stream, when you can |
||
path = path.substring(path.lastIndexOf('/') + 1, path.length) | ||
rs.push(buffered) | ||
rs.push(null) | ||
const filePair = {path: path, stream: rs} | ||
i.write(filePair) | ||
i.end() | ||
} | ||
}) | ||
}) | ||
} | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
'use strict' | ||
|
||
const Command = require('ronin').Command | ||
const debug = require('debug') | ||
const utils = require('../../utils') | ||
const log = debug('cli:files') | ||
log.error = debug('cli:files:error') | ||
|
||
module.exports = Command.extend({ | ||
desc: 'Download IPFS objects', | ||
|
||
options: {}, | ||
|
||
run: (path, options) => { | ||
if (!path) { | ||
throw new Error("Argument 'path' is required") | ||
} | ||
if (!options) { | ||
options = {} | ||
} | ||
utils.getIPFS((err, ipfs) => { | ||
if (err) { | ||
throw err | ||
} | ||
ipfs.files.cat(path, (err, res) => { | ||
if (err) { | ||
throw (err) | ||
} | ||
if (res) { | ||
res.on('file', (data) => { | ||
data.stream.pipe(process.stdout) | ||
}) | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if not There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it will fail silently? not sure what you want me to do here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are return statements the proper way to exit gracefully? |
||
}) | ||
}) | ||
} | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
'use strict' | ||
|
||
const Command = require('ronin').Command | ||
const debug = require('debug') | ||
const utils = require('../../utils') | ||
const log = debug('cli:files') | ||
log.error = debug('cli:files:error') | ||
var fs = require('fs') | ||
const pathj = require('path') | ||
|
||
module.exports = Command.extend({ | ||
desc: 'Download IPFS objects', | ||
|
||
options: {}, | ||
|
||
run: (path, options) => { | ||
let dir | ||
let filepath | ||
let ws | ||
|
||
if (!path) { | ||
throw new Error("Argument 'path' is required") | ||
} | ||
if (!options) { | ||
options = {} | ||
dir = process.cwd() | ||
} else { | ||
if (options.slice(-1) !== '/') { | ||
options += '/' | ||
} | ||
dir = options | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if |
||
} | ||
|
||
utils.getIPFS((err, ipfs) => { | ||
if (err) { | ||
throw err | ||
} | ||
ipfs.files.get(path, (err, data) => { | ||
if (err) { | ||
throw err | ||
} | ||
data.on('file', (data) => { | ||
if (data.path.lastIndexOf('/') === -1) { | ||
filepath = data.path | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. only needed in here, no need to be declared at the top level There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. linter: |
||
if (data.dir === false) { | ||
ws = fs.createWriteStream(pathj.join(dir, data.path)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. only needed in here, no need to be declared at the top level There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 60:13 error 'ws' used outside of binding context block-scoped-var There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. one scope up sorry, |
||
data.stream.pipe(ws) | ||
} else { | ||
try { | ||
fs.mkdirSync(pathj.join(dir, data.path)) | ||
} catch (err) { | ||
throw err | ||
} | ||
} | ||
} else { | ||
filepath = data.path.substring(0, data.path.lastIndexOf('/') + 1) | ||
try { | ||
fs.mkdirSync(pathj.join(dir, filepath)) | ||
} catch (err) { | ||
throw err | ||
} | ||
ws = fs.createWriteStream(pathj.join(dir, data.path)) | ||
data.stream.pipe(ws) | ||
} | ||
}) | ||
}) | ||
}) | ||
} | ||
}) |
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.
use
path
for the name of the variable that contains the module, and use another keyword for the argument path. So that we keep the usage of the module consistent