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

feat: add alert, confirm, and prompt #7507

Merged
merged 6 commits into from
Oct 13, 2020
Merged

Conversation

kt3k
Copy link
Member

@kt3k kt3k commented Sep 16, 2020

This PR adds alert, confirm, and prompt functions from web standards.

You can write simple interaction script like the below with this change.

const name = prompt('What is your name?');
console.log(`Hello ${name}!`);
$ ./target/debug/deno run test.ts
What is your name? John
Hello John!

Maybe these features are not so important for advanced users, but I think these could be very helpful for beginners of Deno or learners of the language.


closes #4257

Copy link
Collaborator

@nayeemrmn nayeemrmn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When not interactive, we should do what browsers do when dialog boxes are disabled.

cli/rt/41_prompt.js Outdated Show resolved Hide resolved
cli/rt/41_prompt.js Outdated Show resolved Hide resolved
cli/rt/41_prompt.js Outdated Show resolved Hide resolved
cli/dts/lib.deno.shared_globals.d.ts Outdated Show resolved Hide resolved
cli/rt/41_prompt.js Outdated Show resolved Hide resolved
cli/rt/41_prompt.js Outdated Show resolved Hide resolved
cli/rt/41_prompt.js Outdated Show resolved Hide resolved
Copy link
Collaborator

@nayeemrmn nayeemrmn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kryptish
Copy link

I like this, anything speaking against merging this? =)

@ry ry added this to the 1.5.0 milestone Sep 18, 2020
@bartlomieju
Copy link
Member

@kt3k this looks great! I love how simple the implementation is and having this API enables users to create simple CLI scripts without additional libraries. Please the branch so we can land it

@kt3k
Copy link
Member Author

kt3k commented Oct 12, 2020

@bartlomieju Thank you for the feedbacks! Rebased the branch! I'll add the handling (and tests) of yes, Yes, etc in confirm function a little later. Keep it as is for now.

Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, great feature @kt3k!

@bartlomieju bartlomieju merged commit 0dcaea7 into denoland:master Oct 13, 2020
@bartlomieju
Copy link
Member

Yikes, it looks like @kitsonk had a lot of concerns for this API in #4257. @kitsonk are you still against them? If so, I will do the revert.

bartlomieju added a commit to bartlomieju/deno that referenced this pull request Oct 13, 2020
@David-Else
Copy link

These are awesome. I hope to teach kids programming and see Deno as a perfect way to provide everything they will need in one executable. I can even teach typed vs untyped going from JS to TS, these kinds of simple API's to do essential things are so important. It is the difference between friendly and easy to understand vs annoying and pointless time-wasting.

@kt3k kt3k deleted the feature/prompt branch October 30, 2020 05:35
@Bestulo
Copy link

Bestulo commented Dec 30, 2020

Is there any way to use these as promise instead of blocking?

edit: guess it could be a child process that reads from console but that sounds over the top

@Soremwar
Copy link
Contributor

@Bestulo Alert, confirm and prompt are all abstractions over a simple stdin/stdout program, it's quite easy to implement asynchronously without this API

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

Successfully merging this pull request may close these issues.

Request: alert, confirm, prompt, and their async counterpart
10 participants