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

stdin handling #20

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

stdin handling #20

sindresorhus opened this issue Dec 14, 2015 · 12 comments

Comments

@sindresorhus
Copy link
Owner

Note, not reading, but handling when to read from stdin.

Currently it's this logic: https://github.com/sindresorhus/find-versions-cli/blob/56dbd77051147819abdf845504af3b3cc1dd2935/cli.js#L30-L41

The logic is, we only try to read from stdin when there's no input and process.isTTY === false. Otherwise it won't work when used with child_process.execFile so the logic is hard to get right for many
if you only check isTTY, which i did with most of my CLIs previously, it wouldn't work with execFile, as isTTY is false there and it tries to read from stdin.

From hangouts chat:

Sindre: might consider handling reading stdin too, though not quite sure about that
maybe half of my CLIs read from stdin

Sam Verschueren: I would say "why not". But then again, it's an extra meow dependency

Sindre: extra deps doesn't matter that much since it's only used top-level by a CLI but the we can begin with the above. I want it separate anyways as some CLIs does streaming stdin reading which would require manual work anyways. so I'm thinking first a cli.stdinOrInput and maybe at a later point we can do cli.stdin() which makes use of the former.

@SamVerschueren
Copy link
Contributor

So the function returns the input (if present) or the data from stdin if not. But the formats are different. The input is an array of arguments while stdin is a string with newlines. Should we parse the stdin to be an array.

Example

$ echo 'hello world' | cli

Will result in hello world\n as input. We could by default parse the first line to the array like this ['hello', 'world'].

But what if you want to parse multiline. We can make a multiline argument that is by default false. If it is set to true, it would pass in a 2D array, for the input as well. This would make it a lot easier because you don't have to check types.

wrap up

default
$ echo 'hello world' | cli
//=> input: ['hello', 'world']
$ cli hello world
//=> input: ['hello', 'world']
multiline
$ echo 'hello world\nfoo bar' | cli
//=> input: [['hello', 'world'], ['foo', 'bar']]
$ cli hello world
//=> input: [['hello', 'world']]

@sindresorhus
Copy link
Owner Author

Most of my CLI's only supports one argument though, so cli.input[0] or stdin. We can deal with multiple arguments later.

@SamVerschueren
Copy link
Contributor

I'm ok with the first argument. But it might be more interesting to return the an array of the first line.

For example, if we have this piece of code

cli.stdinOrInput().then(input => {
    console.log(input);
});

We could make input the first argument and drop others.

$ echo 'hello world' | cli
//=> input: hello
$ cli hello world
//=> input: hello

But if, and it's only a matter of time, we want to support multiple arguments, it's again a breaking change. Because the result of the promise isn't a single value anymore, but an array of values.

So maybe it's better to support multiple arguments, but not multiple lines (in case of stdin). This way, the result of the promise is always an array.

$ echo 'hello world\nfoo bar' | cli
//=> input: ['hello', 'world']
$ cli hello world
//=> input: ['hello', 'world']

Multiple lines are not supported for now. If in the future, we want to add multiple lines as describe in my previous post (with a 2D array). It's not a breaking change but an extra feature because the default result stays the same, only when passing multiline: true, it returns a 2D array.

Just some thoughts, you're the boss ;). So if you don't mind doing another breaking change release in the (near) future, we can just return the first argument for now.

@sindresorhus
Copy link
Owner Author

So maybe it's better to support multiple arguments, but not multiple lines (in case of stdin). This way, the result of the promise is always an array.

Many of my CLI's support multiple arguments in the input, but never on stdin, meaning:

$ echo foo | cli
//=> input: ['foo']
$ cli foo bar
//=> input: ['foo', 'bar']

I can't think of any use-case for supporting multiple arguments on stdin. It's almost always just a streamed string input.

So with this in mind, we could either make it always an array and force us to use the first element if the CLI input only supports one argument, or iterate over it when supporting multiple. Alternatively could have an option for it, but the former might be simpler.

@SamVerschueren
Copy link
Contributor

Many of my CLI's support multiple arguments in the input, but never on stdin

Seems logical

Can you give an example on what you mean with your latest paragraph? Do you mean this?

$ echo 'hello world' | cli
//=> input: ['hello', 'world']
$ cli hello world
//=> input: ['hello', 'world']

@sindresorhus
Copy link
Owner Author

No, I mean that stdin is always one input (regardless of spaces, newlines, etc), while the main CLI input can be either one or multiple, depending on the CLI.

@SamVerschueren
Copy link
Contributor

Oh ok, no problem. Thanks.

@Qix-
Copy link

Qix- commented Dec 24, 2015

Wait... am I missing something? You never support command line arguments coming in from stdin. You still issue flags to the command using the same flags you'd use anyway, but instead of specifying a filename or something you use stdin instead.

@SamVerschueren
Copy link
Contributor

At least it gives the user the option to use both while the developer only has to use one function call instead of working with get-stdin and stuff. It's always the same workflow if you want to support stdin, so why not let meow handle that?

@Qix-
Copy link

Qix- commented Dec 24, 2015

Still confused. Not sure we're talking about the same thing. From what I'm reading, you're wanting to allow something like echo "--some-flag" | app-with-meow in order to allow app-with-meow to use meow meow to parse stdin as command line arguments.

@SamVerschueren
Copy link
Contributor

No, not the flags, the input.

For instance dev-time. It doesn't support stdin at the moment, so this is not possible.

$ echo "SamVerschueren" | dev-time
$ cat user.txt | dev-time

A package of @sindresorhus using stdin is find-versions.

@Qix-
Copy link

Qix- commented Dec 24, 2015

Gotcha. Disregard then.

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

Successfully merging a pull request may close this issue.

3 participants