Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

updated cat core to return just file stream #253

Closed
wants to merge 4 commits into from

Conversation

nginnever
Copy link
Member

This came from conversation over the interface-core cat api talks PR.

This changes the core files cat command to return just the stream part of the unixfs-engine.exporter

This also updates the cat cli, http-api, and tests to respect the change.

@jbenet jbenet added the status/in-progress In progress label May 21, 2016
@@ -45,7 +45,9 @@ module.exports = function files (self) {
callback('This dag node is a directory', null)
} else {
const exportStream = Exporter(hash, self._dagS)
callback(null, exportStream)
exportStream.on('data', (object) => {
Copy link
Member

Choose a reason for hiding this comment

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

s/on/once, so that the listener is cleared after

@daviddias
Copy link
Member

daviddias commented May 21, 2016

Missing

  • Use tests from interface-ipfs-core

@dignifiedquire dignifiedquire mentioned this pull request May 21, 2016
@nginnever
Copy link
Member Author

This needs PR merge for this to pass tests.


module.exports = function files (self) {
return {
add: (arr, callback) => {
add: promisify((arr, cb) => {
Copy link
Member

Choose a reason for hiding this comment

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

our pattern is to use callback as the most top level callback, so that it is easy distinguishable when we jump out the func

Copy link
Member

@dignifiedquire dignifiedquire May 22, 2016

Choose a reason for hiding this comment

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

that's just @diasdavid s pattern 😛

Copy link
Member Author

Choose a reason for hiding this comment

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

haha to callback or cb, i'll switch back to callback here

@nginnever
Copy link
Member Author

nginnever commented May 23, 2016

Closing in favor of PR

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants