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

Allow prompt() and confirm() to read from redirected stdin #22955

Closed
solb opened this issue Mar 16, 2024 · 5 comments
Closed

Allow prompt() and confirm() to read from redirected stdin #22955

solb opened this issue Mar 16, 2024 · 5 comments

Comments

@solb
Copy link
Contributor

solb commented Mar 16, 2024

I love that Deno has built-in prompt() and confirm() functions because they make it easy to write command-line programs. Deno scripts make excellent replacements for Bourne shell scripts in many cases (and make things such as JSON processing trivial!).

However, the input functions' response to input redirection and piping hinders non-interactive use of command-line utilities. Although they have long returned null and false, respectively, I think the functions should instead retain their normal behavior regardless of whether the input is from a terminal.

As a simple example, the standard Unix utility rev lets one reverse strings from standard input:

$ rev
This is a line I typed.
.depyt I enil a si sihT
This is another line I typed.
.depyt I enil rehtona si sihT
$

It is most useful when used in a pipeline, like so:

$ fortune -l | rev
,slevel neves no tliub saw ti taht hcus saw htiriT saniM fo noihsaf eht roF
llaw hcae ni dna ,llaw a tes saw hcae tuoba dna ,llih a otni devled hcae
.etag a saw
"gniK eht fo nruteR ehT" ,neikloT .R.R.J --		

nehw ,4.4V ,"serutcurtS ataD dna slanretnI SMV" ni detouQ[	
].weivrevo metsys ot gnirrefer 	
$

One can trivially implement a rev clone in Deno by putting the following in an executable script named rev:

#!/usr/bin/env -S deno run --ext ts --check

let read;
while((read = prompt("")) != null)
	console.log([...read].reverse().join(""));

However, although the first use case works, the second exits immediately when prompt() returns null:

$ ./rev
 This is a line I typed.
.depyt I enil a si sihT
 This is another line I typed.
.depyt I enil rehtona si sihT

$ fortune -l | ./rev
$

The current behavior is achieved by the stdin.isTerminal() checks in runtime/js/41_prompt.js. They have been present since the input functions were introduced, having been added in response to a reviewer feedback in #7507 with no further discussion. I think that supporting this style of command-line utility is more important than matching browser behavior in this case.

I suspect that removing this special case would obviate other issues such as #22633.

@lucacasonato
Copy link
Member

prompt, confirm, and input are meant for users. They should not block when called in CI for example. Changing this would be a breaking change.

If you want to collect input from a user or machine, I suggest you call Deno.stdin.readSync() to read stdin input.

@solb
Copy link
Contributor Author

solb commented Jul 11, 2024

I acknowledge that this would be a breaking change, but it would significantly ease Deno's use as a scripting language. Given that Deno 2 is getting closer, isn't now the right time to consider breaking changes?

@solb
Copy link
Contributor Author

solb commented Sep 21, 2024

Furthermore, https://deno.com/blog/v2.0-release-candidate lists the Deno.ReaderSync interface that provides Deno.stdin.readSync() as being removed in Deno 2, so the proposed workaround will no longer be available.

@solb
Copy link
Contributor Author

solb commented Sep 21, 2024

@lucacasonato Is there room to consider breaking I/O changes such as this before Deno 2 is final? Together with #22956 and #22957, this minor improvement to the behavior of these functions would make writing simple command-line tools in Deno seamless.

@lucacasonato
Copy link
Member

Furthermore, https://deno.com/blog/v2.0-release-candidate lists the Deno.ReaderSync interface that provides Deno.stdin.readSync() as being removed in Deno 2, so the proposed workaround will no longer be available.

Just the type interface is going away - not the actual method

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

No branches or pull requests

2 participants