Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gather the output #2

Open
sindresorhus opened this issue Dec 13, 2015 · 12 comments
Open

Gather the output #2

sindresorhus opened this issue Dec 13, 2015 · 12 comments

Comments

@sindresorhus
Copy link
Owner

Currently you can do the following if you want the output of multiple calls:

var output = [];
hookStd(str => output.push(str));

// when done logging you can use the `calls` array.

Would be nicer if it did this automagically and offered the output somehow.

Maybe like this?

// callback is now optional
const std = hookStd();

console.log('foo');
console.log('bar');

std.unhook();

console.log(std.output);
//=> 'foo\nbar\n'

@jamestalmage @sotojuan Thoughts? What should the API look like?

@sotojuan
Copy link

That would definitely make multiline output easier 👍 I would argue that's what some users would expect out of the box anyway.

@vadimdemedes
Copy link

Interesting module. I'd suggest unhook.stdout and unhook.stderr instead of one unhook.output. Otherwise, from the first impression, looks fine ;)

@Qix-
Copy link

Qix- commented Dec 13, 2015

Not sure about your last example (unhook is undefined), but I think I get it.

I'd almost want to see something more from this. Being able to hook or filter would be awesome.

const std = hookStd();

std.on('out', str => str.replace(/\n/g, '\n\t'));
std.on('err', str => str.replace(/\n/g, '\n!!\t'));

// or more practically
import chalk from chalk;
std.on('err', str => chalk.bright.red(str));

std.get('out'); // original output contents
std.get('err'); // original output contents

// same could be applied to 'in' but I couldn't think of an example off the top of my head.

Maybe it could also hook fs.write(1, ...) and fs.write(2, ...) as well as fs.read(0, ...) for completeness.

@sindresorhus
Copy link
Owner Author

Interesting module. I'd suggest unhook.stdout and unhook.stderr instead of one unhook.output.

👍

Not sure about your last example (unhook is undefined), but I think I get it.

Typo. Fixed.

std.on('out', str => str.replace(/\n/g, '\n\t'));
std.on('err', str => str.replace(/\n/g, '\n!!\t'));

Why events?

You can already do this with hookStd(str => str.replace(/\n/g, '\n\t'));

Maybe it could also hook fs.write(1, ...) and fs.write(2, ...) as well as fs.read(0, ...) for completeness.

Is 1 the stdout fd? I don't think that works on Windows anyways.

@jamestalmage
Copy link
Contributor

Do you see this as primarily a test tool? If so, would it be valuable to provide convenience methods for accessing individual calls / lines?

// this would be one call but two lines
process.stdout.write('foo\nbar')

@sindresorhus
Copy link
Owner Author

@jamestalmage Yeah, was thinking about that. Could be an option. ?

@jamestalmage
Copy link
Contributor

Could be an option

👍

@Qix-
Copy link

Qix- commented Dec 13, 2015

Why events?

Hooks are usually pretty event driven, and events are a good way to break up similar functionality with different inputs, optionally. I might want to subscribe to just stderr. How would I do that with a simple constructor? null?

Is 1 the stdout fd? I don't think that works on Windows anyways.

Yes; 0 = in, 1 = out, 2 = err. Technically more FDs can be assigned but that's archaic and edge-casey.

Pretty sure libuv abstracts this on windows (and I wouldn't be surprised if windows does work like that, given the C standard and whatnot).

Other fd's are pretty uncommon to use via output methods. Technically you could do this:

(echo "require('fs').createReadStream(null, {fd:3}).pipe(process.stdout);" | node) 3<<< "hello, world"

@sindresorhus
Copy link
Owner Author

Hooks are usually pretty event driven, and events are a good way to break up similar functionality with different inputs, optionally.

But your example won't work with events as events are async and can't take a return value. So modifying the output like you're doing in your example wouldn't work.

I might want to subscribe to just stderr. How would I do that with a simple constructor? null?

See how I already solve it. Main export for both. Sub-prop for specific stream.

Pretty sure libuv abstracts this on windows (and I wouldn't be surprised if windows does work like that, given the C standard and whatnot).

They might, but I've honestly never seen this used and I don't think I want to bother adding support for it until there's demand.

@Qix-
Copy link

Qix- commented Dec 13, 2015

But your example won't work with events as events are async and can't take a return value. So modifying the output like you're doing in your example wouldn't work.

Derp, you're right.

Main export for both. Sub-prop for specific stream.

That doesn't seem intuitive to me, IMO, but I can't think of a better solution.

They might, but I've honestly never seen this used and I don't think I want to bother adding support for it until there's demand.

It's proper FD stuff. It'd be super complete if you did.


For giggles, I wondered if writing to process.stdout called either write or writeSync with the fd of 1.

screen shot 2015-12-13 at 15 00 55

It doesn't.

screen shot 2015-12-13 at 15 02 57

fd 20, as you can see, is just its REPL history

So it's up to you if you want to support it. It would be correct functionality, as anything written to 1 or 2 wouldn't be hooked by this library.

@sindresorhus
Copy link
Owner Author

That doesn't seem intuitive to me, IMO, but I can't think of a better solution.

I guess I could have dropped the main export and done an hookStd.all export instead, but I don't see how that's any better.

So it's up to you if you want to support it. It would be correct functionality, as anything written to 1 or 2 wouldn't be hooked by this library.

I've yet to see anyone use that in Node.js and I don't see why they would when we have a better way.

@Qix-
Copy link

Qix- commented Dec 13, 2015

I don't see why they would when we have a better way

Maybe not explicitly, but if you have a FD configuration and you just want to write to standard output instead of a file descriptor for an actual file, you'd just specify 1 and not have to change any functionality. It's somewhat common practice in C coding and since Node is just a loose javascript wrapper around Unix functionality it would make sense (to me, anyway).

But I am just a C programmer in a Javascript programmer's body. 👻

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

No branches or pull requests

5 participants