-
Notifications
You must be signed in to change notification settings - Fork 296
WIP cat interface core #280
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,7 +1,32 @@ | ||
'use strict' | ||
|
||
const argCommand = require('../cmd-helpers').argCommand | ||
const bs58 = require('bs58') | ||
const isIPFS = require('is-ipfs') | ||
const promisify = require('promisify-es6') | ||
|
||
module.exports = (send) => { | ||
return argCommand(send, 'cat') | ||
const cat = promisify((multihash, callback) => { | ||
try { | ||
multihash = cleanMultihash(multihash) | ||
} catch (err) { | ||
return callback(err) | ||
} | ||
send('cat', multihash, null, null, function (err, result) { | ||
if (err) { | ||
return callback(err) | ||
} | ||
return callback(null, result) | ||
}) | ||
}) | ||
return cat | ||
} | ||
|
||
function cleanMultihash (multihash) { | ||
if (!isIPFS.multihash(multihash)) { | ||
throw new Error('not valid multihash') | ||
} | ||
if (Buffer.isBuffer(multihash)) { | ||
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. Isn't input allowed to be a Buffer containing a base58-encoded multihash? If so, this would double b58-encode it. 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. input is allowed to be a buffer for js-ipfs core, but the go-ipfs and js-ipfs-api could only take bs58 encoded strings of multihashes so i use this check to make sure that if a buffer multihash is supplied, then it is converted to a format everyone can use, bs58 encoded string. It shouldn't double multihash 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 linked to doc says "the raw Buffer of the multihash (or of and encoded version)". Did I misunderstand what "of the encoded version" means? It means "base58 encoded", right? Underyling assumption: this 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 think the encoding mention on raw Buffer is base58, this is borrowed from the object.get definition of a multihash input. The string mentioned after it is the base58 encoded multihash
Right, this upgrade to cat will now take a buffer input so that it is interop with interface-core 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. Maybe I've misunderstood? In "the raw Buffer of the multihash (or of encoded version)" -- what encoding does "the raw Buffer" have vs "encoded version"? 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've updated the readme in interface-core, hopefully that simplifies the statement. A buffer is just a node.js default utf-8 buffer of a multihash, and a 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. YES SO AWESOME |
||
return bs58.encode(multihash) | ||
} | ||
return multihash | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
/* eslint-env mocha */ | ||
/* globals apiClients */ | ||
|
||
'use strict' | ||
|
||
const test = require('interface-ipfs-core') | ||
|
||
const common = { | ||
setup: function (cb) { | ||
cb(null, apiClients.a) | ||
}, | ||
teardown: function (cb) { | ||
cb() | ||
} | ||
} | ||
|
||
test.files(common) |
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 think this is defined in
src/api/object.js
too. Could you toss it into a module insrc/
?