fix stdin.js

[ghost]

get expectPipeIn by the right way

[wooorm]

Nice catch! Do you know the support of Stat#isFIFO? I couldn’t find it anywhere...

[anko]

Is this really correct?

Old method:

$ node -e "console.log(\!process.stdin.isTTY)" <<< "hello"
true
$ node -e "console.log(\!process.stdin.isTTY)"            
false

Proposed method:

$ node -e "console.log(require('fs').fstatSync(0).isFIFO())" <<< "hello"
false
$ node -e "console.log(require('fs').fstatSync(0).isFIFO())"            
false

I think our tests don't touch this, hence why Travis shows OK.

[wooorm]

@yukkurisinai @anko Interesing, they do seem to work for echo "hello" | node ex.js, where ex.js looks as follows:

console.log(!process.stdin.isTTY);
console.log(require('fs').fstatSync(0).isFIFO())

Yields:

true
true

But for node ex.js <<< "hello":

true
false

[wooorm]

@yukkurisinai What was the case where this failed?

[ghost]

when I use mdast in drone.io, !process.stdin.isTTY is always true :(
In addition to this, If I write gulpfile.js as below, it output true.

var shell = require("gulp-shell");
gulp.task("foobar", shell.task("node -e \"console.log(\!process.stdin.isTTY)\""));

when I use mdast by gulp-shell, the same thing happened at line 24.

gulp.task("lint-md", shell.task("mdast -u mdast-lint *.md --frail"));

[ghost]

In the first place, mdast check program.files.length at line 37. why must mdast check !process.stdin.isTTY ?

[ghost]

sorry to my poor english :(

[wooorm]

@yukkurisinai because when not piping in, passing no files is an error. When piping in, passing files is an error!

[ghost]

ok, I understood... I thought that when passing files, mdast can ignore expectPipeIn.

[wooorm]

Okay, I tested a lot, it has to do with subprocesses. And I couldn’t find a method, either !process.stdin.isTTY, require('fs').fstatSync(0).isFIFO(), or !require('tty').isatty(process.stdin) which detects it everywhere...

[ghost]

umm... If you can ignore <<< , I think that require('fs').fstatSync(0).isFIFO() is the best way.

[wooorm]

But I’d rather not ignore it though!

[ghost]

ok, I investigation about "<<<". please wait a moment to conclude plz...

[wooorm]

Yeah I’m testing it out too. Thanks for helping!

[ghost]

if user use "<<<", isFIFO can't catch pipe. so, I wrote a code.

var expectPipeIn = !process.stdin.isTTY || fs.fstatSync(0).isFIFO();

How is this?

[wooorm]

stdin: undefined false false  // `<<<` pipe in bash
stdin: undefined false false  // `<<<` pipe in gulp
stdin: undefined true false   // `|` pipe in bash
stdin: undefined true false   // `|` pipe in gulp
stdin: true      false true   // `files` in bash
stdin: undefined false false  // `files` in gulp

As you can see from line 6, it’s exactly the same as line 1 and 2 (meaning it
can’t be detected).

[wooorm]

@anko How standard is <<< it seems pretty hidden?

[wooorm]

I was able to get almost all cases to work:

mdast # throws, correctly
mdast readme.md
echo 'hello' | mdast
mdast <<< hello

...and in gulp (see below for example code):

mdast readme.md
echo 'hello' mdast
mdast <<< hello

By only throwing if there’s definitly a TTY.

However, this doesn’t work for the below case (which keeps hanging)

var gulp = require('gulp');
var shell = require('gulp-shell');

var task = shell.task('mdast'); // this hangs.

gulp.task('default', task);

[wooorm]

Plus, the system outlined above: In bash and gulp: Throws on echo 'hello' | mdast readme.md, but chooses readme.md on mdast readme.md <<< hello (without error).

I’m not sure if “hanging” / waiting for stdin, in a child-process, when not given files is a big problem. I think I’ll just add it :)

[wooorm]

Thanks for your work and help. This ensures that at least no false errors are thrown. It will be released when GH-57 is merged.

[ghost]

thank you very much!