cli - get - fix formatting when piping out

[kumavis]

Fixes #897

[kumavis]

@diasdavid heres a fix -- seems like circle tests failed due to unrelated chrome upgrade failure

[daviddias]

thanks @kumavis! Yeah circle has been doing that, we haven't had the time to investigate, but it is just how it is installing chrome.

Going to wait for CI on this one.

[kumavis]

the test failure is interesting
it expects there to be a newline but there is not -- however the source file contains a new line so i would expect that to be there

[kumavis]

ill compare against go-ipfs

[kumavis]

 ✘  user@work  ~/dev/js-ipfs   cli-block-get-pipe-fix ±
 ⧖  node -p -e "require('fs').readFileSync('test/test-data/hello')"
<Buffer 68 65 6c 6c 6f 20 77 6f 72 6c 64 0a>

 user@work  ~/dev/js-ipfs   cli-block-get-pipe-fix ±
 ⧖  cat test/test-data/hello| hexdump                            
0000000 6568 6c6c 206f 6f77 6c72 0a64          
000000c

my brain just exploded

[kumavis]

TIL hexdump respects machine endianness in its two-byte display

this is better

user@work  ~/dev/js-ipfs   cli-block-get-pipe-fix ±
 ⧖  cat test/test-data/hello| hexdump -C
00000000  68 65 6c 6c 6f 20 77 6f  72 6c 64 0a              |hello world.|
0000000c

[kumavis]

this is surprisingly complicated because execa is stripping the file contents newline as if it were the EOF

[kumavis]

currently we also append a newline to the CID on a put

./src/cli/bin.js block put test/test-data/hello | hexdump -C
00000000  51 6d 5a 6a 54 6e 59 77  32 54 46 68 6e 39 4e 6e  |QmZjTnYw2TFhn9Nn|
00000010  37 74 6a 6d 50 53 6f 54  42 6f 59 37 59 52 6b 77  |7tjmPSoTBoY7YRkw|
00000020  50 7a 77 53 72 53 62 61  62 59 32 34 4b 70 0a     |PzwSrSbabY24Kp.|
0000002f

go-ipfs also does this

ipfs block put test/test-data/hello | hexdump -C
00000000  51 6d 5a 6a 54 6e 59 77  32 54 46 68 6e 39 4e 6e  |QmZjTnYw2TFhn9Nn|
00000010  37 74 6a 6d 50 53 6f 54  42 6f 59 37 59 52 6b 77  |7tjmPSoTBoY7YRkw|
00000020  50 7a 77 53 72 53 62 61  62 59 32 34 4b 70 0a     |PzwSrSbabY24Kp.|
0000002f

not sure what the unix conversions are here

cat does not append a newline

cat test/test-data/hello | hexdump
0000000 6568 6c6c 206f 6f77 6c72 0a64          
000000c

curl does not apply a newline

[daviddias]

@whyrusleeping @Kubuxu wanna chime in here?

[kumavis]

per discussion in irc, we thought that:

• newlines in "ui" mode (e.g. tty) are expected
• being able to pipe without a newline makes sense
• magically dropping the newline on stdou.isTTY is too magical, so there should be a flag

[kumavis]

somewhere i trolled myself and made some bad assumptions about go-ipfs behavior
seems ipfs cat X does not append a newline

will take another look tomorrow and more closely compare go-ipfs + js-ipfs

[kumavis]

ok so the issue is that we:

• append a newline to jsipfs block get <cid>, its incorrect to do that

this is complicated by the fact that we:

• strip newlines on cli responses inside of ipfsExec (actually in execa)
• test jsipfs block get against a file that has a newline at the end (that gets stripped by ipfsExec when the command correctly does not append one)

so we can:

1. remove new line stripping by ipfsExec/execa and change all related tests to expect newlines (messy)
2. change the jsipfs block get test to not check for the newline even though its part of the file content (this is what we do in jsipfs files cat which correctly does not append a newline but sadly has the file's newline eaten)
3. test jsipfs block get to test against a file that does not end in a newline

[kumavis]

@diasdavid @dignifiedquire pick your poison and I'll update the PR

[daviddias]

@kumavis why isn't the option to have a flag --no-newline (or something) not an option? Wasn't that the cleanest and obvious?

[kumavis]

@diasdavid i previously incorrectly stated go-ipfs behavior
these two commands

• ipfs block get
• ipfs cat
  do NOT append a newline in go-ipfs
  I assume we should match go-ipfs

[daviddias]

yeah, let's match go-ipfs then :)

[daviddias]

btw, you can disable that misleading stripping of EOF from execa https://www.npmjs.com/package/execa#stripeof :)

[kumavis]

btw, you can disable that misleading stripping of EOF from execa https://www.npmjs.com/package/execa#stripeof :)

yeah, though it would mean we'd have to change all the tests to expect a newline

[daviddias]

• remove new line stripping by ipfsExec/execa and change all related tests to expect newlines (messy). Yes with cli - get - fix formatting when piping out #900 (comment)
• Make both implementations match cli - get - fix formatting when piping out #900 (comment)
• add this test test jsipfs block get to test against a file that does not end in a newline

[kumavis]

the trifecta! 😱

[kumavis]

closing in favor of #903