Commander.js: Not compatible with `Node v6.0.0` or `Node v6.1.0`?

[hustcer]

It's strange that when I switch to Node v6.1.0 to execute my app.js -h, just to find the output help content was truncated as:

  Usage: index [options]

  qut is a command line tool for Stock Analysis and Research.

  Options:

    -h, --help             output usage information
    -V, --version          output the version number
    -p, --page   <n>       specify the page index to display.
    -l, --limit  <n>       set total display limit of current page.
    -s, --sort   <sort>    specify field to sort by.
    -f, --format           enable or disable number format.
    -o, --order  <order>   specify order direction: asc or desc.
    -q, --query  <query>   specify collection to query from mongodb. available collections: earning,cal,
                           forecast,fund,gsz,holder,hotspot,insider,investigation,issuance,restrict,
                           shareholder
    -u, --update <update>  update certain datas, available data types: all,quote,cia,range,cal,insider,
                           holder,margin,account,transfer,gsz,forecast,earning,hgt,restrict,price,fprice,
                           dd,fdd,ihustcer@~/github/swift-quant$

When I switch back to Node v5.11.1, it works well:

  Usage: index [options]

  qut is a command line tool for Stock Analysis and Research.

  Options:

    -h, --help             output usage information
    -V, --version          output the version number
    -p, --page   <n>       specify the page index to display.
    -l, --limit  <n>       set total display limit of current page.
    -s, --sort   <sort>    specify field to sort by.
    -f, --format           enable or disable number format.
    -o, --order  <order>   specify order direction: asc or desc.
    -q, --query  <query>   specify collection to query from mongodb. available collections: earning,cal,
                           forecast,fund,gsz,holder,hotspot,insider,investigation,issuance,restrict,
                           shareholder
    -u, --update <update>  update certain datas, available data types: all,quote,cia,range,cal,insider,
                           holder,margin,account,transfer,gsz,forecast,earning,hgt,restrict,price,fprice,
                           dd,fdd,issuance,investigate,shareholder,hotspot.
    -Q, --quant <quant>    filter stocks, available filter types: discount,forecast,earning,insider,gsz,
                           10holder,investigate,shareholder,fund,issuance.
    -S, --stat  <stat>     data statistic, available stat types: insider,insider-week,insider-month,dd,
                           dd-day,dd-week.
    -O, --optimize         filter stocks in optimized way!
    -g, --grep  <kw>       specify the keyword to grep in name or concept, multiple keywords should be
                           separated by ",".

Whether this is a bug or not?

[hustcer]

Node v6.0.0 seems to have the same problem after I update to Node v6.1.0 by homebrew and switch back by nvm use --delete-prefix v6.0.0, however, when I first upgrade to Node v6.0.0, I didn't meet this problem. It's really quite strange.

[nicola]

Same issue here, after an x amount of characters, it returns an EOF

[nicola]

Apparently --help triggers a unknown command found that triggers a process.exit(1) on subcommands

[zhiyelee]

oh, interesting. I will check that, and PR is welcome

[lpinca]

Probably relevant issue: https://github.com/nodejs/node/issues/6456.

[cbetta]

@nicola do you know what the line/lines are that cause this issue?

[nicola]

@cbetta nope, but see here: https://github.com/nodejs/node/issues/6456

[cbetta]

@nicola ok thanks. Was just hoping to see if i could make a local patch that fixes it for us by changing the way we exit.

[bcoe]

here's a workaround I'm considering landing in yargs for this problem:

https://github.com/yargs/yargs/pull/501

[bcoe]

@hustcer @cbetta @zhiyelee I pulled the tiny bit of logic that yargs uses to solve this problem into a shim:

https://github.com/yargs/set-blocking

UglifyJS2 uses this same approach, which I modeled yargs' fix after: https://github.com/mishoo/UglifyJS2/pull/1061/files

[cbetta]

@bcoe that's awesome. Would love to see if we can make a patch for commander that uses this fix. What would that take?

[bcoe]

@cbetta yargs writes all of its output in one call, prior to calling process.exit(code);; If commander uses a similar approach, you should be able to do the following:

1. check whether you are about to call process.exit(0), prior to printing your console output.
2. if you are about to print the output and call process.exit(0):

_set blocking to true:_

const setBlocking = require('set-blocking')
setBlocking(true)

_output your content like usual:_

console.log(commander.generateCLI());

_now exit:_

process.exit(0);

Note that you need to set blocking to true, prior to printing the console output.

[nicola]

@bcoe grat work! Thanks for making it modular!

[cbetta]

@bcoe @nicola sorry not sure i follow. Can I apply this to my CLI program or does this need to be a patch to commander?

[hustcer]

@cbetta
Actually, just add the following lines at the beginning should work:

[process.stdout, process.stderr].forEach(stream => {
  if (stream._handle && typeof stream._handle.setBlocking === 'function') {
    stream._handle.setBlocking(true);
  }
});

[cbetta]

@hustcer my hero! Do you think it would be worth shipping as a module that people can easily include? E.g. blocked-io?

[lpinca]

@cbetta that's what https://github.com/yargs/set-blocking does, also the issue will be fixed soon in core, see https://github.com/nodejs/node/issues/6980.

[cbetta]

@lpinca ah now that earlier comment makes sense. Thanks for clarifying.

[lpinca]

I think this can be closed now, It should have been fixed in node 6.2.1.

[zhiyelee]

thanks @lpinca

[shadowspawn]

Future readers: this may still be an issue on Windows TTY which are asynchronous according to:

• https://nodejs.org/api/process.html#process_a_note_on_process_i_o

Please open a new issue if you are seeing truncated help.